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@xxxxxxxxx> Cc: Thomas Hellström <thomas.hellstrom@xxxxxxxxxxxxxxx> --- 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