Skip to content

Commit 631b117

Browse files
committed
drm/i915/dsb: Correct DSB command buffer cache coherency settings
The display engine does not snoop the caches so we should 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@linux.intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20231009132204.15098-2-ville.syrjala@linux.intel.com Reviewed-by: Uma Shankar <uma.shankar@intel.com>
1 parent bcdcae6 commit 631b117

1 file changed

Lines changed: 11 additions & 4 deletions

File tree

drivers/gpu/drm/i915/display/intel_dsb.c

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ void intel_dsb_finish(struct intel_dsb *dsb)
316316
DSB_FORCE_DEWAKE, 0);
317317

318318
intel_dsb_align_tail(dsb);
319+
320+
i915_gem_object_flush_map(dsb->vma->obj);
319321
}
320322

321323
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,
462464
/* ~1 qword per instruction, full cachelines */
463465
size = ALIGN(max_cmds * 8, CACHELINE_BYTES);
464466

465-
if (HAS_LMEM(i915))
467+
if (HAS_LMEM(i915)) {
466468
obj = i915_gem_object_create_lmem(i915, PAGE_ALIGN(size),
467469
I915_BO_ALLOC_CONTIGUOUS);
468-
else
470+
if (IS_ERR(obj))
471+
goto out_put_rpm;
472+
} else {
469473
obj = i915_gem_object_create_internal(i915, PAGE_ALIGN(size));
470-
if (IS_ERR(obj))
471-
goto out_put_rpm;
474+
if (IS_ERR(obj))
475+
goto out_put_rpm;
476+
477+
i915_gem_object_set_cache_coherency(obj, I915_CACHE_NONE);
478+
}
472479

473480
vma = i915_gem_object_ggtt_pin(obj, NULL, 0, 0, 0);
474481
if (IS_ERR(vma)) {

0 commit comments

Comments
 (0)