Re: [PATCH] drm/i915: Drop the overzealous warning from i915_gem_set_cache_level

[Daniel Vetter <daniel@ffwll.ch>]

On Wed, Aug 14, 2013 at 09:54:05AM +0100, Chris Wilson wrote:
> On Wed, Aug 14, 2013 at 10:49:11AM +0200, Daniel Vetter wrote:
> > On Tue, Aug 13, 2013 at 03:37:56PM +0300, Ville Syrjälä wrote:
> > > On Tue, Aug 13, 2013 at 01:20:13PM +0100, Chris Wilson wrote:
> > > > On Tue, Aug 13, 2013 at 03:12:59PM +0300, Ville Syrjälä wrote:
> > > > > Thinking about this stuff a bit, I think I actually came up with a
> > > > > scenario where we would currently fail to invalidate the CPU cache
> > > > > between non-snooped GPU/GTT access and CPU access:
> > > > > 
> > > > > 1. make bo non-snooped w/ pin_display=true (wd=0, rd|=gtt)
> > > > > 2. set to CPU read domain (wd=0 rd|=cpu)
> > > > > 3. set to GTT (or GPU) write domain (wd=gtt, rd=gtt) -> CPU cache is stale after this point
> > > > > 4. make bo snooped -> pin_display=true still so we directly set (wd=cpu, rd=cpu)
> > > > > 5. set to CPU domain -> CPU cache is still stale
> > > > 
> > > > You will also find the scanout reads stale data as well.
> > > 
> > > Well, assuming you actually write something to the bo w/ the CPU. If
> > > not, then it keeps scanning out the correct data.
> > 
> > I think an if (obj->pin_display) return -EBUSY; in the set_caching ioctl
> > would be good to fix this.
> 
> And we already do that check (as a result of obj->pin_count).
> Sorted.

Indeed. Patch merged to dinq (with a pimped commit message), thanks.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch