[Intel-gfx] [PATCH 1/2] drm/i915: Use Write-Through cacheing for the display plane on Iris

[Daniel Vetter]

On Fri, Aug 2, 2013 at 10:03 PM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Fri, Aug 02, 2013 at 05:51:49PM +0300, Ville Syrjälä wrote:
>> On Thu, Aug 01, 2013 at 06:39:54PM +0100, Chris Wilson wrote:
>> > Haswell GT3e has the unique feature of supporting Write-Through cacheing
>> > of objects within the eLLC/LLC. The purpose of this is to enable the display
>> > plane to remain coherent whilst objects lie resident in the eLLC/LLC - so
>> > that we, in theory, get the best of both worlds, perfect display and fast
>> > access.
>> >
>> > However, we still need to be careful as the CPU does not see the WT when
>> > accessing the cache. In particular, this means that we need to flush the
>> > cache lines after writing to an object through the CPU, and on
>> > transitioning from a cached state to WT.
>> >
>> > v2: Actually do the clflush on transition to WT, nagging by Ville.
>> > v3: Flush the CPU cache after writes into WT objects.
>>
>> v4?
> v3.2: ditto

v3 (or whatever exactly ended up in Ben's tree) slighty changes the
get_caching_ioctl semantics: WT objects are set as cached, which I
think is a slight abi breakage. I didn't check what mesa exactly does
for scanout buffers (and if that concept is even consistent between
all the different winsys) but I think the easy cop-out is to just
treat WT as uncached in get_caching.

That option also avoids me asking for igt coverage before merging ;-)

>> >  {
>> >     if (obj->tiling_mode)
>> > @@ -510,7 +520,7 @@ i915_gem_shmem_pread(struct drm_device *dev,
>> >              * read domain and manually flush cachelines (if required). This
>> >              * optimizes for the case when the gpu will dirty the data
>> >              * anyway again before the next pread happens. */
>> > -           if (obj->cache_level == I915_CACHE_NONE)
>> > +           if (!is_cpu_read_cached(obj->cache_level))
>>
>> Based on what we discussed on irc about GPU snooping even on UC
>> accesses, couldn't this even be:
>>
>> !HAS_LLC(dev) && obj->cache_level == I915_CACHE_NONE
>
> Yes, I came to that conclusion later on as well. I like having the
> is_cached() functions to try and clarify the reason for the tests, but I
> don't think I have the names right yet.

Another idea would be to split the cache_level from the detailed
cache_options (with stuff like aga, L3$ caching/what we currently
still call MLC, ...). Cpu flushing code would then only check for
cache_level.

I like the approach here though, so maybe s/_cached/_coherent/ for bikeshed.

btw I've asked Ben to resend his s/age0/age3/ patch also for Iris, so
yours would need to be rebased on top.

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