[Intel-gfx] [RFC] drm/i915: Move the STC LRA eviction policy workaround after ring init.

[Daniel Vetter]

On Thu, Apr 26, 2012 at 11:45:50PM -0700, Kenneth Graunke wrote:
> Clearing bit 5 of CACHE_MODE_0 is necessary to prevent GPU hangs in
> OpenGL programs such as Google MapsGL, Google Earth, and gzdoom.
> 
> While commit 09be664ecc77d58 introduced the workaround, the register
> write didn't actually stick: a printk and I915_READ immediately after
> the write would return the new value, but a second one would show that
> it had reverted to the original value...even with no intervening code.
> 
> The hardware documentation mentions that the ring must be idle before
> writing CACHE_MODE_0.  This provided a clue that it might be necessary
> to initialize the rings before attempting to program the register.  Sure
> enough, moving the write after ring initialization makes it stick.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=47535
> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
> ---
>  drivers/gpu/drm/i915/i915_dma.c |    7 +++++++
>  drivers/gpu/drm/i915/intel_pm.c |    4 ----
>  2 files changed, 7 insertions(+), 4 deletions(-)
> 
> Here's a horrible patch---putting that one workaround bit directly inside 
> i915_load_modeset_init is clearly a terrible idea.  There will obviously
> be many more, and per-generation.  I suspect that many more of the
> workaround bits we're setting in init_clock_gating may need to be moved
> later in the init process too...but I haven't checked to see which ones
> are failing to stick.
> 
> So I guess there's a couple questions:
> 
> * Does it make sense to move ALL the init_clock_gating stuff to this
>   point in time?  Or are there some registers that need to be done early,
>   where they are today?  (If we can move them all, we could just move the
>   call to init_clock_gating and be done with it...)
> 
>   Apparently CACHE_MODE_0 is a context-specific register, while some others
>   are not.  I like having all the workaround bits in one place, but that
>   may or may not be feasible...
> 
> * Do we want to make an intel_apply_workarounds() function or such?
>   Perhaps use a function pointer that gets filled in on a per-chipset basis?
> 
> * Is this the best time to set it?  Later?  Elsewhere?
> 
> * What should the code look like long-term, and what would be easiest for
>   backporting to stable kernels?

I'm working on a real fix for -next that cleans up our gem init handling
and workaround setting, but for -fixes I guess we just need to add yet
another cludge to init_render_ring ... That way it should get called at
all the right places (driver load, resume and gpu reset).
-Daniel
-- 
Daniel Vetter
Mail: daniel at ffwll.ch
Mobile: +41 (0)79 365 57 48