Re: [PATCH 1/2] drm/i915: Remove use of the autoreported ringbuffer HEAD position

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Feb 08, 2012 at 01:34:13PM +0000, Chris Wilson wrote:
> This is a revert of 6aa56062eaba67adfb247cded244fd877329588d.
> 
> This was originally introduced to workaround reads of the ringbuffer
> registers returning 0 on SandyBridge causing hangs due to ringbuffer
> overflow. The root cause here was reads through the GT powerwell require
> the forcewake dance, something we only learnt of later. Now it appears
> that reading the reported head position from the HWS is returning
> garbage, leading once again to hangs.
> 
> For example, on q35 the autoreported head reports:
>   [  217.975608] head now 00010000, actual 00010000
>   [  436.725613] head now 00200000, actual 00200000
>   [  462.956033] head now 00210000, actual 00210010
>   [  485.501409] head now 00400000, actual 00400020
>   [  508.064280] head now 00410000, actual 00410000
>   [  530.576078] head now 00600000, actual 00600020
>   [  553.273489] head now 00610000, actual 00610018
> which appears reasonably sane. In contrast, if we look at snb:
>   [  141.970680] head now 00e10000, actual 00008238
>   [  141.974062] head now 02734000, actual 000083c8
>   [  141.974425] head now 00e10000, actual 00008488
>   [  141.980374] head now 032b5000, actual 000088b8
>   [  141.980885] head now 03271000, actual 00008950
>   [  142.040628] head now 02101000, actual 00008b40
>   [  142.180173] head now 02734000, actual 00009050
>   [  142.181090] head now 00000000, actual 00000ae0
>   [  142.183737] head now 02734000, actual 00009050
> 
> In addition, the automatic reporting of the head position is scheduled
> to be defeatured in the future. It has no more utility, remove it.
> 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=45492
> Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I'm on the fence whether we should include this in -fixes. On one hand it
fixes a severe issue on snb, but introduces a perf regression without the
second patch. Otoh we've never shipped snb without it broken like this.
But if it turns out that this is broken on ilk and earlier, too, I think
we definitely need these two patches in -fixes.
-Daniel
-- 
Daniel Vetter
Mail: daniel@ffwll.ch
Mobile: +41 (0)79 365 57 48