| Message ID | 20211021114410.2437099-2-matthew.auld@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/4] drm/i915/clflush: fixup handling of cache_dirty | expand |
On 10/21/21 13:44, Matthew Auld wrote: > We seem to have an unfortunate issue where we arrive from: > > i915_gem_object_flush_if_display+0x86/0xd0 [i915] > intel_user_framebuffer_dirty+0x1a/0x50 [i915] > drm_mode_dirtyfb_ioctl+0xfb/0x1b0 > > Which can be before the pages are populated(and pinned for display), and > so i915_gem_object_has_struct_page() might still return true, as per the > ttm backend. We could re-order the later get_pages() call here, but > since on discrete everything should already be coherent, with the > exception of the display engine, and even there display surfaces must be > allocated in device local-memory anyway, so there should in theory be no > conceivable reason to ever call i915_gem_clflush_object() on discrete. > > References: https://gitlab.freedesktop.org/drm/intel/-/issues/4320 > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com>
diff --git a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c index d09365b5eb29..b0822fd99709 100644 --- a/drivers/gpu/drm/i915/gem/i915_gem_clflush.c +++ b/drivers/gpu/drm/i915/gem/i915_gem_clflush.c @@ -70,6 +70,8 @@ static struct clflush *clflush_work_create(struct drm_i915_gem_object *obj) bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, unsigned int flags) { + + struct drm_i915_private *i915 = to_i915(obj->base.dev); struct clflush *clflush; assert_object_held(obj); @@ -81,7 +83,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, * anything not backed by physical memory we consider to be always * coherent and not need clflushing. */ - if (!i915_gem_object_has_struct_page(obj)) { + if (!i915_gem_object_has_struct_page(obj) || IS_DGFX(i915)) { obj->cache_dirty = false; return false; } @@ -106,7 +108,7 @@ bool i915_gem_clflush_object(struct drm_i915_gem_object *obj, if (clflush) { i915_sw_fence_await_reservation(&clflush->base.chain, obj->base.resv, NULL, true, - i915_fence_timeout(to_i915(obj->base.dev)), + i915_fence_timeout(i915), I915_FENCE_GFP); dma_resv_add_excl_fence(obj->base.resv, &clflush->base.dma); dma_fence_work_commit(&clflush->base);
We seem to have an unfortunate issue where we arrive from: i915_gem_object_flush_if_display+0x86/0xd0 [i915] intel_user_framebuffer_dirty+0x1a/0x50 [i915] drm_mode_dirtyfb_ioctl+0xfb/0x1b0 Which can be before the pages are populated(and pinned for display), and so i915_gem_object_has_struct_page() might still return true, as per the ttm backend. We could re-order the later get_pages() call here, but since on discrete everything should already be coherent, with the exception of the display engine, and even there display surfaces must be allocated in device local-memory anyway, so there should in theory be no conceivable reason to ever call i915_gem_clflush_object() on discrete. References: https://gitlab.freedesktop.org/drm/intel/-/issues/4320 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 | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)