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

[Daniel Vetter <daniel@ffwll.ch>]

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