[Intel-gfx] [PATCH] drm/i915: Don't override PPGTT cacheability on HSW

[Daniel Vetter]

On Wed, Apr 3, 2013 at 9:17 PM, Kenneth Graunke <kenneth at whitecape.org> wrote:
> On 04/03/2013 11:06 AM, Ben Widawsky wrote:
>>
>> Apparently these ECOCHK bits changed on HSW and the behavior is not what
>> we want. I've not been able to find VLV definition specifically so I'll
>> assume it's the same as IVB.
>>
>> (Only compile tested)
>>
>> Reported-by: Kenneth Graunke <kenneth at whitecape.org>
>> Signed-off-by: Ben Widawsky <ben at bwidawsk.net>
>
>
> The behavior isn't particularly bad, but the PTEs already take care of this
> for us, so it doesn't do anything.  That said, having random override bits
> set for no purpose is bad...we should just let the PTEs do their job.
>
> Reviewed-and-tested-by: Kenneth Graunke <kenneth at whitecape.org>

This bit is used to set the cachability of the ppgtt PTEs themselves,
_not_ the data pointed at by the ptes. We very much want that, it's
after all the only reason we have enabled aliasing ppgtt support on
snb/ivb. Iirc the speedup was around 10-20% on some benchmarks,
disabling this bit here complety killed these improvements.

Also, if you disable llc caching, the cpu pte writes aren't coherent
any more with what the gt reads, so I'm pretty sure that this will
blow up somewhere on one of our more nasty igt tests.

So I've checked hsw bspec and the problem is that hw guys again
changed the bits around a bit, and I think on HSW we actually want
(0x8 << 3) instead of what's currently there.

Could also be that on hsw the hw now supports pte cachability control
in the pdes, but at least on snb/ivb that part didn't really work. And
bspec is a bit an unclear mess in that area ... Since we should be ok
with the override I don't think it's worth time to investigate this
more.
-Daniel
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch