[Intel-gfx] [PATCH 02/27] drm/i915: Mark CPU cache as dirty on every transition for CPU writes

[Dongwon Kim]

Chris,

Just to make sure, you want to just remove write_domain check from 
if statement before clflush in execbuffer_move_to_gpu. Am I right?
I will try both (cache_dirty only vs cache_dirty & !cache_coherent)
and get back to you shortly. 

On Wed, Apr 19, 2017 at 07:08:33PM +0100, Chris Wilson wrote:
> On Wed, Apr 19, 2017 at 09:52:28AM -0700, Dongwon Kim wrote:
> > I tried your patch but it didn't fix the original 
> > problem. I think it is somehow related to the flushing condition
> > here:
> > 
> > @@ -1129,10 +1129,8 @@ i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req,
> >  	if (vma->exec_entry->flags & EXEC_OBJECT_ASYNC)
> >  		continue;
> > 
> > 	if (obj->base.write_domain & I915_GEM_DOMAIN_CPU) {
> > +	if (obj->base.write_domain & obj->cache_dirty)
> >  		i915_gem_clflush_object(obj, 0);
> > -		obj->base.write_domain = 0;
> > -	}
> > 
> > here, we do clflush only if write_domain is not 0 even if cache_dirty
> > flag is set after your patch is applied.
> 
> This can be just reduced to if (obj->cache_dirty) clflush().
> 
> We're slightly better in that we don't set obj->cache_dirty so often for
> normal gpu rendering (just on transitions away from the gpu now), but it
> still means we will be redundantly checking for clflushes prior to
> rendering.
> 
> Can you double check that this patch + if (obj->cache_dirty) works for you?
> 
> What I guess I really want here is
> 	if (obj->cache_dirty & !obj->cache_coherent)
> essentially inlining the check from clflush.
> -Chris
> 
> -- 
> Chris Wilson, Intel Open Source Technology Centre