[PATCH 1/4] drm/i915: Remove duplicate cache workaround

[PATCH 1/4] drm/i915: Remove duplicate cache workaround

[Ben Widawsky]

This is already achieved in init_clock gating, and is probably the
result of a bad merge from Daniel. I'm too tired to bet on him making a
mistake though.

CC: Daniel Vetter <daniel.vetter@ffwll.ch>
Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
index 984a0c5..625a348 100644
--- a/drivers/gpu/drm/i915/intel_ringbuffer.c
+++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
@@ -521,14 +521,6 @@ static int init_render_ring(struct intel_ring_buffer *ring)
 	}
 
 	if (IS_GEN6(dev)) {
-		/* From the Sandybridge PRM, volume 1 part 3, page 24:
-		 * "If this bit is set, STCunit will have LRA as replacement
-		 *  policy. [...] This bit must be reset.  LRA replacement
-		 *  policy is not supported."
-		 */
-		I915_WRITE(CACHE_MODE_0,
-			   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
-
 		/* This is not explicitly set for GEN6, so read the register.
 		 * see intel_ring_mi_set_context() for why we care.
 		 * TODO: consider explicitly setting the bit for GEN5
-- 
1.7.12.2

[PATCH 2/4] drm/i915: Disable render depth cache pipeline flush

[Ben Widawsky]

Required through gen6.

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/i915_reg.h | 1 +
 drivers/gpu/drm/i915/intel_pm.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index d17bef7..37e62e0 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -662,6 +662,7 @@
 #define   MI_ARB_DISPLAY_PRIORITY_B_A		(1 << 0)	/* display B > display A */
 
 #define CACHE_MODE_0	0x02120 /* 915+ only */
+#define   CM0_RC_PIPELINE_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 a3e4f8b..f1800ca 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3385,6 +3385,8 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 
 	I915_WRITE(CACHE_MODE_0,
 		   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
+	I915_WRITE(CACHE_MODE_0,
+		   _MASKED_BIT_ENABLE(CM0_RC_PIPELINE_FLUSH_DISABLE));
 
 	I915_WRITE(GEN6_UCGCTL1,
 		   I915_READ(GEN6_UCGCTL1) |
-- 
1.7.12.2

[PATCH 3/4] drm/i915: ILK also needs that last fix

[Ben Widawsky]

That fix was the disable render deptch cache pipeline flush

Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
---
 drivers/gpu/drm/i915/intel_pm.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index f1800ca..8aafa45 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3338,6 +3338,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
 	I915_WRITE(WM2_LP_ILK, 0);
 	I915_WRITE(WM1_LP_ILK, 0);
 
+	I915_WRITE(CACHE_MODE_0,
+		   _MASKED_BIT_ENABLE(CM0_RC_PIPELINE_FLUSH_DISABLE));
 	/*
 	 * Based on the document from hardware guys the following bits
 	 * should be set unconditionally in order to enable FBC.
-- 
1.7.12.2

[PATCH 4/4] drm/i915: Fix GT_MODE default value

[Ben Widawsky]

I can't even find how I figured this might be needed anymore. But sure
enough, the value I'm reading back on platforms doesn't match what the
docs recommends.

It seemed to fix Chris' GT1 in limited testing as well.

Tested-by: Chris Wilson <chris@chris-wilson.co.uk>
---
 drivers/gpu/drm/i915/i915_reg.h | 3 +++
 drivers/gpu/drm/i915/intel_pm.c | 5 +++++
 2 files changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 37e62e0..4c1f461 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -531,6 +531,9 @@
 # define VS_TIMER_DISPATCH				(1 << 6)
 # define MI_FLUSH_ENABLE				(1 << 12)
 
+#define GEN6_GT_MODE	0x20d0
+#define   GEN6_GT_MODE_HI	(1 << 9)
+
 #define GFX_MODE	0x02520
 #define GFX_MODE_GEN7	0x0229c
 #define RING_MODE_GEN7(ring)	((ring)->mmio_base+0x29c)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 8aafa45..e0574f9 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -3446,6 +3446,11 @@ static void gen6_init_clock_gating(struct drm_device *dev)
 			   DISPPLANE_TRICKLE_FEED_DISABLE);
 		intel_flush_display_plane(dev_priv, pipe);
 	}
+
+	/* The default value should be 0x200 according to docs, but the two
+	 * platforms I checked have a 0 for this. (Maybe BIOS overrides?) */
+	I915_WRITE(GEN6_GT_MODE, _MASKED_BIT_DISABLE(0xffff));
+	I915_WRITE(GEN6_GT_MODE, _MASKED_BIT_ENABLE(GEN6_GT_MODE_HI));
 }
 
 static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)
-- 
1.7.12.2

Re: [PATCH 1/4] drm/i915: Remove duplicate cache workaround

[Daniel Vetter]

On Wed, Oct 03, 2012 at 07:34:21PM -0700, Ben Widawsky wrote:
> This is already achieved in init_clock gating, and is probably the
> result of a bad merge from Daniel. I'm too tired to bet on him making a
> mistake though.
> 
> CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

Citing from the commit message that added this 2nd copy that you've just
removed:

"This cannot be done in gen6_init_clock_gating with most of the other
workaround bits; the render ring needs to exist.  Otherwise, the
register write gets dropped on the floor (one printk will show it
changed, but a second printk immediately following shows the value
reverts to the old one)."

Congrats, you've removed the wrong copy ;-)

Now I suspect not all of the w/a patches currently floating have seen
level of testing, and I'd wager a few suffer from the same. So I think we
need checks in i-g-t. Eric has started this with the intel_reg_checker
tool, but we lack an aweful lot of recent workarounds. Also, we need to
integrate a call to this tool in the testsuite, once at least at the
beginning somewhere (to check boot-up state) but also in ZZ_hangman (in
case the reset botched things up). We really should add a suspend/resume
testcase in there, too ...

Cheers, Daniel

> ---
>  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
>  1 file changed, 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> index 984a0c5..625a348 100644
> --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> @@ -521,14 +521,6 @@ static int init_render_ring(struct intel_ring_buffer *ring)
>  	}
>  
>  	if (IS_GEN6(dev)) {
> -		/* From the Sandybridge PRM, volume 1 part 3, page 24:
> -		 * "If this bit is set, STCunit will have LRA as replacement
> -		 *  policy. [...] This bit must be reset.  LRA replacement
> -		 *  policy is not supported."
> -		 */
> -		I915_WRITE(CACHE_MODE_0,
> -			   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
> -
>  		/* This is not explicitly set for GEN6, so read the register.
>  		 * see intel_ring_mi_set_context() for why we care.
>  		 * TODO: consider explicitly setting the bit for GEN5
> -- 
> 1.7.12.2
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch

Re: [PATCH 4/4] drm/i915: Fix GT_MODE default value

[Daniel Vetter]

On Wed, Oct 03, 2012 at 07:34:24PM -0700, Ben Widawsky wrote:
> I can't even find how I figured this might be needed anymore. But sure
> enough, the value I'm reading back on platforms doesn't match what the
> docs recommends.
> 
> It seemed to fix Chris' GT1 in limited testing as well.
> 
> Tested-by: Chris Wilson <chris@chris-wilson.co.uk>

Merged to -fixes, with the missing sob line forged.
-Daniel
> ---
>  drivers/gpu/drm/i915/i915_reg.h | 3 +++
>  drivers/gpu/drm/i915/intel_pm.c | 5 +++++
>  2 files changed, 8 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 37e62e0..4c1f461 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -531,6 +531,9 @@
>  # define VS_TIMER_DISPATCH				(1 << 6)
>  # define MI_FLUSH_ENABLE				(1 << 12)
>  
> +#define GEN6_GT_MODE	0x20d0
> +#define   GEN6_GT_MODE_HI	(1 << 9)
> +
>  #define GFX_MODE	0x02520
>  #define GFX_MODE_GEN7	0x0229c
>  #define RING_MODE_GEN7(ring)	((ring)->mmio_base+0x29c)
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index 8aafa45..e0574f9 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3446,6 +3446,11 @@ static void gen6_init_clock_gating(struct drm_device *dev)
>  			   DISPPLANE_TRICKLE_FEED_DISABLE);
>  		intel_flush_display_plane(dev_priv, pipe);
>  	}
> +
> +	/* The default value should be 0x200 according to docs, but the two
> +	 * platforms I checked have a 0 for this. (Maybe BIOS overrides?) */
> +	I915_WRITE(GEN6_GT_MODE, _MASKED_BIT_DISABLE(0xffff));
> +	I915_WRITE(GEN6_GT_MODE, _MASKED_BIT_ENABLE(GEN6_GT_MODE_HI));
>  }
>  
>  static void gen7_setup_fixed_func_scheduler(struct drm_i915_private *dev_priv)
> -- 
> 1.7.12.2
> 
> _______________________________________________
> 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

Re: [PATCH 1/4] drm/i915: Remove duplicate cache workaround

[Ben Widawsky]

On Thu, 4 Oct 2012 09:01:17 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Oct 03, 2012 at 07:34:21PM -0700, Ben Widawsky wrote:
> > This is already achieved in init_clock gating, and is probably the
> > result of a bad merge from Daniel. I'm too tired to bet on him making a
> > mistake though.
> > 
> > CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> Citing from the commit message that added this 2nd copy that you've just
> removed:
> 
> "This cannot be done in gen6_init_clock_gating with most of the other
> workaround bits; the render ring needs to exist.  Otherwise, the
> register write gets dropped on the floor (one printk will show it
> changed, but a second printk immediately following shows the value
> reverts to the old one)."
> 
> Congrats, you've removed the wrong copy ;-)

Would you be willing to verify this again? There is some interesting
information in the bspec that leads me to believe you've jumped to the
wrong conclusion regarding whether or not the render ring needs to
exist (I think there are other preconditions that I can workaround a
bit better).


> 
> Now I suspect not all of the w/a patches currently floating have seen
> level of testing, and I'd wager a few suffer from the same. 

Just to clarify a bit here: I don't have an SNB here with me, so the
patches themselves were only booted with an IVB. I didn't test the ILK
patch at all because I don't have one to test with also. I did test
patch 2, and 4 on Jesse's machine (1 is obviously not testable without
the kernel actually running)

> So I think we
> need checks in i-g-t. Eric has started this with the intel_reg_checker
> tool, but we lack an aweful lot of recent workarounds. Also, we need to
> integrate a call to this tool in the testsuite, once at least at the
> beginning somewhere (to check boot-up state) but also in ZZ_hangman (in
> case the reset botched things up). We really should add a suspend/resume
> testcase in there, too ...
>

I'll leave this discussion to the mail thread started on patch 10.
 
> Cheers, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 984a0c5..625a348 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -521,14 +521,6 @@ static int init_render_ring(struct intel_ring_buffer *ring)
> >  	}
> >  
> >  	if (IS_GEN6(dev)) {
> > -		/* From the Sandybridge PRM, volume 1 part 3, page 24:
> > -		 * "If this bit is set, STCunit will have LRA as replacement
> > -		 *  policy. [...] This bit must be reset.  LRA replacement
> > -		 *  policy is not supported."
> > -		 */
> > -		I915_WRITE(CACHE_MODE_0,
> > -			   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
> > -
> >  		/* This is not explicitly set for GEN6, so read the register.
> >  		 * see intel_ring_mi_set_context() for why we care.
> >  		 * TODO: consider explicitly setting the bit for GEN5
> > -- 
> > 1.7.12.2
> > 
> 



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 1/4] drm/i915: Remove duplicate cache workaround

[Ben Widawsky]

On Thu, 4 Oct 2012 09:01:17 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Oct 03, 2012 at 07:34:21PM -0700, Ben Widawsky wrote:
> > This is already achieved in init_clock gating, and is probably the
> > result of a bad merge from Daniel. I'm too tired to bet on him making a
> > mistake though.
> > 
> > CC: Daniel Vetter <daniel.vetter@ffwll.ch>
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> Citing from the commit message that added this 2nd copy that you've just
> removed:
> 
> "This cannot be done in gen6_init_clock_gating with most of the other
> workaround bits; the render ring needs to exist.  Otherwise, the
> register write gets dropped on the floor (one printk will show it
> changed, but a second printk immediately following shows the value
> reverts to the old one)."
> 
> Congrats, you've removed the wrong copy ;-)

On further thought, I think we need to investigate the original issue
here more anyway. I've been speaking with Ken, and neither of us really
understand why the later write is required (it seems he authored the
original patch). I think we all agree we'd like to jam these kind of
workarounds into the clock gating functions (even though that is now a
terrible name for it). Ken has tried this patch locally, and it works
fine.

It seems Matt Turner has the original machine, so it'd be great if he
can test this patch. Matt this is the t420s you got from Ken.

> 
> Now I suspect not all of the w/a patches currently floating have seen
> level of testing, and I'd wager a few suffer from the same. So I think we
> need checks in i-g-t. Eric has started this with the intel_reg_checker
> tool, but we lack an aweful lot of recent workarounds. Also, we need to
> integrate a call to this tool in the testsuite, once at least at the
> beginning somewhere (to check boot-up state) but also in ZZ_hangman (in
> case the reset botched things up). We really should add a suspend/resume
> testcase in there, too ...
> 
> Cheers, Daniel
> 
> > ---
> >  drivers/gpu/drm/i915/intel_ringbuffer.c | 8 --------
> >  1 file changed, 8 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_ringbuffer.c b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > index 984a0c5..625a348 100644
> > --- a/drivers/gpu/drm/i915/intel_ringbuffer.c
> > +++ b/drivers/gpu/drm/i915/intel_ringbuffer.c
> > @@ -521,14 +521,6 @@ static int init_render_ring(struct intel_ring_buffer *ring)
> >  	}
> >  
> >  	if (IS_GEN6(dev)) {
> > -		/* From the Sandybridge PRM, volume 1 part 3, page 24:
> > -		 * "If this bit is set, STCunit will have LRA as replacement
> > -		 *  policy. [...] This bit must be reset.  LRA replacement
> > -		 *  policy is not supported."
> > -		 */
> > -		I915_WRITE(CACHE_MODE_0,
> > -			   _MASKED_BIT_DISABLE(CM0_STC_EVICT_DISABLE_LRA_SNB));
> > -
> >  		/* This is not explicitly set for GEN6, so read the register.
> >  		 * see intel_ring_mi_set_context() for why we care.
> >  		 * TODO: consider explicitly setting the bit for GEN5
> > -- 
> > 1.7.12.2
> > 
> 



-- 
Ben Widawsky, Intel Open Source Technology Center

Re: [PATCH 3/4] drm/i915: ILK also needs that last fix

[Daniel Vetter]

On Wed, Oct 03, 2012 at 07:34:23PM -0700, Ben Widawsky wrote:
> That fix was the disable render deptch cache pipeline flush
> 
> Signed-off-by: Ben Widawsky <ben@bwidawsk.net>

I've stumbled over the same one, but my docs here suggest i965g/gm45 need
it, too:

http://cgit.freedesktop.org/~danvet/drm/commit/?h=ilk-wa-pile&id=37c4c82b8cdbcf5adccad97f0b45747ba37ed659

Have you checked whether we don't need this on ivb/vlv/hsw, too?

Also, for w/a patches based on the vpg w/a database, please include the
vpg w/a name tag both in the commit message and in a code comment
somewhere.
-Daniel
> ---
>  drivers/gpu/drm/i915/intel_pm.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index f1800ca..8aafa45 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -3338,6 +3338,8 @@ static void ironlake_init_clock_gating(struct drm_device *dev)
>  	I915_WRITE(WM2_LP_ILK, 0);
>  	I915_WRITE(WM1_LP_ILK, 0);
>  
> +	I915_WRITE(CACHE_MODE_0,
> +		   _MASKED_BIT_ENABLE(CM0_RC_PIPELINE_FLUSH_DISABLE));
>  	/*
>  	 * Based on the document from hardware guys the following bits
>  	 * should be set unconditionally in order to enable FBC.
> -- 
> 1.7.12.2
> 
> _______________________________________________
> 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

Re: [PATCH 3/4] drm/i915: ILK also needs that last fix

[Ben Widawsky]

On Mon, 15 Oct 2012 20:59:22 +0200
Daniel Vetter <daniel@ffwll.ch> wrote:

> On Wed, Oct 03, 2012 at 07:34:23PM -0700, Ben Widawsky wrote:
> > That fix was the disable render deptch cache pipeline flush
> > 
> > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> 
> I've stumbled over the same one, but my docs here suggest i965g/gm45
> need it, too:
> 
> http://cgit.freedesktop.org/~danvet/drm/commit/?h=ilk-wa-pile&id=37c4c82b8cdbcf5adccad97f0b45747ba37ed659
> 
> Have you checked whether we don't need this on ivb/vlv/hsw, too?

I did check whether the windows driver does it for those platforms, and
the answer is no. So the answer to your question is maybe because who
knows what exists in some other doc somewhere in the metaverse. I think
this is a good enough start though since it seems SNB was definitely a
bit buggier than IVB.

> 
> Also, for w/a patches based on the vpg w/a database, please include
> the vpg w/a name tag both in the commit message and in a code comment
> somewhere.

Good idea. If you're okay with longer commit message subjects, I'd even
suggest putting it there to make it even a bit easier to search for.

> -Daniel
> > ---
> >  drivers/gpu/drm/i915/intel_pm.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/gpu/drm/i915/intel_pm.c
> > b/drivers/gpu/drm/i915/intel_pm.c index f1800ca..8aafa45 100644
> > --- a/drivers/gpu/drm/i915/intel_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_pm.c
> > @@ -3338,6 +3338,8 @@ static void ironlake_init_clock_gating(struct
> > drm_device *dev) I915_WRITE(WM2_LP_ILK, 0);
> >  	I915_WRITE(WM1_LP_ILK, 0);
> >  
> > +	I915_WRITE(CACHE_MODE_0,
> > +
> > _MASKED_BIT_ENABLE(CM0_RC_PIPELINE_FLUSH_DISABLE)); /*
> >  	 * Based on the document from hardware guys the following
> > bits
> >  	 * should be set unconditionally in order to enable FBC.
> > -- 
> > 1.7.12.2
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 

Re: [PATCH 3/4] drm/i915: ILK also needs that last fix

[Daniel Vetter]

On Mon, Oct 15, 2012 at 08:21:41PM -0700, Ben Widawsky wrote:
> On Mon, 15 Oct 2012 20:59:22 +0200
> Daniel Vetter <daniel@ffwll.ch> wrote:
> 
> > On Wed, Oct 03, 2012 at 07:34:23PM -0700, Ben Widawsky wrote:
> > > That fix was the disable render deptch cache pipeline flush
> > > 
> > > Signed-off-by: Ben Widawsky <ben@bwidawsk.net>
> > 
> > I've stumbled over the same one, but my docs here suggest i965g/gm45
> > need it, too:
> > 
> > http://cgit.freedesktop.org/~danvet/drm/commit/?h=ilk-wa-pile&id=37c4c82b8cdbcf5adccad97f0b45747ba37ed659
> > 
> > Have you checked whether we don't need this on ivb/vlv/hsw, too?
> 
> I did check whether the windows driver does it for those platforms, and
> the answer is no. So the answer to your question is maybe because who
> knows what exists in some other doc somewhere in the metaverse. I think
> this is a good enough start though since it seems SNB was definitely a
> bit buggier than IVB.

Yeah, I've noticed while checking w/as that they're not consistently named
on older platforms. E.g. the above definitely exists on eaglelake, too,
but named slightly different. So the w/a db doesn't pick up all uses.
Hoooray!

> > Also, for w/a patches based on the vpg w/a database, please include
> > the vpg w/a name tag both in the commit message and in a code comment
> > somewhere.
> 
> Good idea. If you're okay with longer commit message subjects, I'd even
> suggest putting it there to make it even a bit easier to search for.

Yeah, I'm fine with putting it into the commit head, I've put it there
myself. If the w/a only affects one platform we could try to squeeze the
platform name into the headline, too. But having to read the commit
message for that doesn't really hurt, either.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch