Re: [Intel-gfx] [PATCH v7 1/5] drm/i915: Implement a framework for batch buffer pools
-Original Message- From: Nguyen, Michael H Sent: Thursday, December 11, 2014 8:13 PM To: intel-gfx@lists.freedesktop.org Cc: Bloomfield, Jon; Brad Volkin Subject: [PATCH v7 1/5] drm/i915: Implement a framework for batch buffer pools From: Brad Volkin bradley.d.vol...@intel.com This adds a small module for managing a pool of batch buffers. The only current use case is for the command parser, as described in the kerneldoc in the patch. The code is simple, but separating it out makes it easier to change the underlying algorithms and to extend to future use cases should they arise. The interface is simple: init to create an empty pool, fini to clean it up, get to obtain a new buffer. Note that all buffers are expected to be inactive before cleaning up the pool. Locking is currently based on the caller holding the struct_mutex. We already do that in the places where we will use the batch pool for the command parser. v2: - s/BUG_ON/WARN_ON/ for locking assertions - Remove the cap on pool size - Switch from alloc/free to init/fini v3: - Idiomatic looping structure in _fini - Correct handling of purged objects - Don't return a buffer that's too much larger than needed v4: - Rebased to latest -nightly v5: - Remove _put() function and clean up comments to match v6: - Move purged check inside the loop (danvet, from v4 1/7 feedback) v7: - Use single list instead of two. (Chris W) - s/active_list/cache_list - Squashed in debug patches (Chris W) drm/i915: Add a batch pool debugfs file It provides some useful information about the buffers in the global command parser batch pool. v2: rebase on global pool instead of per-ring pools v3: rebase drm/i915: Add batch pool details to i915_gem_objects debugfs To better account for the potentially large memory consumption of the batch pool. v8: - Keep cache in LRU order (danvet, from v6 1/5 feedback) Issue: VIZ-4719 Signed-off-by: Brad Volkin bradley.d.vol...@intel.com --- Documentation/DocBook/drm.tmpl | 5 ++ drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/i915_debugfs.c| 71 +-- drivers/gpu/drm/i915/i915_drv.h| 21 + drivers/gpu/drm/i915/i915_gem.c| 1 + drivers/gpu/drm/i915/i915_gem_batch_pool.c | 135 + 6 files changed, 225 insertions(+), 9 deletions(-) create mode 100644 drivers/gpu/drm/i915/i915_gem_batch_pool.c diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 25c23ca..21cbcdb 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -4059,6 +4059,11 @@ int num_ioctls;/synopsis !Idrivers/gpu/drm/i915/i915_cmd_parser.c /sect2 sect2 +titleBatchbuffer Pools/title +!Pdrivers/gpu/drm/i915/i915_gem_batch_pool.c batch pool +!Idrivers/gpu/drm/i915/i915_gem_batch_pool.c + /sect2 + sect2 titleLogical Rings, Logical Ring Contexts and Execlists/title !Pdrivers/gpu/drm/i915/intel_lrc.c Logical Rings, Logical Ring Contexts and Execlists !Idrivers/gpu/drm/i915/intel_lrc.c diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 3cf70a6..1849ffa 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -19,6 +19,7 @@ i915-$(CONFIG_DEBUG_FS) += i915_debugfs.o # GEM code i915-y += i915_cmd_parser.o \ + i915_gem_batch_pool.o \ i915_gem_context.o \ i915_gem_render_state.o \ i915_gem_debug.o \ diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c index 479e0c1..5d96d94 100644 --- a/drivers/gpu/drm/i915/i915_debugfs.c +++ b/drivers/gpu/drm/i915/i915_debugfs.c @@ -359,6 +359,33 @@ static int per_file_stats(int id, void *ptr, void *data) return 0; } +#define print_file_stats(m, name, stats) \ + seq_printf(m, %s: %u objects, %zu bytes (%zu active, %zu inactive, %zu global, %zu shared, %zu unbound)\n, \ +name, \ +stats.count, \ +stats.total, \ +stats.active, \ +stats.inactive, \ +stats.global, \ +stats.shared, \ +stats.unbound) + +static void print_batch_pool_stats(struct seq_file *m, +struct drm_i915_private *dev_priv) { + struct drm_i915_gem_object *obj; + struct file_stats stats; + + memset(stats, 0, sizeof(stats)); + + list_for_each_entry(obj, + dev_priv-mm.batch_pool.cache_list, + batch_pool_list) + per_file_stats(0, obj, stats); + + print_file_stats(m, batch pool, stats); } + #define count_vmas(list, member) do { \ list_for_each_entry(vma, list, member) { \ size += i915_gem_obj_ggtt_size(vma-obj); \ @@ -441,6
Re: [Intel-gfx] [PATCH v7 2/5] drm/i915: Use batch pools with the command parser
-Original Message- From: Nguyen, Michael H Sent: Thursday, December 11, 2014 8:13 PM To: intel-gfx@lists.freedesktop.org Cc: Bloomfield, Jon; Brad Volkin Subject: [PATCH v7 2/5] drm/i915: Use batch pools with the command parser From: Brad Volkin bradley.d.vol...@intel.com This patch sets up all of the tracking and copying necessary to use batch pools with the command parser and dispatches the copied (shadow) batch to the hardware. After this patch, the parser is in 'enabling' mode. Note that performance takes a hit from the copy in some cases and will likely need some work. At a rough pass, the memcpy appears to be the bottleneck. Without having done a deeper analysis, two ideas that come to mind are: 1) Copy sections of the batch at a time, as they are reached by parsing. Might improve cache locality. 2) Copy only up to the userspace-supplied batch length and memset the rest of the buffer. Reduces the number of reads. v2: - Remove setting the capacity of the pool - One global pool instead of per-ring pools - Replace batch_obj with shadow_batch_obj and hook into eb-vmas - Memset any space in the shadow batch beyond what gets copied - Rebased on execlist prep refactoring v3: - Rebase on chained batch handling - Squash in setting the secure dispatch flag - Add a note about the interaction w/secure dispatch pinning - Check for request-batch_obj == NULL in i915_gem_free_request v4: - Fix read domains for shadow_batch_obj - Remove the set_to_gtt_domain call from i915_parse_cmds - ggtt_pin/unpin in the parser block to simplify error handling - Check USES_FULL_PPGTT before setting DISPATCH_SECURE flag - Remove i915_gem_batch_pool_put calls v5: - Move 'pending_read_domains |= I915_GEM_DOMAIN_COMMAND' after the parser (danvet, from v4 0/7 feedback) Issue: VIZ-4719 Signed-off-by: Brad Volkin bradley.d.vol...@intel.com --- Reviewed-by: Jon Bloomfield jon.bloomfi...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Refactor work that can sleep out of commit (v4)
On 12/12/2014 01:53 AM, Matt Roper wrote: Once we integrate our work into the atomic pipeline, plane commit operations will need to happen with interrupts disabled, due to vblank evasion. Our commit functions today include sleepable work, so those operations need to be split out and run either before or after the atomic register programming. The solution here calculates which of those operations will need to be performed during the 'check' phase and sets flags in an intel_crtc sub-struct. New intel_begin_crtc_commit() and intel_finish_crtc_commit() functions are added before and after the actual register programming; these will eventually be called from the atomic plane helper's .atomic_begin() and .atomic_end() entrypoints. v2: Fix broken sprite code split v3: Make the pre/post commit work crtc-based to match how we eventually want this to be called from the atomic plane helpers. v4: Some platforms that haven't had their watermark code reworked were waiting for vblank, then calling update_sprite_watermarks in their platform-specific disable code. These also need to be flagged out of the critical section. Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 180 ++- drivers/gpu/drm/i915/intel_drv.h | 31 ++ drivers/gpu/drm/i915/intel_sprite.c | 78 ++- 3 files changed, 178 insertions(+), 111 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index 3044af5..5d90114 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11737,7 +11737,11 @@ static int intel_check_primary_plane(struct drm_plane *plane, struct intel_plane_state *state) { + struct drm_device *dev = plane-dev; + struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc = state-base.crtc; + struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state-base.fb; struct drm_rect *dest = state-dst; struct drm_rect *src = state-src; @@ -11758,6 +11762,40 @@ intel_check_primary_plane(struct drm_plane *plane, return -EBUSY; } + if (intel_crtc-active) { + /* +* FBC does not work on some platforms for rotated +* planes, so disable it when rotation is not 0 and +* update it when rotation is set back to 0. +* +* FIXME: This is redundant with the fbc update done in +* the primary plane enable function except that that +* one is done too late. We eventually need to unify +* this. +*/ + if (intel_crtc-primary_enabled + INTEL_INFO(dev)-gen = 4 !IS_G4X(dev) + dev_priv-fbc.plane == intel_crtc-plane + intel_plane-rotation != BIT(DRM_ROTATE_0)) { + intel_crtc-atomic.disable_fbc = true; + } + + if (state-visible) { + /* +* BDW signals flip done immediately if the plane +* is disabled, even if the plane enable is already +* armed to occur at the next vblank :( +*/ + if (IS_BROADWELL(dev) !intel_crtc-primary_enabled) + intel_crtc-atomic.wait_vblank = true; + } + + intel_crtc-atomic.fb_bits |= + INTEL_FRONTBUFFER_PRIMARY(intel_crtc-pipe); + + intel_crtc-atomic.update_fbc = true; + } + return 0; } @@ -11773,18 +11811,6 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_rect *src = state-src; - enum pipe pipe = intel_plane-pipe; - - if (!fb) { - /* -* 'prepare' is never called when plane is being disabled, so -* we need to handle frontbuffer tracking here -*/ - mutex_lock(dev-struct_mutex); - i915_gem_track_fb(intel_fb_obj(plane-fb), NULL, - INTEL_FRONTBUFFER_PRIMARY(pipe)); - mutex_unlock(dev-struct_mutex); - } plane-fb = fb; crtc-x = src-x1 16; @@ -11801,26 +11827,7 @@ intel_commit_primary_plane(struct drm_plane *plane, intel_plane-obj = obj; if (intel_crtc-active) { - /* -* FBC does not work on some platforms for rotated -* planes, so disable it when rotation is not 0 and -* update it when rotation is set back to 0.
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Force the CS stall for invalidate flushes
On Thu, Dec 11, 2014 at 08:17:00AM +, Chris Wilson wrote: In order to act as a full command barrier by itself, we need to tell the pipecontrol to actually stall the command streamer while the flush runs. We require the full command barrier before operations like MI_SET_CONTEXT, which currently rely on a prior invalidate flush. References: https://bugs.freedesktop.org/show_bug.cgi?id=83677 Cc: Simon Farnsworth si...@farnz.org.uk Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 282279b83ca4..02fb478a2867 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -380,6 +380,8 @@ gen7_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_QW_WRITE; flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; + flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD; + Hmm. BSpec says that the render cache won't be flushed when this bit is set. Is that going to cause problems for the gpu_cache_dirty cases where seem to do invalidate+flush with a single PIPE_CONTROL? /* Workaround: we must issue a pipe_control with CS-stall bit * set before a pipe_control command that has the state cache * invalidate bit set. */ -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 3/5] drm/i915: Use batch length instead of object size in command parser
-Original Message- From: Nguyen, Michael H Sent: Thursday, December 11, 2014 8:13 PM To: intel-gfx@lists.freedesktop.org Cc: Bloomfield, Jon; Brad Volkin Subject: [PATCH v7 3/5] drm/i915: Use batch length instead of object size in command parser From: Brad Volkin bradley.d.vol...@intel.com Previously we couldn't trust the user-supplied batch length because it came directly from userspace (i.e. untrusted code). It would have affected what commands software parsed without regard to what hardware would actually execute, leaving a potential hole. With the parser now copying the user supplied batch buffer and writing MI_NOP commands to any space after the copied region, we can safely use the batch length input. This should be a performance win as the actual batch length is frequently much smaller than the allocated object size. v2: Fix handling of non-zero batch_start_offset Issue: VIZ-4719 Signed-off-by: Brad Volkin bradley.d.vol...@intel.com Reviewed-by: Jon Bloomfield jon.bloomfi...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/i9xx: check for panel on pipe before asserting panel unlock bits
On Wed, Dec 10, 2014 at 02:00:02PM -0800, Jesse Barnes wrote: Should address a warning reported in #79824. References: https://bugs.freedesktop.org/show_bug.cgi?id=79824 Signed-off-by: Jesse Barnes jbar...@virtuousgeek.org Isn's this fixed by https://freedesktop.org/patch/37900/ ? --- drivers/gpu/drm/i915/intel_display.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index c424c36..a19544b 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -1607,7 +1607,9 @@ static void i9xx_enable_pll(struct intel_crtc *crtc) BUG_ON(INTEL_INFO(dev)-gen = 5); /* PLL is protected by panel, make sure we can write it */ - if (IS_MOBILE(dev) !IS_I830(dev)) + if (IS_MOBILE(dev) !IS_I830(dev) + (intel_pipe_has_type(crtc, INTEL_OUTPUT_LVDS) || + intel_pipe_has_type(crtc, INTEL_OUTPUT_EDP))) assert_panel_unlocked(dev_priv, crtc-pipe); /* Enable DVO 2x clock on both PLLs if necessary */ -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 4/5] drm/i915: Mark shadow batch buffers as purgeable
-Original Message- From: Nguyen, Michael H Sent: Thursday, December 11, 2014 8:13 PM To: intel-gfx@lists.freedesktop.org Cc: Bloomfield, Jon; Brad Volkin Subject: [PATCH v7 4/5] drm/i915: Mark shadow batch buffers as purgeable From: Brad Volkin bradley.d.vol...@intel.com By adding a new exec_entry flag, we cleanly mark the shadow objects as purgeable after they are on the active list. v2: - Move 'shadow_batch_obj-madv = I915_MADV_WILLNEED' inside _get fnc (danvet, from v4 6/7 feedback) v3: - Remove duplicate 'madv = I915_MADV_WILLNEED' (danvet, from v6 4/5) Issue: VIZ-4719 Signed-off-by: Brad Volkin bradley.d.vol...@intel.com Reviewed-by: Jon Bloomfield jon.bloomfi...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Force the CS stall for invalidate flushes
On Fri, Dec 12, 2014 at 11:09:15AM +0200, Ville Syrjälä wrote: On Thu, Dec 11, 2014 at 08:17:00AM +, Chris Wilson wrote: In order to act as a full command barrier by itself, we need to tell the pipecontrol to actually stall the command streamer while the flush runs. We require the full command barrier before operations like MI_SET_CONTEXT, which currently rely on a prior invalidate flush. References: https://bugs.freedesktop.org/show_bug.cgi?id=83677 Cc: Simon Farnsworth si...@farnz.org.uk Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 282279b83ca4..02fb478a2867 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -380,6 +380,8 @@ gen7_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_QW_WRITE; flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; + flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD; + Hmm. BSpec says that the render cache won't be flushed when this bit is set. Is that going to cause problems for the gpu_cache_dirty cases where seem to do invalidate+flush with a single PIPE_CONTROL? I thought it was DEPTH_STALL that disabled the write flush. It is redundant in the case where the write flush is taking place though and you can do: /* bspec is not entirely clear when the render target cache flush is * disabled with other stall bits set, so don't set any additional * stalls if we are already using the cache flush. */ if ((flags PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH) == 0) flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD; -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Invalidate media caches on gen7
On Thu, Dec 11, 2014 at 02:24:53PM +0200, Ville Syrjälä wrote: On Thu, Dec 11, 2014 at 08:16:59AM +, Chris Wilson wrote: In the gen7 pipe control there is an extra bit to flush the media caches, so let's set it during cache invalidation flushes. Bspec is telling me this bit is already present in snb, and calls it 'Generic Media State Clear'. Older Bspec seems to suggest we should set it here, and maybe that we should also issue another PIPE_CONTROL with the bit set after MI_SET_CONTEXT when switching from media to 3D context. These notes don't seem to be present in the current BSpec, so I'm not sure what the deal is. In an alternative universe, I do the barrier + context switch first, invalidate later. However, there is also a benefit to doing the invalidate before context switch as bspec talks about not reloading invalidated indirect state. Not sure if that ever applies to us though. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v7 5/5] drm/i915: Tidy up execbuffer command parsing code
-Original Message- From: Nguyen, Michael H Sent: Thursday, December 11, 2014 8:13 PM To: intel-gfx@lists.freedesktop.org Cc: Bloomfield, Jon; Brad Volkin Subject: [PATCH v7 5/5] drm/i915: Tidy up execbuffer command parsing code From: Brad Volkin bradley.d.vol...@intel.com Move it to a separate function since the main do_execbuffer function already has so much going on. v2: - Move pin/unpin calls inside i915_parse_cmds() (Chris W, v4 7/7 feedback) Issue: VIZ-4719 Signed-off-by: Brad Volkin bradley.d.vol...@intel.com Reviewed-by: Jon Bloomfield jon.bloomfi...@intel.com? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] Android.mk: replace std=c99 with std=gnu99
From: Tim Gore tim.g...@intel.com The android makefiles were passing the -std=c99 flag to the compiler which disables the typeof keyword. This causes a build fail for a recent addition to igt_aux.h. Change this to -std=gnu99, which is the flag used in the linux build Signed-off-by: Tim Gore tim.g...@intel.com --- benchmarks/Android.mk | 2 +- lib/Android.mk| 2 +- tests/Android.mk | 2 +- tools/Android.mk | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk index 5bb8ef5..14fc0a7 100644 --- a/benchmarks/Android.mk +++ b/benchmarks/Android.mk @@ -11,7 +11,7 @@ define add_benchmark LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -UNDEBUG -include check-ndebug.h -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. diff --git a/lib/Android.mk b/lib/Android.mk index bd8cf58..548bca4 100644 --- a/lib/Android.mk +++ b/lib/Android.mk @@ -25,7 +25,7 @@ LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH) LOCAL_CFLAGS += -DHAVE_LIBDRM_ATOMIC_PRIMITIVES LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 LOCAL_MODULE:= libintel_gpu_tools LOCAL_SHARED_LIBRARIES := libpciaccess \ diff --git a/tests/Android.mk b/tests/Android.mk index 519852a..814b846 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -33,7 +33,7 @@ skip_tests_list += pm_rpm # set local compilation flags for IGT tests IGT_LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM -DANDROID -UNDEBUG -IGT_LOCAL_CFLAGS += -include check-ndebug.h -std=c99 +IGT_LOCAL_CFLAGS += -include check-ndebug.h -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit IGT_LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. diff --git a/tools/Android.mk b/tools/Android.mk index 8ca67f4..39f4512 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -12,7 +12,7 @@ define add_tool LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 0/3] Rearranging eDP PPS code
Gentle reminder for review.. - Vandana On 10-Nov-14 4:58 PM, Kannan, Vandana wrote: In the earlier RFC patch series, PPS related code was moved to intel_panel.c to make it usable for all internal panels. In this patch series, the PPS related functions are split based on platform in intel_dp.c itself. This avoids having code split across intel_dp.c and intel_panel.c w.r.t pps_lock/unlock, other pps delay functions. Also, the code rearrangement is for eDP alone. Vandana Kannan (3): drm/i915: Move PPS calls to edp_init drm/i915: Use vlv_power_sequencer_pipe() only to get pipe drm/i915: Splitting PPS functions based on platform drivers/gpu/drm/i915/intel_dp.c | 226 +-- drivers/gpu/drm/i915/intel_drv.h | 3 + 2 files changed, 147 insertions(+), 82 deletions(-) ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] Demos/Android.mk: dont build intel_sprite_on
From: Tim Gore tim.g...@intel.com intel_sprite_on wont build on Android, due to use of a particular API that has changed in Gmin Signed-off-by: Tim Gore tim.g...@intel.com --- demos/Android.mk | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/demos/Android.mk b/demos/Android.mk index 6227e06..be7f3c2 100644 --- a/demos/Android.mk +++ b/demos/Android.mk @@ -4,11 +4,13 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) -LOCAL_SRC_FILES := intel_sprite_on.c +# This demo wont build on android (from Gmin on). +#LOCAL_SRC_FILES := intel_sprite_on.c + LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # Excessive complaining for established cases. Rely on the Linux version warnings. LOCAL_CFLAGS += -Wno-sign-compare -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 1/3] drm/i915: Move PPS calls to edp_init
On Mon, 10 Nov 2014, Vandana Kannan vandana.kan...@intel.com wrote: Calls to setup eDP panel power sequencer were there in dp_init_connector() function. Moving these calls to edp_init_connector() to keep all PPS calls together and under edp init. It's a bit tricky, but I think you need to setup the power sequencer stuff on eDP before you do AUX communication. BR, Jani. Signed-off-by: Vandana Kannan vandana.kan...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 15 +-- 1 file changed, 5 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index ceb528f..34d8105 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5258,6 +5258,11 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp, /* We now know it's not a ghost, init power sequence regs. */ pps_lock(intel_dp); + intel_dp_init_panel_power_timestamps(intel_dp); + if (IS_VALLEYVIEW(dev)) + vlv_initial_power_sequencer_setup(intel_dp); + else + intel_dp_init_panel_power_sequencer(dev, intel_dp); intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); pps_unlock(intel_dp); @@ -5402,16 +5407,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, BUG(); } - if (is_edp(intel_dp)) { - pps_lock(intel_dp); - intel_dp_init_panel_power_timestamps(intel_dp); - if (IS_VALLEYVIEW(dev)) - vlv_initial_power_sequencer_setup(intel_dp); - else - intel_dp_init_panel_power_sequencer(dev, intel_dp); - pps_unlock(intel_dp); - } - intel_dp_aux_init(intel_dp, intel_connector); /* init MST on ports that can support it */ -- 2.0.1 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Use vlv_power_sequencer_pipe() only to get pipe
On Mon, 10 Nov 2014, Vandana Kannan vandana.kan...@intel.com wrote: vlv_power_sequencer_pipe() calls into init PPS functions. Changing this function to make it only return pipe and not call PPS init. This is because PPS init calls into this function to get a pipe ID and all other callers just need the pipe ID. Signed-off-by: Vandana Kannan vandana.kan...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 4 1 file changed, 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 34d8105..1d4bf78 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -434,10 +434,6 @@ vlv_power_sequencer_pipe(struct intel_dp *intel_dp) pipe_name(intel_dp-pps_pipe), port_name(intel_dig_port-port)); - /* init power sequencer on this pipe and port */ - intel_dp_init_panel_power_sequencer(dev, intel_dp); - intel_dp_init_panel_power_sequencer_registers(dev, intel_dp); IIUC if you end up this far in the function it's a special case, in most cases it'll return with intel_dp-pps_pipe. And the special case probably means you'll need to setup the registers and everything. Ville? BR, Jani. - /* * Even vdd force doesn't work until we've made * the power sequencer lock in on the port. -- 2.0.1 -- Jani Nikula, Intel Open Source Technology Center ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/3] drm/i915: Force the CS stall for invalidate flushes
On Fri, Dec 12, 2014 at 09:20:49AM +, Chris Wilson wrote: On Fri, Dec 12, 2014 at 11:09:15AM +0200, Ville Syrjälä wrote: On Thu, Dec 11, 2014 at 08:17:00AM +, Chris Wilson wrote: In order to act as a full command barrier by itself, we need to tell the pipecontrol to actually stall the command streamer while the flush runs. We require the full command barrier before operations like MI_SET_CONTEXT, which currently rely on a prior invalidate flush. References: https://bugs.freedesktop.org/show_bug.cgi?id=83677 Cc: Simon Farnsworth si...@farnz.org.uk Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Cc: sta...@vger.kernel.org --- drivers/gpu/drm/i915/intel_ringbuffer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 282279b83ca4..02fb478a2867 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -380,6 +380,8 @@ gen7_render_ring_flush(struct intel_engine_cs *ring, flags |= PIPE_CONTROL_QW_WRITE; flags |= PIPE_CONTROL_GLOBAL_GTT_IVB; + flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD; + Hmm. BSpec says that the render cache won't be flushed when this bit is set. Is that going to cause problems for the gpu_cache_dirty cases where seem to do invalidate+flush with a single PIPE_CONTROL? I thought it was DEPTH_STALL that disabled the write flush. Hmm. Yeah, the previous sentence talks about the depth stall bit. So I suppose it could still be referring to the depth stall bit when it says the render cache flush won't be flushed. It is redundant in the case where the write flush is taking place though and you can do: /* bspec is not entirely clear when the render target cache flush is * disabled with other stall bits set, so don't set any additional * stalls if we are already using the cache flush. */ if ((flags PIPE_CONTROL_RENDER_TARGET_CACHE_FLUSH) == 0) flags |= PIPE_CONTROL_STALL_AT_SCOREBOARD; -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t] Demos/Android.mk: dont build intel_sprite_on
On 12 December 2014 at 10:18, tim.g...@intel.com wrote: From: Tim Gore tim.g...@intel.com intel_sprite_on wont build on Android, due to use of a particular API that has changed in Gmin Signed-off-by: Tim Gore tim.g...@intel.com --- demos/Android.mk | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/demos/Android.mk b/demos/Android.mk index 6227e06..be7f3c2 100644 --- a/demos/Android.mk +++ b/demos/Android.mk @@ -4,11 +4,13 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) -LOCAL_SRC_FILES := intel_sprite_on.c +# This demo wont build on android (from Gmin on). +#LOCAL_SRC_FILES := intel_sprite_on.c + LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 I think this change should have been in your previous patch (Android.mk: replace std=c99 with std=gnu99)? # Excessive complaining for established cases. Rely on the Linux version warnings. LOCAL_CFLAGS += -Wno-sign-compare -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2] Demos/Android.mk: dont build intel_sprite_on
From: Tim Gore tim.g...@intel.com intel_sprite_on wont build on Android. Signed-off-by: Tim Gore tim.g...@intel.com --- demos/Android.mk | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/demos/Android.mk b/demos/Android.mk index 6227e06..d2592b1 100644 --- a/demos/Android.mk +++ b/demos/Android.mk @@ -4,7 +4,9 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) -LOCAL_SRC_FILES := intel_sprite_on.c +# This demo wont build on android +#LOCAL_SRC_FILES := intel_sprite_on.c + LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DANDROID -UNDEBUG -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t v2] Android.mk: replace std=c99 with std=gnu99
From: Tim Gore tim.g...@intel.com The android makefiles were passing the -std=c99 flag to the compiler which disables the typeof keyword. This causes a build fail for a recent addition to igt_aux.h. Change this to -std=gnu99, which is the flag used in the linux build Signed-off-by: Tim Gore tim.g...@intel.com --- benchmarks/Android.mk | 2 +- demos/Android.mk | 2 +- lib/Android.mk| 2 +- tests/Android.mk | 2 +- tools/Android.mk | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk index 5bb8ef5..14fc0a7 100644 --- a/benchmarks/Android.mk +++ b/benchmarks/Android.mk @@ -11,7 +11,7 @@ define add_benchmark LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -UNDEBUG -include check-ndebug.h -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. diff --git a/demos/Android.mk b/demos/Android.mk index 6227e06..309ab96 100644 --- a/demos/Android.mk +++ b/demos/Android.mk @@ -8,7 +8,7 @@ LOCAL_SRC_FILES := intel_sprite_on.c LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # Excessive complaining for established cases. Rely on the Linux version warnings. LOCAL_CFLAGS += -Wno-sign-compare diff --git a/lib/Android.mk b/lib/Android.mk index bd8cf58..548bca4 100644 --- a/lib/Android.mk +++ b/lib/Android.mk @@ -25,7 +25,7 @@ LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH) LOCAL_CFLAGS += -DHAVE_LIBDRM_ATOMIC_PRIMITIVES LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 LOCAL_MODULE:= libintel_gpu_tools LOCAL_SHARED_LIBRARIES := libpciaccess \ diff --git a/tests/Android.mk b/tests/Android.mk index 519852a..814b846 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -33,7 +33,7 @@ skip_tests_list += pm_rpm # set local compilation flags for IGT tests IGT_LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM -DANDROID -UNDEBUG -IGT_LOCAL_CFLAGS += -include check-ndebug.h -std=c99 +IGT_LOCAL_CFLAGS += -include check-ndebug.h -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit IGT_LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. diff --git a/tools/Android.mk b/tools/Android.mk index 8ca67f4..39f4512 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -12,7 +12,7 @@ define add_tool LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH i-g-t v2] Android.mk: replace std=c99 with std=gnu99
On 12 December 2014 at 12:14, tim.g...@intel.com wrote: From: Tim Gore tim.g...@intel.com The android makefiles were passing the -std=c99 flag to the compiler which disables the typeof keyword. This causes a build fail for a recent addition to igt_aux.h. Change this to -std=gnu99, which is the flag used in the linux build Signed-off-by: Tim Gore tim.g...@intel.com Thanks, I've merged both this and the demos/Android.mk patch. --- benchmarks/Android.mk | 2 +- demos/Android.mk | 2 +- lib/Android.mk| 2 +- tests/Android.mk | 2 +- tools/Android.mk | 2 +- 5 files changed, 5 insertions(+), 5 deletions(-) diff --git a/benchmarks/Android.mk b/benchmarks/Android.mk index 5bb8ef5..14fc0a7 100644 --- a/benchmarks/Android.mk +++ b/benchmarks/Android.mk @@ -11,7 +11,7 @@ define add_benchmark LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -UNDEBUG -include check-ndebug.h -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. diff --git a/demos/Android.mk b/demos/Android.mk index 6227e06..309ab96 100644 --- a/demos/Android.mk +++ b/demos/Android.mk @@ -8,7 +8,7 @@ LOCAL_SRC_FILES := intel_sprite_on.c LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # Excessive complaining for established cases. Rely on the Linux version warnings. LOCAL_CFLAGS += -Wno-sign-compare diff --git a/lib/Android.mk b/lib/Android.mk index bd8cf58..548bca4 100644 --- a/lib/Android.mk +++ b/lib/Android.mk @@ -25,7 +25,7 @@ LOCAL_EXPORT_C_INCLUDE_DIRS := $(LOCAL_PATH) LOCAL_CFLAGS += -DHAVE_LIBDRM_ATOMIC_PRIMITIVES LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 LOCAL_MODULE:= libintel_gpu_tools LOCAL_SHARED_LIBRARIES := libpciaccess \ diff --git a/tests/Android.mk b/tests/Android.mk index 519852a..814b846 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -33,7 +33,7 @@ skip_tests_list += pm_rpm # set local compilation flags for IGT tests IGT_LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM -DANDROID -UNDEBUG -IGT_LOCAL_CFLAGS += -include check-ndebug.h -std=c99 +IGT_LOCAL_CFLAGS += -include check-ndebug.h -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit IGT_LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. diff --git a/tools/Android.mk b/tools/Android.mk index 8ca67f4..39f4512 100644 --- a/tools/Android.mk +++ b/tools/Android.mk @@ -12,7 +12,7 @@ define add_tool LOCAL_CFLAGS += -DHAVE_TERMIOS_H LOCAL_CFLAGS += -DHAVE_STRUCT_SYSINFO_TOTALRAM LOCAL_CFLAGS += -DANDROID -UNDEBUG -LOCAL_CFLAGS += -std=c99 +LOCAL_CFLAGS += -std=gnu99 # FIXME: drop once Bionic correctly annotates noreturn on pthread_exit LOCAL_CFLAGS += -Wno-error=return-type # Excessive complaining for established cases. Rely on the Linux version warnings. -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915: Prepare for atomic plane helpers (v7)
On 12/12/2014 01:54 AM, Matt Roper wrote: Add the new driver entrypoints that will be called by the atomic plane helpers. This patch does not actually switch over to the new plane helpers yet, so there should be no functional change here. Also note that although plane programming was already split into check/prepare/commit steps, some of the semantics of those individual functions will need to change slightly when we do make the jump so that they match the behavior the plane helpers expect. v2: - Renamed file from intel_atomic.c to intel_atomic_plane.c (Daniel) - Fix a copy/paste comment mistake (Bob) v3: - Use prepare/cleanup functions that we've already factored out - Use newly refactored pre_commit/commit/post_commit to avoid sleeping during vblank evasion v4: - Rebase to latest di-nightly requires adding an 'old_state' parameter to atomic_update; v5: - Must have botched a rebase somewhere and lost some work. Restore state 'dirty' flag to let begin/end code know which planes to run the pre_commit/post_commit hooks for. This would have actually shown up as broken in the next commit rather than this one. v6: - Squash kerneldoc patch into this one. - Previous patches have now already taken care of most of the infrastructure that used to be in this patch. All we're adding here now is some thin wrappers. v7: - Check return of intel_plane_duplicate_state() for allocation failures. Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- Documentation/DocBook/drm.tmpl| 5 + drivers/gpu/drm/i915/Makefile | 1 + drivers/gpu/drm/i915/intel_atomic_plane.c | 150 ++ drivers/gpu/drm/i915/intel_display.c | 32 ++- drivers/gpu/drm/i915/intel_drv.h | 8 ++ drivers/gpu/drm/i915/intel_sprite.c | 12 +++ 6 files changed, 207 insertions(+), 1 deletion(-) create mode 100644 drivers/gpu/drm/i915/intel_atomic_plane.c diff --git a/Documentation/DocBook/drm.tmpl b/Documentation/DocBook/drm.tmpl index 25c23ca..7c5207b 100644 --- a/Documentation/DocBook/drm.tmpl +++ b/Documentation/DocBook/drm.tmpl @@ -3932,6 +3932,11 @@ int num_ioctls;/synopsis /para /sect2 sect2 +titleAtomic Plane Helpers/title +!Pdrivers/gpu/drm/i915/intel_atomic_plane.c atomic plane helpers +!Idrivers/gpu/drm/i915/intel_atomic_plane.c + /sect2 + sect2 titleOutput Probing/title para This section covers output probing and related infrastructure like the diff --git a/drivers/gpu/drm/i915/Makefile b/drivers/gpu/drm/i915/Makefile index 3cf70a6..4a6ced7 100644 --- a/drivers/gpu/drm/i915/Makefile +++ b/drivers/gpu/drm/i915/Makefile @@ -65,6 +65,7 @@ i915-y += dvo_ch7017.o \ dvo_ns2501.o \ dvo_sil164.o \ dvo_tfp410.o \ + intel_atomic_plane.o \ intel_crt.o \ intel_ddi.o \ intel_dp.o \ diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c new file mode 100644 index 000..286fec8 --- /dev/null +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c @@ -0,0 +1,150 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER + * DEALINGS IN THE SOFTWARE. + */ + +/** + * DOC: atomic plane helper support + * + * The functions here are used by the atomic plane helper functions to + * implement legacy plane updates (i.e., drm_plane-update_plane() and + * drm_plane-disable_plane()). This allows plane updates to use the + * atomic state infrastructure and perform plane updates as separate + * prepare/check/commit/cleanup steps. + */ + +#include drm/drmP.h +#include drm/drm_atomic_helper.h +#include drm/drm_plane_helper.h +#include intel_drv.h + +/** + * intel_plane_duplicate_state - duplicate plane state + * @plane: drm plane + * + * Allocates and returns a copy of the plane state
Re: [Intel-gfx] [PATCH 6/7] drm/i915: Switch plane handling to atomic helpers (v5)
On 12/09/2014 09:53 PM, Matt Roper wrote: Now that we have hooks to enable the atomic plane helpers, we can use the plane helpers for our .update_plane() and .disable_plane() entrypoints. Note that we still need to make a few small behavioral changes to the driver entrypoints here as we make the transition, due to differences between how the atomic helpers set things up for us vs how our internal functions were setting things up. v2: Fix commit message typo (Bob) v3: Rebased on top of Gustavo Padovan's patches to kill off intel_crtc_cursor_set_obj() and intel_pipe_set_base(). v4: Rebase again on latest display refactor series. Now that previous patches have smoothed the rough edges of the existing display code, the transition here is much less painful. v5: Plane helpers will re-prepare the same fb for a plane. Drop the WARN on this condition. I think it would make sense to squash this patch with the previous one. That way it becomes clearer where the intel_update_plane() goes. Also, the atomic.track_fb crtc field is added in the previous patch but only used here. I would also split the change from state.orig_{dst,src} into a separate patch, but I guess that's not really necessary. Cheers, Ander Testcase: igt/kms_plane Testcase: igt/kms_universal_plane Testcase: igt/kms_cursor_crc Signed-off-by: Matt Roper matthew.d.ro...@intel.com --- drivers/gpu/drm/i915/intel_display.c | 222 --- drivers/gpu/drm/i915/intel_drv.h | 2 - drivers/gpu/drm/i915/intel_sprite.c | 38 +++--- 3 files changed, 98 insertions(+), 164 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index ea47923..2f80236 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -11678,7 +11678,7 @@ intel_prepare_plane_fb(struct drm_plane *plane, unsigned frontbuffer_bits = 0; int ret = 0; - if (WARN_ON(fb == plane-fb || !obj)) + if (!obj) return 0; switch (plane-type) { @@ -11745,7 +11745,7 @@ intel_check_primary_plane(struct drm_plane *plane, struct drm_device *dev = plane-dev; struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc = state-base.crtc; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *intel_crtc; struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_framebuffer *fb = state-base.fb; struct drm_rect *dest = state-dst; @@ -11753,6 +11753,9 @@ intel_check_primary_plane(struct drm_plane *plane, const struct drm_rect *clip = state-clip; int ret; + crtc = crtc ? crtc : plane-crtc; + intel_crtc = to_intel_crtc(crtc); + ret = drm_plane_helper_check_update(plane, crtc, fb, src, dest, clip, DRM_PLANE_HELPER_NO_SCALING, @@ -11812,23 +11815,26 @@ intel_commit_primary_plane(struct drm_plane *plane, struct drm_framebuffer *fb = state-base.fb; struct drm_device *dev = plane-dev; struct drm_i915_private *dev_priv = dev-dev_private; - struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_crtc *intel_crtc; struct drm_i915_gem_object *obj = intel_fb_obj(fb); struct intel_plane *intel_plane = to_intel_plane(plane); struct drm_rect *src = state-src; + crtc = crtc ? crtc : plane-crtc; + intel_crtc = to_intel_crtc(crtc); + plane-fb = fb; crtc-x = src-x1 16; crtc-y = src-y1 16; - intel_plane-crtc_x = state-orig_dst.x1; - intel_plane-crtc_y = state-orig_dst.y1; - intel_plane-crtc_w = drm_rect_width(state-orig_dst); - intel_plane-crtc_h = drm_rect_height(state-orig_dst); - intel_plane-src_x = state-orig_src.x1; - intel_plane-src_y = state-orig_src.y1; - intel_plane-src_w = drm_rect_width(state-orig_src); - intel_plane-src_h = drm_rect_height(state-orig_src); + intel_plane-crtc_x = state-base.crtc_x; + intel_plane-crtc_y = state-base.crtc_y; + intel_plane-crtc_w = state-base.crtc_w; + intel_plane-crtc_h = state-base.crtc_h; + intel_plane-src_x = state-base.src_x; + intel_plane-src_y = state-base.src_y; + intel_plane-src_w = state-base.src_w; + intel_plane-src_h = state-base.src_h; intel_plane-obj = obj; if (intel_crtc-active) { @@ -11857,6 +11863,32 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc) { struct drm_device *dev = crtc-dev; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); + struct intel_plane *intel_plane; + struct drm_plane *p; + unsigned fb_bits = 0; + + /* Track fb's for any planes being disabled */ + list_for_each_entry(p, dev-mode_config.plane_list, head) { + intel_plane =
Re: [Intel-gfx] [PATCH v3 2/8] drm/i915: Adds graphic address space ballooning logic
On 11/13/2014 12:02 PM, Yu Zhang wrote: With Intel GVT-g, the global graphic memory space is partitioned by multiple vGPU instances in different VMs. The ballooning code is called in i915_gem_setup_global_gtt(), utilizing the drm mm allocator APIs to mark the graphic address space which are partitioned out to other vGPUs as reserved. v2: take Chris and Daniel's comments: - no guard page between different VMs - use drm_mm_reserve_node() to do the reservation for ballooning, instead of the previous drm_mm_insert_node_in_range_generic() v3: take Daniel's comments: - move ballooning functions into i915_vgpu.c - add kerneldoc to ballooning functions Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com Signed-off-by: Zhi Wang zhi.a.w...@intel.com Signed-off-by: Eddie Dong eddie.d...@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 17 +++- drivers/gpu/drm/i915/i915_vgpu.c| 149 drivers/gpu/drm/i915/i915_vgpu.h| 2 + 3 files changed, 165 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index de12017..2dfac13 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -27,6 +27,7 @@ #include drm/drmP.h #include drm/i915_drm.h #include i915_drv.h +#include i915_vgpu.h #include i915_trace.h #include intel_drv.h @@ -1683,6 +1684,16 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, /* Subtract the guard page ... */ drm_mm_init(ggtt_vm-mm, start, end - start - PAGE_SIZE); + + dev_priv-gtt.base.start = start; + dev_priv-gtt.base.total = end - start; + + if (intel_vgpu_active(dev)) { + ret = intel_vgt_balloon(dev); + if (ret) + return ret; + } + Out of curiosity, what will be the mechanism to prevent a vGPU instance from ignoring the ballooning data? Must be something in the hypervisor blocking pass-through access to such domains? And probably GPU reset should also be disallowed for vGPU instances? if (!HAS_LLC(dev)) dev_priv-gtt.base.mm.color_adjust = i915_gtt_color_adjust; @@ -1702,9 +1713,6 @@ int i915_gem_setup_global_gtt(struct drm_device *dev, vma-bound |= GLOBAL_BIND; } - dev_priv-gtt.base.start = start; - dev_priv-gtt.base.total = end - start; - /* Clear any non-preallocated blocks */ drm_mm_for_each_hole(entry, ggtt_vm-mm, hole_start, hole_end) { DRM_DEBUG_KMS(clearing unused GTT space: [%lx, %lx]\n, @@ -1756,6 +1764,9 @@ void i915_global_gtt_cleanup(struct drm_device *dev) } if (drm_mm_initialized(vm-mm)) { + if (intel_vgpu_active(dev)) + intel_vgt_deballoon(); + drm_mm_takedown(vm-mm); list_del(vm-global_link); } diff --git a/drivers/gpu/drm/i915/i915_vgpu.c b/drivers/gpu/drm/i915/i915_vgpu.c index 3f6b797..ff5fba3 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.c +++ b/drivers/gpu/drm/i915/i915_vgpu.c @@ -83,3 +83,152 @@ void i915_check_vgpu(struct drm_device *dev) dev_priv-vgpu.active = true; DRM_INFO(Virtual GPU for Intel GVT-g detected.\n); } + +struct _balloon_info_ { + /* +* There are up to 2 regions per low/high graphic memory that +* might be ballooned. Here, index 0/1 is for low +* graphic memory, 2/3 for high graphic memory. +*/ + struct drm_mm_node space[4]; +} bl_info; This should be static I think. +/** + * intel_vgt_deballoon - deballoon reserved graphics address trunks + * + * This function is called to deallocate the ballooned-out graphic memory, when + * driver is unloaded or when ballooning fails. + */ +void intel_vgt_deballoon(void) +{ + int i; + + DRM_INFO(VGT deballoon.\n); Would debug be more appropriate? I don't see much value of saying this on driver unload - it's not that it is optional at this point. Also for all logging, is intended human readable name VGT or vGT? If the latter it would be nicer to log it in that form. + + for (i = 0; i 4; i++) { + if (bl_info.space[i].allocated) + drm_mm_remove_node(bl_info.space[i]); + } + + memset(bl_info, 0, sizeof(bl_info)); +} + +static int vgt_balloon_space(struct drm_mm *mm, +struct drm_mm_node *node, +unsigned long start, unsigned long end) +{ + unsigned long size = end - start; + + if (start == end) + return -EINVAL; + + DRM_INFO(balloon space: range [ 0x%lx - 0x%lx ] %lu KB.\n, +start, end, size / 1024); KiB ? + node-start = start; + node-size = size; + + return drm_mm_reserve_node(mm, node); +} + +/** + * intel_vgt_balloon - balloon out
Re: [Intel-gfx] [PATCH v3 3/8] drm/i915: Partition the fence registers for vGPU in i915 driver
On 11/13/2014 12:02 PM, Yu Zhang wrote: With Intel GVT-g, the fence registers are partitioned by multiple vGPU instances in different VMs. Routine i915_gem_load() is modified to reset the num_fence_regs, when the driver detects it's running in a VM. And the allocated fence number is provided in PV INFO page structure. Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com Signed-off-by: Eddie Dong eddie.d...@intel.com --- drivers/gpu/drm/i915/i915_gem.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 1de94cc..0c8b32e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -29,6 +29,7 @@ #include drm/drm_vma_manager.h #include drm/i915_drm.h #include i915_drv.h +#include i915_vgpu.h #include i915_trace.h #include intel_drv.h #include linux/oom.h @@ -5014,6 +5015,10 @@ i915_gem_load(struct drm_device *dev) else dev_priv-num_fence_regs = 8; + if (intel_vgpu_active(dev)) + dev_priv-num_fence_regs = + I915_READ(vgtif_reg(avail_rs.fence_num)); + /* Initialize fence registers to zero */ INIT_LIST_HEAD(dev_priv-mm.fence_list); i915_gem_restore_fences(dev); You don't need a start offset and number of allocated fences per domain? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 4/8] drm/i915: Disable framebuffer compression for i915 driver in VM
On 11/13/2014 12:02 PM, Yu Zhang wrote: Framebuffer compression is disabled when driver detects it's running in a Intel GVT-g enlightened VM, because FBC is not emulated and there is no stolen memory for a vGPU. v2: take Chris' comments: - move the code into intel_update_fbc() Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com Signed-off-by: Zhiyuan Lv zhiyuan...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 7a69eba..3bc5d93 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -544,6 +544,10 @@ void intel_update_fbc(struct drm_device *dev) return; } + /* disable framebuffer compression in vGPU */ + if (intel_vgpu_active(dev)) + i915.enable_fbc = 0; + /* * If FBC is already on, we just have to verify that we can * keep it that way... Looks like you'll need to rebase this one, I see no intel_update_fbc in my tree. :( Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 5/8] drm/i915: Add the display switch logic for vGPU in i915 driver
On 11/13/2014 12:02 PM, Yu Zhang wrote: Display switch logic is added to notify the host side that current vGPU have a valid surface to show. It does so by writing the display_ready field in PV INFO page, and then will be handled in the host side. This is useful to avoid trickiness when the VM's framebuffer is being accessed in the middle of VM modesetting, e.g. compositing the framebuffer in the host side. v2: - move the notification code outside the 'else' in load sequence - remove the notification code in intel_crtc_set_config() Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com Signed-off-by: Zhiyuan Lv zhiyuan...@intel.com --- drivers/gpu/drm/i915/i915_dma.c | 11 +++ drivers/gpu/drm/i915/i915_vgpu.h | 7 +++ 2 files changed, 18 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 9a73533..06daa5c 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -36,6 +36,7 @@ #include intel_drv.h #include drm/i915_drm.h #include i915_drv.h +#include i915_vgpu.h #include i915_trace.h #include linux/pci.h #include linux/console.h @@ -1780,6 +1781,16 @@ int i915_driver_load(struct drm_device *dev, unsigned long flags) dev_priv-ums.mm_suspended = 1; } + if (intel_vgpu_active(dev)) { + /* +* Notify a valid surface after modesetting, +* when running inside a VM. +*/ + struct drm_i915_private *dev_priv = to_i915(dev); + + I915_WRITE(vgtif_reg(display_ready), VGT_DRV_DISPLAY_READY); + } + Current code base already has dev_priv so looks like another rebase. Otherwise pretty simple so: Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com i915_setup_sysfs(dev); if (INTEL_INFO(dev)-num_pipes) { diff --git a/drivers/gpu/drm/i915/i915_vgpu.h b/drivers/gpu/drm/i915/i915_vgpu.h index f538b18..9d35fb8 100644 --- a/drivers/gpu/drm/i915/i915_vgpu.h +++ b/drivers/gpu/drm/i915/i915_vgpu.h @@ -80,6 +80,13 @@ struct vgt_if { #define vgtif_reg(x) \ (VGT_PVINFO_PAGE + (long)((struct vgt_if *) NULL)-x) +/* vGPU display status to be used by the host side */ +enum vgt_display_status { + VGT_DRV_DISPLAY_NOT_READY = 0, + VGT_DRV_DISPLAY_READY, /* ready for display switch */ +}; + + extern void i915_check_vgpu(struct drm_device *dev); extern int intel_vgt_balloon(struct drm_device *dev); extern void intel_vgt_deballoon(void); ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 6/8] drm/i915: Disable power management for i915 driver in VM
On 11/13/2014 12:02 PM, Yu Zhang wrote: With Intel GVT-g, GPU power management is controlled by host driver, so there is no need to provide virtualized GPU PM support. In the future it might be useful to gather VM input for freq boost, but now let's disable it simply. v2: take Chris' comments: - do not special case this to gen6+ Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com --- drivers/gpu/drm/i915/intel_pm.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 3bc5d93..3722bd4 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -5314,6 +5314,10 @@ void intel_enable_gt_powersave(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; + /* Powersaving is controlled by the host when inside a VM */ + if (intel_vgpu_active(dev)) + return; + if (IS_IRONLAKE_M(dev)) { mutex_lock(dev-struct_mutex); ironlake_enable_drps(dev); I was looking for potential call sites of this which come earlier than i915_check_vgpu. Didn't find any but found RPS (intel_pm_setup) - what's with that - should it be disabled as well? Otherwise, Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] lib: Fix out of tree build of version.h
On 11 December 2014 at 13:11, Joonas Lahtinen joonas.lahti...@linux.intel.com wrote: Currently out of tree build fails because the version.h.tmp is generated into the source directory instead of the build directory where it is later looked for. This commit fixes it. From 2c0617e21101d69e7219c6660936c0015f93f8ee Mon Sep 17 00:00:00 2001 From: Joonas Lahtinen joonas.lahti...@linux.intel.com Date: Thu, 11 Dec 2014 15:05:11 +0200 Subject: [PATCH] lib: Fix out of tree build of version.h Write the version.h.tmp file into the build directory instead of source directory. This allows out of tree building when those two are not the same. Signed-off-by: Joonas Lahtinen joonas.lahti...@linux.intel.com Patch merged, thanks. --- lib/Makefile.sources | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/Makefile.sources b/lib/Makefile.sources index 819b21a..34a3d31 100644 --- a/lib/Makefile.sources +++ b/lib/Makefile.sources @@ -55,16 +55,16 @@ libintel_tools_la_SOURCES = \ $(IGT_LIB_PATH)/version.h.tmp: @touch $@ @if test -d $(GPU_TOOLS_PATH)/.git; then \ - if which git /dev/null 21; then cd $(@D); \ + if which git /dev/null 21; then \ + cd $(GPU_TOOLS_PATH); \ git log -n 1 --oneline | \ - sed 's/^\([^ ]*\) .*/#define IGT_GIT_SHA1 g\1/' \ -$(@F) ; \ + sed 's/^\([^ ]*\) .*/#define IGT_GIT_SHA1 g\1/' ; \ else \ - echo '#define IGT_GIT_SHA1 NO-GIT' $@ ; \ + echo '#define IGT_GIT_SHA1 NO-GIT' ; \ fi \ else \ - echo '#define IGT_GIT_SHA1 NOT-GIT' $@ ; \ - fi + echo '#define IGT_GIT_SHA1 NOT-GIT' ; \ + fi $@ $(IGT_LIB_PATH)/version.h: $(IGT_LIB_PATH)/version.h.tmp -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 7/8] drm/i915: Create vGPU specific write MMIO to reduce traps
On 11/13/2014 12:02 PM, Yu Zhang wrote: In the virtualized environment, forcewake operations are not necessory for the driver, because mmio accesses will be trapped necessary and emulated by the host side, and real forcewake operations are also done in the host. New mmio write handlers are added to directly call the __raw_i915_write, therefore will reduce many traps and increase the overall performance for drivers running in the VM with Intel GVT-g enhancement. v2: take Chris' comments: - register the mmio hooks in intel_uncore_init() v3: take Daniel's comments: - use macros to assign mmio write functions for vGPU Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com Signed-off-by: Kevin Tian kevin.t...@intel.com --- drivers/gpu/drm/i915/intel_uncore.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index cae27bb..b76c21d 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -727,6 +727,14 @@ hsw_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) REG_WRITE_FOOTER; \ } +#define __vgpu_write(x) \ +static void \ +vgpu_write##x(struct drm_i915_private *dev_priv, off_t reg, u##x val, bool trace) { \ + REG_WRITE_HEADER; \ + __raw_i915_write##x(dev_priv, reg, val); \ + REG_WRITE_FOOTER; \ +} + static const u32 gen8_shadowed_regs[] = { FORCEWAKE_MT, GEN6_RPNSWREQ, @@ -821,6 +829,10 @@ __gen4_write(8) __gen4_write(16) __gen4_write(32) __gen4_write(64) +__vgpu_write(8) +__vgpu_write(16) +__vgpu_write(32) +__vgpu_write(64) #undef __chv_write #undef __gen8_write @@ -828,6 +840,7 @@ __gen4_write(64) #undef __gen6_write #undef __gen5_write #undef __gen4_write +#undef __vgpu_write #undef REG_WRITE_FOOTER #undef REG_WRITE_HEADER @@ -939,6 +952,9 @@ void intel_uncore_init(struct drm_device *dev) break; } + if (intel_vgpu_active(dev)) + ASSIGN_WRITE_MMIO_VFUNCS(vgpu); + i915_check_and_clear_faults(dev); } #undef ASSIGN_WRITE_MMIO_VFUNCS Maybe I am missing something obvious, but why are read variants not needed? Regards, Tvrtko ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v3 8/8] drm/i915: Support alias ppgtt in VM if ppgtt is enabled
On 11/13/2014 12:02 PM, Yu Zhang wrote: The current Intel GVT-g only supports alias ppgtt. And the emulation is done in the host by first trapping PP_DIR_BASE mmio accesses. Updating PP_DIR_BASE by using instructions such as MI_LOAD_REGISTER_IMM are hard to detect and are not supported in current code. Therefore this patch also added a new callback routine - vgpu_mm_switch() to set the PP_DIR_BASE by mmio writes. v2: take Chris' comments: - move the code into sanitize_enable_ppgtt() Signed-off-by: Yu Zhang yu.c.zh...@linux.intel.com Signed-off-by: Jike Song jike.s...@intel.com --- drivers/gpu/drm/i915/i915_gem_gtt.c | 16 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c index 2dfac13..55307eb 100644 --- a/drivers/gpu/drm/i915/i915_gem_gtt.c +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c @@ -44,6 +44,9 @@ static int sanitize_enable_ppgtt(struct drm_device *dev, int enable_ppgtt) if (IS_GEN8(dev)) has_full_ppgtt = false; /* XXX why? */ + if (intel_vgpu_active(dev)) + has_full_ppgtt = false; /* emulation is too hard */ + if (enable_ppgtt == 0 || !has_aliasing_ppgtt) return 0; @@ -733,6 +736,16 @@ static int hsw_mm_switch(struct i915_hw_ppgtt *ppgtt, return 0; } +static int vgpu_mm_switch(struct i915_hw_ppgtt *ppgtt, +struct intel_engine_cs *ring) +{ + struct drm_i915_private *dev_priv = to_i915(ppgtt-base.dev); + + I915_WRITE(RING_PP_DIR_DCLV(ring), PP_DIR_DCLV_2G); + I915_WRITE(RING_PP_DIR_BASE(ring), get_pd_offset(ppgtt)); + return 0; +} + static int gen7_mm_switch(struct i915_hw_ppgtt *ppgtt, struct intel_engine_cs *ring) { @@ -1059,6 +1072,9 @@ static int gen6_ppgtt_init(struct i915_hw_ppgtt *ppgtt) } else BUG(); + if (intel_vgpu_active(dev)) + ppgtt-switch_mm = vgpu_mm_switch; + ret = gen6_ppgtt_alloc(ppgtt); if (ret) return ret; Pending potential checkpatch.pl --strict fixes, Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/2] drm/i915/skl: Skylake also supports DP MST
I've checked that TRANS_DDI_MODE, DP_TP_CTL MST bits are identical to HSW/BDW on SKL, as well as the long vs short HPD bits. So we have a good chance to be working as well as prevous platforms. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3fc3296..8e276c4 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -5085,7 +5085,7 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port, intel_dp_aux_init(intel_dp, intel_connector); /* init MST on ports that can support it */ - if (IS_HASWELL(dev) || IS_BROADWELL(dev)) { + if (IS_HASWELL(dev) || IS_BROADWELL(dev) || INTEL_INFO(dev)-gen = 9) { if (port == PORT_B || port == PORT_C || port == PORT_D) { intel_dp_mst_encoder_init(intel_dig_port, intel_connector-base.base.id); -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/2] drm/i915: Consolidate DDI clock reading out in a single function
2 pieces of code need to read out the DDI clock: the DDI encoder and the MST encoder .get_config() vfuncs. Until now the SKL read out code was only in the former, so let's move the pre and post SKL logic in intel_ddi_clock_get() and this this one everywhere. Signed-off-by: Damien Lespiau damien.lesp...@intel.com --- drivers/gpu/drm/i915/intel_ddi.c | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c index 4e2e860..1c92ad4 100644 --- a/drivers/gpu/drm/i915/intel_ddi.c +++ b/drivers/gpu/drm/i915/intel_ddi.c @@ -834,7 +834,12 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder, void intel_ddi_clock_get(struct intel_encoder *encoder, struct intel_crtc_config *pipe_config) { - hsw_ddi_clock_get(encoder, pipe_config); + struct drm_device *dev = encoder-base.dev; + + if (INTEL_INFO(dev)-gen = 8) + hsw_ddi_clock_get(encoder, pipe_config); + else + skl_ddi_clock_get(encoder, pipe_config); } static void @@ -2029,7 +2034,6 @@ void intel_ddi_get_config(struct intel_encoder *encoder, enum transcoder cpu_transcoder = intel_crtc-config.cpu_transcoder; struct intel_hdmi *intel_hdmi; u32 temp, flags = 0; - struct drm_device *dev = dev_priv-dev; temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder)); if (temp TRANS_DDI_PHSYNC) @@ -2106,10 +2110,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder, dev_priv-vbt.edp_bpp = pipe_config-pipe_bpp; } - if (INTEL_INFO(dev)-gen = 8) - hsw_ddi_clock_get(encoder, pipe_config); - else - skl_ddi_clock_get(encoder, pipe_config); + intel_ddi_clock_get(encoder, pipe_config); } static void intel_ddi_destroy(struct drm_encoder *encoder) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/2] Enable DP MST encoders for SKL
The programming seems fairly similar to previous platforms, so let's create the DP encoders on SKL as well. -- Damien Damien Lespiau (2): drm/i915: Consolidate DDI clock reading out in a single function drm/i915/skl: Skylake also supports DP MST drivers/gpu/drm/i915/intel_ddi.c | 13 +++-- drivers/gpu/drm/i915/intel_dp.c | 2 +- 2 files changed, 8 insertions(+), 7 deletions(-) -- 1.8.3.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH i-g-t] Demos/Android.mk: dont build intel_sprite_on
From: Tim Gore tim.g...@intel.com intel_sprite_on wont build on Android. Previous attempt to disable was just wrong! Signed-off-by: Tim Gore tim.g...@intel.com --- demos/Android.mk | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/demos/Android.mk b/demos/Android.mk index b20d61e..5a00116 100644 --- a/demos/Android.mk +++ b/demos/Android.mk @@ -4,8 +4,7 @@ LOCAL_PATH := $(call my-dir) include $(CLEAR_VARS) -# This demo wont build on android -#LOCAL_SRC_FILES := intel_sprite_on.c +LOCAL_SRC_FILES := intel_sprite_on.c LOCAL_CFLAGS += -DHAVE_TERMIOS_H @@ -24,6 +23,7 @@ LOCAL_STATIC_LIBRARIES := libintel_gpu_tools LOCAL_SHARED_LIBRARIES := libdrm -include $(BUILD_EXECUTABLE) +# This demo (intel_sprite_on.c) wont build on android +#include $(BUILD_EXECUTABLE) ## -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH] drm/i915/skl: Correctly updating sprite wm parameter
On 12/09/2014 05:29 AM, sonika.jin...@intel.com wrote: From: Sonika Jindal sonika.jin...@intel.com The pipe wm parameters is not correctly updated with sprite parameters because it copies them for each plane from plane_list to the sprite offset in pipe wm parameters. Since plane_list also contains primary and cursor planes, we end up updating wrong params for sprites. Signed-off-by: Sonika Jindal sonika.jin...@intel.com --- drivers/gpu/drm/i915/intel_pm.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8d182a6..4a16f9b 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -3301,7 +3301,8 @@ static void skl_compute_wm_pipe_parameters(struct drm_crtc *crtc, list_for_each_entry(plane, dev-mode_config.plane_list, head) { struct intel_plane *intel_plane = to_intel_plane(plane); - if (intel_plane-pipe == pipe) + if (intel_plane-pipe == pipe + plane-type == DRM_PLANE_TYPE_OVERLAY) p-plane[i++] = intel_plane-wm; } } Reviewed-by: Tvrtko Ursulin tvrtko.ursu...@intel.com ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 2/2] drm/i915: sanitize RPS resetting during GPU reset
2014-12-04 12:07 GMT-02:00 Imre Deak imre.d...@intel.com: On Thu, 2014-12-04 at 14:58 +0100, Daniel Vetter wrote: On Thu, Dec 04, 2014 at 02:59:32PM +0200, Imre Deak wrote: Atm, we don't disable RPS interrupts and related work items before resetting the GPU. This may interfere with the following GPU initialization and cause RPS interrupts to show up in PM_IIR too early before calling gen6_enable_rps_interrupts() (triggering a WARN there). Solve this by disabling RPS interrupts and flushing any related work items before resetting the GPU. v2: - split out the common parts of the gt suspend and the new gt reset functions (Paulo) Reported-by: He, Shuang shuang...@intel.com Testcase: igt/gem_reset_stats/ban-render Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=86644 Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/i915_drv.c | 5 - drivers/gpu/drm/i915/intel_pm.c | 28 +++- 2 files changed, 23 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 71be3c9..8377249 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -810,6 +810,9 @@ int i915_reset(struct drm_device *dev) if (!i915.reset) return 0; + if (drm_core_check_feature(dev, DRIVER_MODESET)) + intel_reset_gt_powersave(dev); UMS support is dead, so you can leave this hunk out. Ok, we can call here intel_reset_gt_powersave() unconditionally. Well, it still looks correct, so for both patches: Reviewed-by: Paulo Zanoni paulo.r.zan...@intel.com -Daniel + mutex_lock(dev-struct_mutex); i915_gem_reset(dev); @@ -881,7 +884,7 @@ int i915_reset(struct drm_device *dev) * of re-init after reset. */ if (INTEL_INFO(dev)-gen 5) - intel_reset_gt_powersave(dev); + intel_enable_gt_powersave(dev); } else { mutex_unlock(dev-struct_mutex); } diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 78911e2..45c786f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -6072,6 +6072,20 @@ void intel_cleanup_gt_powersave(struct drm_device *dev) valleyview_cleanup_gt_powersave(dev); } +static void gen6_suspend_rps(struct drm_device *dev) +{ + struct drm_i915_private *dev_priv = dev-dev_private; + + flush_delayed_work(dev_priv-rps.delayed_resume_work); + + /* +* TODO: disable RPS interrupts on GEN9+ too once RPS support +* is added for it. +*/ + if (INTEL_INFO(dev)-gen 9) + gen6_disable_rps_interrupts(dev); +} + /** * intel_suspend_gt_powersave - suspend PM work and helper threads * @dev: drm device @@ -6087,14 +6101,7 @@ void intel_suspend_gt_powersave(struct drm_device *dev) if (INTEL_INFO(dev)-gen 6) return; - flush_delayed_work(dev_priv-rps.delayed_resume_work); - - /* -* TODO: disable RPS interrupts on GEN9+ too once RPS support -* is added for it. -*/ - if (INTEL_INFO(dev)-gen 9) - gen6_disable_rps_interrupts(dev); + gen6_suspend_rps(dev); /* Force GPU to min freq during suspend */ gen6_rps_idle(dev_priv); @@ -6197,8 +6204,11 @@ void intel_reset_gt_powersave(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; + if (INTEL_INFO(dev)-gen 6) + return; + + gen6_suspend_rps(dev); dev_priv-rps.enabled = false; - intel_enable_gt_powersave(dev); } static void ibx_init_clock_gating(struct drm_device *dev) -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] drm/i915: fix use after free during eDP encoder destroying
After commit a18c0af171bfb875012da26f23df051004726973 uthor: Thierry Reding tred...@nvidia.com Date: Wed Dec 10 11:38:49 2014 +0100 drm: Zero out DRM object memory upon cleanup we will use the eDP encoder during destroying it. Fix this by calling drm_encoder_cleanup() at a point when the encoder is not used any more. This caused a NULL pointer dereference in pps_lock(), I can't see that it caused any other problem. All the other encoders seem to call drm_encoder_cleanup() at a safe place. Signed-off-by: Imre Deak imre.d...@intel.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3fc3296..0a55623 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4310,7 +4310,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) drm_dp_aux_unregister(intel_dp-aux); intel_dp_mst_encoder_cleanup(intel_dig_port); - drm_encoder_cleanup(encoder); if (is_edp(intel_dp)) { cancel_delayed_work_sync(intel_dp-panel_vdd_work); /* @@ -4326,6 +4325,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) intel_dp-edp_notifier.notifier_call = NULL; } } + drm_encoder_cleanup(encoder); kfree(intel_dig_port); } -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 0/3] Three small patches to ringbuffer calculations
[PATCH 1/3] drm/i915: use effective_size for ringbuffer calculations When calculating the available space in a ringbuffer, we should use the effective_size rather than the true size of the ring. [PATCH 2/3] drm/i915: recheck ringspace after wait+retire When a ringbuffer is nearly full, we often wait for some request to complete and so free up some ringspace. This code checks that we actually got at least as much space as we expected and need. [PATCH 3/3] drm/i915: Track check calls to intel(_logical)_ring_{begin,advance} Check that these functions are used in a logically consistent manner. I've dropped the patch to allow nested begin/advance pairs; instead we will create a pre-allocate/reserve call as part of the scheduler or preemption work. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 2/3] drm/i915: recheck ringspace after wait+retire
In {intel,logical}_ring_wait_request(), we try to find a request whose completion will release the amount of ring space required. If we find such a request, we wait for it, and then retire it, in the expectation that we will now have at least the required amount of free space in the ring. But it's good to check that this has actually happened, so we can back out with a meaningful error code if something unexpected has happened, such as wait_request returning early. This code was already in the execlist version, so the change to intel_lrc.c is just to add a comment; but we want the same check in the legacy ringbuffer mode too. Signed-off-by: Dave Gordon david.s.gor...@intel.com --- drivers/gpu/drm/i915/intel_lrc.c|9 + drivers/gpu/drm/i915/intel_ringbuffer.c | 12 +++- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 8545dbd..69b042f 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -956,6 +956,15 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, i915_gem_retire_requests_ring(ring); + /* +* According to our calculation above, retiring the request we just +* waited for should have resulted in there being enough space in +* the ringbuffer; but let's check. +* +* If there is not now enough space, something has gone horribly worng +* (such as wait_request returning early, but with no error, or +* retire_requests failing to retire the request we expected it to). +*/ return intel_ring_space(ringbuf) = bytes ? 0 : -ENOSPC; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 9def83c..660d10d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1912,6 +1912,7 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) return 0; list_for_each_entry(request, ring-request_list, list) { + /* Would completion of this request free enough space? */ if (__intel_ring_space(request-tail, ringbuf-tail, ringbuf-effective_size) = n) { break; @@ -1927,7 +1928,16 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) i915_gem_retire_requests_ring(ring); - return 0; + /* +* According to our calculation above, retiring the request we just +* waited for should have resulted in there being enough space in +* the ringbuffer; but let's check. +* +* If there is not now enough space, something has gone horribly worng +* (such as wait_request returning early, but with no error, or +* retire_requests failing to retire the request we expected it to). +*/ + return intel_ring_space(ringbuf) = n ? 0 : -ENOSPC; } static int ring_wait_for_space(struct intel_engine_cs *ring, int n) -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 1/3] drm/i915: use effective_size for ringbuffer calculations
When calculating the available space in a ringbuffer, we should use the effective_size rather than the true size of the ring. Signed-off-by: Dave Gordon david.s.gor...@intel.com --- drivers/gpu/drm/i915/intel_lrc.c|2 +- drivers/gpu/drm/i915/intel_ringbuffer.c |5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index a82020e..8545dbd 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -942,7 +942,7 @@ static int logical_ring_wait_request(struct intel_ringbuffer *ringbuf, /* Would completion of this request free enough space? */ if (__intel_ring_space(request-tail, ringbuf-tail, - ringbuf-size) = bytes) { + ringbuf-effective_size) = bytes) { break; } } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 83accb7..9def83c 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -66,7 +66,8 @@ void intel_ring_update_space(struct intel_ringbuffer *ringbuf) } ringbuf-space = __intel_ring_space(ringbuf-head HEAD_ADDR, - ringbuf-tail, ringbuf-size); + ringbuf-tail, + ringbuf-effective_size); } int intel_ring_space(struct intel_ringbuffer *ringbuf) @@ -1912,7 +1913,7 @@ static int intel_ring_wait_request(struct intel_engine_cs *ring, int n) list_for_each_entry(request, ring-request_list, list) { if (__intel_ring_space(request-tail, ringbuf-tail, - ringbuf-size) = n) { + ringbuf-effective_size) = n) { break; } } -- 1.7.9.5 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 3/3] drm/i915: Track check calls to intel(_logical)_ring_{begin, advance}
When adding instructions to a legacy or LRC ringbuffer, the sequence of emit() calls must be preceded by a call to intel(_logical)_ring_begin() to reserve the required amount of space, and followed by a matching call to intel(_logical)_ring_advance() (note that this used to trigger immediate submission to the h/w, but now actual submission is deferred until all the instructions for a single batch submission have been assembled). Historically some (display) code didn't use begin/advance, but just inserted instructions ad hoc, which would then be sent to the hardware along with the current or next batch, but this is not supported and is now regarded as incorrect. This commit therefore adds begin/advance tracking, with WARNings where various forms of misuse are detected. These include: * advance without begin * begin without advance before submission to h/w * multiple begins without an advance between * exceeding the space reserved by begin * leaving the ring misaligned * ring buffer overrun (negative freespace) Signed-off-by: Dave Gordon david.s.gor...@intel.com --- drivers/gpu/drm/i915/intel_lrc.c|4 ++- drivers/gpu/drm/i915/intel_lrc.h| 11 ++- drivers/gpu/drm/i915/intel_ringbuffer.c |9 -- drivers/gpu/drm/i915/intel_ringbuffer.h | 54 ++- 4 files changed, 73 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_lrc.c b/drivers/gpu/drm/i915/intel_lrc.c index 69b042f..422f3b9 100644 --- a/drivers/gpu/drm/i915/intel_lrc.c +++ b/drivers/gpu/drm/i915/intel_lrc.c @@ -825,6 +825,7 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf) struct intel_context *ctx = ringbuf-FIXME_lrc_ctx; intel_logical_ring_advance(ringbuf); + WARN_ON(ringbuf-rsv_level != 0); if (intel_ring_stopped(ring)) return; @@ -1093,7 +1094,8 @@ int intel_logical_ring_begin(struct intel_ringbuffer *ringbuf, int num_dwords) if (ret) return ret; - ringbuf-space -= num_dwords * sizeof(uint32_t); + __intel_ringbuffer_begin(ringbuf, num_dwords); + return 0; } diff --git a/drivers/gpu/drm/i915/intel_lrc.h b/drivers/gpu/drm/i915/intel_lrc.h index 14b216b..9a0457e 100644 --- a/drivers/gpu/drm/i915/intel_lrc.h +++ b/drivers/gpu/drm/i915/intel_lrc.h @@ -48,8 +48,17 @@ void intel_logical_ring_advance_and_submit(struct intel_ringbuffer *ringbuf); */ static inline void intel_logical_ring_advance(struct intel_ringbuffer *ringbuf) { - ringbuf-tail = ringbuf-size - 1; + __intel_ringbuffer_check(ringbuf); + + /* +* Tail == effecive_size is legitimate (buffer exactly full). +* Tail effective_size is not, and should give a warning, +* but we'll reset tail in both cases to prevent further chaos +*/ + if (ringbuf-tail = ringbuf-effective_size) + ringbuf-tail -= ringbuf-effective_size; } + /** * intel_logical_ring_emit() - write a DWORD to the ringbuffer. * @ringbuf: Ringbuffer to write to. diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 660d10d..7ffad3a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -55,6 +55,7 @@ int __intel_ring_space(int head, int tail, int size) int space = head - tail; if (space = 0) space += size; + WARN_ON(space I915_RING_FREE_SPACE); return space - I915_RING_FREE_SPACE; } @@ -85,7 +86,10 @@ bool intel_ring_stopped(struct intel_engine_cs *ring) void __intel_ring_advance(struct intel_engine_cs *ring) { struct intel_ringbuffer *ringbuf = ring-buffer; - ringbuf-tail = ringbuf-size - 1; + + intel_ring_advance(ring); + WARN_ON(ringbuf-rsv_level != 0); + if (intel_ring_stopped(ring)) return; ring-write_tail(ring, ringbuf-tail); @@ -2107,7 +2111,8 @@ int intel_ring_begin(struct intel_engine_cs *ring, if (ret) return ret; - ring-buffer-space -= num_dwords * sizeof(uint32_t); + __intel_ringbuffer_begin(ring-buffer, num_dwords); + return 0; } diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 6dbb6f4..a6660c1 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -112,6 +112,11 @@ struct intel_ringbuffer { int size; int effective_size; + /* these let advance() check for misuse */ + int rsv_level; /* reservation nesting level */ + int rsv_size; /* size passed to begin() */ + int rsv_start; /* tail when begin() last returned */ + /** We track the position of the requests in the ring buffer, and * when each is retired we increment last_retired_head as the GPU * must have finished processing the request and so we know we @@ -401,11
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Rebalance runtime pm vs forcewake
2014-12-08 16:27 GMT-02:00 Mika Kuoppala mika.kuopp...@linux.intel.com: From: Chris Wilson ch...@chris-wilson.co.uk Calling intel_runtime_pm_put() is illegal from a soft-irq context, so revert the crude hack commit aa0b3b5bb8768c1a6a6788869d9c7015eae7e80c Author: Paulo Zanoni paulo.r.zan...@intel.com Date: Tue Apr 1 14:55:07 2014 -0300 drm/i915: don't schedule force_wake_timer at gen6_read and apply the single line corrective instead. References: https://bugs.freedesktop.org/show_bug.cgi?id=80913 Cc: Paulo Zanoni paulo.r.zan...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk This patch adds tons and tons of new WARNs when running igt/tests/pm_rpm on BDW, including: WARNING: CPU: 1 PID: 228 at drivers/gpu/drm/i915/intel_uncore.c:623 assert_force_wake_inactive+0x36/0x40 [i915]() WARN_ON(dev_priv-uncore.forcewake_count 0) --- drivers/gpu/drm/i915/i915_drv.c | 1 + drivers/gpu/drm/i915/intel_uncore.c | 18 ++ 2 files changed, 7 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index 71be3c9..706b122 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -1402,6 +1402,7 @@ static int intel_runtime_suspend(struct device *device) } del_timer_sync(dev_priv-gpu_error.hangcheck_timer); + intel_uncore_forcewake_reset(dev, false); dev_priv-pm.suspended = true; /* diff --git a/drivers/gpu/drm/i915/intel_uncore.c b/drivers/gpu/drm/i915/intel_uncore.c index 46de8d7..38ac389 100644 --- a/drivers/gpu/drm/i915/intel_uncore.c +++ b/drivers/gpu/drm/i915/intel_uncore.c @@ -449,8 +449,6 @@ static void gen6_force_wake_timer(unsigned long arg) if (--dev_priv-uncore.forcewake_count == 0) dev_priv-uncore.funcs.force_wake_put(dev_priv, FORCEWAKE_ALL); spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags); - - intel_runtime_pm_put(dev_priv); } void intel_uncore_forcewake_reset(struct drm_device *dev, bool restore) @@ -586,7 +584,6 @@ void gen6_gt_force_wake_get(struct drm_i915_private *dev_priv, int fw_engine) void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine) { unsigned long irqflags; - bool delayed = false; if (!dev_priv-uncore.funcs.force_wake_put) return; @@ -603,21 +600,19 @@ void gen6_gt_force_wake_put(struct drm_i915_private *dev_priv, int fw_engine) goto out; } - spin_lock_irqsave(dev_priv-uncore.lock, irqflags); WARN_ON(!dev_priv-uncore.forcewake_count); if (--dev_priv-uncore.forcewake_count == 0) { dev_priv-uncore.forcewake_count++; - delayed = true; mod_timer_pinned(dev_priv-uncore.force_wake_timer, jiffies + 1); } + spin_unlock_irqrestore(dev_priv-uncore.lock, irqflags); out: - if (!delayed) - intel_runtime_pm_put(dev_priv); + intel_runtime_pm_put(dev_priv); } void assert_force_wake_inactive(struct drm_i915_private *dev_priv) @@ -777,12 +772,11 @@ gen6_read##x(struct drm_i915_private *dev_priv, off_t reg, bool trace) { \ NEEDS_FORCE_WAKE((dev_priv), (reg))) { \ dev_priv-uncore.funcs.force_wake_get(dev_priv, \ FORCEWAKE_ALL); \ - val = __raw_i915_read##x(dev_priv, reg); \ - dev_priv-uncore.funcs.force_wake_put(dev_priv, \ - FORCEWAKE_ALL); \ - } else { \ - val = __raw_i915_read##x(dev_priv, reg); \ + dev_priv-uncore.forcewake_count++; \ + mod_timer_pinned(dev_priv-uncore.force_wake_timer, \ +jiffies + 1); \ } \ + val = __raw_i915_read##x(dev_priv, reg); \ hsw_unclaimed_reg_debug(dev_priv, reg, true, false); \ REG_READ_FOOTER; \ } -- 1.9.1 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Paulo Zanoni ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2] drm/i915/chv: Use timeout mode for RC6 on chv
On Sat, Dec 13, 2014 at 11:43:27AM +0530, deepa...@linux.intel.com wrote: From: Deepak S deepa...@linux.intel.com Higher RC6 residency is observed using timeout mode instead of EI mode. It's Recommended to use TO Method for RC6. v2: Add comment about timeout threshold. (Tom) Yeah if TO is better let's just use it. The 1750us value is what the BIOS spec recommends, so: Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com Signed-off-by: Deepak S deepa...@linux.intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Why is Rodrigo's sob here? --- drivers/gpu/drm/i915/intel_pm.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2316d23..2acb3de 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4689,7 +4689,8 @@ static void cherryview_enable_rps(struct drm_device *dev) I915_WRITE(RING_MAX_IDLE(ring-mmio_base), 10); I915_WRITE(GEN6_RC_SLEEP, 0); - I915_WRITE(GEN6_RC6_THRESHOLD, 5); /* 50/125ms per EI */ + /* TO threshold set to 1750 us ( 0x557 * 1.28 us) */ + I915_WRITE(GEN6_RC6_THRESHOLD, 0x557); /* allows RC6 residency counter to work */ I915_WRITE(VLV_COUNTER_CONTROL, @@ -4703,7 +4704,7 @@ static void cherryview_enable_rps(struct drm_device *dev) /* 3: Enable RC6 */ if ((intel_enable_rc6(dev) INTEL_RC6_ENABLE) (pcbr VLV_PCBR_ADDR_SHIFT)) - rc6_mode = GEN6_RC_CTL_EI_MODE(1); + rc6_mode = GEN7_RC_CTL_TO_MODE; I915_WRITE(GEN6_RC_CONTROL, rc6_mode); -- 1.9.1 -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [ANNOUNCE] intel-gpu-tools 1.9
A new intel-gpu-tools quarterly release is available with the following changes: - New test cases added: drm_import_export, gem_gpgpu_fill, gem_ppgtt, gem_tiled_wb, kms_pwrite_crc. - New helper for interactive progress indicators (see igt_print_activity and igt_progress), which can be disabled by setting the log-level to warn (Thomas and Daniel). - Basic skl support: pci ids, rendercopy mediafill (Damien, Zhao Yakui). - chv support for the iosf sideband tools and a few other improvements (Ville). - Fence register support for intel_reg_dumper on bdw+ (Rodrigo). - Support for skl in quick_dump (Damien). - Golden state generation infrastructure (Mika). - New skl watermark tool (Damien). - New EDID test block that includes multiple display modes (Thomas). - Individual test documentation available in generated documentation and from the test binaries (Thomas). - New logging domains and log filtering (Thomas). - Various API documentation fixes and improvements (Thomas). Complete list of changes since 1.8: Adam Sampson (2): Use = rather than == in test. Don't use += to append to a shell variable. Armin Reese (1): tools/null_state_gen: Add GEN9 golden context batch buffer creation Brad Volkin (4): tests/gem_exec_parse: test for chained batch buffers tests/drv_hangman: skip a few asserts when using the cmd parser tests/gem_exec_parse: fix batch_len setting for cmd-crossing-page tests/gem_madvise: set execbuf.batch_len before doing an execbuf Chris Wilson (25): test: Exercise full ppgtt switching between multiple fd igt/gem_ppgtt: Create secondary contexts and mm igt/pm_rps: Fix the batch count for emitting the flush igt/gem_userptr_blits: GTT mmaping a userptr requires llc igt/kms_render: Iterate only through the formats lib: Try harder to drop-caches igt/pm_rps: Fix STORE_DWORD for pre-gen8 igt/gem_reloc_vs_gpu: Fix reloc.presumed_offset value igt/gem_concurrent_blit: Only read back a few GTT values igt/gem_userptr_blits: Test interruptible create-destroy igt/kms_flip/nonblocking_read: Demonstrate that O_NONBLOCK is a myth igt/gem_negative_reloc: Execute a BLT operation with a negative reloc igt/gem_bad_reloc: Handle real offset being 0 igt/gem_userptr_blits/dmabuf: Provide partial coverage on !llc platforms igt/gem_userptr_blits/dmabuf: Map the right pointer for !llc igt/gem_linear_blits: Add sufficient RAM check ioctl_wrappers: Pass in offset to CPU mmaps igt/gem_tiled_wb: Exercise CPU mmaps with swizzling igt/gem_tiled_wb: Remove extraneous mmap(wc) requirement tests: Remove spurious binaries from gem_tiled_wb commit igt/gem_linear_blits: Require that we do the full test igt/gem_tiled_blits: Require that we do the full test intel_error_decode: Decode the ERROR register on Sandybridge and Ivybridge igt/drm_read: Abuse read(drm) igt/drm_read: Require that pipe 0 is active Damien Lespiau (24): kms_cursor_crc: Remove two unused local variables skl: Add SKL PCI ids skl: Add gen9 to intel_gen() skl: initialize instdone bits for gen9 list-workarounds/skl: Add Skylake to the list of valid platorms rendercopy/skl: Start the gen9 rendercopy from the gen8 version rendercopy/skl: Set the 3DSTATE_VF state rendercopy/skl: Update 3DSTATE_SBE assembler/skl: Add gen 9 to the -g option gem_seqno_wrap: Remove unused variable overlay: Fix compilation warning when not having xrandr gem_wait: Use PRIu64 in format string quick-dump: Make quick dump link against libintel_tools quick_dump: Move base_display.txt to indivual platforms quick_dump/skl: Add some display registers quick_dump/skl: Make quick_dump SKL aware quick_dump: Drop common_display.txt from VLV/CHV skl_ddb_allocation: Add a standalone version of the DDB allocator skl_ddb_allocation: Add checks on the DDB entries skl_ddb_allocation: Make 'end' exclusive in the DDB allocation entry skl_ddb_allocation: Respect the minimum number of blocks gem_bad_reloc: Don't flip-flop between SKIP and PASS kms_cursor_crc: Remove value to 'return' in a void function drv_hangman: Remove unused function Daniel Vetter (25): NEWS: New heading for 1.9 tests: Add drm_import_export tests/drm_import_export: Add subtest for prime tests/kms_psr_sink_crc: Use options tests/gem_wait_render_timeout: Drop local structs tests/gem_wait_render_timeout: Convert to subtests tests/gem_wait: argument validation tests lib/aux: Print progress output at INFO level NEWS: Updates tests: Sprinkle missing igt_exit() where needed. tests/gem_wait: Don't close drmfd in subtest tests/*: lib/igt.cocci found something! Move watermark code from tests to tools Move library selftests
Re: [Intel-gfx] [PATCH 1/8] drm/i915: Rebalance runtime pm vs forcewake
On Fri, Dec 12, 2014 at 02:22:31PM -0200, Paulo Zanoni wrote: 2014-12-08 16:27 GMT-02:00 Mika Kuoppala mika.kuopp...@linux.intel.com: From: Chris Wilson ch...@chris-wilson.co.uk Calling intel_runtime_pm_put() is illegal from a soft-irq context, so revert the crude hack commit aa0b3b5bb8768c1a6a6788869d9c7015eae7e80c Author: Paulo Zanoni paulo.r.zan...@intel.com Date: Tue Apr 1 14:55:07 2014 -0300 drm/i915: don't schedule force_wake_timer at gen6_read and apply the single line corrective instead. References: https://bugs.freedesktop.org/show_bug.cgi?id=80913 Cc: Paulo Zanoni paulo.r.zan...@intel.com Cc: Daniel Vetter daniel.vet...@ffwll.ch Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk This patch adds tons and tons of new WARNs when running igt/tests/pm_rpm on BDW, including: WARNING: CPU: 1 PID: 228 at drivers/gpu/drm/i915/intel_uncore.c:623 assert_force_wake_inactive+0x36/0x40 [i915]() WARN_ON(dev_priv-uncore.forcewake_count 0) The assert is in the incorrect place, it should be after the suspend disables forcewake. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH] tests: Add gem_blt_copy_align test
Copy a block into destination object with varying dst/src offsets. Put guard area before and after the blit target to see that it didn't touch memory out of blit boundaries. References: https://bugs.freedesktop.org/show_bug.cgi?id=79053 Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- gem_blt_copy_align.c | 230 +++ tests/Makefile.sources | 1 + tests/gem_blt_copy_align.c | 237 + 3 files changed, 468 insertions(+) create mode 100644 gem_blt_copy_align.c create mode 100644 tests/gem_blt_copy_align.c diff --git a/gem_blt_copy_align.c b/gem_blt_copy_align.c new file mode 100644 index 000..84693da --- /dev/null +++ b/gem_blt_copy_align.c @@ -0,0 +1,230 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Chris Wilson ch...@chris-wilson.co.uk + *Mika Kuoppala mika.kuopp...@intel.com + * + */ + +#include stdio.h +#include string.h +#include stdint.h +#include i915_drm.h +#include intel_chipset.h +#include intel_reg.h +#include ioctl_wrappers.h +#include drm.h +#include drmtest.h + +static int gen = 0; + +#define WIDTH 32 +#define HEIGHT 32 + +static uint32_t linear[WIDTH*HEIGHT]; + +static void +copy_align(int fd, + uint32_t dst, uint32_t dst_offset, + uint32_t src, uint32_t src_offset, + uint32_t w, uint32_t h) +{ + uint32_t batch[12]; + struct drm_i915_gem_relocation_entry reloc[2]; + struct drm_i915_gem_exec_object2 obj[3]; + struct drm_i915_gem_execbuffer2 exec; + uint32_t handle; + int i=0; + + batch[i++] = XY_SRC_COPY_BLT_CMD | + XY_SRC_COPY_BLT_WRITE_ALPHA | + XY_SRC_COPY_BLT_WRITE_RGB; + if (gen = 8) + batch[i - 1] |= 8; + else + batch[i - 1] |= 6; + + batch[i++] = (3 24) | /* 32 bits */ + (0xcc 16) | /* copy ROP */ + w * 4; + + /* The = gen8 blitter needs to have dst/src base +* addresses aligned to 4k. So we need to handle the +* offsets with with dst/src coordinates */ + batch[i++] = dst_offset; /* dst, x1,y2 */ + batch[i++] = ((h) 16) | (w + dst_offset); /* dst x2,y2 */ + batch[i++] = 0; + if (gen = 8) + batch[i++] = 0; + batch[i++] = src_offset; /* src x1,y1 */ + batch[i++] = w * 4; + +#if 0 + /* Bspec says this should be aligned to quad with gen8 but it doesn't + seem to matter */ + if (gen = 8 (w * 4) % 8) + igt_warn(src pitch NOT aligned to quad\n); +#endif + + batch[i++] = 0; + if (gen = 8) + batch[i++] = 0; + + batch[i++] = MI_BATCH_BUFFER_END; + batch[i++] = MI_NOOP; + + handle = gem_create(fd, 4096); + gem_write(fd, handle, 0, batch, sizeof(batch)); + + reloc[0].target_handle = dst; + reloc[0].delta = 0; + reloc[0].offset = 4 * sizeof(batch[0]); + reloc[0].presumed_offset = 0; + reloc[0].read_domains = I915_GEM_DOMAIN_RENDER; + reloc[0].write_domain = I915_GEM_DOMAIN_RENDER; + + reloc[1].target_handle = src; + reloc[1].delta = 0; + reloc[1].offset = 7 * sizeof(batch[0]); + if (gen = 8) + reloc[1].offset += sizeof(batch[0]); + reloc[1].presumed_offset = 0; + reloc[1].read_domains = I915_GEM_DOMAIN_RENDER; + reloc[1].write_domain = 0; + + memset(obj, 0, sizeof(obj)); + exec.buffer_count = 0; + obj[exec.buffer_count++].handle = dst; + if (src != dst) + obj[exec.buffer_count++].handle = src; + obj[exec.buffer_count].handle = handle; + obj[exec.buffer_count].relocation_count = 2; + obj[exec.buffer_count].relocs_ptr = (uintptr_t)reloc; +
Re: [Intel-gfx] [PATCH] tests: Add gem_blt_copy_align test
On 12 December 2014 at 16:53, Mika Kuoppala mika.kuopp...@linux.intel.com wrote: Copy a block into destination object with varying dst/src offsets. Put guard area before and after the blit target to see that it didn't touch memory out of blit boundaries. References: https://bugs.freedesktop.org/show_bug.cgi?id=79053 Cc: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Mika Kuoppala mika.kuopp...@intel.com --- gem_blt_copy_align.c | 230 +++ Did you mean to add this file? tests/Makefile.sources | 1 + tests/gem_blt_copy_align.c | 237 + 3 files changed, 468 insertions(+) create mode 100644 gem_blt_copy_align.c create mode 100644 tests/gem_blt_copy_align.c Please also add the test binary name to .gitignore and add a short description of the test using the IGT_TEST_DESCRIPTION macro. diff --git a/gem_blt_copy_align.c b/gem_blt_copy_align.c new file mode 100644 index 000..84693da --- /dev/null +++ b/gem_blt_copy_align.c @@ -0,0 +1,230 @@ +/* + * Copyright © 2014 Intel Corporation + * + * Permission is hereby granted, free of charge, to any person obtaining a + * copy of this software and associated documentation files (the Software), + * to deal in the Software without restriction, including without limitation + * the rights to use, copy, modify, merge, publish, distribute, sublicense, + * and/or sell copies of the Software, and to permit persons to whom the + * Software is furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice (including the next + * paragraph) shall be included in all copies or substantial portions of the + * Software. + * + * THE SOFTWARE IS PROVIDED AS IS, WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL + * THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING + * FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS + * IN THE SOFTWARE. + * + * Authors: + *Chris Wilson ch...@chris-wilson.co.uk + *Mika Kuoppala mika.kuopp...@intel.com + * + */ + +#include stdio.h +#include string.h +#include stdint.h +#include i915_drm.h +#include intel_chipset.h +#include intel_reg.h +#include ioctl_wrappers.h +#include drm.h +#include drmtest.h + +static int gen = 0; + +#define WIDTH 32 +#define HEIGHT 32 + +static uint32_t linear[WIDTH*HEIGHT]; + +static void +copy_align(int fd, + uint32_t dst, uint32_t dst_offset, + uint32_t src, uint32_t src_offset, + uint32_t w, uint32_t h) +{ + uint32_t batch[12]; + struct drm_i915_gem_relocation_entry reloc[2]; + struct drm_i915_gem_exec_object2 obj[3]; + struct drm_i915_gem_execbuffer2 exec; + uint32_t handle; + int i=0; + + batch[i++] = XY_SRC_COPY_BLT_CMD | + XY_SRC_COPY_BLT_WRITE_ALPHA | + XY_SRC_COPY_BLT_WRITE_RGB; + if (gen = 8) + batch[i - 1] |= 8; + else + batch[i - 1] |= 6; + + batch[i++] = (3 24) | /* 32 bits */ + (0xcc 16) | /* copy ROP */ + w * 4; + + /* The = gen8 blitter needs to have dst/src base +* addresses aligned to 4k. So we need to handle the +* offsets with with dst/src coordinates */ + batch[i++] = dst_offset; /* dst, x1,y2 */ + batch[i++] = ((h) 16) | (w + dst_offset); /* dst x2,y2 */ + batch[i++] = 0; + if (gen = 8) + batch[i++] = 0; + batch[i++] = src_offset; /* src x1,y1 */ + batch[i++] = w * 4; + +#if 0 + /* Bspec says this should be aligned to quad with gen8 but it doesn't + seem to matter */ + if (gen = 8 (w * 4) % 8) + igt_warn(src pitch NOT aligned to quad\n); +#endif + + batch[i++] = 0; + if (gen = 8) + batch[i++] = 0; + + batch[i++] = MI_BATCH_BUFFER_END; + batch[i++] = MI_NOOP; + + handle = gem_create(fd, 4096); + gem_write(fd, handle, 0, batch, sizeof(batch)); + + reloc[0].target_handle = dst; + reloc[0].delta = 0; + reloc[0].offset = 4 * sizeof(batch[0]); + reloc[0].presumed_offset = 0; + reloc[0].read_domains = I915_GEM_DOMAIN_RENDER; + reloc[0].write_domain = I915_GEM_DOMAIN_RENDER; + + reloc[1].target_handle = src; + reloc[1].delta = 0; + reloc[1].offset = 7 * sizeof(batch[0]); + if (gen = 8) + reloc[1].offset += sizeof(batch[0]); + reloc[1].presumed_offset = 0; + reloc[1].read_domains = I915_GEM_DOMAIN_RENDER; +
Re: [Intel-gfx] [PATCH] drm/i915: fix use after free during eDP encoder destroying
On Fri, Dec 12, 2014 at 05:57:38PM +0200, Imre Deak wrote: After commit a18c0af171bfb875012da26f23df051004726973 uthor: Thierry Reding tred...@nvidia.com Date: Wed Dec 10 11:38:49 2014 +0100 drm: Zero out DRM object memory upon cleanup we will use the eDP encoder during destroying it. Fix this by calling drm_encoder_cleanup() at a point when the encoder is not used any more. This caused a NULL pointer dereference in pps_lock(), I can't see that it caused any other problem. All the other encoders seem to call drm_encoder_cleanup() at a safe place. Signed-off-by: Imre Deak imre.d...@intel.com drm_encoder_cleanup() doesn't appear to have any nasty side effects so this looks sane. Reviewed-by: Ville Syrjälä ville.syrj...@linux.intel.com --- drivers/gpu/drm/i915/intel_dp.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3fc3296..0a55623 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -4310,7 +4310,6 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) drm_dp_aux_unregister(intel_dp-aux); intel_dp_mst_encoder_cleanup(intel_dig_port); - drm_encoder_cleanup(encoder); if (is_edp(intel_dp)) { cancel_delayed_work_sync(intel_dp-panel_vdd_work); /* @@ -4326,6 +4325,7 @@ void intel_dp_encoder_destroy(struct drm_encoder *encoder) intel_dp-edp_notifier.notifier_call = NULL; } } + drm_encoder_cleanup(encoder); kfree(intel_dig_port); } -- 1.8.4 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Ville Syrjälä Intel OTC ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Track check calls to intel(_logical)_ring_{begin, advance}
On Fri, Dec 12, 2014 at 04:13:03PM +, Dave Gordon wrote: static inline void intel_ring_advance(struct intel_engine_cs *ring) { struct intel_ringbuffer *ringbuf = ring-buffer; - ringbuf-tail = ringbuf-size - 1; + + __intel_ringbuffer_check(ringbuf); + + /* + * Tail == effecive_size is legitimate (buffer exactly full). + * Tail effective_size is not, and should give a warning, + * but we'll reset tail in both cases to prevent further chaos + */ + if (ringbuf-tail = ringbuf-effective_size) + ringbuf-tail -= ringbuf-effective_size; Urm. No. If you never write into the reserved pair of cachelines at the end of the ringbuffer but the hw reads garbage from it, you lose. tail = size - 1; is a nice description of how the hw works that is suitable for inlining, with all the magic in begin(). The goal is to remove the duplicated logic from intel_lrc.c, use requests completely, and remove the dri1 hangover. To repeat what I said last time: http://cgit.freedesktop.org/~ickle/linux-2.6/tree/drivers/gpu/drm/i915/intel_ringbuffer.c?h=requests#n2447 is where I want us to go, a single piece of logic for ring management. -Chris -- Chris Wilson, Intel Open Source Technology Centre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/3] drm/i915: Track check calls to intel(_logical)_ring_{begin, advance}
On 12/12/14 17:12, Chris Wilson wrote: On Fri, Dec 12, 2014 at 04:13:03PM +, Dave Gordon wrote: static inline void intel_ring_advance(struct intel_engine_cs *ring) { struct intel_ringbuffer *ringbuf = ring-buffer; -ringbuf-tail = ringbuf-size - 1; + +__intel_ringbuffer_check(ringbuf); + +/* + * Tail == effecive_size is legitimate (buffer exactly full). + * Tail effective_size is not, and should give a warning, + * but we'll reset tail in both cases to prevent further chaos + */ +if (ringbuf-tail = ringbuf-effective_size) +ringbuf-tail -= ringbuf-effective_size; Urm. No. If you never write into the reserved pair of cachelines at the end of the ringbuffer but the hw reads garbage from it, you lose. This won't happen, because (with the first patch of this set applied) the check for sufficient space uses effective_size, but the wrap code uses the actual size. Thus, once the next chunk won't fit in the available space up to the effective_size, we write NOPs all over whatever is left up to the total size. The part between the effective and the actual sizes is therefore always (and only) written with NOPs. tail = size - 1; is a nice description of how the hw works that is suitable for inlining, with all the magic in begin(). The goal is to remove the duplicated logic from intel_lrc.c, use requests completely, and remove the dri1 hangover. To repeat what I said last time: http://cgit.freedesktop.org/~ickle/linux-2.6/tree/drivers/gpu/drm/i915/intel_ringbuffer.c?h=requests#n2447 is where I want us to go, a single piece of logic for ring management. -Chris I agree with the deduplication, which is why nearly all these changes are in functions operating on ringbuffers (not rings), and located in intel_ringbuffer.h, with only minimal changes in the execlist and legacy paths to use these common functions rather than having two implementations. So, what I think I'll do is rework this third patch to merge the tail-masking into the common function; rename it to something other than /check/; and then the legacy and LRC versions become trivial wrappers round this. Are you happy with the first two patches? .Dave. ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 3/4] drm/i915: New offset for reading frequencies on CHV.
On Fri, Dec 12, 2014 at 02:18:15PM +0530, deepa...@linux.intel.com wrote: From: Deepak S deepa...@linux.intel.com Use new Sideband offset to read max/min/gaur freq based on the SKU it is running on. Based on the Number of EU, we read different bits to identify the max frequencies at which system can run. Signed-off-by: Deepak S deepa...@linux.intel.com --- drivers/gpu/drm/i915/i915_drv.h | 4 +-- drivers/gpu/drm/i915/i915_reg.h | 12 drivers/gpu/drm/i915/intel_pm.c | 52 ++- drivers/gpu/drm/i915/intel_sideband.c | 4 +-- 4 files changed, 61 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b58bad4..0690dff 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -3016,8 +3016,8 @@ int sandybridge_pcode_read(struct drm_i915_private *dev_priv, u32 mbox, u32 *val int sandybridge_pcode_write(struct drm_i915_private *dev_priv, u32 mbox, u32 val); /* intel_sideband.c */ -u32 vlv_punit_read(struct drm_i915_private *dev_priv, u8 addr); -void vlv_punit_write(struct drm_i915_private *dev_priv, u8 addr, u32 val); +u32 vlv_punit_read(struct drm_i915_private *dev_priv, u32 addr); +void vlv_punit_write(struct drm_i915_private *dev_priv, u32 addr, u32 val); u32 vlv_nc_read(struct drm_i915_private *dev_priv, u8 addr); u32 vlv_gpio_nc_read(struct drm_i915_private *dev_priv, u32 reg); void vlv_gpio_nc_write(struct drm_i915_private *dev_priv, u32 reg, u32 val); diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index b57cba3..f41290c 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -602,6 +602,18 @@ enum punit_power_well { #define PUNIT_FUSE_BUS2 0xf6 /* bits 47:40 */ #define PUNIT_FUSE_BUS1 0xf5 /* bits 55:48 */ +#define FB_GFX_FMAX_AT_VMAX_FUSE 0x136 +#define FB_GFX_FMAX_AT_VMAX_FUSE_MASK0xff +#define FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT24 +#define FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT16 +#define FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT8 + This blank line makes me think FB_GFX_GUAR_FREQ_FUSE_MASK isn't part of this register. So best not leave such blank line here. I have 0x3c3c3c28 in this register, which matches what I get using the old method. +#define FB_GFX_GUAR_FREQ_FUSE_MASK 0xff + +#define FB_GFX_FMIN_AT_VMIN_FUSE 0x137 +#define FB_GFX_FMIN_AT_VMIN_FUSE_MASK0xff +#define FB_GFX_FMIN_AT_VMIN_FUSE_SHIFT 8 I have 0x69841428 here. The low 8 bits look like another freq value. What is it? I have no docs for this stuff so can't really review apart from looking at what my hardware reports. Actually, since all the values are 8 bits maybe it would be neater to just #define FB_GFX_FREQ_FUSE_MASK 0xff and use that everywhere instead of having three different definitions for the same 0xff value. + #define PUNIT_GPU_STATUS_REG 0xdb #define PUNIT_GPU_STATUS_MAX_FREQ_SHIFT 16 #define PUNIT_GPU_STATUS_MAX_FREQ_MASK 0xff diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 2acb3de..71b8e2f 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -4346,11 +4346,29 @@ void gen6_update_ring_freq(struct drm_device *dev) static int cherryview_rps_max_freq(struct drm_i915_private *dev_priv) { + struct drm_device *dev = dev_priv-dev; + struct intel_device_info *info; u32 val, rp0; - val = vlv_punit_read(dev_priv, PUNIT_GPU_STATUS_REG); - rp0 = (val PUNIT_GPU_STATUS_MAX_FREQ_SHIFT) PUNIT_GPU_STATUS_MAX_FREQ_MASK; - + info = (struct intel_device_info *)dev_priv-info; Pointless cast. Also the assignment could be done when declaring info, and we usually use INTEL_INFO() to get at it. + + if (dev-pdev-revision = 0x20) { Do we really need this check? I would think it would be up to the Punit firmware version rather the stepping. My BSW has PCI rev 0x15, but with the latest BIOS both fuse registers already contain correct looking information. + val = vlv_punit_read(dev_priv, FB_GFX_FMAX_AT_VMAX_FUSE); + + if (info-eu_total == 8) /* (2 * 4) config */ + rp0 = (val FB_GFX_FMAX_AT_VMAX_2SS4EU_FUSE_SHIFT); + else if (info-eu_total == 12) /* (2 * 6) config */ + rp0 = (val FB_GFX_FMAX_AT_VMAX_2SS6EU_FUSE_SHIFT); + else if (info-eu_total == 16) /* (2 * 8) config */ + rp0 = (val FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); + else /* Setting (2 * 8) Min RP0 for any other combination */ + rp0 = (val FB_GFX_FMAX_AT_VMAX_2SS8EU_FUSE_SHIFT); 'switch (INTEL_INFO(dev)-eu_total)' perhaps? +
Re: [Intel-gfx] [PATCH 01/17] drm/i915: Add automated testing support for Displayport compliance testing
2014-12-10 21:53 GMT-02:00 Todd Previte tprev...@gmail.com: Add the skeleton framework for supporting automation for Displayport compliance testing. This patch adds the necessary framework for the source device to appropriately respond to test automation requests from a sink device. V2: - Addressed previous mailing list feedback - Fixed compilation issue (struct members declared in a later patch) - Updated debug messages to be more accurate - Added status checks for the DPCD read/write calls - Removed excess comments and debug messages - Fixed debug message compilation warnings - Fixed compilation issue with missing variables - Updated link training autotest to ACK Signed-off-by: Todd Previte tprev...@gmail.com --- drivers/gpu/drm/i915/intel_dp.c | 72 +--- drivers/gpu/drm/i915/intel_drv.h | 4 +++ 2 files changed, 72 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c index 3fc3296..3dc92a3 100644 --- a/drivers/gpu/drm/i915/intel_dp.c +++ b/drivers/gpu/drm/i915/intel_dp.c @@ -3744,11 +3744,75 @@ intel_dp_get_sink_irq_esi(struct intel_dp *intel_dp, u8 *sink_irq_vector) return true; } -static void -intel_dp_handle_test_request(struct intel_dp *intel_dp) +static uint8_t intel_dp_autotest_link_training(struct intel_dp *intel_dp) +{ + uint8_t test_result = DP_TEST_ACK; + return test_result; +} + +static uint8_t intel_dp_autotest_video_pattern(struct intel_dp *intel_dp) +{ + uint8_t test_result = DP_TEST_NAK; + return test_result; +} + +static uint8_t intel_dp_autotest_edid(struct intel_dp *intel_dp) +{ + uint8_t test_result = DP_TEST_NAK; + return test_result; +} + +static uint8_t intel_dp_autotest_phy_pattern(struct intel_dp *intel_dp) +{ + uint8_t test_result = DP_TEST_NAK; + return test_result; +} + +static void intel_dp_handle_test_request(struct intel_dp *intel_dp) { - /* NAK by default */ - drm_dp_dpcd_writeb(intel_dp-aux, DP_TEST_RESPONSE, DP_TEST_NAK); + uint8_t response = DP_TEST_NAK; + uint8_t rxdata = 0; + int status = 0; + + status = drm_dp_dpcd_read(intel_dp-aux, DP_TEST_REQUEST, rxdata, 1); + if (status != 0) { Why are we checking for zero here? In the happy case, shouldn't this function return 1? To my understanding, we would be ignoring all test requests from the users, which means you wouldn't be able to test anything in your series at all... I see that you don't change this line at all in the rest of your series, so maybe I'm just crazy and failing to notice some detail... + response = DP_TEST_NAK; + DRM_DEBUG_KMS(Could not read test request from sink\n); You assign a value to response but don't do anything to it. Shouldn't we be trying to send the NAK in this case? If yes, then the code is missing, if no, then I guess we can remove the response assignment (well, we could remove it in both cases since it's already initialized to DP_TEST_NAK anyway). + return; + } + + switch (rxdata) { + case DP_TEST_LINK_TRAINING: + DRM_DEBUG_KMS(LINK_TRAINING test requested\n); + response = intel_dp_autotest_link_training(intel_dp); + break; + case DP_TEST_LINK_VIDEO_PATTERN: + DRM_DEBUG_KMS(TEST_PATTERN test requested\n); + response = intel_dp_autotest_video_pattern(intel_dp); + break; + case DP_TEST_LINK_EDID_READ: + DRM_DEBUG_KMS(EDID test requested\n); + response = intel_dp_autotest_edid(intel_dp); + break; + case DP_TEST_LINK_PHY_TEST_PATTERN: + DRM_DEBUG_KMS(PHY_PATTERN test requested\n); + response = intel_dp_autotest_phy_pattern(intel_dp); + break; + /* FAUX is optional in DP 1.2*/ + case DP_TEST_LINK_FAUX_PATTERN: + DRM_DEBUG_KMS(FAUX_PATTERN testing not supported\n); + break; + default: + DRM_DEBUG_KMS(Invalid test request '%02x'\n, rxdata); + break; + } + status = drm_dp_dpcd_write(intel_dp-aux, + DP_TEST_RESPONSE, + response, 1); + if (status != 0) Same here... + DRM_DEBUG_KMS(Could not write test response to sink\n); + + intel_dp-compliance_testing_active = 0; } static int diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 588b618..d1a807a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -638,6 +638,10 @@ struct intel_dp { struct mutex mutex; } drrs_state; + /* Displayport compliance testing */ + unsigned long compliance_test_data; +
Re: [Intel-gfx] [PATCH 03/11] drm/i915: don't try to find crtcs for FBC if it's disabled
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com .. because it would be a waste of time, so move the place where the check is done. Also, with this we won't risk printing more than one pipe active, disabling compression or no output, disabling when FBC is actually disabled. This patch also represents a small behavior difference when using i915.powersave=0: it is now exactly the same as i915.enable_fbc=0 on this part of the code. I always ask myself if we should continue using this i915.powersave here. I vaguely remember someone complaining when I tried to re-org this i915.powersave making it a umbrealla for all powersaving features and the complain was to avoid more than one variable for feature... to avoid superset of parameters, or something like that. Or if we keep this here shouldn't we also put this to disable psr? Anyway this can be another patch. Daniel, please let me know what you want with this parameters that I can change that later. So for this patch: Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 20 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 7686573..4647d57 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -521,10 +521,16 @@ void intel_fbc_update(struct drm_device *dev) return; } - if (!i915.powersave) { + if (i915.enable_fbc 0) { + if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) + DRM_DEBUG_KMS(disabled per chip default\n); + goto out_disable; + } + + if (!i915.enable_fbc || !i915.powersave) { if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) DRM_DEBUG_KMS(fbc disabled per module param\n); - return; + goto out_disable; } /* @@ -559,16 +565,6 @@ void intel_fbc_update(struct drm_device *dev) obj = intel_fb_obj(fb); adjusted_mode = intel_crtc-config.adjusted_mode; - if (i915.enable_fbc 0) { - if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) - DRM_DEBUG_KMS(disabled per chip default\n); - goto out_disable; - } - if (!i915.enable_fbc) { - if (set_no_fbc_reason(dev_priv, FBC_MODULE_PARAM)) - DRM_DEBUG_KMS(fbc disabled per module param\n); - goto out_disable; - } if ((adjusted_mode-flags DRM_MODE_FLAG_INTERLACE) || (adjusted_mode-flags DRM_MODE_FLAG_DBLSCAN)) { if (set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED_MODE)) -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 04/11] drm/i915: don't keep reassigning FBC_UNSUPPORTED
On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com This may save a few picoseconds on !HAS_FBC platforms. And it also satisfies my OCD. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 4647d57..f3d5764 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -516,10 +516,8 @@ void intel_fbc_update(struct drm_device *dev) const struct drm_display_mode *adjusted_mode; unsigned int max_width, max_height; - if (!HAS_FBC(dev)) { - set_no_fbc_reason(dev_priv, FBC_UNSUPPORTED); I really don't like this fbc_reasons... I believe a drm_debug here is enough. but I can try to kill that later. For this patch: Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com + if (!HAS_FBC(dev)) return; - } if (i915.enable_fbc 0) { if (set_no_fbc_reason(dev_priv, FBC_CHIP_DEFAULT)) @@ -684,6 +682,7 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) { if (!HAS_FBC(dev_priv)) { dev_priv-fbc.enabled = false; + dev_priv-fbc.no_fbc_reason = FBC_UNSUPPORTED; return; } -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 05/11] drm/i915: change dev_priv-fbc.plane to dev_priv-fbc.crtc
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Since the mapping from CRTCs to planes is fixed, looking at the CRTC is essentially the same as looking at the plane. Also, the next patches wil start using the frontbuffer_bits macros, and they take the pipe as the parameter instead of the plane, and this could differ on gens 2 and 3. Another nice thing is that we don't risk accidentally initializing things to PLANE_A if we don't set the value before it is used for the first time. But this shouldn't be a problem with the current code. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 2 +- drivers/gpu/drm/i915/intel_display.c | 5 ++--- drivers/gpu/drm/i915/intel_fbc.c | 6 +++--- drivers/gpu/drm/i915/intel_sprite.c | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index b5260bf..9d694f1 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -689,7 +689,7 @@ struct i915_fbc { unsigned long size; unsigned threshold; unsigned int fb_id; - enum plane plane; + struct intel_crtc *crtc; int y; struct drm_mm_node compressed_fb; diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c index e4789f4..88f3652 100644 --- a/drivers/gpu/drm/i915/intel_display.c +++ b/drivers/gpu/drm/i915/intel_display.c @@ -4218,11 +4218,10 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc) struct drm_i915_private *dev_priv = dev-dev_private; struct intel_crtc *intel_crtc = to_intel_crtc(crtc); int pipe = intel_crtc-pipe; - int plane = intel_crtc-plane; intel_crtc_wait_for_pending_flips(crtc); - if (dev_priv-fbc.plane == plane) + if (dev_priv-fbc.crtc == intel_crtc) intel_fbc_disable(dev); hsw_disable_ips(intel_crtc); @@ -11813,7 +11812,7 @@ intel_commit_primary_plane(struct drm_plane *plane, */ if (intel_crtc-primary_enabled INTEL_INFO(dev)-gen = 4 !IS_G4X(dev) - dev_priv-fbc.plane == intel_crtc-plane + dev_priv-fbc.crtc == intel_crtc intel_plane-rotation != BIT(DRM_ROTATE_0)) { intel_fbc_disable(dev); } diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index f3d5764..88d00d3 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -383,7 +383,7 @@ static void intel_fbc_work_fn(struct work_struct *__work) if (work-crtc-primary-fb == work-fb) { dev_priv-display.enable_fbc(work-crtc); - dev_priv-fbc.plane = to_intel_crtc(work-crtc)-plane; + dev_priv-fbc.crtc = to_intel_crtc(work-crtc); dev_priv-fbc.fb_id = work-crtc-primary-fb-base.id; dev_priv-fbc.y = work-crtc-y; } @@ -474,7 +474,7 @@ void intel_fbc_disable(struct drm_device *dev) return; dev_priv-display.disable_fbc(dev); - dev_priv-fbc.plane = -1; + dev_priv-fbc.crtc = NULL; } static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, @@ -626,7 +626,7 @@ void intel_fbc_update(struct drm_device *dev) * cannot be unpinned (and have its GTT offset and fence revoked) * without first being decoupled from the scanout and FBC disabled. */ - if (dev_priv-fbc.plane == intel_crtc-plane + if (dev_priv-fbc.crtc == intel_crtc dev_priv-fbc.fb_id == fb-base.id dev_priv-fbc.y == crtc-y) return; diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c index c18e57d..942daca 100644 --- a/drivers/gpu/drm/i915/intel_sprite.c +++ b/drivers/gpu/drm/i915/intel_sprite.c @@ -1016,7 +1016,7 @@ intel_pre_disable_primary(struct drm_crtc *crtc) struct intel_crtc *intel_crtc = to_intel_crtc(crtc); mutex_lock(dev-struct_mutex); - if (dev_priv-fbc.plane == intel_crtc-plane) + if (dev_priv-fbc.crtc == intel_crtc) intel_fbc_disable(dev); mutex_unlock(dev-struct_mutex); -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 06/11] drm/i915: pass which operation triggered the frontbuffer tracking
On Mon, Dec 8, 2014 at 8:53 AM, Daniel Vetter dan...@ffwll.ch wrote: On Mon, Dec 08, 2014 at 02:09:15PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We want to port FBC to the frontbuffer tracking infrastructure, but for that we need to know what caused the object invalidation/flush so we can react accordingly: CPU mmaps need manual, GTT mmaps and flips don't need handling and ring rendering needs nukes. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h| 7 +++ drivers/gpu/drm/i915/i915_gem.c| 10 +- drivers/gpu/drm/i915/i915_gem_execbuffer.c | 2 +- drivers/gpu/drm/i915/intel_drv.h | 11 +++ drivers/gpu/drm/i915/intel_frontbuffer.c | 15 ++- 5 files changed, 30 insertions(+), 15 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9d694f1..ea3cc81 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -685,6 +685,13 @@ struct intel_context { struct list_head link; }; +enum fb_op_origin { + ORIGIN_GTT, + ORIGIN_CPU, + ORIGIN_RENDER, Maybe GPU instead of RENDER since it includes the blitter? Render typically only means the render ring in gem code. And maybe add an I915_ prefix or so at least to the enum. I agree with Daniel that RENDER isn't a good name, but also not sure about GPU... Maybe ENGINE_CS or RINGS... Anyway that's it from me with bikesheds, lgtm overall. I bikesheded the bikeshed! :D Anyway lgtm as well so anyway you prefer to go feel free to use Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com -Daniel + ORIGIN_FLIP, +}; + struct i915_fbc { unsigned long size; unsigned threshold; diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index de241eb..7ef12e8 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -2321,7 +2321,7 @@ i915_gem_object_move_to_inactive(struct drm_i915_gem_object *obj) list_move_tail(vma-mm_list, vm-inactive_list); } - intel_fb_obj_flush(obj, true); + intel_fb_obj_flush(obj, true, ORIGIN_RENDER); list_del_init(obj-ring_list); @@ -3665,7 +3665,7 @@ i915_gem_object_flush_gtt_write_domain(struct drm_i915_gem_object *obj) old_write_domain = obj-base.write_domain; obj-base.write_domain = 0; - intel_fb_obj_flush(obj, false); + intel_fb_obj_flush(obj, false, ORIGIN_GTT); trace_i915_gem_object_change_domain(obj, obj-base.read_domains, @@ -3688,7 +3688,7 @@ i915_gem_object_flush_cpu_write_domain(struct drm_i915_gem_object *obj, old_write_domain = obj-base.write_domain; obj-base.write_domain = 0; - intel_fb_obj_flush(obj, false); + intel_fb_obj_flush(obj, false, ORIGIN_CPU); trace_i915_gem_object_change_domain(obj, obj-base.read_domains, @@ -3745,7 +3745,7 @@ i915_gem_object_set_to_gtt_domain(struct drm_i915_gem_object *obj, bool write) } if (write) - intel_fb_obj_invalidate(obj, NULL); + intel_fb_obj_invalidate(obj, NULL, ORIGIN_GTT); trace_i915_gem_object_change_domain(obj, old_read_domains, @@ -4072,7 +4072,7 @@ i915_gem_object_set_to_cpu_domain(struct drm_i915_gem_object *obj, bool write) } if (write) - intel_fb_obj_invalidate(obj, NULL); + intel_fb_obj_invalidate(obj, NULL, ORIGIN_CPU); trace_i915_gem_object_change_domain(obj, old_read_domains, diff --git a/drivers/gpu/drm/i915/i915_gem_execbuffer.c b/drivers/gpu/drm/i915/i915_gem_execbuffer.c index 0c25f62..af290e6 100644 --- a/drivers/gpu/drm/i915/i915_gem_execbuffer.c +++ b/drivers/gpu/drm/i915/i915_gem_execbuffer.c @@ -965,7 +965,7 @@ i915_gem_execbuffer_move_to_active(struct list_head *vmas, obj-dirty = 1; i915_gem_request_assign(obj-last_write_req, req); - intel_fb_obj_invalidate(obj, ring); + intel_fb_obj_invalidate(obj, ring, ORIGIN_RENDER); /* update for the implicit flush after a batch */ obj-base.write_domain = ~I915_GEM_GPU_DOMAINS; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 588b618..633fb9a 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -841,13 +841,15 @@ void intel_ddi_set_vc_payload_alloc(struct drm_crtc *crtc, bool state); /* intel_frontbuffer.c */ void intel_fb_obj_invalidate(struct drm_i915_gem_object *obj, - struct intel_engine_cs *ring); + struct intel_engine_cs *ring,
Re: [Intel-gfx] [PATCH 07/11] drm/i915: also do frontbuffer tracking on pwrites
wondering if this fixes: https://bugs.freedesktop.org/show_bug.cgi?id=87143 Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Mon, Dec 8, 2014 at 8:55 AM, Daniel Vetter dan...@ffwll.ch wrote: On Mon, Dec 08, 2014 at 02:09:16PM -0200, Paulo Zanoni wrote: From: Paulo Zanoni paulo.r.zan...@intel.com We need this for FBC, and possibly for PSR too. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Hm, this might fix some cursor update failures if userspace only pwrites and doesn't follow suite with a cursor ioctl update call. Not sure this ever happens, but definitely makes sense. -Daniel --- drivers/gpu/drm/i915/i915_gem.c | 4 1 file changed, 4 insertions(+) diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c index 7ef12e8..b8c906e 100644 --- a/drivers/gpu/drm/i915/i915_gem.c +++ b/drivers/gpu/drm/i915/i915_gem.c @@ -1117,6 +1117,10 @@ i915_gem_pwrite_ioctl(struct drm_device *dev, void *data, ret = i915_gem_phys_pwrite(obj, args, file); else ret = i915_gem_shmem_pwrite(dev, obj, args, file); + + intel_fb_obj_flush(obj, false, ORIGIN_CPU); + } else { + intel_fb_obj_flush(obj, false, ORIGIN_GTT); } out: -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] [PATCH 6/9] drm/i915: add frontbuffer tracking to FBC
From: Paulo Zanoni paulo.r.zan...@intel.com Kill the blt/render tracking we currently have and use the frontbuffer tracking infrastructure. Don't enable things by default yet. v2: (Rodrigo) Fix small conflict on rebase and typo at subject. Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 9 +-- drivers/gpu/drm/i915/intel_drv.h | 6 +- drivers/gpu/drm/i915/intel_fbc.c | 107 --- drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +--- drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +--- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 6 files changed, 80 insertions(+), 98 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index 9efeec2..11a492a 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -708,6 +708,7 @@ struct i915_fbc { unsigned long size; unsigned threshold; unsigned int fb_id; + unsigned int possible_framebuffer_bits; struct intel_crtc *crtc; int y; @@ -720,14 +721,6 @@ struct i915_fbc { * possible. */ bool enabled; - /* On gen8 some rings cannont perform fbc clean operation so for now -* we are doing this on SW with mmio. -* This variable works in the opposite information direction -* of ring-fbc_dirty telling software on frontbuffer tracking -* to perform the cache clean on sw side. -*/ - bool need_sw_cache_clean; - struct intel_fbc_work { struct delayed_work work; struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 633fb9a..3a1c3d8 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1071,7 +1071,11 @@ bool intel_fbc_enabled(struct drm_device *dev); void intel_fbc_update(struct drm_device *dev); void intel_fbc_init(struct drm_i915_private *dev_priv); void intel_fbc_disable(struct drm_device *dev); -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value); +void intel_fbc_invalidate(struct drm_i915_private *dev_priv, + unsigned int frontbuffer_bits, + enum fb_op_origin origin); +void intel_fbc_flush(struct drm_i915_private *dev_priv, +unsigned int frontbuffer_bits, enum fb_op_origin origin); /* intel_hdmi.c */ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index f658dc0..303470c 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -173,29 +173,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev) return I915_READ(DPFC_CONTROL) DPFC_CTL_EN; } -static void snb_fbc_blit_update(struct drm_device *dev) +static void intel_fbc_nuke(struct drm_i915_private *dev_priv) { - struct drm_i915_private *dev_priv = dev-dev_private; - u32 blt_ecoskpd; - - /* Make sure blitter notifies FBC of writes */ - - /* Blitter is part of Media powerwell on VLV. No impact of -* his param in other platforms for now */ - gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA); - - blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD); - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY - GEN6_BLITTER_LOCK_SHIFT; - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY; - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - blt_ecoskpd = ~(GEN6_BLITTER_FBC_NOTIFY -GEN6_BLITTER_LOCK_SHIFT); - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - POSTING_READ(GEN6_BLITTER_ECOSKPD); - - gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA); + I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE); + POSTING_READ(MSG_FBC_REND_STATE); } static void ilk_fbc_enable(struct drm_crtc *crtc) @@ -238,9 +219,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc) I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); - snb_fbc_blit_update(dev); } + intel_fbc_nuke(dev_priv); + DRM_DEBUG_KMS(enabled fbc on plane %c\n, plane_name(intel_crtc-plane)); } @@ -319,7 +301,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc) SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); - snb_fbc_blit_update(dev); + intel_fbc_nuke(dev_priv); DRM_DEBUG_KMS(enabled fbc on plane %c\n, plane_name(intel_crtc-plane)); } @@ -339,19 +321,6 @@ bool intel_fbc_enabled(struct drm_device *dev) return dev_priv-fbc.enabled; } -void
Re: [Intel-gfx] [PATCH 08/11] drm/i915: add fronbuffer tracking to FBC
just a note that I sent a reviwed v2: rebased and with typo fix. On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com Kill the blt/render tracking we currently have and use the frontbuffer tracking infrastructure. Don't enable things by default yet. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/i915_drv.h | 9 +-- drivers/gpu/drm/i915/intel_drv.h | 6 +- drivers/gpu/drm/i915/intel_fbc.c | 119 ++- drivers/gpu/drm/i915/intel_frontbuffer.c | 14 +--- drivers/gpu/drm/i915/intel_ringbuffer.c | 41 +-- drivers/gpu/drm/i915/intel_ringbuffer.h | 1 - 6 files changed, 80 insertions(+), 110 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index ea3cc81..22285c2 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -696,6 +696,7 @@ struct i915_fbc { unsigned long size; unsigned threshold; unsigned int fb_id; + unsigned int possible_framebuffer_bits; struct intel_crtc *crtc; int y; @@ -708,14 +709,6 @@ struct i915_fbc { * possible. */ bool enabled; - /* On gen8 some rings cannont perform fbc clean operation so for now -* we are doing this on SW with mmio. -* This variable works in the opposite information direction -* of ring-fbc_dirty telling software on frontbuffer tracking -* to perform the cache clean on sw side. -*/ - bool need_sw_cache_clean; - struct intel_fbc_work { struct delayed_work work; struct drm_crtc *crtc; diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h index 633fb9a..3a1c3d82 100644 --- a/drivers/gpu/drm/i915/intel_drv.h +++ b/drivers/gpu/drm/i915/intel_drv.h @@ -1071,7 +1071,11 @@ bool intel_fbc_enabled(struct drm_device *dev); void intel_fbc_update(struct drm_device *dev); void intel_fbc_init(struct drm_i915_private *dev_priv); void intel_fbc_disable(struct drm_device *dev); -void bdw_fbc_sw_flush(struct drm_device *dev, u32 value); +void intel_fbc_invalidate(struct drm_i915_private *dev_priv, + unsigned int frontbuffer_bits, + enum fb_op_origin origin); +void intel_fbc_flush(struct drm_i915_private *dev_priv, +unsigned int frontbuffer_bits, enum fb_op_origin origin); /* intel_hdmi.c */ void intel_hdmi_init(struct drm_device *dev, int hdmi_reg, enum port port); diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 88d00d3..03ba43d 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -178,29 +178,10 @@ static bool g4x_fbc_enabled(struct drm_device *dev) return I915_READ(DPFC_CONTROL) DPFC_CTL_EN; } -static void snb_fbc_blit_update(struct drm_device *dev) +static void intel_fbc_nuke(struct drm_i915_private *dev_priv) { - struct drm_i915_private *dev_priv = dev-dev_private; - u32 blt_ecoskpd; - - /* Make sure blitter notifies FBC of writes */ - - /* Blitter is part of Media powerwell on VLV. No impact of -* his param in other platforms for now */ - gen6_gt_force_wake_get(dev_priv, FORCEWAKE_MEDIA); - - blt_ecoskpd = I915_READ(GEN6_BLITTER_ECOSKPD); - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY - GEN6_BLITTER_LOCK_SHIFT; - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - blt_ecoskpd |= GEN6_BLITTER_FBC_NOTIFY; - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - blt_ecoskpd = ~(GEN6_BLITTER_FBC_NOTIFY -GEN6_BLITTER_LOCK_SHIFT); - I915_WRITE(GEN6_BLITTER_ECOSKPD, blt_ecoskpd); - POSTING_READ(GEN6_BLITTER_ECOSKPD); - - gen6_gt_force_wake_put(dev_priv, FORCEWAKE_MEDIA); + I915_WRITE(MSG_FBC_REND_STATE, FBC_REND_NUKE); + POSTING_READ(MSG_FBC_REND_STATE); } static void ilk_fbc_enable(struct drm_crtc *crtc) @@ -243,9 +224,10 @@ static void ilk_fbc_enable(struct drm_crtc *crtc) I915_WRITE(SNB_DPFC_CTL_SA, SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); - snb_fbc_blit_update(dev); } + intel_fbc_nuke(dev_priv); + DRM_DEBUG_KMS(enabled fbc on plane %c\n, plane_name(intel_crtc-plane)); } @@ -324,7 +306,7 @@ static void gen7_fbc_enable(struct drm_crtc *crtc) SNB_CPU_FENCE_ENABLE | obj-fence_reg); I915_WRITE(DPFC_CPU_FENCE_OFFSET, crtc-y); - snb_fbc_blit_update(dev); + intel_fbc_nuke(dev_priv); DRM_DEBUG_KMS(enabled fbc on plane %c\n, plane_name(intel_crtc-plane)); } @@ -342,31 +324,6 @@ bool
Re: [Intel-gfx] [PATCH 09/11] drm/i915: extract intel_fbc_find_crtc()
Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com I want to make this code a little more complicated, so let's extract the function first. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 46 +--- 1 file changed, 29 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 03ba43d..450d0be 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -444,6 +444,32 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, return true; } +static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) +{ + struct drm_device *dev = dev_priv-dev; + struct drm_crtc *crtc = NULL, *tmp_crtc; + + for_each_crtc(dev, tmp_crtc) { + if (intel_crtc_active(tmp_crtc) + to_intel_crtc(tmp_crtc)-primary_enabled) { + if (crtc) { + if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES)) + DRM_DEBUG_KMS(more than one pipe active, disabling compression\n); + return NULL; + } + crtc = tmp_crtc; + } + } + + if (!crtc || crtc-primary-fb == NULL) { + if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT)) + DRM_DEBUG_KMS(no output, disabling\n); + return NULL; + } + + return crtc; +} + /** * intel_fbc_update - enable/disable FBC as needed * @dev: the drm_device @@ -466,7 +492,7 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, void intel_fbc_update(struct drm_device *dev) { struct drm_i915_private *dev_priv = dev-dev_private; - struct drm_crtc *crtc = NULL, *tmp_crtc; + struct drm_crtc *crtc = NULL; struct intel_crtc *intel_crtc; struct drm_framebuffer *fb; struct drm_i915_gem_object *obj; @@ -497,23 +523,9 @@ void intel_fbc_update(struct drm_device *dev) * - new fb is too large to fit in compressed buffer * - going to an unsupported config (interlace, pixel multiply, etc.) */ - for_each_crtc(dev, tmp_crtc) { - if (intel_crtc_active(tmp_crtc) - to_intel_crtc(tmp_crtc)-primary_enabled) { - if (crtc) { - if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES)) - DRM_DEBUG_KMS(more than one pipe active, disabling compression\n); - goto out_disable; - } - crtc = tmp_crtc; - } - } - - if (!crtc || crtc-primary-fb == NULL) { - if (set_no_fbc_reason(dev_priv, FBC_NO_OUTPUT)) - DRM_DEBUG_KMS(no output, disabling\n); + crtc = intel_fbc_find_crtc(dev_priv); + if (!crtc) goto out_disable; - } intel_crtc = to_intel_crtc(crtc); fb = crtc-primary-fb; -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 10/11] drm/i915: HSW+ FBC is tied to pipe A
I always ask myself if we should just clean the code and remove all platforms before HSW that always had many fbc issues. So we could make it simple and just do for pipe A for all platforms. Anyway, this looks ok for now Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So add code to consider this case. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 20 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index 450d0be..e8dc1d5 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -446,10 +446,16 @@ static bool set_no_fbc_reason(struct drm_i915_private *dev_priv, static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) { - struct drm_device *dev = dev_priv-dev; struct drm_crtc *crtc = NULL, *tmp_crtc; + enum pipe pipe; + bool pipe_a_only = false; + + if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)-gen = 8) + pipe_a_only = true; + + for_each_pipe(dev_priv, pipe) { + tmp_crtc = dev_priv-pipe_to_crtc_mapping[pipe]; - for_each_crtc(dev, tmp_crtc) { if (intel_crtc_active(tmp_crtc) to_intel_crtc(tmp_crtc)-primary_enabled) { if (crtc) { @@ -459,6 +465,9 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) } crtc = tmp_crtc; } + + if (pipe_a_only) + break; } if (!crtc || crtc-primary-fb == NULL) { @@ -714,11 +723,14 @@ void intel_fbc_init(struct drm_i915_private *dev_priv) return; } - /* TODO: some platforms have FBC tied to a specific plane! */ - for_each_pipe(dev_priv, pipe) + for_each_pipe(dev_priv, pipe) { dev_priv-fbc.possible_framebuffer_bits |= INTEL_FRONTBUFFER_PRIMARY(pipe); + if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)-gen = 8) + break; + } + if (INTEL_INFO(dev_priv)-gen = 7) { dev_priv-display.fbc_enabled = ilk_fbc_enabled; dev_priv-display.enable_fbc = gen7_fbc_enable; -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 11/11] drm/i915: gen5+ can have FBC with multiple pipes
I keep my with to remove support for old gens but anyway: Reviewed-by: Rodrigo Vivi rodrigo.v...@intel.com On Mon, Dec 8, 2014 at 8:09 AM, Paulo Zanoni przan...@gmail.com wrote: From: Paulo Zanoni paulo.r.zan...@intel.com So allow it. Signed-off-by: Paulo Zanoni paulo.r.zan...@intel.com --- drivers/gpu/drm/i915/intel_fbc.c | 6 -- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c index e8dc1d5..1c22922 100644 --- a/drivers/gpu/drm/i915/intel_fbc.c +++ b/drivers/gpu/drm/i915/intel_fbc.c @@ -448,17 +448,19 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv) { struct drm_crtc *crtc = NULL, *tmp_crtc; enum pipe pipe; - bool pipe_a_only = false; + bool pipe_a_only = false, one_pipe_only = false; if (IS_HASWELL(dev_priv) || INTEL_INFO(dev_priv)-gen = 8) pipe_a_only = true; + else if (INTEL_INFO(dev_priv)-gen = 4) + one_pipe_only = true; for_each_pipe(dev_priv, pipe) { tmp_crtc = dev_priv-pipe_to_crtc_mapping[pipe]; if (intel_crtc_active(tmp_crtc) to_intel_crtc(tmp_crtc)-primary_enabled) { - if (crtc) { + if (one_pipe_only crtc) { if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES)) DRM_DEBUG_KMS(more than one pipe active, disabling compression\n); return NULL; -- 2.1.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 2/2] drm/i915: VLV/CHV PSR: Increase wait delay time before active PSR.
On Mon, Dec 8, 2014 at 1:35 AM, Daniel Vetter dan...@ffwll.ch wrote: On Fri, Dec 05, 2014 at 08:40:42PM -0500, Rodrigo Vivi wrote: Since active function on VLV immediately activate PSR let's give more time for idleness. v2: Rebase over intel_psr.c and fix typo. v3: Revival: Manual tests indicated that this is needed. With a short delay there is a huge risk of getting blank screens when planes are being enabled. Signed-off-by: Rodrigo Vivi rodrigo.v...@intel.com Reviewed-by: Durgadoss R durgados...@intel.com --- drivers/gpu/drm/i915/intel_psr.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/intel_psr.c b/drivers/gpu/drm/i915/intel_psr.c index afb8b8c..a6028b6 100644 --- a/drivers/gpu/drm/i915/intel_psr.c +++ b/drivers/gpu/drm/i915/intel_psr.c @@ -597,6 +597,12 @@ void intel_psr_flush(struct drm_device *dev, struct drm_i915_private *dev_priv = dev-dev_private; struct drm_crtc *crtc; enum pipe pipe; + /* On HSW/BDW Hardware controls idle_frames to go to PSR entry state + * However on VLV we go to PSR active state with psr work. So let's + * wait more time. The main reason is to give more time when primary + * plane is getting enabled avoiding blank screens. + */ + int delay = msecs_to_jiffies(HAS_DDI(dev) ? 100 : 5000); Same question as before: Shouldn't we look at vbt perhaps like on ddi platforms to compute a reasonable delay? 5s is kinda not reasonable I think ;-) I agree 5s is an eternity here... to be honest I was using 1 sec here but when sending the patch I reused the one I had sent before without noticing it was 5. Anyway, what kind of value at vbt do you suggest? If we use the tp1 wake for instance that is 50 here in multiple of 100 it will end up in 5 sec again. Another thing that I notice today is that I also got that sink crc issue on BDW. So it isn't only on VLV/CHV that we are unable to use sink crc for test. I still believe that is the link fully off that is making us to be unable to get panel crc when in psr. But there is hope. I tested different values here and if we use 400 ms I could use automated kms_psr_sink_crc here... with 300 and bellow it fails. The issue is that I'm not sure if it depends on different panel so I'll test this value on other platforms and different panels. Do you think 500ms reasonable to apply for all platforms in order to keep automated tests and everything working well? What do you think? Thanks, Rodrigo. -Daniel mutex_lock(dev_priv-psr.lock); if (!dev_priv-psr.enabled) { @@ -629,7 +635,7 @@ void intel_psr_flush(struct drm_device *dev, if (!dev_priv-psr.active !dev_priv-psr.busy_frontbuffer_bits) schedule_delayed_work(dev_priv-psr.work, - msecs_to_jiffies(100)); + msecs_to_jiffies(delay)); mutex_unlock(dev_priv-psr.lock); } -- 1.9.3 ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx -- Rodrigo Vivi Blog: http://blog.vivi.eng.br ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx