From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> The display engine does not snoop the caches so shoukd to mark the DSB command buffer as I915_CACHE_NONE. i915_gem_object_create_internal() always gives us I915_CACHE_LLC on LLC platforms. And to make things 100% correct we should also clflush at the end, if necessary. Note that currently this is a non-issue as we always write the command buffer through a WC mapping, so a cache flush is not actually needed. But we might actually want to consider a WB mapping since we also end up reading from the command buffer (in the indexed reg write handling). Either that or we should do something else to avoid those reads (might actually be even more sensible on DGFX since we end up reading over PCIe). But we should measure the overhead first... Anyways, no real harm in adding the belts and suspenders here so that the code will work correctly regardless of how we map the buffer. If we do get a WC mapping (as we request) i915_gem_object_flush_map() will be a nop. Well, apart form a wmb() which may just flush the WC buffer a bit earlier than would otherwise happen (at the latest the mmio accesses would trigger the WC flush). Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> --- drivers/gpu/drm/i915/display/intel_dsb.c | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c index 7410ba3126f9..78b6fe24dcd8 100644 --- a/drivers/gpu/drm/i915/display/intel_dsb.c +++ b/drivers/gpu/drm/i915/display/intel_dsb.c @@ -316,6 +316,8 @@ void intel_dsb_finish(struct intel_dsb *dsb) DSB_FORCE_DEWAKE, 0); intel_dsb_align_tail(dsb); + + i915_gem_object_flush_map(dsb->vma->obj); } static int intel_dsb_dewake_scanline(const struct intel_crtc_state *crtc_state) @@ -462,13 +464,18 @@ struct intel_dsb *intel_dsb_prepare(const struct intel_crtc_state *crtc_state, /* ~1 qword per instruction, full cachelines */ size = ALIGN(max_cmds * 8, CACHELINE_BYTES); - if (HAS_LMEM(i915)) + if (HAS_LMEM(i915)) { obj = i915_gem_object_create_lmem(i915, PAGE_ALIGN(size), I915_BO_ALLOC_CONTIGUOUS); - else + if (IS_ERR(obj)) + goto out_put_rpm; + } else { obj = i915_gem_object_create_internal(i915, PAGE_ALIGN(size)); - if (IS_ERR(obj)) - goto out_put_rpm; + if (IS_ERR(obj)) + goto out_put_rpm; + + i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE); + } vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0); if (IS_ERR(vma)) { -- 2.41.0