From f01db988ef6f6c70a6cc36ee71e4a98a68901229 Mon Sep 17 00:00:00 2001 From: Sean Paul Date: Fri, 16 Mar 2012 12:43:22 -0400 Subject: drm/i915: Add wait_for in init_ring_common I have seen a number of "blt ring initialization failed" messages where the ctl or start registers are not the correct value. Upon further inspection, if the code just waited a little bit, it would read the correct value. Adding the wait_for to these reads should eliminate the issue. Signed-off-by: Sean Paul Reviewed-by: Ben Widawsky Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index ca3972f2c6f5..b7b50a5b0478 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -290,9 +290,9 @@ static int init_ring_common(struct intel_ring_buffer *ring) | RING_REPORT_64K | RING_VALID); /* If the head is still not zero, the ring is dead */ - if ((I915_READ_CTL(ring) & RING_VALID) == 0 || - I915_READ_START(ring) != obj->gtt_offset || - (I915_READ_HEAD(ring) & HEAD_ADDR) != 0) { + if (wait_for((I915_READ_CTL(ring) & RING_VALID) != 0 && + I915_READ_START(ring) == obj->gtt_offset && + (I915_READ_HEAD(ring) & HEAD_ADDR) == 0, 50)) { DRM_ERROR("%s initialization failed " "ctl %08x head %08x tail %08x start %08x\n", ring->name, -- cgit v1.2.3 From 7e231dbe0c2a4359472f929732d4200479d8f939 Mon Sep 17 00:00:00 2001 From: Jesse Barnes Date: Wed, 28 Mar 2012 13:39:38 -0700 Subject: drm/i915: ValleyView IRQ support ValleyView has a new interrupt architecture; best to put it in a new set of functions. Also make sure the ring mask functions handle ValleyView. FIXME: fix flipping; need to enable interrupts and call prepare/finish Signed-off-by: Jesse Barnes Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index b7b50a5b0478..75081f146390 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -687,7 +687,7 @@ render_ring_get_irq(struct intel_ring_buffer *ring) spin_lock(&ring->irq_lock); if (ring->irq_refcount++ == 0) { - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) ironlake_enable_irq(dev_priv, GT_PIPE_NOTIFY | GT_USER_INTERRUPT); else @@ -706,7 +706,7 @@ render_ring_put_irq(struct intel_ring_buffer *ring) spin_lock(&ring->irq_lock); if (--ring->irq_refcount == 0) { - if (HAS_PCH_SPLIT(dev)) + if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) ironlake_disable_irq(dev_priv, GT_USER_INTERRUPT | GT_PIPE_NOTIFY); -- cgit v1.2.3 From e2a1e2f0242c363ed80458282d67039c373fbb1f Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Thu, 29 Mar 2012 19:11:26 -0700 Subject: drm/i915: ring irq cleanups - gen6 put/get only need one argument rflags and gflags are always the same (see above explanation) - remove a couple redundantly defined IRQs - reordered some lines to make things go in descending order Every ring has its own interrupts, enables, masks, and status bits that are fed into the main interrupt enable/mask/status registers. At one point in time it seemed like a good idea to make our functions support the notion that each interrupt may have a different bit position in the corresponding register (blitter parser error may be bit n in IMR, but bit m in blitter IMR). It turned out though that the HW designers did us a solid on Gen6+ and this unfortunate situation has been avoided. This allows our interrupt code to be cleaned up a bit. I jammed this into one commit because there should be no functional change with this commit, and staging it into multiple commits was unnecessarily artificial IMO. CC: Chris Wilson CC: Jesse Barnes Signed-off-by: Ben Widawsky Reviewed-by: Chris Wilson [danvet: - fixed up merged conflict with vlv changes. - added GEN6 to GT blitter bit, we only use it on gen6+. - added a comment to both ring irq bits and GT irq bits that on gen6+ these alias. - added comment that GT_BSD_USER_INTERRUPT is ilk-only. - I've got confused a bit that we still use GT_USER_INTERRUPT on ivb for the render ring - but this goes back to ilk where we have only gt interrupt bits and so we be equally confusing if changed.] Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 36 +++++++++++---------------------- 1 file changed, 12 insertions(+), 24 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 75081f146390..c7eea7fad16f 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -788,7 +788,7 @@ ring_add_request(struct intel_ring_buffer *ring, } static bool -gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) +gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 mask) { struct drm_device *dev = ring->dev; drm_i915_private_t *dev_priv = dev->dev_private; @@ -803,9 +803,9 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) spin_lock(&ring->irq_lock); if (ring->irq_refcount++ == 0) { - ring->irq_mask &= ~rflag; + ring->irq_mask &= ~mask; I915_WRITE_IMR(ring, ring->irq_mask); - ironlake_enable_irq(dev_priv, gflag); + ironlake_enable_irq(dev_priv, mask); } spin_unlock(&ring->irq_lock); @@ -813,16 +813,16 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) } static void -gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 gflag, u32 rflag) +gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 mask) { struct drm_device *dev = ring->dev; drm_i915_private_t *dev_priv = dev->dev_private; spin_lock(&ring->irq_lock); if (--ring->irq_refcount == 0) { - ring->irq_mask |= rflag; + ring->irq_mask |= mask; I915_WRITE_IMR(ring, ring->irq_mask); - ironlake_disable_irq(dev_priv, gflag); + ironlake_disable_irq(dev_priv, mask); } spin_unlock(&ring->irq_lock); @@ -1376,33 +1376,25 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, static bool gen6_render_ring_get_irq(struct intel_ring_buffer *ring) { - return gen6_ring_get_irq(ring, - GT_USER_INTERRUPT, - GEN6_RENDER_USER_INTERRUPT); + return gen6_ring_get_irq(ring, GT_USER_INTERRUPT); } static void gen6_render_ring_put_irq(struct intel_ring_buffer *ring) { - return gen6_ring_put_irq(ring, - GT_USER_INTERRUPT, - GEN6_RENDER_USER_INTERRUPT); + return gen6_ring_put_irq(ring, GT_USER_INTERRUPT); } static bool gen6_bsd_ring_get_irq(struct intel_ring_buffer *ring) { - return gen6_ring_get_irq(ring, - GT_GEN6_BSD_USER_INTERRUPT, - GEN6_BSD_USER_INTERRUPT); + return gen6_ring_get_irq(ring, GEN6_BSD_USER_INTERRUPT); } static void gen6_bsd_ring_put_irq(struct intel_ring_buffer *ring) { - return gen6_ring_put_irq(ring, - GT_GEN6_BSD_USER_INTERRUPT, - GEN6_BSD_USER_INTERRUPT); + return gen6_ring_put_irq(ring, GEN6_BSD_USER_INTERRUPT); } /* ring buffer for Video Codec for Gen6+ */ @@ -1431,17 +1423,13 @@ static const struct intel_ring_buffer gen6_bsd_ring = { static bool blt_ring_get_irq(struct intel_ring_buffer *ring) { - return gen6_ring_get_irq(ring, - GT_BLT_USER_INTERRUPT, - GEN6_BLITTER_USER_INTERRUPT); + return gen6_ring_get_irq(ring, GEN6_BLITTER_USER_INTERRUPT); } static void blt_ring_put_irq(struct intel_ring_buffer *ring) { - gen6_ring_put_irq(ring, - GT_BLT_USER_INTERRUPT, - GEN6_BLITTER_USER_INTERRUPT); + gen6_ring_put_irq(ring, GEN6_BLITTER_USER_INTERRUPT); } static int blt_ring_flush(struct intel_ring_buffer *ring, -- cgit v1.2.3 From 25c063004a6d0048c4dece74db4da117b9ae623e Mon Sep 17 00:00:00 2001 From: Ben Widawsky Date: Thu, 29 Mar 2012 19:11:27 -0700 Subject: drm/i915: open code gen6+ ring irqs We can now open-code the get/put irq functions as they were just abstracting single register definitions. It would be nice to merge this in with the IRQ handling code... but that is too much work for me at present. In addition I could probably collapse this in to a lot of the Ironlake stuff, but I don't think it's worth the potential regressions. This patch itself should not effect functionality. CC: Jesse Barnes Signed-off-by: Ben Widawsky Reviewed-by: Chris Wilson Signed-off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 62 +++++++++------------------------ 1 file changed, 16 insertions(+), 46 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index c7eea7fad16f..98ac5c0ca37a 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -788,10 +788,11 @@ ring_add_request(struct intel_ring_buffer *ring, } static bool -gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 mask) +gen6_ring_get_irq(struct intel_ring_buffer *ring) { struct drm_device *dev = ring->dev; drm_i915_private_t *dev_priv = dev->dev_private; + u32 mask = ring->irq_enable; if (!dev->irq_enabled) return false; @@ -813,10 +814,11 @@ gen6_ring_get_irq(struct intel_ring_buffer *ring, u32 mask) } static void -gen6_ring_put_irq(struct intel_ring_buffer *ring, u32 mask) +gen6_ring_put_irq(struct intel_ring_buffer *ring) { struct drm_device *dev = ring->dev; drm_i915_private_t *dev_priv = dev->dev_private; + u32 mask = ring->irq_enable; spin_lock(&ring->irq_lock); if (--ring->irq_refcount == 0) { @@ -1373,30 +1375,6 @@ gen6_ring_dispatch_execbuffer(struct intel_ring_buffer *ring, return 0; } -static bool -gen6_render_ring_get_irq(struct intel_ring_buffer *ring) -{ - return gen6_ring_get_irq(ring, GT_USER_INTERRUPT); -} - -static void -gen6_render_ring_put_irq(struct intel_ring_buffer *ring) -{ - return gen6_ring_put_irq(ring, GT_USER_INTERRUPT); -} - -static bool -gen6_bsd_ring_get_irq(struct intel_ring_buffer *ring) -{ - return gen6_ring_get_irq(ring, GEN6_BSD_USER_INTERRUPT); -} - -static void -gen6_bsd_ring_put_irq(struct intel_ring_buffer *ring) -{ - return gen6_ring_put_irq(ring, GEN6_BSD_USER_INTERRUPT); -} - /* ring buffer for Video Codec for Gen6+ */ static const struct intel_ring_buffer gen6_bsd_ring = { .name = "gen6 bsd ring", @@ -1408,8 +1386,9 @@ static const struct intel_ring_buffer gen6_bsd_ring = { .flush = gen6_ring_flush, .add_request = gen6_add_request, .get_seqno = gen6_ring_get_seqno, - .irq_get = gen6_bsd_ring_get_irq, - .irq_put = gen6_bsd_ring_put_irq, + .irq_enable = GEN6_BSD_USER_INTERRUPT, + .irq_get = gen6_ring_get_irq, + .irq_put = gen6_ring_put_irq, .dispatch_execbuffer = gen6_ring_dispatch_execbuffer, .sync_to = gen6_bsd_ring_sync_to, .semaphore_register = {MI_SEMAPHORE_SYNC_VR, @@ -1420,18 +1399,6 @@ static const struct intel_ring_buffer gen6_bsd_ring = { /* Blitter support (SandyBridge+) */ -static bool -blt_ring_get_irq(struct intel_ring_buffer *ring) -{ - return gen6_ring_get_irq(ring, GEN6_BLITTER_USER_INTERRUPT); -} - -static void -blt_ring_put_irq(struct intel_ring_buffer *ring) -{ - gen6_ring_put_irq(ring, GEN6_BLITTER_USER_INTERRUPT); -} - static int blt_ring_flush(struct intel_ring_buffer *ring, u32 invalidate, u32 flush) { @@ -1463,8 +1430,9 @@ static const struct intel_ring_buffer gen6_blt_ring = { .flush = blt_ring_flush, .add_request = gen6_add_request, .get_seqno = gen6_ring_get_seqno, - .irq_get = blt_ring_get_irq, - .irq_put = blt_ring_put_irq, + .irq_get = gen6_ring_get_irq, + .irq_put = gen6_ring_put_irq, + .irq_enable = GEN6_BLITTER_USER_INTERRUPT, .dispatch_execbuffer = gen6_ring_dispatch_execbuffer, .sync_to = gen6_blt_ring_sync_to, .semaphore_register = {MI_SEMAPHORE_SYNC_BR, @@ -1482,8 +1450,9 @@ int intel_init_render_ring_buffer(struct drm_device *dev) if (INTEL_INFO(dev)->gen >= 6) { ring->add_request = gen6_add_request; ring->flush = gen6_render_ring_flush; - ring->irq_get = gen6_render_ring_get_irq; - ring->irq_put = gen6_render_ring_put_irq; + ring->irq_get = gen6_ring_get_irq; + ring->irq_put = gen6_ring_put_irq; + ring->irq_enable = GT_USER_INTERRUPT; ring->get_seqno = gen6_ring_get_seqno; } else if (IS_GEN5(dev)) { ring->add_request = pc_render_add_request; @@ -1506,8 +1475,9 @@ int intel_render_ring_init_dri(struct drm_device *dev, u64 start, u32 size) *ring = render_ring; if (INTEL_INFO(dev)->gen >= 6) { ring->add_request = gen6_add_request; - ring->irq_get = gen6_render_ring_get_irq; - ring->irq_put = gen6_render_ring_put_irq; + ring->irq_get = gen6_ring_get_irq; + ring->irq_put = gen6_ring_put_irq; + ring->irq_enable = GT_USER_INTERRUPT; } else if (IS_GEN5(dev)) { ring->add_request = pc_render_add_request; ring->get_seqno = pc_render_get_seqno; -- cgit v1.2.3 From 901781b997570e55af950348d7cd5cef64fb6d7c Mon Sep 17 00:00:00 2001 From: Daniel Vetter Date: Fri, 30 Mar 2012 20:24:34 +0200 Subject: drm/i915: use render gen to switch ring irq functions Top-level interrupt bits are usually found in the display block. It therefore makes sense to use HAS_PCH_SPLIT in i915_irq.c But the irq stuff in intel_ring.c only concerns itself with render core/gt-level interrupt sources. It therefore makes more sense to switch based on gpu gen. Kills a vlv special case. Reviewed-by: Jesse Barnes Signed-Off-by: Daniel Vetter --- drivers/gpu/drm/i915/intel_ringbuffer.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'drivers/gpu/drm/i915/intel_ringbuffer.c') diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c index 98ac5c0ca37a..465a7da3b30d 100644 --- a/drivers/gpu/drm/i915/intel_ringbuffer.c +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c @@ -687,7 +687,7 @@ render_ring_get_irq(struct intel_ring_buffer *ring) spin_lock(&ring->irq_lock); if (ring->irq_refcount++ == 0) { - if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) + if (INTEL_INFO(dev)->gen >= 5) ironlake_enable_irq(dev_priv, GT_PIPE_NOTIFY | GT_USER_INTERRUPT); else @@ -706,7 +706,7 @@ render_ring_put_irq(struct intel_ring_buffer *ring) spin_lock(&ring->irq_lock); if (--ring->irq_refcount == 0) { - if (HAS_PCH_SPLIT(dev) || IS_VALLEYVIEW(dev)) + if (INTEL_INFO(dev)->gen >= 5) ironlake_disable_irq(dev_priv, GT_USER_INTERRUPT | GT_PIPE_NOTIFY); -- cgit v1.2.3