Re: [PATCH 1/2] drm/i915: Mark CPU cache as dirty on every transition for CPU writes

[Dongwon Kim <dongwon.kim@intel.com>]

Chris,

In set_cache_level, we change obj->cache_level then
update obj->cache_coherent but I think this order
has to be reversed because coherency needs to be
determined based on the previous cache_level, not
the new one. 

After chaning code as shown below:

obj->cache_coherent = i915_gem_object_is_coherent(obj);
obj->cache_level = cache level;

, I see all tests are passing..

-DW

On Sat, Apr 29, 2017 at 09:43:52AM +0100, Chris Wilson wrote:
> On Fri, Apr 28, 2017 at 03:55:56PM -0700, Dongwon Kim wrote:
> > Hi Chris,
> > 
> > I tried this but I still see tests are failing. 
> > I wanted to debug it little further to find a specific
> > condition where clflush is missing but didn't have 
> > enough time. I will look into this early next week.
> 
> Did you check this patch separately?
> 
> So we are still missing a transition where we need to flag the cache as
> becoming dirty. And I still believe you have a
> "set-cache-level(snooped); gpu write; set-cache-level(none); gpu access"
> sequence. 
> 
> This patch should be marking as any write to a snooped bo as making the
> cache-dirty. So we should be caching any and all transitions from snoop
> to none, as that cache_dirty flag will not go away until we clflush.
> 
> And the real active ingredient of this patch is to always flush the
> dirty_cache before rendering, not just if the object was in the CPU
> write domain at that time.
> 
> Hmm, one thing to check is that if your userspace is not declaring some
> domain access that is dirtying the cache. Or if you are using mocs that
> override the cache tracking, without adjusting the PTE. If you are doing
> the latter, there isn't much the kernel can do to help.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx