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

[Chris Wilson]

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.
> 
> And now please look at this:
> 
> @@ -753,6 +766,11 @@ flush_write_domain(struct drm_i915_gem_object *obj, unsigned int flush_domains)
>         case I915_GEM_DOMAIN_CPU:
>                 i915_gem_clflush_object(obj, I915_CLFLUSH_SYNC);
>                 break;
> +
> +       case I915_GEM_DOMAIN_RENDER:
> +               if (gpu_write_needs_clflush(obj))
> +                       obj->cache_dirty = true;
> +               break;
>         }
> 
>         obj->base.write_domain=0;
> 
> So here, if the write_domain is I915_GEM_DOMAIN_RENDER, we set cache_dirty to true
> then reset write_domain.
> 
> So right after this flush_write_domain call, write_domain will be 0 but cache is
> still dirty. I am wondering if this is where that condition (write_domain==0 and 
> cache_dirty==1) originally came from.

And we definitely do not want to be flushing the cache for the GPU after
a GPU write. We only want for that cache to be flushed after the GPU if
it is moved to a non-coherent domain. That's the challenge.

I was also expecting that (incoherent) reads from the GPU would go through
the cache - we make the same assumption for CPU reads.

Thanks for testing, definitely back to the drawing board. Hmm, might be
worth taking the alternative approach and to always schedule an async
clflush in set-cache-domain. Just have to check that we have appropriate
waits and don't have any inappropriate ones.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre