[PATCH 0/5] pile of ilk related workarounds

[PATCH 0/5] pile of ilk related workarounds

[Daniel Vetter]

Hi all,

Dave reported a gpu hang regression on his ilk, likely due to the re-enabled rc6
support (but not confirmed). So I've scrounged through the w/a database and
found a few bits. Preliminary verdict is that this seems to prevent the hangs.

If that stands for a few more days, I'd like to merge patches 1-3 to -fixes, the
last two for dinq.

Please review.

Thanks, Daniel

Daniel Vetter (5):
  drm/i915: implement WaIssueDummyWriteToWakupFromRC6
  drm/i915: implement WaDisableRenderCachePipelinedFlush
  drm/i915: WaInsertNoopAfterBatchEndCommand
  drm/i915: implement WaDisableSpriteDestColorKey
  drm/i915: document a few more already implemented ilk w/as

 drivers/gpu/drm/i915/i915_drv.c         |   13 +++++++++++++
 drivers/gpu/drm/i915/i915_reg.h         |    1 +
 drivers/gpu/drm/i915/intel_drv.h        |    2 ++
 drivers/gpu/drm/i915/intel_pm.c         |   11 ++++++++++-
 drivers/gpu/drm/i915/intel_ringbuffer.c |   18 +++++++++++++++---
 drivers/gpu/drm/i915/intel_sprite.c     |   27 +++++++++++++++++++++++----
 6 files changed, 64 insertions(+), 8 deletions(-)

-- 
1.7.10.4

[PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6

[Daniel Vetter]

Or at least our best understanding of it.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..c3f4f04 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg)
 	return true;
 }
 
+static void
+ilk_dummy_write(struct drm_i915_private *dev_priv)
+{
+	/* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before
+	 * touching it for real. MI_MODE is masked, hence harmless to write 0
+	 * into. */
+	I915_WRITE_NOTRACE(MI_MODE, 0);
+}
+
 #define __i915_read(x, y) \
 u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
 	u##x val = 0; \
+	if (IS_GEN5(dev_priv->dev)) \
+		ilk_dummy_write(dev_priv); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		unsigned long irqflags; \
 		spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
@@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
+	if (IS_GEN5(dev_priv->dev)) \
+		ilk_dummy_write(dev_priv); \
 	if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
 		write##y(val, dev_priv->regs + reg + 0x180000);		\
 	} else {							\
-- 
1.7.10.4

Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6

[Chris Wilson]

On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Or at least our best understanding of it.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Looks legit. Just a minor alteration to include the w/a name inside
ilk_dummy_write() as well.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6

[Daniel Vetter]

Or at least our best understanding of it.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..c3f4f04 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg)
 	return true;
 }
 
+static void
+ilk_dummy_write(struct drm_i915_private *dev_priv)
+{
+	/* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before
+	 * touching it for real. MI_MODE is masked, hence harmless to write 0
+	 * into. */
+	I915_WRITE_NOTRACE(MI_MODE, 0);
+}
+
 #define __i915_read(x, y) \
 u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
 	u##x val = 0; \
+	if (IS_GEN5(dev_priv->dev)) \
+		ilk_dummy_write(dev_priv); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		unsigned long irqflags; \
 		spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
@@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
+	if (IS_GEN5(dev_priv->dev)) \
+		ilk_dummy_write(dev_priv); \
 	if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
 		write##y(val, dev_priv->regs + reg + 0x180000);		\
 	} else {							\
-- 
1.7.10.4

[PATCH] drm/i915: implement WaIssueDummyWriteToWakeupFromRC6

[Daniel Vetter]

Or at least our best understanding of it.

v2: Fixup commit message and put the wa name into the comment block.
And actually update the commit, too.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_drv.c |   13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index a7837e5..fb6b633 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1131,9 +1131,20 @@ static bool IS_DISPLAYREG(u32 reg)
 	return true;
 }
 
+static void
+ilk_dummy_write(struct drm_i915_private *dev_priv)
+{
+	/* WaIssueDummyWriteToWakeupFromRC6: Issue a dummy write to wake up the
+	 * chip from rc6 before touching it for real. MI_MODE is masked, hence
+	 * harmless to write 0 into. */
+	I915_WRITE_NOTRACE(MI_MODE, 0);
+}
+
 #define __i915_read(x, y) \
 u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
 	u##x val = 0; \
+	if (IS_GEN5(dev_priv->dev)) \
+		ilk_dummy_write(dev_priv); \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		unsigned long irqflags; \
 		spin_lock_irqsave(&dev_priv->gt_lock, irqflags); \
@@ -1165,6 +1176,8 @@ void i915_write##x(struct drm_i915_private *dev_priv, u32 reg, u##x val) { \
 	if (NEEDS_FORCE_WAKE((dev_priv), (reg))) { \
 		__fifo_ret = __gen6_gt_wait_for_fifo(dev_priv); \
 	} \
+	if (IS_GEN5(dev_priv->dev)) \
+		ilk_dummy_write(dev_priv); \
 	if (IS_VALLEYVIEW(dev_priv->dev) && IS_DISPLAYREG(reg)) { \
 		write##y(val, dev_priv->regs + reg + 0x180000);		\
 	} else {							\
-- 
1.7.10.4

Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6

[Chris Wilson]

On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>  #define __i915_read(x, y) \
>  u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
>  	u##x val = 0; \
> +	if (IS_GEN5(dev_priv->dev)) \
> +		ilk_dummy_write(dev_priv); \

IS_GEN5(dev_priv->dev) just makes me want to puke. At some point we must
go through and add an __INTEL_INFO(dev_priv) and so
  #define __IS_GEN5(dev_priv__) ((dev_priv__)->info->->gen == 5)
  #define IS_GEN5(dev) __IS_GEN5(to_drm_i915_private(dev))
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6

[Daniel Vetter]

On Thu, Oct 18, 2012 at 1:17 PM, Chris Wilson <chris@chris-wilson.co.uk> wrote:
> On Thu, 18 Oct 2012 11:49:50 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
>>  #define __i915_read(x, y) \
>>  u##x i915_read##x(struct drm_i915_private *dev_priv, u32 reg) { \
>>       u##x val = 0; \
>> +     if (IS_GEN5(dev_priv->dev)) \
>> +             ilk_dummy_write(dev_priv); \
>
> IS_GEN5(dev_priv->dev) just makes me want to puke. At some point we must
> go through and add an __INTEL_INFO(dev_priv) and so
>   #define __IS_GEN5(dev_priv__) ((dev_priv__)->info->->gen == 5)
>   #define IS_GEN5(dev) __IS_GEN5(to_drm_i915_private(dev))

We could also teach the drm setup and teardown code manners and embed
struct drm_device into our own device struct and call it a day. Would
kill all that pointless circular pointer chasing. Been on my todo
since ages, will probably stay there for a while ...
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 1/5] drm/i915: implement WaIssueDummyWriteToWakupFromRC6

[Lespiau, Damien]

On Thu, Oct 18, 2012 at 10:49 AM, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> +static void
> +ilk_dummy_write(struct drm_i915_private *dev_priv)
> +{
> +       /* Ilk w/a: Issue a dummy write to wake up the chip from rc6 before
> +        * touching it for real. MI_MODE is masked, hence harmless to write 0
> +        * into. */

I'd document the wa implemented here.

The summary line is missing an 'e' in Wake, it makes it harder to look for it.

-- 
Damien

[PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush

[Daniel Vetter]

Comment says for eaglelake/cantiga, but it's listed in the ilk table,
too. So apply it to both.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/i915_reg.h |    1 +
 drivers/gpu/drm/i915/intel_pm.c |    8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index a4162dd..1970c54 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -661,6 +661,7 @@
 #define   MI_ARB_DISPLAY_PRIORITY_B_A		(1 << 0)	/* display B > display A */
 
 #define CACHE_MODE_0	0x02120 /* 915+ only */
+#define   CM0_PIPELINED_RENDER_FLUSH_DISABLE (1<<8)
 #define   CM0_IZ_OPT_DISABLE      (1<<6)
 #define   CM0_ZR_OPT_DISABLE      (1<<5)
 #define	  CM0_STC_EVICT_DISABLE_LRA_SNB	(1<<5)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 72f41aa..aa77639 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3397,6 +3397,10 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(_3D_CHICKEN2,
 		   _3D_CHICKEN2_WM_READ_PIPELINED << 16 |
 		   _3D_CHICKEN2_WM_READ_PIPELINED);
+
+	/* WaDisableRenderCachePipelinedFlush */
+	I915_WRITE(CACHE_MODE_0,
+		   _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE));
 }
 
 static void gen6_init_clock_gating(struct drm_device *dev)
@@ -3728,6 +3732,10 @@ static void g4x_init_clock_gating(struct drm_device *dev)
 	if (IS_GM45(dev))
 		dspclk_gate |= DSSUNIT_CLOCK_GATE_DISABLE;
 	I915_WRITE(DSPCLK_GATE_D, dspclk_gate);
+
+	/* WaDisableRenderCachePipelinedFlush */
+	I915_WRITE(CACHE_MODE_0,
+		   _MASKED_BIT_ENABLE(CM0_PIPELINED_RENDER_FLUSH_DISABLE));
 }
 
 static void crestline_init_clock_gating(struct drm_device *dev)
-- 
1.7.10.4

Re: [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush

[Chris Wilson]

On Thu, 18 Oct 2012 11:49:51 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Comment says for eaglelake/cantiga, but it's listed in the ilk table,
> too. So apply it to both.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Looks harmless due to the massive number of other p/c errata and that
pipelined render cache flushes have never been relied upon.

Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

Re: [PATCH 2/5] drm/i915: implement WaDisableRenderCachePipelinedFlush

[Daniel Vetter]

On Thu, Oct 18, 2012 at 12:21:17PM +0100, Chris Wilson wrote:
> On Thu, 18 Oct 2012 11:49:51 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> > Comment says for eaglelake/cantiga, but it's listed in the ilk table,
> > too. So apply it to both.
> > 
> > Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> 
> Looks harmless due to the massive number of other p/c errata and that
> pipelined render cache flushes have never been relied upon.
> 
> Reviewed-by: Chris Wilson <chris@chris-wilson.co.uk>

I've queued the first two here for -next (since they don't seem to fix any
reported bugs).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

[PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand

[Daniel Vetter]

Comment says that this applies to earlier gens, too. Since two more
MI_NOOP's can't hurt that much, I've figured I'll apply this w/a down
to gen2.

v2: Correct the ringbuffer dword count for gen3, spotted by Chris
Wilson.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 984a0c5..38092dc 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -969,7 +969,7 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
 {
 	int ret;
 
-	ret = intel_ring_begin(ring, 2);
+	ret = intel_ring_begin(ring, 4);
 	if (ret)
 		return ret;
 
@@ -978,6 +978,11 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
 			MI_BATCH_GTT |
 			MI_BATCH_NON_SECURE_I965);
 	intel_ring_emit(ring, offset);
+	/* WaInsertNoopAfterBatchEndCommand: Command says to do the same after
+	 * the batchbuffer start command. Unclear whether really required on
+	 * gen3, but better safe than sorry. */
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
@@ -996,7 +1001,10 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, MI_BATCH_BUFFER);
 	intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
 	intel_ring_emit(ring, offset + len - 8);
-	intel_ring_emit(ring, 0);
+	/* WaInsertNoopAfterBatchEndCommand: Command says to do the same after
+	 * the batchbuffer start command. Unclear whether really required on
+	 * gen2, but better safe than sorry. */
+	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
@@ -1008,12 +1016,16 @@ i915_dispatch_execbuffer(struct intel_ring_buffer *ring,
 {
 	int ret;
 
-	ret = intel_ring_begin(ring, 2);
+	ret = intel_ring_begin(ring, 4);
 	if (ret)
 		return ret;
 
 	intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT);
 	intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
+	/* WaInsertNoopAfterBatchEndCommand: Command says to do the same after
+	 * the batchbuffer start command. */
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
-- 
1.7.10.4

Re: [PATCH 3/5] drm/i915: WaInsertNoopAfterBatchEndCommand

[Chris Wilson]

On Thu, 18 Oct 2012 11:49:52 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Comment says that this applies to earlier gens, too. Since two more
> MI_NOOP's can't hurt that much, I've figured I'll apply this w/a down
> to gen2.
> 
> v2: Correct the ringbuffer dword count for gen3, spotted by Chris
> Wilson.
> 
> Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm just not sold on this one, surely we would have spotted a need
before now? 12 extra cycles after every batch! :-p

Can you please fix up the cut'n'paste comments to not refer to
generations that never call the function?
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH] drm/i915: WaInsertNoopAfterBatchEndCommand

[Daniel Vetter]

Comment says that this applies to earlier gens, too. Since two more
MI_NOOP's can't hurt that much, I've figured I'll apply this w/a down
to gen2.

v2: Correct the ringbuffer dword count for gen3, spotted by Chris
Wilson.

v3: Fixup the comments.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c |   18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 984a0c5..90db51d 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -969,7 +969,7 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
 {
 	int ret;
 
-	ret = intel_ring_begin(ring, 2);
+	ret = intel_ring_begin(ring, 4);
 	if (ret)
 		return ret;
 
@@ -978,6 +978,10 @@ i965_dispatch_execbuffer(struct intel_ring_buffer *ring, u32 offset, u32 length)
 			MI_BATCH_GTT |
 			MI_BATCH_NON_SECURE_I965);
 	intel_ring_emit(ring, offset);
+	/* WaInsertNoopAfterBatchEndCommand: Comment says to do the same after
+	 * the batchbuffer start command. */
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
@@ -996,7 +1000,10 @@ i830_dispatch_execbuffer(struct intel_ring_buffer *ring,
 	intel_ring_emit(ring, MI_BATCH_BUFFER);
 	intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
 	intel_ring_emit(ring, offset + len - 8);
-	intel_ring_emit(ring, 0);
+	/* WaInsertNoopAfterBatchEndCommand: Comment says to do the same after
+	 * the batchbuffer start command. Unclear whether really required on
+	 * gen2, but better safe than sorry. */
+	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
@@ -1008,12 +1015,17 @@ i915_dispatch_execbuffer(struct intel_ring_buffer *ring,
 {
 	int ret;
 
-	ret = intel_ring_begin(ring, 2);
+	ret = intel_ring_begin(ring, 4);
 	if (ret)
 		return ret;
 
 	intel_ring_emit(ring, MI_BATCH_BUFFER_START | MI_BATCH_GTT);
 	intel_ring_emit(ring, offset | MI_BATCH_NON_SECURE);
+	/* WaInsertNoopAfterBatchEndCommand: Comment says to do the same after
+	 * the batchbuffer start command. Unclear whether really required on
+	 * gen3, but better safe than sorry. */
+	intel_ring_emit(ring, MI_NOOP);
+	intel_ring_emit(ring, MI_NOOP);
 	intel_ring_advance(ring);
 
 	return 0;
-- 
1.7.10.4

[PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey

[Daniel Vetter]

Unfortunately this requires a bit of book-keeping to return the
right values for get_colorkey and to set things up correctly
when re-enabling the plane.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 drivers/gpu/drm/i915/intel_drv.h    |    2 ++
 drivers/gpu/drm/i915/intel_sprite.c |   27 +++++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 79f8ed6..e935e75 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -220,6 +220,8 @@ struct intel_plane {
 	struct drm_i915_gem_object *obj;
 	int max_downscale;
 	u32 lut_r[1024], lut_g[1024], lut_b[1024];
+	bool restore_ilk_dest_key; /* for a w/a */
+
 	void (*update_plane)(struct drm_plane *plane,
 			     struct drm_framebuffer *fb,
 			     struct drm_i915_gem_object *obj,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7644f31..8bcacd5 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -230,6 +230,13 @@ ilk_update_plane(struct drm_plane *plane, struct drm_framebuffer *fb,
 
 	dvscntr = I915_READ(DVSCNTR(pipe));
 
+	WARN_ON(!(dvscntr & DVS_ENABLE) && (dvscntr && DVS_DEST_KEY));
+
+	if (intel_plane->restore_ilk_dest_key) {
+		dvscntr |= DVS_DEST_KEY;
+		intel_plane->restore_ilk_dest_key = false;
+	}
+
 	/* Mask out pixel format bits in case we change it */
 	dvscntr &= ~DVS_PIXFORMAT_MASK;
 	dvscntr &= ~DVS_RGB_ORDER_XBGR;
@@ -311,8 +318,16 @@ ilk_disable_plane(struct drm_plane *plane)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	int pipe = intel_plane->pipe;
+	uint32_t tmp;
 
-	I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
+	/* WaDisableSpriteDestColorKey: We need to disable the dest key when
+	 * disabling the sprite. */
+	tmp = I915_READ(DVSCNTR(pipe));
+	if (IS_GEN5(dev) &&(tmp & DVS_DEST_KEY))
+		intel_plane->restore_ilk_dest_key = true;
+	tmp &= ~(DVS_ENABLE | DVS_DEST_KEY);
+
+	I915_WRITE(DVSCNTR(pipe), tmp);
 	/* Disable the scaler */
 	I915_WRITE(DVSSCALE(pipe), 0);
 	/* Flush double buffered register updates */
@@ -365,6 +380,7 @@ ilk_update_colorkey(struct drm_plane *plane,
 	int ret = 0;
 
 	intel_plane = to_intel_plane(plane);
+	intel_plane->restore_ilk_dest_key = false;
 
 	I915_WRITE(DVSKEYVAL(intel_plane->pipe), key->min_value);
 	I915_WRITE(DVSKEYMAX(intel_plane->pipe), key->max_value);
@@ -372,9 +388,12 @@ ilk_update_colorkey(struct drm_plane *plane,
 
 	dvscntr = I915_READ(DVSCNTR(intel_plane->pipe));
 	dvscntr &= ~(DVS_SOURCE_KEY | DVS_DEST_KEY);
-	if (key->flags & I915_SET_COLORKEY_DESTINATION)
-		dvscntr |= DVS_DEST_KEY;
-	else if (key->flags & I915_SET_COLORKEY_SOURCE)
+	if (key->flags & I915_SET_COLORKEY_DESTINATION) {
+		if ((dvscntr & DVS_ENABLE) || !IS_GEN5(dev))
+			dvscntr |= DVS_DEST_KEY;
+		else
+			intel_plane->restore_ilk_dest_key = true;
+	} else if (key->flags & I915_SET_COLORKEY_SOURCE)
 		dvscntr |= DVS_SOURCE_KEY;
 	I915_WRITE(DVSCNTR(intel_plane->pipe), dvscntr);
 
-- 
1.7.10.4

Re: [PATCH 4/5] drm/i915: implement WaDisableSpriteDestColorKey

[Chris Wilson]

On Thu, 18 Oct 2012 11:49:53 +0200, Daniel Vetter <daniel.vetter@ffwll.ch> wrote:
> Unfortunately this requires a bit of book-keeping to return the
> right values for get_colorkey and to set things up correctly
> when re-enabling the plane.

Needs to worry about per-pipe values and ilk_get_colorkey().
Maybe the code would be simpler if we unconditionally shadowed
DVSCNTR[I915_NUM_PIPES].
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre

[PATCH 5/5] drm/i915: document a few more already implemented ilk w/as

[Daniel Vetter]

Found while strolling docs.

Signed-off-by: Daniel Vetter <daniel.vetter@ffwll.ch>
---
 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 aa77639..45ffb2c 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3342,6 +3342,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	/* Required for CxSR */
 	dspclk_gate |= DPARBUNIT_CLOCK_GATE_DISABLE;
 
+	/* WaDisableSVSMUnitClockGating */
 	I915_WRITE(PCH_3DCGDIS0,
 		   MARIUNIT_CLOCK_GATE_DISABLE |
 		   SVSMUNIT_CLOCK_GATE_DISABLE);
@@ -3353,7 +3354,7 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	/*
 	 * According to the spec the following bits should be set in
 	 * order to enable memory self-refresh
-	 * The bit 22/21 of 0x42004
+	 * The bit 22/21 of 0x42004 (WaDisableDisplayFetchStrideStreching)
 	 * The bit 5 of 0x42020
 	 * The bit 15 of 0x45000
 	 */
-- 
1.7.10.4