Re: [Intel-gfx] [PATCH 1/4] drm/i915/clflush: fixup handling of cache_dirty

["Thomas Hellström" <thomas.hellstrom@linux.intel.com>]

On 10/21/21 13:44, Matthew Auld wrote:
> In theory if clflush_work_create() somehow fails here, and we don't yet
> have mm.pages populated then we end up resetting cache_dirty, which is
> likely wrong, since that will potentially skip the flush-on-acquire, if
> it was needed.
>
> It looks like intel_user_framebuffer_dirty() can arrive here before the
> pages are populated.
>
> Signed-off-by: Matthew Auld <matthew.auld@intel.com>
> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/gem/i915_gem_clflush.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
> index f0435c6feb68..d09365b5eb29 100644
> --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
> +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c
> @@ -20,6 +20,7 @@ static void __do_clflush(struct drm_i915_gem_object *obj)
>   {
>   	GEM_BUG_ON(!i915_gem_object_has_pages(obj));
>   	drm_clflush_sg(obj->mm.pages);
> +	obj->cache_dirty = false;
>   

I think the guidelines are to avoid updating state in async work if at 
all possible, so we need to add this after __do_clflush() in the sync 
path and after dma_fence_work_commit() in the async path.

Will that work?

/Thomas