[PATCH 1/2] drm/i915: Remove BLT workaround for pre-production Sandybridge systems.

[PATCH 1/2] drm/i915: Remove BLT workaround for pre-production Sandybridge systems.

[Kenneth Graunke]

This code was effectively dead, as < rev08 never shipped.

With the workaround gone, blt_ring_init became just another name for
init_ring_common, so delete it and just call init_ring_common directly.

This patch should not change behavior on Gen6 rev >= 8, nor Gen7.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   55 ++-----------------------------
 1 files changed, 3 insertions(+), 52 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ca70e2f..8198ab3 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1345,66 +1345,17 @@ blt_ring_put_irq(struct intel_ring_buffer *ring)
 			  GEN6_BLITTER_USER_INTERRUPT);
 }
 
-
-/* Workaround for some stepping of SNB,
- * each time when BLT engine ring tail moved,
- * the first command in the ring to be parsed
- * should be MI_BATCH_BUFFER_START
- */
-#define NEED_BLT_WORKAROUND(dev) \
-	(IS_GEN6(dev) && (dev->pdev->revision < 8))
-
-static inline struct drm_i915_gem_object *
-to_blt_workaround(struct intel_ring_buffer *ring)
-{
-	return ring->private;
-}
-
-static int blt_ring_init(struct intel_ring_buffer *ring)
-{
-	if (NEED_BLT_WORKAROUND(ring->dev)) {
-		struct drm_i915_gem_object *obj;
-		u32 *ptr;
-		int ret;
-
-		obj = i915_gem_alloc_object(ring->dev, 4096);
-		if (obj == NULL)
-			return -ENOMEM;
-
-		ret = i915_gem_object_pin(obj, 4096, true);
-		if (ret) {
-			drm_gem_object_unreference(&obj->base);
-			return ret;
-		}
-
-		ptr = kmap(obj->pages[0]);
-		*ptr++ = MI_BATCH_BUFFER_END;
-		*ptr++ = MI_NOOP;
-		kunmap(obj->pages[0]);
-
-		ret = i915_gem_object_set_to_gtt_domain(obj, false);
-		if (ret) {
-			i915_gem_object_unpin(obj);
-			drm_gem_object_unreference(&obj->base);
-			return ret;
-		}
-
-		ring->private = obj;
-	}
-
-	return init_ring_common(ring);
-}
-
 static int blt_ring_begin(struct intel_ring_buffer *ring,
 			  int num_dwords)
 {
 	if (ring->private) {
+		struct drm_i915_gem_object *obj = ring->private;
 		int ret = intel_ring_begin(ring, num_dwords+2);
 		if (ret)
 			return ret;
 
 		intel_ring_emit(ring, MI_BATCH_BUFFER_START);
-		intel_ring_emit(ring, to_blt_workaround(ring)->gtt_offset);
+		intel_ring_emit(ring, obj->gtt_offset);
 
 		return 0;
 	} else
@@ -1447,7 +1398,7 @@ static const struct intel_ring_buffer gen6_blt_ring = {
 	.id			= RING_BLT,
 	.mmio_base		= BLT_RING_BASE,
 	.size			= 32 * PAGE_SIZE,
-	.init			= blt_ring_init,
+	.init			= init_ring_common,
 	.write_tail		= ring_write_tail,
 	.flush			= blt_ring_flush,
 	.add_request		= gen6_add_request,
-- 
1.7.7.1

[PATCH 2/2] drm/i915: Remove extraneous MI_BATCHBUFFER_START from the BLT ring.

[Kenneth Graunke]

This looks like it was part of the BLT ring workaround which the prior
patch removed.  However, this is a separate patch because it changes
behavior on shipping platforms.

Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |    4 ----
 1 files changed, 0 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 8198ab3..9f11751 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1353,10 +1353,6 @@ static int blt_ring_begin(struct intel_ring_buffer *ring,
 		int ret = intel_ring_begin(ring, num_dwords+2);
 		if (ret)
 			return ret;
-
-		intel_ring_emit(ring, MI_BATCH_BUFFER_START);
-		intel_ring_emit(ring, obj->gtt_offset);
-
 		return 0;
 	} else
 		return intel_ring_begin(ring, 4);
-- 
1.7.7.1

Re: [PATCH 2/2] drm/i915: Remove extraneous MI_BATCHBUFFER_START from the BLT ring.

[Daniel Vetter]

On Sun, Nov 06, 2011 at 09:51:38AM -0800, Kenneth Graunke wrote:
> This looks like it was part of the BLT ring workaround which the prior
> patch removed.  However, this is a separate patch because it changes
> behavior on shipping platforms.

Nope, this should actually be part of your previous patch. ring->private
for the blt ring is only set for this gen6 preproduction-hw w/a, so this
entire if block should go. You're missing a similar if block in the ring
cleanup.

I've posted that patch a few months back, but due to the Giant Whitespace
Patch haven't yet bothered to forward port it ...

Cheers, Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48

[PATCH] drm/i915/ringbuffer: kill snb blt workaround

[Kenneth Graunke]

From: Daniel Vetter <daniel.vetter@ffwll.ch>

This was just to facilitate product enablement with pre-production hw.
Allows us to kill quite a bit of cruft.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
Reviewed-by: Eric Anholt <eric@anholt.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   81 +------------------------------
 1 files changed, 2 insertions(+), 79 deletions(-)

Thanks Daniel, I missed that!  This is much nicer.

Here's your original patch rebased against drm-intel-next.  I dropped the
first hunk because the render ring now uses the cleanup hook for PIPE_CONTROL.
Otherwise, it's the same.

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index ca70e2f..0602bb6 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -1345,79 +1345,13 @@ blt_ring_put_irq(struct intel_ring_buffer *ring)
 			  GEN6_BLITTER_USER_INTERRUPT);
 }
 
-
-/* Workaround for some stepping of SNB,
- * each time when BLT engine ring tail moved,
- * the first command in the ring to be parsed
- * should be MI_BATCH_BUFFER_START
- */
-#define NEED_BLT_WORKAROUND(dev) \
-	(IS_GEN6(dev) && (dev->pdev->revision < 8))
-
-static inline struct drm_i915_gem_object *
-to_blt_workaround(struct intel_ring_buffer *ring)
-{
-	return ring->private;
-}
-
-static int blt_ring_init(struct intel_ring_buffer *ring)
-{
-	if (NEED_BLT_WORKAROUND(ring->dev)) {
-		struct drm_i915_gem_object *obj;
-		u32 *ptr;
-		int ret;
-
-		obj = i915_gem_alloc_object(ring->dev, 4096);
-		if (obj == NULL)
-			return -ENOMEM;
-
-		ret = i915_gem_object_pin(obj, 4096, true);
-		if (ret) {
-			drm_gem_object_unreference(&obj->base);
-			return ret;
-		}
-
-		ptr = kmap(obj->pages[0]);
-		*ptr++ = MI_BATCH_BUFFER_END;
-		*ptr++ = MI_NOOP;
-		kunmap(obj->pages[0]);
-
-		ret = i915_gem_object_set_to_gtt_domain(obj, false);
-		if (ret) {
-			i915_gem_object_unpin(obj);
-			drm_gem_object_unreference(&obj->base);
-			return ret;
-		}
-
-		ring->private = obj;
-	}
-
-	return init_ring_common(ring);
-}
-
-static int blt_ring_begin(struct intel_ring_buffer *ring,
-			  int num_dwords)
-{
-	if (ring->private) {
-		int ret = intel_ring_begin(ring, num_dwords+2);
-		if (ret)
-			return ret;
-
-		intel_ring_emit(ring, MI_BATCH_BUFFER_START);
-		intel_ring_emit(ring, to_blt_workaround(ring)->gtt_offset);
-
-		return 0;
-	} else
-		return intel_ring_begin(ring, 4);
-}
-
 static int blt_ring_flush(struct intel_ring_buffer *ring,
 			  u32 invalidate, u32 flush)
 {
 	uint32_t cmd;
 	int ret;
 
-	ret = blt_ring_begin(ring, 4);
+	ret = intel_ring_begin(ring, 4);
 	if (ret)
 		return ret;
 
@@ -1432,22 +1366,12 @@ static int blt_ring_flush(struct intel_ring_buffer *ring,
 	return 0;
 }
 
-static void blt_ring_cleanup(struct intel_ring_buffer *ring)
-{
-	if (!ring->private)
-		return;
-
-	i915_gem_object_unpin(ring->private);
-	drm_gem_object_unreference(ring->private);
-	ring->private = NULL;
-}
-
 static const struct intel_ring_buffer gen6_blt_ring = {
 	.name			= "blt ring",
 	.id			= RING_BLT,
 	.mmio_base		= BLT_RING_BASE,
 	.size			= 32 * PAGE_SIZE,
-	.init			= blt_ring_init,
+	.init			= init_ring_common,
 	.write_tail		= ring_write_tail,
 	.flush			= blt_ring_flush,
 	.add_request		= gen6_add_request,
@@ -1455,7 +1379,6 @@ static const struct intel_ring_buffer gen6_blt_ring = {
 	.irq_get		= blt_ring_get_irq,
 	.irq_put		= blt_ring_put_irq,
 	.dispatch_execbuffer	= gen6_ring_dispatch_execbuffer,
-	.cleanup		= blt_ring_cleanup,
 	.sync_to		= gen6_blt_ring_sync_to,
 	.semaphore_register	= {MI_SEMAPHORE_SYNC_BR,
 				   MI_SEMAPHORE_SYNC_BV,
-- 
1.7.7.1

Re: [PATCH] drm/i915/ringbuffer: kill snb blt workaround

[Daniel Vetter]

On Sun, Nov 06, 2011 at 10:50:46AM -0800, Kenneth Graunke wrote:
> From: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> This was just to facilitate product enablement with pre-production hw.
> Allows us to kill quite a bit of cruft.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Kenneth Graunke <kenneth@whitecape.org>
> Reviewed-by: Eric Anholt <eric@anholt.net>
> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c |   81 +------------------------------
>  1 files changed, 2 insertions(+), 79 deletions(-)
> 
> Thanks Daniel, I missed that!  This is much nicer.

Cool, you've found it! I've had a bit a problem with my maze of patches
;-) And thanks a lot for forward porting it through the whitespace
changes.
-Daniel

-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48