Re: [Intel-gfx] [PATCH 09/18] drm/i915: make PM interrupt writes non-destructive
On Tue, 6 Nov 2012 16:25:33 +, Ben Widawsky b...@bwidawsk.net wrote: diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8f15616..5477454 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev) I915_WRITE(GEN6_RC_CONTROL, 0); I915_WRITE(GEN6_RPNSWREQ, 1 31); I915_WRITE(GEN6_PMINTRMSK, 0x); - I915_WRITE(GEN6_PMIER, 0); + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) ~GEN6_PM_RPS_EVENTS); /* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev) dev_priv-rps.pm_iir = 0; spin_unlock_irq(dev_priv-rps.lock); - I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR)); + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) ~GEN6_PM_RPS_EVENTS); } int intel_enable_rc6(const struct drm_device *dev) @@ -2518,10 +2518,10 @@ static void gen6_enable_rps(struct drm_device *dev) gen6_set_rps(dev_priv-dev, (gt_perf_status 0xff00) 8); /* requires MSI enabled */ - I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS); spin_lock_irq(dev_priv-rps.lock); WARN_ON(dev_priv-rps.pm_iir != 0); - I915_WRITE(GEN6_PMIMR, 0); + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) GEN6_PM_RPS_EVENTS); spin_unlock_irq(dev_priv-rps.lock); /* enable all PM interrupts */ I915_WRITE(GEN6_PMINTRMSK, 0); Totally confused. IER = IER ~RPS // only disable the RPS events IIR = IIR RPS // clear the asserted RPS bits IMR != IIR. -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 v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
On Wed, 2012-11-07 at 18:31 +0900, Inki Dae wrote: 2012/11/2 Imre Deak imre.d...@intel.com The patchset adds the missing event_lock when accessing the vblank_event_list in drm_vblank_off() and as preparation for this also fixes a few other issues in the exynos driver. This is also a dependency for Rob Clark's drm_send_vblank_event() rework as that would trigger a warning for the unhold event_lock without this changeset. The exynos changes are only compile tested, the rest is tested on an Intel IVB machine on top of drm-intel-nightly + drm_send_vblank_event() rework, with i-g-t/flip_test. Hi Imre, Works fine. But we should wait for Rob's patch set to be merged to -next, and this may be rebased on top of latest Rob's patch set again. Ok, thanks for checking this. I assume then that this patchset will get merged through your tree. I think Rob's patchset depends on this, so ideally this should go first. Otherwise the i915 driver would trigger the WARN in his patchset due to the unheld event_lock. --Imre ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 09/18] drm/i915: make PM interrupt writes non-destructive
On Wed, 07 Nov 2012 10:17:09 + Chris Wilson ch...@chris-wilson.co.uk wrote: On Tue, 6 Nov 2012 16:25:33 +, Ben Widawsky b...@bwidawsk.net wrote: diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c index 8f15616..5477454 100644 --- a/drivers/gpu/drm/i915/intel_pm.c +++ b/drivers/gpu/drm/i915/intel_pm.c @@ -2354,7 +2354,7 @@ static void gen6_disable_rps(struct drm_device *dev) I915_WRITE(GEN6_RC_CONTROL, 0); I915_WRITE(GEN6_RPNSWREQ, 1 31); I915_WRITE(GEN6_PMINTRMSK, 0x); - I915_WRITE(GEN6_PMIER, 0); + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) ~GEN6_PM_RPS_EVENTS); /* Complete PM interrupt masking here doesn't race with the rps work * item again unmasking PM interrupts because that is using a different * register (PMIMR) to mask PM interrupts. The only risk is in leaving @@ -2364,7 +2364,7 @@ static void gen6_disable_rps(struct drm_device *dev) dev_priv-rps.pm_iir = 0; spin_unlock_irq(dev_priv-rps.lock); - I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR)); + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) ~GEN6_PM_RPS_EVENTS); } int intel_enable_rc6(const struct drm_device *dev) @@ -2518,10 +2518,10 @@ static void gen6_enable_rps(struct drm_device *dev) gen6_set_rps(dev_priv-dev, (gt_perf_status 0xff00) 8); /* requires MSI enabled */ - I915_WRITE(GEN6_PMIER, GEN6_PM_DEFERRED_EVENTS); + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS); spin_lock_irq(dev_priv-rps.lock); WARN_ON(dev_priv-rps.pm_iir != 0); - I915_WRITE(GEN6_PMIMR, 0); + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) GEN6_PM_RPS_EVENTS); spin_unlock_irq(dev_priv-rps.lock); /* enable all PM interrupts */ I915_WRITE(GEN6_PMINTRMSK, 0); Totally confused. IER = IER ~RPS // only disable the RPS events IIR = IIR RPS // clear the asserted RPS bits IMR != IIR. -Chris So yes, there is a bug here I think. To make sure you aren't referring to something else though I'll elaborate, and add this to the commit message in v2. At preinstall all interrupts are masked and disabled. That will only happen at driver load time. Then the above code enable/disable rc6 happens also at resume, so a little more care is taken. The IMR is only touched by the workqueue, so enable/disable touch IER and IIR. Disable (there is a bug here you found): Disable RPS events, optionally clear IIR RPS interrupts. + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) ~GEN6_PM_RPS_EVENTS); should be either nothing at all (since we clear on enable anyway), or: + I915_WRITE(GEN6_PMIIR, GEN6_PM_RPS_EVENTS); Enable (this is okay as it stands, I think): Enable RPS events, clear IIR RPS interrupts. + I915_WRITE(GEN6_PMIER, I915_READ(GEN6_PMIER) | GEN6_PM_RPS_EVENTS); + I915_WRITE(GEN6_PMIIR, I915_READ(GEN6_PMIIR) GEN6_PM_RPS_EVENTS); -- Ben Widawsky, 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 00/18] [RFC] Introduce the Haswell VECS
The basic usage of this feature is already upstream in libva. git://anongit.freedesktop.org/vaapi/intel-driver On Tue, 6 Nov 2012 16:25:24 + Ben Widawsky b...@bwidawsk.net wrote: The VECS is a new command streamer introduced in Haswell which allows offloading of video post processing to a new engine called the VEBOX. Like other rings, it is up to userspace to take advantage of it. Also like the other rings, the primary enabling is turning on interrupts, adding the new semaphores, and other basic bits. Somewhat more complicated than the other rings, the VECS has its interrupts in the PMIMR, so as a result, I did a bunch of interrupt naming cleanups because I thought it made more sense. Coming separately will be the basic i-g-t test cases. I still have some work to do on those, but figure that the review can go on in parallel. In terms of the work history, these started as patches by me, finished by Haihao, and then cleaned up and rebased by me. I tried to leave credit to Haihao where applicable, but I rewrote a lot of his stuff, so blame me if something is bad. Ben Widawsky (14): drm/i915: Comments for semaphore clarification drm/i915: Semaphore MBOX update generalization drm/i915: Introduce VECS: the 4th ring drm/i915: Add VECS semaphore bits drm/i915: Rename ring flush functions drm/i915: Vebox ringbuffer init drm/i915: Create a more generic pm handler for hsw+ drm/i915: make PM interrupt writes non-destructive drm/i915: Create an ivybridge_irq_preinstall drm/i915: Add PM regs to pre install drm/i915: Convert irq_refounct to struct drm/i915: consolidate interrupt naming scheme drm/i915: vebox interrupt get/put drm/i915: Enable vebox interrupts Xiang, Haihao (4): drm/i915: add HAS_VEBOX drm/i915: add VEBOX into debugfs drm/i915: add I915_EXEC_VEBOX to i915_gem_do_execbuffer() drm/i915: add I915_PARAM_HAS_VEBOX to i915_getparam drivers/gpu/drm/i915/i915_debugfs.c| 13 ++ drivers/gpu/drm/i915/i915_dma.c| 5 +- drivers/gpu/drm/i915/i915_drv.c| 2 + drivers/gpu/drm/i915/i915_drv.h| 3 + drivers/gpu/drm/i915/i915_gem.c| 8 ++ drivers/gpu/drm/i915/i915_gem_execbuffer.c | 9 ++ drivers/gpu/drm/i915/i915_irq.c| 153 +-- drivers/gpu/drm/i915/i915_reg.h| 148 --- drivers/gpu/drm/i915/intel_pm.c| 8 +- drivers/gpu/drm/i915/intel_ringbuffer.c| 188 + drivers/gpu/drm/i915/intel_ringbuffer.h| 13 +- include/uapi/drm/i915_drm.h| 3 +- 12 files changed, 383 insertions(+), 170 deletions(-) -- Ben Widawsky, 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 v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
2012/11/2 Imre Deak imre.d...@intel.com The patchset adds the missing event_lock when accessing the vblank_event_list in drm_vblank_off() and as preparation for this also fixes a few other issues in the exynos driver. This is also a dependency for Rob Clark's drm_send_vblank_event() rework as that would trigger a warning for the unhold event_lock without this changeset. The exynos changes are only compile tested, the rest is tested on an Intel IVB machine on top of drm-intel-nightly + drm_send_vblank_event() rework, with i-g-t/flip_test. Hi Imre, Works fine. But we should wait for Rob's patch set to be merged to -next, and this may be rebased on top of latest Rob's patch set again. Thanks, Inki Dae In v2: - Instead of fixing the event_lock vs. vblank_time_lock lockdep issue in drm_handle_vblank(), fix it in the exynos driver. This looks like the more logical thing to do and this way we also have a smaller impact on the rest of DRM code. Imre Deak (4): drm/exynos: hold event_lock while accessing pageflip_event_list drm/exynos: call drm_vblank_put for each queued flip event drm/exynos: fix lockdep for event_lock wrt. vbl_time_lock drm: hold event_lock while accessing vblank_event_list drivers/gpu/drm/drm_irq.c|3 +++ drivers/gpu/drm/exynos/exynos_drm_crtc.c |5 + drivers/gpu/drm/exynos/exynos_drm_fimd.c | 20 +--- drivers/gpu/drm/exynos/exynos_drm_vidi.c | 20 +--- drivers/gpu/drm/exynos/exynos_mixer.c| 11 +-- 5 files changed, 11 insertions(+), 48 deletions(-) -- 1.7.9.5 ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH 01/18] drm/i915: Comments for semaphore clarification
On Tue, 06 Nov 2012, Ben Widawsky b...@bwidawsk.net wrote: Semaphores are tied very closely to the rings in the GPU. Trivial patch adds comments to the existing code so that when we add new rings we can include comments there as well. It also helps distinguish the ring to semaphore mailbox interactions by using the ringname in the semaphore data structures. This patch should have no functional impact. Reviewed-by: Jani Nikula jani.nik...@intel.com A subset of this patch was: Reviewed-by: Chris Wilson ch...@chris-wilson.co.uk Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_reg.h | 12 ++-- drivers/gpu/drm/i915/intel_ringbuffer.c | 18 +- drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 3 files changed, 16 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index 9118bd1..f82755e 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -252,12 +252,12 @@ #define MI_SEMAPHORE_UPDATE (121) #define MI_SEMAPHORE_COMPARE(120) #define MI_SEMAPHORE_REGISTER (118) -#define MI_SEMAPHORE_SYNC_RV(216) -#define MI_SEMAPHORE_SYNC_RB(016) -#define MI_SEMAPHORE_SYNC_VR(016) -#define MI_SEMAPHORE_SYNC_VB(216) -#define MI_SEMAPHORE_SYNC_BR(216) -#define MI_SEMAPHORE_SYNC_BV(016) +#define MI_SEMAPHORE_SYNC_RB(016) /* RCS wait for BCS (BRSYNC) */ +#define MI_SEMAPHORE_SYNC_RV(216) /* RCS wait for VCS (VRSYNC) */ +#define MI_SEMAPHORE_SYNC_VR(016) /* VCS wait for RCS (RVSYNC) */ +#define MI_SEMAPHORE_SYNC_VB(216) /* VCS wait for BCS (BVSYNC) */ +#define MI_SEMAPHORE_SYNC_BV(016) /* BCS wait for VCS (VBSYNC) */ +#define MI_SEMAPHORE_SYNC_BR(216) /* BCS wait for RCS (RBSYNC) */ #define MI_SEMAPHORE_SYNC_INVALID (10) /* * 3D instructions used by the kernel diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index a035ac2..423948f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1503,9 +1503,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring-irq_enable_mask = GT_USER_INTERRUPT; ring-get_seqno = gen6_ring_get_seqno; ring-sync_to = gen6_ring_sync; - ring-semaphore_register[0] = MI_SEMAPHORE_SYNC_INVALID; - ring-semaphore_register[1] = MI_SEMAPHORE_SYNC_RV; - ring-semaphore_register[2] = MI_SEMAPHORE_SYNC_RB; + ring-semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID; + ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV; + ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB; ring-signal_mbox[0] = GEN6_VRSYNC; ring-signal_mbox[1] = GEN6_BRSYNC; } else if (IS_GEN5(dev)) { @@ -1639,9 +1639,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ring-irq_put = gen6_ring_put_irq; ring-dispatch_execbuffer = gen6_ring_dispatch_execbuffer; ring-sync_to = gen6_ring_sync; - ring-semaphore_register[0] = MI_SEMAPHORE_SYNC_VR; - ring-semaphore_register[1] = MI_SEMAPHORE_SYNC_INVALID; - ring-semaphore_register[2] = MI_SEMAPHORE_SYNC_VB; + ring-semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR; + ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID; + ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB; ring-signal_mbox[0] = GEN6_RVSYNC; ring-signal_mbox[1] = GEN6_BVSYNC; } else { @@ -1684,9 +1684,9 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) ring-irq_put = gen6_ring_put_irq; ring-dispatch_execbuffer = gen6_ring_dispatch_execbuffer; ring-sync_to = gen6_ring_sync; - ring-semaphore_register[0] = MI_SEMAPHORE_SYNC_BR; - ring-semaphore_register[1] = MI_SEMAPHORE_SYNC_BV; - ring-semaphore_register[2] = MI_SEMAPHORE_SYNC_INVALID; + ring-semaphore_register[RCS] = MI_SEMAPHORE_SYNC_BR; + ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_BV; + ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_INVALID; ring-signal_mbox[0] = GEN6_RBSYNC; ring-signal_mbox[1] = GEN6_VBSYNC; ring-init = init_ring_common; diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.h b/drivers/gpu/drm/i915/intel_ringbuffer.h index 5af65b8..df1a0a2 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.h +++ b/drivers/gpu/drm/i915/intel_ringbuffer.h @@ -89,7 +89,7 @@ struct intel_ring_buffer { struct intel_ring_buffer *to, u32 seqno); - u32 semaphore_register[3]; /*our mbox written
Re: [Intel-gfx] [PATCH 02/18] drm/i915: Semaphore MBOX update generalization
On Tue, 06 Nov 2012, Ben Widawsky b...@bwidawsk.net wrote: This replaces the existing MBOX update code with a more generalized calculation for emitting mbox updates. We also create a sentinel for doing the updates so we can more abstractly deal with the rings. When doing MBOX updates the code must be aware of the /other/ rings. Until now the platforms which supported semaphores had a fixed number of rings and so it made sense for the code to be very specialized (hardcoded). The patch does contain a functional change, but should have no behavioral changes. Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_reg.h | 1 + drivers/gpu/drm/i915/intel_ringbuffer.c | 40 - drivers/gpu/drm/i915/intel_ringbuffer.h | 2 +- 3 files changed, 27 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h index f82755e..d220410 100644 --- a/drivers/gpu/drm/i915/i915_reg.h +++ b/drivers/gpu/drm/i915/i915_reg.h @@ -438,6 +438,7 @@ #define GEN6_VRSYNC (RING_SYNC_1(GEN6_BSD_RING_BASE)) #define GEN6_VBSYNC (RING_SYNC_0(GEN6_BSD_RING_BASE)) #define GEN6_BRSYNC (RING_SYNC_0(BLT_RING_BASE)) +#define GEN6_NOSYNC 0 #define GEN6_BVSYNC (RING_SYNC_1(BLT_RING_BASE)) #define RING_MAX_IDLE(base) ((base)+0x54) #define RING_HWS_PGA(base) ((base)+0x80) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 423948f..ace1176 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -558,12 +558,14 @@ update_mboxes(struct intel_ring_buffer *ring, u32 seqno, u32 mmio_offset) { +#define MBOX_UPDATE_DWORDS 4 intel_ring_emit(ring, MI_SEMAPHORE_MBOX | MI_SEMAPHORE_GLOBAL_GTT | MI_SEMAPHORE_REGISTER | MI_SEMAPHORE_UPDATE); intel_ring_emit(ring, seqno); intel_ring_emit(ring, mmio_offset); + intel_ring_emit(ring, MI_NOOP); I take it this is the functional change you refer to in the commit message, but the reason why is missing. } /** @@ -579,21 +581,26 @@ static int gen6_add_request(struct intel_ring_buffer *ring, u32 *seqno) { - u32 mbox1_reg; - u32 mbox2_reg; - int ret; + struct drm_device *dev = ring-dev; + struct drm_i915_private *dev_priv = dev-dev_private; + struct intel_ring_buffer *useless; + int i, ret; - ret = intel_ring_begin(ring, 10); + ret = intel_ring_begin(ring, ((I915_NUM_RINGS-1) * + MBOX_UPDATE_DWORDS) + + 4); My only (possibly unwarranted) worry about this patch is that this and the for_each_ring below implicitly expect each ring-signal_mbox to have *exactly* one GEN6_NOSYNC, and there are no comments or checks about it. How does it blow up if that is not the case? Other than the comments above, Reviewed-by: Jani Nikula jani.nik...@intel.com if (ret) return ret; - - mbox1_reg = ring-signal_mbox[0]; - mbox2_reg = ring-signal_mbox[1]; +#undef MBOX_UPDATE_DWORDS *seqno = i915_gem_next_request_seqno(ring); - update_mboxes(ring, *seqno, mbox1_reg); - update_mboxes(ring, *seqno, mbox2_reg); + for_each_ring(useless, dev_priv, i) { + u32 mbox_reg = ring-signal_mbox[i]; + if (mbox_reg != GEN6_NOSYNC) + update_mboxes(ring, *seqno, mbox_reg); + } + intel_ring_emit(ring, MI_STORE_DWORD_INDEX); intel_ring_emit(ring, I915_GEM_HWS_INDEX MI_STORE_DWORD_INDEX_SHIFT); intel_ring_emit(ring, *seqno); @@ -1506,8 +1513,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev) ring-semaphore_register[RCS] = MI_SEMAPHORE_SYNC_INVALID; ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_RV; ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_RB; - ring-signal_mbox[0] = GEN6_VRSYNC; - ring-signal_mbox[1] = GEN6_BRSYNC; + ring-signal_mbox[RCS] = GEN6_NOSYNC; + ring-signal_mbox[VCS] = GEN6_VRSYNC; + ring-signal_mbox[BCS] = GEN6_BRSYNC; } else if (IS_GEN5(dev)) { ring-add_request = pc_render_add_request; ring-flush = gen4_render_ring_flush; @@ -1642,8 +1650,9 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) ring-semaphore_register[RCS] = MI_SEMAPHORE_SYNC_VR; ring-semaphore_register[VCS] = MI_SEMAPHORE_SYNC_INVALID; ring-semaphore_register[BCS] = MI_SEMAPHORE_SYNC_VB; - ring-signal_mbox[0] = GEN6_RVSYNC; - ring-signal_mbox[1] = GEN6_BVSYNC; + ring-signal_mbox[RCS] = GEN6_RVSYNC; + ring-signal_mbox[VCS] = GEN6_NOSYNC; +
Re: [Intel-gfx] [PATCH 17/18] drm/i915: Use a slab for object allocation
On Mon, 05 Nov 2012 20:57:05 + Chris Wilson ch...@chris-wilson.co.uk wrote: On Mon, 5 Nov 2012 17:49:35 +, Ben Widawsky b...@bwidawsk.net wrote: On Fri, 19 Oct 2012 18:03:23 +0100 Chris Wilson ch...@chris-wilson.co.uk wrote: The primary purpose of this was to debug some use-after-free memory corruption that was causing an OOPS inside drm/i915. As it turned out the corruption was being caused elsewhere and i915.ko as a major user of many objects was being hit hardest. Indeed as we do frequent the generic kmalloc caches, dedicating one to ourselves (or at least naming one for us depending upon the core) aids debugging our own slab usage. Signed-off-by: Chris Wilson ch...@chris-wilson.co.uk Reviewed-by: Jesse Barnes jbar...@virtuousgeek.org So I previously acked this patch, but you dropped that, so now you get more words. Why not go all the way and move all allocations we do in i915 to this slab (adding a flag with whether or not to zero the memory first)? While I agree in terms of space, nothing takes up as much as objects, I think it would be nice to do this so we can totally track what our driver is doing. In fact I would suggest this is something which core drm should provide to all of the drivers (we already use drm_alloc calls in some places). I thought the slab cache was a fixed size, so presumably you mean introduce a slab per object we allocate? Of which the only other interesting one is the requests. And we don't tend to have many requests pending at any one time (outside of exceptional error cases), unlike the thousands of bo that tend to be allocated. So is it worth dedicating a separate slab to the few but frequently allocated requests? -Chris I guess I wasn't really thinking about the details. I knew in the back of my mind that the slab cache was a fixed size, but was thinking we could have a generically large enough object for most of our other things (I think they're mostly small except for dev_priv). But, now thinking about it a bit, I guess that wastes the memory we're trying to scrutinize, so it's probably silly. In any case, this made me review kmem_cache_create a bit, so we can bump this patch to: Reviewed-by: Ben Widawsky b...@bwidawsk.net -- Ben Widawsky, 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 05/18] drm/i915: Rename ring flush functions
On Tue, 06 Nov 2012, Ben Widawsky b...@bwidawsk.net wrote: Historically we considered the render ring to have special flush semantics and everything else to fall under a more general umbrella. Probably by coincidence more than anything we decided to make the bsd ring have the default *other* flush. As the new vebox ring exposes, the bsd ring is actually the weird one. Doing this allows us to call gen6_ring_flush for the vebox because calling blt_ring_flush would be weird... This patch should have no functional change. Reviewed-by: Jani Nikula jani.nik...@intel.com Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/intel_ringbuffer.c | 12 ++-- 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 2d0c3bb..11f3698 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -1390,8 +1390,8 @@ static void gen6_bsd_ring_write_tail(struct intel_ring_buffer *ring, _MASKED_BIT_DISABLE(GEN6_BSD_SLEEP_MSG_DISABLE)); } -static int gen6_ring_flush(struct intel_ring_buffer *ring, -u32 invalidate, u32 flush) +static int gen6_bsd_ring_flush(struct intel_ring_buffer *ring, +u32 invalidate, u32 flush) { uint32_t cmd; int ret; @@ -1462,8 +1462,8 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, /* Blitter support (SandyBridge+) */ -static int blt_ring_flush(struct intel_ring_buffer *ring, - u32 invalidate, u32 flush) +static int gen6_ring_flush(struct intel_ring_buffer *ring, +u32 invalidate, u32 flush) { uint32_t cmd; int ret; @@ -1640,7 +1640,7 @@ int intel_init_bsd_ring_buffer(struct drm_device *dev) /* gen6 bsd needs a special wa for tail updates */ if (IS_GEN6(dev)) ring-write_tail = gen6_bsd_ring_write_tail; - ring-flush = gen6_ring_flush; + ring-flush = gen6_bsd_ring_flush; ring-add_request = gen6_add_request; ring-get_seqno = gen6_ring_get_seqno; ring-irq_enable_mask = GEN6_BSD_USER_INTERRUPT; @@ -1688,7 +1688,7 @@ int intel_init_blt_ring_buffer(struct drm_device *dev) ring-mmio_base = BLT_RING_BASE; ring-write_tail = ring_write_tail; - ring-flush = blt_ring_flush; + ring-flush = gen6_ring_flush; ring-add_request = gen6_add_request; ring-get_seqno = gen6_ring_get_seqno; ring-irq_enable_mask = GEN6_BLITTER_USER_INTERRUPT; -- 1.8.0 ___ 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
Re: [Intel-gfx] [PATCH 06/18] drm/i915: add HAS_VEBOX
On Tue, 06 Nov 2012, Ben Widawsky b...@bwidawsk.net wrote: From: Xiang, Haihao haihao.xi...@intel.com The flag will be useful to help share code between IVB, and HSW as the programming is similar in many places with this as one of the major differences. Reviewed-by: Jani Nikula jani.nik...@intel.com Signed-off-by: Xiang, Haihao haihao.xi...@intel.com [Commit message + small fix by] Signed-off-by: Ben Widawsky b...@bwidawsk.net --- drivers/gpu/drm/i915/i915_dma.c | 2 +- drivers/gpu/drm/i915/i915_drv.c | 2 ++ drivers/gpu/drm/i915/i915_drv.h | 3 +++ 3 files changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/i915_dma.c b/drivers/gpu/drm/i915/i915_dma.c index 1eea5be..80511fc 100644 --- a/drivers/gpu/drm/i915/i915_dma.c +++ b/drivers/gpu/drm/i915/i915_dma.c @@ -1442,7 +1442,7 @@ static void i915_dump_device_info(struct drm_i915_private *dev_priv) #define DEV_INFO_FLAG(name) info-name ? #name , : #define DEV_INFO_SEP , DRM_DEBUG_DRIVER(i915 device info: gen=%i, pciid=0x%04x flags= - %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s, + %s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s%s, info-gen, dev_priv-dev-pdev-device, DEV_INFO_FLAGS); diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c index f8ba5fe..106becf5c 100644 --- a/drivers/gpu/drm/i915/i915_drv.c +++ b/drivers/gpu/drm/i915/i915_drv.c @@ -287,6 +287,7 @@ static const struct intel_device_info intel_haswell_d_info = { .has_blt_ring = 1, .has_llc = 1, .has_force_wake = 1, + .has_vebox_ring = 1, }; static const struct intel_device_info intel_haswell_m_info = { @@ -296,6 +297,7 @@ static const struct intel_device_info intel_haswell_m_info = { .has_blt_ring = 1, .has_llc = 1, .has_force_wake = 1, + .has_vebox_ring = 1, }; static const struct pci_device_id pciidlist[] = {/* aka */ diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h index f316916..c103b9d 100644 --- a/drivers/gpu/drm/i915/i915_drv.h +++ b/drivers/gpu/drm/i915/i915_drv.h @@ -311,6 +311,7 @@ struct drm_i915_gt_funcs { DEV_INFO_FLAG(supports_tv) DEV_INFO_SEP \ DEV_INFO_FLAG(has_bsd_ring) DEV_INFO_SEP \ DEV_INFO_FLAG(has_blt_ring) DEV_INFO_SEP \ + DEV_INFO_FLAG(has_vebox_ring) DEV_INFO_SEP \ DEV_INFO_FLAG(has_llc) struct intel_device_info { @@ -339,6 +340,7 @@ struct intel_device_info { u8 has_bsd_ring:1; u8 has_blt_ring:1; u8 has_llc:1; + u8 has_vebox_ring:1; }; #define I915_PPGTT_PD_ENTRIES 512 @@ -1177,6 +1179,7 @@ struct drm_i915_file_private { #define HAS_BSD(dev)(INTEL_INFO(dev)-has_bsd_ring) #define HAS_BLT(dev)(INTEL_INFO(dev)-has_blt_ring) +#define HAS_VEBOX(dev) (INTEL_INFO(dev)-has_vebox_ring) #define HAS_LLC(dev)(INTEL_INFO(dev)-has_llc) #define I915_NEED_GFX_HWS(dev) (INTEL_INFO(dev)-need_gfx_hws) -- 1.8.0 ___ 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
[Intel-gfx] REg: Doubt in drm kernel driver
Hi all, I am working on a project related to MFX part of Gen7 GPU. I am working on dumping the input commands and data being passed to MFX pipeline while decoding. And to dump the decoded output data from the MFX pipeline. I am using mplayer to decode the h264 file with vaapi support to use the MFX pipeline. I have modified the intel_driver code - gen7_mfd.c file, and I am able to dump the input commands and data being sent to the MFX into a file. Now I need to dump the decoded output of the MFX pipeline. I am not able to find it in user space intel-driver and drm driver code. I believe we need to modify the drm kernel driver code for dumping the decoded output data. It will be really helpful if anyone can provide me information on where to modify the drm kernel driver code to dump the following data 1. The output of the current decoded frame. 2. The output of the decoded motion vectors of the current frame. The decoded motion vector will be stored in address passed in MFX_AVC_DIRECTMODE_STATE command. 3. The motion vector information for the reference frame needed for decoding the current frame. This information will be in the address passed in MFX_AVC_REF_IDX_STATE command. 4. The decoder reference picture data used for motion compensation. These data can be read from the MFX_PIPE_BUF_ADDR_STATE command. Please let me know if you have any questions. Thanks in advance. Srinath ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] S3 causing IRQ delivery mismatch - i915 hotplug storm
I'm trying to debug an issue on an older lapop (Toshiba Satellite A505) - that has an i3 processor (M330) - and intel graphics. This is running under Xen-unstable, and a 3.7-rc4 pvops kernel - but can also be reproduced using kernels as old as 3.2.23 - and hypervisors as old as 4.0.4 (I have cross posted here, because I am not yet sure if this is a Xen, pvops, or i915 issue - and would appreciate opinions in sorting it out.) The symptoms of the problem exhibit themselves by a lagging mouse in X11 after resume, when using the trackpad. After digging in a bit, the problem seems a bit more insidious - the i915 kworker responsible for hotplug detection (i915_hotplug_work_func) seems to be getting triggered for every PS2 IRQ - every trackpad mouse movement, and keypress triggers tracing (with drm.debug=0x06) like the following: Nov 6 21:41:58 rusting kernel: [ 263.924454] [drm:i915_hotplug_work_func], running encoder hotplug functions Nov 6 21:41:58 rusting kernel: [ 263.924468] [drm:intel_ironlake_crt_detect_hotplug], ironlake hotplug adpa=0xf40018, result 0 Nov 6 21:41:58 rusting kernel: [ 263.924472] [drm:intel_crt_detect], CRT not detected via hotplug Nov 6 21:41:58 rusting kernel: [ 263.924475] [drm:output_poll_execute], [CONNECTOR:11:VGA-1] status updated from 2 to 2 Nov 6 21:41:58 rusting kernel: [ 263.926771] [drm:drm_do_probe_ddc_edid], drm: skipping non-existent adapter i915 gmbus dpc Nov 6 21:41:58 rusting kernel: [ 263.926775] [drm:output_poll_execute], [CONNECTOR:14:HDMI-A-1] status updated from 2 to 2 Nov 6 21:41:58 rusting kernel: [ 263.927291] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5145003e Nov 6 21:41:58 rusting kernel: [ 263.944207] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5145003e Nov 6 21:41:58 rusting kernel: [ 263.964201] [drm:intel_dp_aux_ch], dp_aux_ch timeout status 0x5145003e Nov 6 21:41:58 rusting kernel: [ 263.983694] [drm:intel_dp_detect], DPCD: Nov 6 21:41:58 rusting kernel: [ 263.983704] [drm:output_poll_execute], [CONNECTOR:17:DP-1] status updated from 2 to 2 Additionally, this same trace stack is printed out at a regular 10s interval, after resume - where prior to resuming from S3 it is printed out once at boot time. This kworker consumes a significant portion of the cpu, and essentially grinds Xorg to a halt, until the probing can catch up with the user moving the cursor. There seems to be a mismatch for these IRQ delivery - or at least exhibits the behavior similar to such a problem. Does anyone have any thoughts as to where in the software stack I should start to dig in? Any opinions on which component likely contains the issue is appreciated. /btg ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
2012/11/7 Imre Deak imre.d...@intel.com On Wed, 2012-11-07 at 18:31 +0900, Inki Dae wrote: 2012/11/2 Imre Deak imre.d...@intel.com The patchset adds the missing event_lock when accessing the vblank_event_list in drm_vblank_off() and as preparation for this also fixes a few other issues in the exynos driver. This is also a dependency for Rob Clark's drm_send_vblank_event() rework as that would trigger a warning for the unhold event_lock without this changeset. The exynos changes are only compile tested, the rest is tested on an Intel IVB machine on top of drm-intel-nightly + drm_send_vblank_event() rework, with i-g-t/flip_test. Hi Imre, Works fine. But we should wait for Rob's patch set to be merged to -next, and this may be rebased on top of latest Rob's patch set again. Ok, thanks for checking this. I assume then that this patchset will get merged through your tree. I think Rob's patchset depends on this, so ideally this should go first. Otherwise the i915 driver would trigger the WARN in his patchset due to the unheld event_lock. Ok, but I merge it first, shouldn't Rob's patch set be rebased? Anyway this is minor issue so I could resolve it. And it seems like that your patch set has no dependency of Rob's. I mean that your patch set worked fine without Rob's. Thanks, Inki Dae --Imre ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] [PATCH v2 0/4] drm/exynos, intel: fix locking for flip/vbl event list
On Wed, Nov 7, 2012 at 10:25 AM, Inki Dae inki@samsung.com wrote: 2012/11/7 Imre Deak imre.d...@intel.com On Wed, 2012-11-07 at 18:31 +0900, Inki Dae wrote: 2012/11/2 Imre Deak imre.d...@intel.com The patchset adds the missing event_lock when accessing the vblank_event_list in drm_vblank_off() and as preparation for this also fixes a few other issues in the exynos driver. This is also a dependency for Rob Clark's drm_send_vblank_event() rework as that would trigger a warning for the unhold event_lock without this changeset. The exynos changes are only compile tested, the rest is tested on an Intel IVB machine on top of drm-intel-nightly + drm_send_vblank_event() rework, with i-g-t/flip_test. Hi Imre, Works fine. But we should wait for Rob's patch set to be merged to -next, and this may be rebased on top of latest Rob's patch set again. Ok, thanks for checking this. I assume then that this patchset will get merged through your tree. I think Rob's patchset depends on this, so ideally this should go first. Otherwise the i915 driver would trigger the WARN in his patchset due to the unheld event_lock. Ok, but I merge it first, shouldn't Rob's patch set be rebased? Anyway this is minor issue so I could resolve it. And it seems like that your patch set has no dependency of Rob's. I mean that your patch set worked fine without Rob's. I think there should be no hard dependency on my patch set.. the only connection is that my patchset without this patch will start showing the WARN_ON() traces BR, -R Thanks, Inki Dae --Imre ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ dri-devel mailing list dri-de...@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/dri-devel ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] S3 causing IRQ delivery mismatch - i915 hotplug storm
On Wed, Nov 7, 2012 at 11:22 AM, Ben Guthro b...@guthro.net wrote: I'm trying to debug an issue on an older lapop (Toshiba Satellite A505) - that has an i3 processor (M330) - and intel graphics. This is running under Xen-unstable, and a 3.7-rc4 pvops kernel - but can also be reproduced using kernels as old as 3.2.23 - and hypervisors as old as 4.0.4 (I have cross posted here, because I am not yet sure if this is a Xen, pvops, or i915 issue - and would appreciate opinions in sorting it out.) This appears to be unrelated to Xen / pvops, at the moment, after some additional debugging, and appears to be an issue with the i915 driver with older hardware. I'll remove xen-devel, and Konrad from future replies to this thread. Additionally, this same trace stack is printed out at a regular 10s interval, after resume - where prior to resuming from S3 it is printed out once at boot time. 10*HZ seems to be the normal hotplug interval, when an interrupt doesn't fire There seems to be a mismatch for these IRQ delivery - or at least exhibits the behavior similar to such a problem. I was mistaken here. The i8042 IRQ would just start up the IRQ handling - but the i915 driver always thinks it has pending work, and schedules it. It seems that the hotplug mask is not getting cleared in pch_iir (in i915_irq.c) Manually clearing this bit with pch_iir = pch_iir ^ hotplug_mask; in the ironlake_irq_handler() function seems to resolve the issue - making it so I don't get the flurry of hotplug work bogging down the system. ...but is this disabling hotplug detection entirely? ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
Re: [Intel-gfx] S3 causing IRQ delivery mismatch - i915 hotplug storm
On Wed, Nov 7, 2012 at 2:43 PM, Ben Guthro b...@guthro.net wrote: There seems to be a mismatch for these IRQ delivery - or at least exhibits the behavior similar to such a problem. I was mistaken here. The i8042 IRQ would just start up the IRQ handling - but the i915 driver always thinks it has pending work, and schedules it. It seems that the hotplug mask is not getting cleared in pch_iir (in i915_irq.c) Manually clearing this bit with pch_iir = pch_iir ^ hotplug_mask; in the ironlake_irq_handler() function seems to resolve the issue - making it so I don't get the flurry of hotplug work bogging down the system. ...but is this disabling hotplug detection entirely? The attached patch seems to resolve the issue of the interrupt storm after S3 for this Ibex Peak PCH system. Could someone comment on whether this will have any unintended side effects? diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c index 32e1bda..d8f2f79 100644 --- a/drivers/gpu/drm/i915/i915_irq.c +++ b/drivers/gpu/drm/i915/i915_irq.c @@ -802,8 +802,13 @@ static irqreturn_t ironlake_irq_handler(DRM_IRQ_ARGS) /* check event from PCH */ if (de_iir DE_PCH_EVENT) { - if (pch_iir hotplug_mask) + if (pch_iir hotplug_mask) { queue_work(dev_priv-wq, dev_priv-hotplug_work); + + if (!HAS_PCH_CPT(dev)) + pch_iir = pch_iir ^ SDE_HOTPLUG_MASK; + } + if (HAS_PCH_CPT(dev)) cpt_irq_handler(dev, pch_iir); else hotplug.patch Description: Binary data ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx
[Intel-gfx] About HD3000(sandybridge) driver on vxWorks
Dear Sir: My name is wenjie zhou.I come from Wuhan China. Now I am writing HD3000(sandybridge) driver on vxWorks.But,there are some problems confusing me for a long time.Please help me!Thank you! My problme is:When I use the blitter engine to realize filling the color to the rectangle,CP will stop in the first instruction(BR00).In the progrm,there are six instuction:br00,br13,br14,br09,br16 and MI_NOP.After allowing the CP to excute ,the Mode Register's bit9(Rings Idle) is set to 0.It means parser is not idle or ring arbiter is not idle.And the Error Status Register (ESR)'s bit0 will be set to 1. It means instruction error.The RING_BUFFER_HEADER register's value is 0x4.So I think the problem happened when the CP excuted the br00 instruction. When the CP excutes flush command, it works very well. I wonder why it happens? Is it relate with cache?PS:this ringbuffer program works very well on the ironlake platform. Thank you! Sincerely yours, Wenjie Zhou ___ Intel-gfx mailing list Intel-gfx@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/intel-gfx