* [PATCH v2 1/3] drm/xe/dma-buf: keep non-p2p imported buffers in system memory [not found] <20261007153605.1743626-5-matthew.auld@intel.com> @ 2026-10-07 15:36 ` Matthew Auld 2026-10-07 15:36 ` [PATCH v2 2/3] drm/xe/vm: make PREFETCH to TT sticky Matthew Auld 2026-10-07 15:36 ` [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers Matthew Auld 2 siblings, 0 replies; 23+ messages in thread From: Matthew Auld @ 2026-10-07 15:36 UTC (permalink / raw) To: intel-xe; +Cc: stable, Thomas Hellström, Matthew Brost When an exported buffer is mapped by a foreign device lacking peer-to-peer DMA support (such as an integrated GPU for display offload), xe_dma_buf_map() migrates the buffer to XE_PL_TT so that system memory pages can be accessed by the importer. However, since commit 5c87fee3c96c ("drm/xe: Attempt to bring bos back to VRAM after eviction"), XE_PL_TT is marked with TTM_PL_FLAG_FALLBACK in the buffer's placement. When DMABUF_MOVE_NOTIFY was enabled by default (meaning dynamic attachments are no longer pinned on map), the next xe_bo_validate() during render submission sees TT as a fallback placement and attempts to migrate the buffer back to VRAM. On the next frame, the foreign importer accesses the buffer, requiring another migration to TT, resulting in a continuous ping-pong between VRAM and system memory every frame and causing severe rendering performance degradations. To fix this, introduce xe_bo_migrate_tt_sticky() which records a temporary sticky placement in XE_PL_TT on successful migration. Subsequent validations use this sticky placement to keep the buffer in system memory until the non-p2p attachment is detached or memory pressure forces a fallback to the default placement. User is reporting what looks to be exactly this, with horrible performance when DMABUF_MOVE_NOTIFY was enabled by default. Assisted-by: LLM Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 Fixes: 5c87fee3c96c ("drm/xe: Attempt to bring bos back to VRAM after eviction") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: <stable@vger.kernel.org> # v6.12+ Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> --- drivers/gpu/drm/xe/xe_bo.c | 59 ++++++++++++++++++++++++++++++-- drivers/gpu/drm/xe/xe_bo.h | 3 ++ drivers/gpu/drm/xe/xe_bo_types.h | 4 +++ drivers/gpu/drm/xe/xe_dma_buf.c | 16 ++++++++- 4 files changed, 79 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c index 469fa4d5feb5..5065a7bac39c 100644 --- a/drivers/gpu/drm/xe/xe_bo.c +++ b/drivers/gpu/drm/xe/xe_bo.c @@ -3395,6 +3395,9 @@ int xe_bo_validate(struct xe_bo *bo, struct xe_vm *vm, bool allow_res_evict, .no_wait_gpu = false, .gfp_retry_mayfail = true, }; + struct ttm_placement *placement = bo->sticky_placement.num_placement ? + &bo->sticky_placement : + &bo->placement; int ret; if (xe_bo_is_pinned(bo)) @@ -3411,7 +3414,12 @@ int xe_bo_validate(struct xe_bo *bo, struct xe_vm *vm, bool allow_res_evict, xe_vm_set_validating(vm, allow_res_evict); trace_xe_bo_validate(bo); xe_validation_assert_exec(xe_bo_device(bo), exec, &bo->ttm.base); - ret = ttm_bo_validate(&bo->ttm, &bo->placement, &ctx); + ret = ttm_bo_validate(&bo->ttm, placement, &ctx); + if (ret && ret != -EINTR && ret != -ERESTARTSYS && + bo->sticky_placement.num_placement) { + xe_bo_reset_sticky_placement(bo); + ret = ttm_bo_validate(&bo->ttm, &bo->placement, &ctx); + } xe_vm_clear_validating(vm, allow_res_evict); return ret; @@ -3962,6 +3970,7 @@ int xe_bo_migrate(struct xe_bo *bo, u32 mem_type, struct ttm_operation_ctx *tctx }; struct ttm_placement placement; struct ttm_place requested; + int ret; xe_bo_assert_held(bo); tctx = tctx ? tctx : &ctx; @@ -3993,7 +4002,53 @@ int xe_bo_migrate(struct xe_bo *bo, u32 mem_type, struct ttm_operation_ctx *tctx if (!tctx->no_wait_gpu) xe_validation_assert_exec(xe_bo_device(bo), exec, &bo->ttm.base); - return ttm_bo_validate(&bo->ttm, &placement, tctx); + ret = ttm_bo_validate(&bo->ttm, &placement, tctx); + if (!ret) + xe_bo_reset_sticky_placement(bo); + return ret; +} + +/** + * xe_bo_migrate_tt_sticky - Migrate an object to TT and record its placement as sticky + * @bo: The buffer object to migrate. + * @tctx: The ttm_operation_ctx to use for migration, or NULL for default. + * @exec: The drm_exec transaction to use for exhaustive eviction. + * + * Like xe_bo_migrate() to XE_PL_TT, but on success records the resulting placement + * so that subsequent validations try to keep the object in TT instead of falling + * back to the default placement. The stickiness is removed if validation falls + * back to the default placement (e.g. under memory pressure), on any non-sticky + * migration, or via an explicit call to xe_bo_reset_sticky_placement(). + * + * Return: 0 on success. Negative error code on failure. + */ +int xe_bo_migrate_tt_sticky(struct xe_bo *bo, + struct ttm_operation_ctx *tctx, + struct drm_exec *exec) +{ + int ret; + + ret = xe_bo_migrate(bo, XE_PL_TT, tctx, exec); + if (!ret) { + xe_place_from_ttm_type(XE_PL_TT, &bo->sticky_place); + bo->sticky_placement = (struct ttm_placement){ + .num_placement = 1, + .placement = &bo->sticky_place, + }; + } + return ret; +} + +/** + * xe_bo_reset_sticky_placement - Reset sticky placement for an object + * @bo: The buffer object whose sticky placement should be cleared. + * + * Clear any sticky placement recorded by xe_bo_migrate_tt_sticky(), + * returning subsequent validations to the default placement. + */ +void xe_bo_reset_sticky_placement(struct xe_bo *bo) +{ + bo->sticky_placement.num_placement = 0; } /** diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h index 77b3a02e9b8e..1ddf3b6d4a9e 100644 --- a/drivers/gpu/drm/xe/xe_bo.h +++ b/drivers/gpu/drm/xe/xe_bo.h @@ -436,6 +436,9 @@ bool xe_bo_can_migrate(struct xe_bo *bo, u32 mem_type); int xe_bo_migrate(struct xe_bo *bo, u32 mem_type, struct ttm_operation_ctx *ctc, struct drm_exec *exec); +int xe_bo_migrate_tt_sticky(struct xe_bo *bo, struct ttm_operation_ctx *ctc, + struct drm_exec *exec); +void xe_bo_reset_sticky_placement(struct xe_bo *bo); int xe_bo_evict(struct xe_bo *bo, struct drm_exec *exec); int xe_bo_evict_pinned(struct xe_bo *bo); diff --git a/drivers/gpu/drm/xe/xe_bo_types.h b/drivers/gpu/drm/xe/xe_bo_types.h index 8ec4a01a0092..253a1dba55a7 100644 --- a/drivers/gpu/drm/xe/xe_bo_types.h +++ b/drivers/gpu/drm/xe/xe_bo_types.h @@ -56,6 +56,10 @@ struct xe_bo { struct ttm_place placements[XE_BO_MAX_PLACEMENTS]; /** @placement: current placement for this BO */ struct ttm_placement placement; + /** @sticky_placement: target placement from forced migration */ + struct ttm_placement sticky_placement; + /** @sticky_place: place for sticky_placement */ + struct ttm_place sticky_place; /** @ggtt_node: Array of GGTT nodes if this BO is mapped in the GGTTs */ struct xe_ggtt_node *ggtt_node[XE_MAX_TILES_PER_DEVICE]; /** @vmap: iosys map of this buffer */ diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c index 6a85b292dee7..b973579a69b8 100644 --- a/drivers/gpu/drm/xe/xe_dma_buf.c +++ b/drivers/gpu/drm/xe/xe_dma_buf.c @@ -59,6 +59,20 @@ static void xe_dma_buf_detach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) { struct drm_gem_object *obj = attach->dmabuf->priv; + struct xe_bo *bo = gem_to_xe_bo(obj); + bool has_non_p2p = false; + struct dma_buf_attachment *a; + + dma_resv_lock(dmabuf->resv, NULL); + list_for_each_entry(a, &dmabuf->attachments, node) { + if (!a->peer2peer) { + has_non_p2p = true; + break; + } + } + if (!has_non_p2p) + xe_bo_reset_sticky_placement(bo); + dma_resv_unlock(dmabuf->resv); xe_pm_runtime_put(to_xe_device(obj->dev)); } @@ -129,7 +143,7 @@ static struct sg_table *xe_dma_buf_map(struct dma_buf_attachment *attach, if (!xe_bo_is_pinned(bo)) { if (!attach->peer2peer) - r = xe_bo_migrate(bo, XE_PL_TT, NULL, exec); + r = xe_bo_migrate_tt_sticky(bo, NULL, exec); else r = xe_bo_validate(bo, NULL, false, exec); if (r) -- 2.55.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 2/3] drm/xe/vm: make PREFETCH to TT sticky [not found] <20261007153605.1743626-5-matthew.auld@intel.com> 2026-10-07 15:36 ` [PATCH v2 1/3] drm/xe/dma-buf: keep non-p2p imported buffers in system memory Matthew Auld @ 2026-10-07 15:36 ` Matthew Auld 2026-10-07 15:36 ` [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers Matthew Auld 2 siblings, 0 replies; 23+ messages in thread From: Matthew Auld @ 2026-10-07 15:36 UTC (permalink / raw) To: intel-xe; +Cc: Thomas Hellström, Matthew Brost, stable When userspace explicitly requests DRM_XE_VM_BIND_OP_PREFETCH to system memory (XE_PL_TT), xe_bo_migrate() moves the buffer object to TT. However, because XE_PL_TT is marked with TTM_PL_FLAG_FALLBACK in the buffer's default placement (introduced in commit 5c87fee3c96c ("drm/xe: Attempt to bring bos back to VRAM after eviction")), any subsequent validation during exec immediately migrates the buffer back to VRAM, completely undoing the userspace prefetch request. Use xe_bo_migrate_tt_sticky() when prefetching to XE_PL_TT so that the buffer remains resident in system memory across subsequent validations. Assisted-by: LLM Reported-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> Fixes: 5c87fee3c96c ("drm/xe: Attempt to bring bos back to VRAM after eviction") Signed-off-by: Matthew Auld <matthew.auld@intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: <stable@vger.kernel.org> # v6.12+ Reviewed-by: Thomas Hellström <thomas.hellstrom@linux.intel.com> --- drivers/gpu/drm/xe/xe_vm.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_vm.c b/drivers/gpu/drm/xe/xe_vm.c index ef369df4aa26..093ad3e39608 100644 --- a/drivers/gpu/drm/xe/xe_vm.c +++ b/drivers/gpu/drm/xe/xe_vm.c @@ -3428,7 +3428,10 @@ static int op_lock_and_prep(struct drm_exec *exec, struct xe_vm *vm, mem_type = region_to_mem_type[region]; } - err = xe_bo_migrate(bo, mem_type, NULL, exec); + if (mem_type == XE_PL_TT) + err = xe_bo_migrate_tt_sticky(bo, NULL, exec); + else + err = xe_bo_migrate(bo, mem_type, NULL, exec); } break; } -- 2.55.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers [not found] <20261007153605.1743626-5-matthew.auld@intel.com> 2026-10-07 15:36 ` [PATCH v2 1/3] drm/xe/dma-buf: keep non-p2p imported buffers in system memory Matthew Auld 2026-10-07 15:36 ` [PATCH v2 2/3] drm/xe/vm: make PREFETCH to TT sticky Matthew Auld @ 2026-10-07 15:36 ` Matthew Auld 2026-10-07 18:11 ` Thomas Hellström 2 siblings, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-07 15:36 UTC (permalink / raw) To: intel-xe; +Cc: Thomas Hellström, Matthew Brost, stable If p2p is allowed on the CPU (like on AMD), we currently permit p2p for all exported VRAM buffers. However, in some multi-GPU setups, like with prime rendering on a dgpu and scanout/compositor using the igpu, it looks like the exported dma-buf window buffer can in some cases end up using p2p. However, user is reporting a kernel regression where performance here massively tanks across many games/workloads, where it looks like on each frame, the igpu is reading/sampling from VRAM over pcie for the window buffer, since it uses p2p. p2p is currently more intended for dgpu <-> dgpu, and not so much display buffers with igpu <-> dgpu, at least for now. We don't currently have any hints to say "please use p2p for this buffer, or please don't", however we can use some heuristics to try to determine if this is likely a display buffer being exported to an igpu, which probably never intended for p2p: 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get tagged with this, but not always. 2. Importer being a PCI device with display, which is not xe driver. We also rely on the existing p2p Intel CPU whitelist to stop this happening on xe igpu. Currently p2p is only enabled for server CPUs, on Intel. We also limit the above to dual placement buffers. The original regression was bisected to commit 609fc8766d68 ("dma-buf: Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was already present when enabling that knob, so this goes back even further. User has confirmed that along with the sticky dma-buf TT placement fix, to prevent ping-ponging, the performance is back to normal. Assisted-by: LLM Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 Signed-off-by: Matthew Auld <matthew.auld@intel.com> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> Cc: Matthew Brost <matthew.brost@intel.com> Cc: <stable@vger.kernel.org> # v6.12 --- drivers/gpu/drm/xe/xe_bo.c | 3 +- drivers/gpu/drm/xe/xe_bo.h | 1 + drivers/gpu/drm/xe/xe_dma_buf.c | 63 +++++++++++++++++++++++++-------- 3 files changed, 51 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c index 5065a7bac39c..e63437a261cb 100644 --- a/drivers/gpu/drm/xe/xe_bo.c +++ b/drivers/gpu/drm/xe/xe_bo.c @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, void *data, * Display scanout is always non-coherent with the CPU cache. */ if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) - bo_flags |= XE_BO_FLAG_FORCE_WC; + bo_flags |= XE_BO_FLAG_FORCE_WC | XE_BO_FLAG_SCANOUT; if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file *file_priv, DRM_XE_GEM_CPU_CACHING_WC, XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | XE_BO_FLAG_FORCE_WC | + XE_BO_FLAG_SCANOUT | XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); if (IS_ERR(bo)) return PTR_ERR(bo); diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h index 1ddf3b6d4a9e..6965c2655731 100644 --- a/drivers/gpu/drm/xe/xe_bo.h +++ b/drivers/gpu/drm/xe/xe_bo.h @@ -56,6 +56,7 @@ #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) #define XE_BO_FLAG_NO_COMPRESSION BIT(26) #define XE_BO_FLAG_NEEDS_1G BIT(27) +#define XE_BO_FLAG_SCANOUT BIT(28) /* this one is trigger internally only */ #define XE_BO_FLAG_INTERNAL_TEST BIT(30) diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c b/drivers/gpu/drm/xe/xe_dma_buf.c index b973579a69b8..0a347f09cee4 100644 --- a/drivers/gpu/drm/xe/xe_dma_buf.c +++ b/drivers/gpu/drm/xe/xe_dma_buf.c @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct _ddebug *desc, bool *flag) #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) #endif +static void xe_dma_buf_move_notify(struct dma_buf_attachment *attach) +{ + struct drm_gem_object *obj = attach->importer_priv; + struct xe_bo *bo = gem_to_xe_bo(obj); + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; + + XE_WARN_ON(xe_bo_evict(bo, exec)); +} + +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { + .allow_peer2peer = true, + .invalidate_mappings = xe_dma_buf_move_notify +}; + +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) +{ + while (dev) { + if (dev_is_pci(dev)) + return to_pci_dev(dev); + dev = dev->parent; + } + return NULL; +} + +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment *attach) +{ + return attach->importer_ops == &xe_dma_buf_attach_ops; +} + static int xe_dma_buf_attach(struct dma_buf *dmabuf, struct dma_buf_attachment *attach) { struct drm_gem_object *obj = attach->dmabuf->priv; + struct xe_bo *bo = gem_to_xe_bo(obj); bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf *dmabuf, pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach->dev, verbose_p2p_dist) < 0) attach->peer2peer = false; - if (!attach->peer2peer && !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { + struct pci_dev *client = xe_find_parent_pci_dev(attach->dev); + + /* + * There are perf issues with using p2p with display buffers, + * like with igpu (compositor) <-> dgpu (exported p2p window + * buffer). Probably we need some actual userspace hints. We + * shouldn't really assume anything. Also, note that some of + * these buffers are not always tagged with SCANOUT. + */ + if (bo->flags & XE_BO_FLAG_SCANOUT) + attach->peer2peer = false; + else if (client && pci_is_display(client) && + !xe_dma_buf_attach_is_xe_device(attach)) + attach->peer2peer = false; + } + + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) return -EOPNOTSUPP; xe_pm_runtime_get(to_xe_device(obj->dev)); @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, struct dma_buf *dma_buf) return ret ? ERR_PTR(ret) : &bo->ttm.base; } -static void xe_dma_buf_move_notify(struct dma_buf_attachment *attach) -{ - struct drm_gem_object *obj = attach->importer_priv; - struct xe_bo *bo = gem_to_xe_bo(obj); - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; - - XE_WARN_ON(xe_bo_evict(bo, exec)); -} - -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { - .allow_peer2peer = true, - .invalidate_mappings = xe_dma_buf_move_notify -}; - #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) struct dma_buf_test_params { -- 2.55.0 ^ permalink raw reply related [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-07 15:36 ` [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers Matthew Auld @ 2026-10-07 18:11 ` Thomas Hellström 2026-10-08 9:46 ` Matthew Auld 0 siblings, 1 reply; 23+ messages in thread From: Thomas Hellström @ 2026-10-07 18:11 UTC (permalink / raw) To: Matthew Auld, intel-xe; +Cc: Matthew Brost, stable On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: > If p2p is allowed on the CPU (like on AMD), we currently permit p2p > for > all exported VRAM buffers. However, in some multi-GPU setups, like > with > prime rendering on a dgpu and scanout/compositor using the igpu, it > looks like the exported dma-buf window buffer can in some cases end > up > using p2p. > > However, user is reporting a kernel regression where performance here > massively tanks across many games/workloads, where it looks like on > each > frame, the igpu is reading/sampling from VRAM over pcie for the > window > buffer, since it uses p2p. p2p is currently more intended for dgpu <- > > > dgpu, and not so much display buffers with igpu <-> dgpu, at least > for > now. > > We don't currently have any hints to say "please use p2p for this > buffer, or please don't", however we can use some heuristics to try > to > determine if this is likely a display buffer being exported to an > igpu, > which probably never intended for p2p: > > 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get > tagged with this, but not always. > > 2. Importer being a PCI device with display, which is not xe driver. > > We also rely on the existing p2p Intel CPU whitelist to stop this > happening on xe igpu. Currently p2p is only enabled for server CPUs, > on > Intel. We also limit the above to dual placement buffers. > > The original regression was bisected to commit 609fc8766d68 ("dma- > buf: > Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was > already present when enabling that knob, so this goes back even > further. > > User has confirmed that along with the sticky dma-buf TT placement > fix, > to prevent ping-ponging, the performance is back to normal. > Actually, I think this needs to be fixed on the importer side. Until we have some some user-space hints, I don't think the exporter should second-guess. Most sane solution IMO is if igfx importers could just not advertise pcie p2p support? Thanks, Thomas > Assisted-by: LLM > Closes: > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel > GPUs") > Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> > Cc: Matthew Brost <matthew.brost@intel.com> > Cc: <stable@vger.kernel.org> # v6.12 > --- > drivers/gpu/drm/xe/xe_bo.c | 3 +- > drivers/gpu/drm/xe/xe_bo.h | 1 + > drivers/gpu/drm/xe/xe_dma_buf.c | 63 +++++++++++++++++++++++++------ > -- > 3 files changed, 51 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c > index 5065a7bac39c..e63437a261cb 100644 > --- a/drivers/gpu/drm/xe/xe_bo.c > +++ b/drivers/gpu/drm/xe/xe_bo.c > @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, > void *data, > * Display scanout is always non-coherent with the CPU > cache. > */ > if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) > - bo_flags |= XE_BO_FLAG_FORCE_WC; > + bo_flags |= XE_BO_FLAG_FORCE_WC | > XE_BO_FLAG_SCANOUT; > > if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { > if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) > @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file > *file_priv, > DRM_XE_GEM_CPU_CACHING_WC, > > XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | > XE_BO_FLAG_FORCE_WC | > + XE_BO_FLAG_SCANOUT | > XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); > if (IS_ERR(bo)) > return PTR_ERR(bo); > diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h > index 1ddf3b6d4a9e..6965c2655731 100644 > --- a/drivers/gpu/drm/xe/xe_bo.h > +++ b/drivers/gpu/drm/xe/xe_bo.h > @@ -56,6 +56,7 @@ > #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) > #define XE_BO_FLAG_NO_COMPRESSION BIT(26) > #define XE_BO_FLAG_NEEDS_1G BIT(27) > +#define XE_BO_FLAG_SCANOUT BIT(28) > > /* this one is trigger internally only */ > #define XE_BO_FLAG_INTERNAL_TEST BIT(30) > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c > b/drivers/gpu/drm/xe/xe_dma_buf.c > index b973579a69b8..0a347f09cee4 100644 > --- a/drivers/gpu/drm/xe/xe_dma_buf.c > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c > @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct > _ddebug *desc, bool *flag) > #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) > #endif > > +static void xe_dma_buf_move_notify(struct dma_buf_attachment > *attach) > +{ > + struct drm_gem_object *obj = attach->importer_priv; > + struct xe_bo *bo = gem_to_xe_bo(obj); > + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; > + > + XE_WARN_ON(xe_bo_evict(bo, exec)); > +} > + > +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { > + .allow_peer2peer = true, > + .invalidate_mappings = xe_dma_buf_move_notify > +}; > + > +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) > +{ > + while (dev) { > + if (dev_is_pci(dev)) > + return to_pci_dev(dev); > + dev = dev->parent; > + } > + return NULL; > +} > + > +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment > *attach) > +{ > + return attach->importer_ops == &xe_dma_buf_attach_ops; > +} > + > static int xe_dma_buf_attach(struct dma_buf *dmabuf, > struct dma_buf_attachment *attach) > { > struct drm_gem_object *obj = attach->dmabuf->priv; > + struct xe_bo *bo = gem_to_xe_bo(obj); > bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); > > xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); > @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf > *dmabuf, > pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- > >dev, verbose_p2p_dist) < 0) > attach->peer2peer = false; > > - if (!attach->peer2peer && > !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) > + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { > + struct pci_dev *client = > xe_find_parent_pci_dev(attach->dev); > + > + /* > + * There are perf issues with using p2p with display > buffers, > + * like with igpu (compositor) <-> dgpu (exported > p2p window > + * buffer). Probably we need some actual userspace > hints. We > + * shouldn't really assume anything. Also, note that > some of > + * these buffers are not always tagged with SCANOUT. > + */ > + if (bo->flags & XE_BO_FLAG_SCANOUT) > + attach->peer2peer = false; > + else if (client && pci_is_display(client) && > + !xe_dma_buf_attach_is_xe_device(attach)) > + attach->peer2peer = false; > + } > + > + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) > return -EOPNOTSUPP; > > xe_pm_runtime_get(to_xe_device(obj->dev)); > @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, > struct dma_buf *dma_buf) > return ret ? ERR_PTR(ret) : &bo->ttm.base; > } > > -static void xe_dma_buf_move_notify(struct dma_buf_attachment > *attach) > -{ > - struct drm_gem_object *obj = attach->importer_priv; > - struct xe_bo *bo = gem_to_xe_bo(obj); > - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; > - > - XE_WARN_ON(xe_bo_evict(bo, exec)); > -} > - > -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { > - .allow_peer2peer = true, > - .invalidate_mappings = xe_dma_buf_move_notify > -}; > - > #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) > > struct dma_buf_test_params { ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-07 18:11 ` Thomas Hellström @ 2026-10-08 9:46 ` Matthew Auld 2026-10-08 9:55 ` Christian König 0 siblings, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-08 9:46 UTC (permalink / raw) To: Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König, Christian König On 07/10/2026 19:11, Thomas Hellström wrote: > On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >> for >> all exported VRAM buffers. However, in some multi-GPU setups, like >> with >> prime rendering on a dgpu and scanout/compositor using the igpu, it >> looks like the exported dma-buf window buffer can in some cases end >> up >> using p2p. >> >> However, user is reporting a kernel regression where performance here >> massively tanks across many games/workloads, where it looks like on >> each >> frame, the igpu is reading/sampling from VRAM over pcie for the >> window >> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>> >> dgpu, and not so much display buffers with igpu <-> dgpu, at least >> for >> now. >> >> We don't currently have any hints to say "please use p2p for this >> buffer, or please don't", however we can use some heuristics to try >> to >> determine if this is likely a display buffer being exported to an >> igpu, >> which probably never intended for p2p: >> >> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >> tagged with this, but not always. >> >> 2. Importer being a PCI device with display, which is not xe driver. >> >> We also rely on the existing p2p Intel CPU whitelist to stop this >> happening on xe igpu. Currently p2p is only enabled for server CPUs, >> on >> Intel. We also limit the above to dual placement buffers. >> >> The original regression was bisected to commit 609fc8766d68 ("dma- >> buf: >> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >> already present when enabling that knob, so this goes back even >> further. >> >> User has confirmed that along with the sticky dma-buf TT placement >> fix, >> to prevent ping-ponging, the performance is back to normal. >> > > Actually, I think this needs to be fixed on the importer side. Until we > have some some user-space hints, I don't think the exporter should > second-guess. Most sane solution IMO is if igfx importers could just > not advertise pcie p2p support? Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. Some questions: 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? > > Thanks, > Thomas > >> Assisted-by: LLM >> Closes: >> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >> GPUs") >> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >> Cc: Matthew Brost <matthew.brost@intel.com> >> Cc: <stable@vger.kernel.org> # v6.12 >> --- >> drivers/gpu/drm/xe/xe_bo.c | 3 +- >> drivers/gpu/drm/xe/xe_bo.h | 1 + >> drivers/gpu/drm/xe/xe_dma_buf.c | 63 +++++++++++++++++++++++++------ >> -- >> 3 files changed, 51 insertions(+), 16 deletions(-) >> >> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >> index 5065a7bac39c..e63437a261cb 100644 >> --- a/drivers/gpu/drm/xe/xe_bo.c >> +++ b/drivers/gpu/drm/xe/xe_bo.c >> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >> void *data, >> * Display scanout is always non-coherent with the CPU >> cache. >> */ >> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >> - bo_flags |= XE_BO_FLAG_FORCE_WC; >> + bo_flags |= XE_BO_FLAG_FORCE_WC | >> XE_BO_FLAG_SCANOUT; >> >> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >> *file_priv, >> DRM_XE_GEM_CPU_CACHING_WC, >> >> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >> XE_BO_FLAG_FORCE_WC | >> + XE_BO_FLAG_SCANOUT | >> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >> if (IS_ERR(bo)) >> return PTR_ERR(bo); >> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >> index 1ddf3b6d4a9e..6965c2655731 100644 >> --- a/drivers/gpu/drm/xe/xe_bo.h >> +++ b/drivers/gpu/drm/xe/xe_bo.h >> @@ -56,6 +56,7 @@ >> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >> #define XE_BO_FLAG_NEEDS_1G BIT(27) >> +#define XE_BO_FLAG_SCANOUT BIT(28) >> >> /* this one is trigger internally only */ >> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >> b/drivers/gpu/drm/xe/xe_dma_buf.c >> index b973579a69b8..0a347f09cee4 100644 >> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >> _ddebug *desc, bool *flag) >> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >> #endif >> >> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >> *attach) >> +{ >> + struct drm_gem_object *obj = attach->importer_priv; >> + struct xe_bo *bo = gem_to_xe_bo(obj); >> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >> + >> + XE_WARN_ON(xe_bo_evict(bo, exec)); >> +} >> + >> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >> + .allow_peer2peer = true, >> + .invalidate_mappings = xe_dma_buf_move_notify >> +}; >> + >> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >> +{ >> + while (dev) { >> + if (dev_is_pci(dev)) >> + return to_pci_dev(dev); >> + dev = dev->parent; >> + } >> + return NULL; >> +} >> + >> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >> *attach) >> +{ >> + return attach->importer_ops == &xe_dma_buf_attach_ops; >> +} >> + >> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >> struct dma_buf_attachment *attach) >> { >> struct drm_gem_object *obj = attach->dmabuf->priv; >> + struct xe_bo *bo = gem_to_xe_bo(obj); >> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >> >> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >> *dmabuf, >> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>> dev, verbose_p2p_dist) < 0) >> attach->peer2peer = false; >> >> - if (!attach->peer2peer && >> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >> + struct pci_dev *client = >> xe_find_parent_pci_dev(attach->dev); >> + >> + /* >> + * There are perf issues with using p2p with display >> buffers, >> + * like with igpu (compositor) <-> dgpu (exported >> p2p window >> + * buffer). Probably we need some actual userspace >> hints. We >> + * shouldn't really assume anything. Also, note that >> some of >> + * these buffers are not always tagged with SCANOUT. >> + */ >> + if (bo->flags & XE_BO_FLAG_SCANOUT) >> + attach->peer2peer = false; >> + else if (client && pci_is_display(client) && >> + !xe_dma_buf_attach_is_xe_device(attach)) >> + attach->peer2peer = false; >> + } >> + >> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >> return -EOPNOTSUPP; >> >> xe_pm_runtime_get(to_xe_device(obj->dev)); >> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >> struct dma_buf *dma_buf) >> return ret ? ERR_PTR(ret) : &bo->ttm.base; >> } >> >> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >> *attach) >> -{ >> - struct drm_gem_object *obj = attach->importer_priv; >> - struct xe_bo *bo = gem_to_xe_bo(obj); >> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >> - >> - XE_WARN_ON(xe_bo_evict(bo, exec)); >> -} >> - >> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >> - .allow_peer2peer = true, >> - .invalidate_mappings = xe_dma_buf_move_notify >> -}; >> - >> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >> >> struct dma_buf_test_params { ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-08 9:46 ` Matthew Auld @ 2026-10-08 9:55 ` Christian König 2026-10-08 11:17 ` Matthew Auld 0 siblings, 1 reply; 23+ messages in thread From: Christian König @ 2026-10-08 9:55 UTC (permalink / raw) To: Matthew Auld, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 10/8/26 11:46, Matthew Auld wrote: > On 07/10/2026 19:11, Thomas Hellström wrote: >> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>> for >>> all exported VRAM buffers. However, in some multi-GPU setups, like >>> with >>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>> looks like the exported dma-buf window buffer can in some cases end >>> up >>> using p2p. >>> >>> However, user is reporting a kernel regression where performance here >>> massively tanks across many games/workloads, where it looks like on >>> each >>> frame, the igpu is reading/sampling from VRAM over pcie for the >>> window >>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>> >>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>> for >>> now. >>> >>> We don't currently have any hints to say "please use p2p for this >>> buffer, or please don't", however we can use some heuristics to try >>> to >>> determine if this is likely a display buffer being exported to an >>> igpu, >>> which probably never intended for p2p: >>> >>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>> tagged with this, but not always. >>> >>> 2. Importer being a PCI device with display, which is not xe driver. >>> >>> We also rely on the existing p2p Intel CPU whitelist to stop this >>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>> on >>> Intel. We also limit the above to dual placement buffers. >>> >>> The original regression was bisected to commit 609fc8766d68 ("dma- >>> buf: >>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>> already present when enabling that knob, so this goes back even >>> further. >>> >>> User has confirmed that along with the sticky dma-buf TT placement >>> fix, >>> to prevent ping-ponging, the performance is back to normal. >>> >> >> Actually, I think this needs to be fixed on the importer side. Until we >> have some some user-space hints, I don't think the exporter should >> second-guess. Most sane solution IMO is if igfx importers could just >> not advertise pcie p2p support? > > Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. > > Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. > > Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. > > Some questions: > > 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. > > 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. Regards, Christian. > >> >> Thanks, >> Thomas >> >>> Assisted-by: LLM >>> Closes: >>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>> GPUs") >>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>> Cc: Matthew Brost <matthew.brost@intel.com> >>> Cc: <stable@vger.kernel.org> # v6.12 >>> --- >>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 +++++++++++++++++++++++++------ >>> -- >>> 3 files changed, 51 insertions(+), 16 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>> index 5065a7bac39c..e63437a261cb 100644 >>> --- a/drivers/gpu/drm/xe/xe_bo.c >>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>> void *data, >>> * Display scanout is always non-coherent with the CPU >>> cache. >>> */ >>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>> XE_BO_FLAG_SCANOUT; >>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>> *file_priv, >>> DRM_XE_GEM_CPU_CACHING_WC, >>> >>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>> XE_BO_FLAG_FORCE_WC | >>> + XE_BO_FLAG_SCANOUT | >>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>> if (IS_ERR(bo)) >>> return PTR_ERR(bo); >>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>> index 1ddf3b6d4a9e..6965c2655731 100644 >>> --- a/drivers/gpu/drm/xe/xe_bo.h >>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>> @@ -56,6 +56,7 @@ >>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>> /* this one is trigger internally only */ >>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>> index b973579a69b8..0a347f09cee4 100644 >>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>> _ddebug *desc, bool *flag) >>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>> #endif >>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>> *attach) >>> +{ >>> + struct drm_gem_object *obj = attach->importer_priv; >>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>> + >>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>> +} >>> + >>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>> + .allow_peer2peer = true, >>> + .invalidate_mappings = xe_dma_buf_move_notify >>> +}; >>> + >>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>> +{ >>> + while (dev) { >>> + if (dev_is_pci(dev)) >>> + return to_pci_dev(dev); >>> + dev = dev->parent; >>> + } >>> + return NULL; >>> +} >>> + >>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>> *attach) >>> +{ >>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>> +} >>> + >>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>> struct dma_buf_attachment *attach) >>> { >>> struct drm_gem_object *obj = attach->dmabuf->priv; >>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>> *dmabuf, >>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>> dev, verbose_p2p_dist) < 0) >>> attach->peer2peer = false; >>> - if (!attach->peer2peer && >>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>> + struct pci_dev *client = >>> xe_find_parent_pci_dev(attach->dev); >>> + >>> + /* >>> + * There are perf issues with using p2p with display >>> buffers, >>> + * like with igpu (compositor) <-> dgpu (exported >>> p2p window >>> + * buffer). Probably we need some actual userspace >>> hints. We >>> + * shouldn't really assume anything. Also, note that >>> some of >>> + * these buffers are not always tagged with SCANOUT. >>> + */ >>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>> + attach->peer2peer = false; >>> + else if (client && pci_is_display(client) && >>> + !xe_dma_buf_attach_is_xe_device(attach)) >>> + attach->peer2peer = false; >>> + } >>> + >>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>> return -EOPNOTSUPP; >>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>> struct dma_buf *dma_buf) >>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>> } >>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>> *attach) >>> -{ >>> - struct drm_gem_object *obj = attach->importer_priv; >>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>> - >>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>> -} >>> - >>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>> - .allow_peer2peer = true, >>> - .invalidate_mappings = xe_dma_buf_move_notify >>> -}; >>> - >>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>> struct dma_buf_test_params { > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-08 9:55 ` Christian König @ 2026-10-08 11:17 ` Matthew Auld 2026-10-09 9:16 ` Matthew Auld 0 siblings, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-08 11:17 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 08/10/2026 10:55, Christian König wrote: > On 10/8/26 11:46, Matthew Auld wrote: >> On 07/10/2026 19:11, Thomas Hellström wrote: >>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>>> for >>>> all exported VRAM buffers. However, in some multi-GPU setups, like >>>> with >>>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>>> looks like the exported dma-buf window buffer can in some cases end >>>> up >>>> using p2p. >>>> >>>> However, user is reporting a kernel regression where performance here >>>> massively tanks across many games/workloads, where it looks like on >>>> each >>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>> window >>>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>>> >>>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>>> for >>>> now. >>>> >>>> We don't currently have any hints to say "please use p2p for this >>>> buffer, or please don't", however we can use some heuristics to try >>>> to >>>> determine if this is likely a display buffer being exported to an >>>> igpu, >>>> which probably never intended for p2p: >>>> >>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>>> tagged with this, but not always. >>>> >>>> 2. Importer being a PCI device with display, which is not xe driver. >>>> >>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>>> on >>>> Intel. We also limit the above to dual placement buffers. >>>> >>>> The original regression was bisected to commit 609fc8766d68 ("dma- >>>> buf: >>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>> already present when enabling that knob, so this goes back even >>>> further. >>>> >>>> User has confirmed that along with the sticky dma-buf TT placement >>>> fix, >>>> to prevent ping-ponging, the performance is back to normal. >>>> >>> >>> Actually, I think this needs to be fixed on the importer side. Until we >>> have some some user-space hints, I don't think the exporter should >>> second-guess. Most sane solution IMO is if igfx importers could just >>> not advertise pcie p2p support? >> >> Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. >> >> Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. >> >> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >> >> Some questions: >> >> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >> >> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? > > If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? > > At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. > > And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. > > Regards, > Christian. > >> >>> >>> Thanks, >>> Thomas >>> >>>> Assisted-by: LLM >>>> Closes: >>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>> GPUs") >>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>> Cc: <stable@vger.kernel.org> # v6.12 >>>> --- >>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 +++++++++++++++++++++++++------ >>>> -- >>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>> >>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>>> index 5065a7bac39c..e63437a261cb 100644 >>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>> void *data, >>>> * Display scanout is always non-coherent with the CPU >>>> cache. >>>> */ >>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>> XE_BO_FLAG_SCANOUT; >>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>> *file_priv, >>>> DRM_XE_GEM_CPU_CACHING_WC, >>>> >>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>> XE_BO_FLAG_FORCE_WC | >>>> + XE_BO_FLAG_SCANOUT | >>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>> if (IS_ERR(bo)) >>>> return PTR_ERR(bo); >>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>> @@ -56,6 +56,7 @@ >>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>> /* this one is trigger internally only */ >>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>> index b973579a69b8..0a347f09cee4 100644 >>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>> _ddebug *desc, bool *flag) >>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>> #endif >>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>> *attach) >>>> +{ >>>> + struct drm_gem_object *obj = attach->importer_priv; >>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>> + >>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>> +} >>>> + >>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>> + .allow_peer2peer = true, >>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>> +}; >>>> + >>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>> +{ >>>> + while (dev) { >>>> + if (dev_is_pci(dev)) >>>> + return to_pci_dev(dev); >>>> + dev = dev->parent; >>>> + } >>>> + return NULL; >>>> +} >>>> + >>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>> *attach) >>>> +{ >>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>> +} >>>> + >>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>> struct dma_buf_attachment *attach) >>>> { >>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>> *dmabuf, >>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>> dev, verbose_p2p_dist) < 0) >>>> attach->peer2peer = false; >>>> - if (!attach->peer2peer && >>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>> + struct pci_dev *client = >>>> xe_find_parent_pci_dev(attach->dev); >>>> + >>>> + /* >>>> + * There are perf issues with using p2p with display >>>> buffers, >>>> + * like with igpu (compositor) <-> dgpu (exported >>>> p2p window >>>> + * buffer). Probably we need some actual userspace >>>> hints. We >>>> + * shouldn't really assume anything. Also, note that >>>> some of >>>> + * these buffers are not always tagged with SCANOUT. >>>> + */ >>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>> + attach->peer2peer = false; >>>> + else if (client && pci_is_display(client) && >>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>> + attach->peer2peer = false; >>>> + } >>>> + >>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>> return -EOPNOTSUPP; >>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>> struct dma_buf *dma_buf) >>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>> } >>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>> *attach) >>>> -{ >>>> - struct drm_gem_object *obj = attach->importer_priv; >>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>> - >>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>> -} >>>> - >>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>> - .allow_peer2peer = true, >>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>> -}; >>>> - >>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>> struct dma_buf_test_params { >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-08 11:17 ` Matthew Auld @ 2026-10-09 9:16 ` Matthew Auld 2026-10-09 9:26 ` Christian König 0 siblings, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-09 9:16 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 08/10/2026 12:17, Matthew Auld wrote: > On 08/10/2026 10:55, Christian König wrote: >> On 10/8/26 11:46, Matthew Auld wrote: >>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>>>> for >>>>> all exported VRAM buffers. However, in some multi-GPU setups, like >>>>> with >>>>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>>>> looks like the exported dma-buf window buffer can in some cases end >>>>> up >>>>> using p2p. >>>>> >>>>> However, user is reporting a kernel regression where performance here >>>>> massively tanks across many games/workloads, where it looks like on >>>>> each >>>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>>> window >>>>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>>>> >>>>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>>>> for >>>>> now. >>>>> >>>>> We don't currently have any hints to say "please use p2p for this >>>>> buffer, or please don't", however we can use some heuristics to try >>>>> to >>>>> determine if this is likely a display buffer being exported to an >>>>> igpu, >>>>> which probably never intended for p2p: >>>>> >>>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>>>> tagged with this, but not always. >>>>> >>>>> 2. Importer being a PCI device with display, which is not xe >>>>> driver. >>>>> >>>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>>>> on >>>>> Intel. We also limit the above to dual placement buffers. >>>>> >>>>> The original regression was bisected to commit 609fc8766d68 ("dma- >>>>> buf: >>>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>>> already present when enabling that knob, so this goes back even >>>>> further. >>>>> >>>>> User has confirmed that along with the sticky dma-buf TT placement >>>>> fix, >>>>> to prevent ping-ponging, the performance is back to normal. >>>>> >>>> >>>> Actually, I think this needs to be fixed on the importer side. Until we >>>> have some some user-space hints, I don't think the exporter should >>>> second-guess. Most sane solution IMO is if igfx importers could just >>>> not advertise pcie p2p support? >>> >>> Yeah, not sure tbh. I'm not a huge fan of this patch either. But you >>> could argue that the SCANOUT flag is already a good implicit hint/ >>> signal on the exporter side, but unfortunately there were some games >>> where this did not reliably work. The other implicit hint/signal is >>> perhaps the dual placement angle. For example, in compute-runtime, it >>> might be that they use VRAM-only if they want p2p? AFAICT mesa for us >>> always uses a dual placement or just system memory for these types of >>> buffers. >>> >>> Christian, do you have any thoughts here? We have a kernel perf >>> regression after DMABUF_MOVE_NOTIFY was made the default after >>> updating to a new kernel. The gist is that this then turned on the >>> p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu >>> doing prime rendering. From the user bug report it sounds like the >>> dgpu linear window buffer exported to the compositor (igpu) is dual >>> placement VRAM + TT, and so this now attempts to keep that memory in >>> VRAM instead of moving it. Compositor then uses p2p to access it. It >>> works but performance falls off a massive cliff here. >>> >>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), >>> but since this is a kernel regression I was hoping we could fix this >>> on the kernel side. >>> >>> Some questions: >>> >>> 1. Do you know what is the general story with p2p between igpu and >>> dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be >>> dropped? It clearly seems to work though. >>> >>> 2. If this is legit, do you guys then also prevent p2p with display >>> buffers? Are you just doing that at the Mesa level, or is there some >>> magic in the kernel also? >> >> If I understand that correctly that the igpu is the importer and the >> dgpu is the exporter then yeah Thomas is right that definately needs >> to be fixed on the importer side. > > Ideas on how do that? Could we do something like allow_peer2peer = false > in amdgpu_gem_prime_import() if this is an APU device, or further check > if the exporter side is not amdgpu, like APU + Xe? For example: https://gitlab.freedesktop.org/mwa/kernel/-/commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c Is that something you would accept? > >> >> At least for AMD APUs we clearly support P2P both as exporter and >> importer, but what isn't supported is to scan out over P2P. We >> explicitely reject that inside DC/DAL. >> >> And I'm pretty sure that you should do that as well. I'm actually >> really suprised that this works technically and doesn't cause >> corruption or anything. > > AFAICT on Intel iGPUs it will currently always end up with peer2peer = > false on attach. Not 100% sure if direct scanout was happening on the > p2p buffer itself, for the user report. > >> >> Regards, >> Christian. >> >>> >>>> >>>> Thanks, >>>> Thomas >>>> >>>>> Assisted-by: LLM >>>>> Closes: >>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>> GPUs") >>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>> --- >>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++++++ >>>>> +------ >>>>> -- >>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>> >>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>> void *data, >>>>> * Display scanout is always non-coherent with the CPU >>>>> cache. >>>>> */ >>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>> XE_BO_FLAG_SCANOUT; >>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>> *file_priv, >>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>> XE_BO_FLAG_FORCE_WC | >>>>> + XE_BO_FLAG_SCANOUT | >>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>> if (IS_ERR(bo)) >>>>> return PTR_ERR(bo); >>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>> @@ -56,6 +56,7 @@ >>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>> /* this one is trigger internally only */ >>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>> index b973579a69b8..0a347f09cee4 100644 >>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>> _ddebug *desc, bool *flag) >>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>> #endif >>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>> *attach) >>>>> +{ >>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>> + >>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>> +} >>>>> + >>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>> + .allow_peer2peer = true, >>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>> +}; >>>>> + >>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>> +{ >>>>> + while (dev) { >>>>> + if (dev_is_pci(dev)) >>>>> + return to_pci_dev(dev); >>>>> + dev = dev->parent; >>>>> + } >>>>> + return NULL; >>>>> +} >>>>> + >>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>> *attach) >>>>> +{ >>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>> +} >>>>> + >>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>> struct dma_buf_attachment *attach) >>>>> { >>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>> *dmabuf, >>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>> dev, verbose_p2p_dist) < 0) >>>>> attach->peer2peer = false; >>>>> - if (!attach->peer2peer && >>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>> + struct pci_dev *client = >>>>> xe_find_parent_pci_dev(attach->dev); >>>>> + >>>>> + /* >>>>> + * There are perf issues with using p2p with display >>>>> buffers, >>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>> p2p window >>>>> + * buffer). Probably we need some actual userspace >>>>> hints. We >>>>> + * shouldn't really assume anything. Also, note that >>>>> some of >>>>> + * these buffers are not always tagged with SCANOUT. >>>>> + */ >>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>> + attach->peer2peer = false; >>>>> + else if (client && pci_is_display(client) && >>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>> + attach->peer2peer = false; >>>>> + } >>>>> + >>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>> return -EOPNOTSUPP; >>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>> struct dma_buf *dma_buf) >>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>> } >>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>> *attach) >>>>> -{ >>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>> - >>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>> -} >>>>> - >>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>> - .allow_peer2peer = true, >>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>> -}; >>>>> - >>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>> struct dma_buf_test_params { >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-09 9:16 ` Matthew Auld @ 2026-10-09 9:26 ` Christian König 2026-10-09 10:29 ` Matthew Auld 0 siblings, 1 reply; 23+ messages in thread From: Christian König @ 2026-10-09 9:26 UTC (permalink / raw) To: Matthew Auld, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 10/9/26 11:16, Matthew Auld wrote: > On 08/10/2026 12:17, Matthew Auld wrote: >> On 08/10/2026 10:55, Christian König wrote: >>> On 10/8/26 11:46, Matthew Auld wrote: >>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>>>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>>>>> for >>>>>> all exported VRAM buffers. However, in some multi-GPU setups, like >>>>>> with >>>>>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>>>>> looks like the exported dma-buf window buffer can in some cases end >>>>>> up >>>>>> using p2p. >>>>>> >>>>>> However, user is reporting a kernel regression where performance here >>>>>> massively tanks across many games/workloads, where it looks like on >>>>>> each >>>>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>>>> window >>>>>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>>>>> >>>>>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>>>>> for >>>>>> now. >>>>>> >>>>>> We don't currently have any hints to say "please use p2p for this >>>>>> buffer, or please don't", however we can use some heuristics to try >>>>>> to >>>>>> determine if this is likely a display buffer being exported to an >>>>>> igpu, >>>>>> which probably never intended for p2p: >>>>>> >>>>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>>>>> tagged with this, but not always. >>>>>> >>>>>> 2. Importer being a PCI device with display, which is not xe driver. >>>>>> >>>>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>>>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>>>>> on >>>>>> Intel. We also limit the above to dual placement buffers. >>>>>> >>>>>> The original regression was bisected to commit 609fc8766d68 ("dma- >>>>>> buf: >>>>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>>>> already present when enabling that knob, so this goes back even >>>>>> further. >>>>>> >>>>>> User has confirmed that along with the sticky dma-buf TT placement >>>>>> fix, >>>>>> to prevent ping-ponging, the performance is back to normal. >>>>>> >>>>> >>>>> Actually, I think this needs to be fixed on the importer side. Until we >>>>> have some some user-space hints, I don't think the exporter should >>>>> second-guess. Most sane solution IMO is if igfx importers could just >>>>> not advertise pcie p2p support? >>>> >>>> Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/ signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. >>>> >>>> Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. >>>> >>>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >>>> >>>> Some questions: >>>> >>>> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >>>> >>>> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? >>> >>> If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. >> >> Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? > > For example: https://gitlab.freedesktop.org/mwa/kernel/-/commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c > > Is that something you would accept? No, you misunderstood me. Enabling P2P is perfectly fine for APUs and mandatory for a couple of customer projects. The buffer should just be rejected from scanout in the display code and on the AMDGPU side we already do that if I'm not completely mistaken. Userspace has fallback path that when a DRM atomic requests fails for an imported buffer it falls back to copying the buffer first. Regards, Christian. >> >>> >>> At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. >>> >>> And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. >> >> AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. >> >>> >>> Regards, >>> Christian. >>> >>>> >>>>> >>>>> Thanks, >>>>> Thomas >>>>> >>>>>> Assisted-by: LLM >>>>>> Closes: >>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>>> GPUs") >>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>> --- >>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++++++ +------ >>>>>> -- >>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>> >>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>>> void *data, >>>>>> * Display scanout is always non-coherent with the CPU >>>>>> cache. >>>>>> */ >>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>> XE_BO_FLAG_SCANOUT; >>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>> *file_priv, >>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>> XE_BO_FLAG_FORCE_WC | >>>>>> + XE_BO_FLAG_SCANOUT | >>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>> if (IS_ERR(bo)) >>>>>> return PTR_ERR(bo); >>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>> @@ -56,6 +56,7 @@ >>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>> /* this one is trigger internally only */ >>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>> _ddebug *desc, bool *flag) >>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>> #endif >>>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>> *attach) >>>>>> +{ >>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>> + >>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>> +} >>>>>> + >>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>> + .allow_peer2peer = true, >>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>> +}; >>>>>> + >>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>>> +{ >>>>>> + while (dev) { >>>>>> + if (dev_is_pci(dev)) >>>>>> + return to_pci_dev(dev); >>>>>> + dev = dev->parent; >>>>>> + } >>>>>> + return NULL; >>>>>> +} >>>>>> + >>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>>> *attach) >>>>>> +{ >>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>> +} >>>>>> + >>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>> struct dma_buf_attachment *attach) >>>>>> { >>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>> *dmabuf, >>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>>> dev, verbose_p2p_dist) < 0) >>>>>> attach->peer2peer = false; >>>>>> - if (!attach->peer2peer && >>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>> + struct pci_dev *client = >>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>> + >>>>>> + /* >>>>>> + * There are perf issues with using p2p with display >>>>>> buffers, >>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>> p2p window >>>>>> + * buffer). Probably we need some actual userspace >>>>>> hints. We >>>>>> + * shouldn't really assume anything. Also, note that >>>>>> some of >>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>> + */ >>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>> + attach->peer2peer = false; >>>>>> + else if (client && pci_is_display(client) && >>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>> + attach->peer2peer = false; >>>>>> + } >>>>>> + >>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>> return -EOPNOTSUPP; >>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>>> struct dma_buf *dma_buf) >>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>> } >>>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>> *attach) >>>>>> -{ >>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>> - >>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>> -} >>>>>> - >>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>> - .allow_peer2peer = true, >>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>> -}; >>>>>> - >>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>> struct dma_buf_test_params { >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-09 9:26 ` Christian König @ 2026-10-09 10:29 ` Matthew Auld 2026-10-09 11:38 ` Christian König 0 siblings, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-09 10:29 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 09/10/2026 10:26, Christian König wrote: > On 10/9/26 11:16, Matthew Auld wrote: >> On 08/10/2026 12:17, Matthew Auld wrote: >>> On 08/10/2026 10:55, Christian König wrote: >>>> On 10/8/26 11:46, Matthew Auld wrote: >>>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>>>>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>>>>>> for >>>>>>> all exported VRAM buffers. However, in some multi-GPU setups, like >>>>>>> with >>>>>>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>>>>>> looks like the exported dma-buf window buffer can in some cases end >>>>>>> up >>>>>>> using p2p. >>>>>>> >>>>>>> However, user is reporting a kernel regression where performance here >>>>>>> massively tanks across many games/workloads, where it looks like on >>>>>>> each >>>>>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>>>>> window >>>>>>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>>>>>> >>>>>>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>>>>>> for >>>>>>> now. >>>>>>> >>>>>>> We don't currently have any hints to say "please use p2p for this >>>>>>> buffer, or please don't", however we can use some heuristics to try >>>>>>> to >>>>>>> determine if this is likely a display buffer being exported to an >>>>>>> igpu, >>>>>>> which probably never intended for p2p: >>>>>>> >>>>>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>>>>>> tagged with this, but not always. >>>>>>> >>>>>>> 2. Importer being a PCI device with display, which is not xe driver. >>>>>>> >>>>>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>>>>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>>>>>> on >>>>>>> Intel. We also limit the above to dual placement buffers. >>>>>>> >>>>>>> The original regression was bisected to commit 609fc8766d68 ("dma- >>>>>>> buf: >>>>>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>>>>> already present when enabling that knob, so this goes back even >>>>>>> further. >>>>>>> >>>>>>> User has confirmed that along with the sticky dma-buf TT placement >>>>>>> fix, >>>>>>> to prevent ping-ponging, the performance is back to normal. >>>>>>> >>>>>> >>>>>> Actually, I think this needs to be fixed on the importer side. Until we >>>>>> have some some user-space hints, I don't think the exporter should >>>>>> second-guess. Most sane solution IMO is if igfx importers could just >>>>>> not advertise pcie p2p support? >>>>> >>>>> Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/ signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. >>>>> >>>>> Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. >>>>> >>>>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >>>>> >>>>> Some questions: >>>>> >>>>> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >>>>> >>>>> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? >>>> >>>> If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. >>> >>> Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? >> >> For example: https://gitlab.freedesktop.org/mwa/kernel/-/commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c >> >> Is that something you would accept? > > No, you misunderstood me. Enabling P2P is perfectly fine for APUs and mandatory for a couple of customer projects. > > The buffer should just be rejected from scanout in the display code and on the AMDGPU side we already do that if I'm not completely mistaken. Reporter confirmed seeing the issue in windowed mode also. So I don't think it's just a case of rejecting from scanout? > > Userspace has fallback path that when a DRM atomic requests fails for an imported buffer it falls back to copying the buffer first. > > Regards, > Christian. >>> >>>> >>>> At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. >>>> >>>> And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. >>> >>> AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. >>> >>>> >>>> Regards, >>>> Christian. >>>> >>>>> >>>>>> >>>>>> Thanks, >>>>>> Thomas >>>>>> >>>>>>> Assisted-by: LLM >>>>>>> Closes: >>>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>>>> GPUs") >>>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>>> --- >>>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++++++ +------ >>>>>>> -- >>>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>>> >>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>>>> void *data, >>>>>>> * Display scanout is always non-coherent with the CPU >>>>>>> cache. >>>>>>> */ >>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>>> XE_BO_FLAG_SCANOUT; >>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>>> *file_priv, >>>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>>> XE_BO_FLAG_FORCE_WC | >>>>>>> + XE_BO_FLAG_SCANOUT | >>>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>>> if (IS_ERR(bo)) >>>>>>> return PTR_ERR(bo); >>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>>> @@ -56,6 +56,7 @@ >>>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>>> /* this one is trigger internally only */ >>>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>>> _ddebug *desc, bool *flag) >>>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>>> #endif >>>>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>> *attach) >>>>>>> +{ >>>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>> + >>>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>> +} >>>>>>> + >>>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>> + .allow_peer2peer = true, >>>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>>> +}; >>>>>>> + >>>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>>>> +{ >>>>>>> + while (dev) { >>>>>>> + if (dev_is_pci(dev)) >>>>>>> + return to_pci_dev(dev); >>>>>>> + dev = dev->parent; >>>>>>> + } >>>>>>> + return NULL; >>>>>>> +} >>>>>>> + >>>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>>>> *attach) >>>>>>> +{ >>>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>>> +} >>>>>>> + >>>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>>> struct dma_buf_attachment *attach) >>>>>>> { >>>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>>> *dmabuf, >>>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>>>> dev, verbose_p2p_dist) < 0) >>>>>>> attach->peer2peer = false; >>>>>>> - if (!attach->peer2peer && >>>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>>> + struct pci_dev *client = >>>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>>> + >>>>>>> + /* >>>>>>> + * There are perf issues with using p2p with display >>>>>>> buffers, >>>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>>> p2p window >>>>>>> + * buffer). Probably we need some actual userspace >>>>>>> hints. We >>>>>>> + * shouldn't really assume anything. Also, note that >>>>>>> some of >>>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>>> + */ >>>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>>> + attach->peer2peer = false; >>>>>>> + else if (client && pci_is_display(client) && >>>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>>> + attach->peer2peer = false; >>>>>>> + } >>>>>>> + >>>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>>> return -EOPNOTSUPP; >>>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>>>> struct dma_buf *dma_buf) >>>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>>> } >>>>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>> *attach) >>>>>>> -{ >>>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>> - >>>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>> -} >>>>>>> - >>>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>> - .allow_peer2peer = true, >>>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>>> -}; >>>>>>> - >>>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>>> struct dma_buf_test_params { >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-09 10:29 ` Matthew Auld @ 2026-10-09 11:38 ` Christian König 2026-10-09 13:02 ` Matthew Auld 2026-10-09 13:06 ` Thomas Hellström 0 siblings, 2 replies; 23+ messages in thread From: Christian König @ 2026-10-09 11:38 UTC (permalink / raw) To: Matthew Auld, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 10/9/26 12:29, Matthew Auld wrote: > On 09/10/2026 10:26, Christian König wrote: >> On 10/9/26 11:16, Matthew Auld wrote: >>> On 08/10/2026 12:17, Matthew Auld wrote: >>>> On 08/10/2026 10:55, Christian König wrote: >>>>> On 10/8/26 11:46, Matthew Auld wrote: >>>>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>>>>>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>>>>>>> for >>>>>>>> all exported VRAM buffers. However, in some multi-GPU setups, like >>>>>>>> with >>>>>>>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>>>>>>> looks like the exported dma-buf window buffer can in some cases end >>>>>>>> up >>>>>>>> using p2p. >>>>>>>> >>>>>>>> However, user is reporting a kernel regression where performance here >>>>>>>> massively tanks across many games/workloads, where it looks like on >>>>>>>> each >>>>>>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>>>>>> window >>>>>>>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>>>>>>> >>>>>>>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>>>>>>> for >>>>>>>> now. >>>>>>>> >>>>>>>> We don't currently have any hints to say "please use p2p for this >>>>>>>> buffer, or please don't", however we can use some heuristics to try >>>>>>>> to >>>>>>>> determine if this is likely a display buffer being exported to an >>>>>>>> igpu, >>>>>>>> which probably never intended for p2p: >>>>>>>> >>>>>>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>>>>>>> tagged with this, but not always. >>>>>>>> >>>>>>>> 2. Importer being a PCI device with display, which is not xe driver. >>>>>>>> >>>>>>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>>>>>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>>>>>>> on >>>>>>>> Intel. We also limit the above to dual placement buffers. >>>>>>>> >>>>>>>> The original regression was bisected to commit 609fc8766d68 ("dma- >>>>>>>> buf: >>>>>>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>>>>>> already present when enabling that knob, so this goes back even >>>>>>>> further. >>>>>>>> >>>>>>>> User has confirmed that along with the sticky dma-buf TT placement >>>>>>>> fix, >>>>>>>> to prevent ping-ponging, the performance is back to normal. >>>>>>>> >>>>>>> >>>>>>> Actually, I think this needs to be fixed on the importer side. Until we >>>>>>> have some some user-space hints, I don't think the exporter should >>>>>>> second-guess. Most sane solution IMO is if igfx importers could just >>>>>>> not advertise pcie p2p support? >>>>>> >>>>>> Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/ signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. >>>>>> >>>>>> Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. >>>>>> >>>>>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >>>>>> >>>>>> Some questions: >>>>>> >>>>>> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >>>>>> >>>>>> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? >>>>> >>>>> If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. >>>> >>>> Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? >>> >>> For example: https://gitlab.freedesktop.org/mwa/kernel/-/commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c >>> >>> Is that something you would accept? >> >> No, you misunderstood me. Enabling P2P is perfectly fine for APUs and mandatory for a couple of customer projects. >> >> The buffer should just be rejected from scanout in the display code and on the AMDGPU side we already do that if I'm not completely mistaken. > > Reporter confirmed seeing the issue in windowed mode also. So I don't think it's just a case of rejecting from scanout? Then I don't understand what the problem is. When the dGPU renders and the iGPU displays you have only two possibilities, either the dGPU pushes the frames over the bus or the iGPU pulls the frames over the bus. Both should be equaly slow or fast, so what exactly is the expectation here? The only thing you need to avoid is that the iGPU starts to scan out over the PCIe bus directly, cause then the frame is pulled over the bus multiple times (if that actually works correctly). Do you have a link to the original bug report? I was only added later to the discussion so I don't have that. Regards, Christian. > >> >> Userspace has fallback path that when a DRM atomic requests fails for an imported buffer it falls back to copying the buffer first. >> >> Regards, >> Christian. >>>> >>>>> >>>>> At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. >>>>> >>>>> And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. >>>> >>>> AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. >>>> >>>>> >>>>> Regards, >>>>> Christian. >>>>> >>>>>> >>>>>>> >>>>>>> Thanks, >>>>>>> Thomas >>>>>>> >>>>>>>> Assisted-by: LLM >>>>>>>> Closes: >>>>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>>>>> GPUs") >>>>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>>>> --- >>>>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++++++ +------ >>>>>>>> -- >>>>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>>>>> void *data, >>>>>>>> * Display scanout is always non-coherent with the CPU >>>>>>>> cache. >>>>>>>> */ >>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>>>> XE_BO_FLAG_SCANOUT; >>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>>>> *file_priv, >>>>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>>>> XE_BO_FLAG_FORCE_WC | >>>>>>>> + XE_BO_FLAG_SCANOUT | >>>>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>>>> if (IS_ERR(bo)) >>>>>>>> return PTR_ERR(bo); >>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>> @@ -56,6 +56,7 @@ >>>>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>>>> /* this one is trigger internally only */ >>>>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>>>> _ddebug *desc, bool *flag) >>>>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>>>> #endif >>>>>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>> *attach) >>>>>>>> +{ >>>>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>> + >>>>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>> +} >>>>>>>> + >>>>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>> + .allow_peer2peer = true, >>>>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>> +}; >>>>>>>> + >>>>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>>>>> +{ >>>>>>>> + while (dev) { >>>>>>>> + if (dev_is_pci(dev)) >>>>>>>> + return to_pci_dev(dev); >>>>>>>> + dev = dev->parent; >>>>>>>> + } >>>>>>>> + return NULL; >>>>>>>> +} >>>>>>>> + >>>>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>>>>> *attach) >>>>>>>> +{ >>>>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>>>> +} >>>>>>>> + >>>>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>>>> struct dma_buf_attachment *attach) >>>>>>>> { >>>>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>>>> *dmabuf, >>>>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>>>>> dev, verbose_p2p_dist) < 0) >>>>>>>> attach->peer2peer = false; >>>>>>>> - if (!attach->peer2peer && >>>>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>>>> + struct pci_dev *client = >>>>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>>>> + >>>>>>>> + /* >>>>>>>> + * There are perf issues with using p2p with display >>>>>>>> buffers, >>>>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>>>> p2p window >>>>>>>> + * buffer). Probably we need some actual userspace >>>>>>>> hints. We >>>>>>>> + * shouldn't really assume anything. Also, note that >>>>>>>> some of >>>>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>>>> + */ >>>>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>>>> + attach->peer2peer = false; >>>>>>>> + else if (client && pci_is_display(client) && >>>>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>>>> + attach->peer2peer = false; >>>>>>>> + } >>>>>>>> + >>>>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>>>> return -EOPNOTSUPP; >>>>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>>>>> struct dma_buf *dma_buf) >>>>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>>>> } >>>>>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>> *attach) >>>>>>>> -{ >>>>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>> - >>>>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>> -} >>>>>>>> - >>>>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>> - .allow_peer2peer = true, >>>>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>> -}; >>>>>>>> - >>>>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>>>> struct dma_buf_test_params { >>>>>> >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-09 11:38 ` Christian König @ 2026-10-09 13:02 ` Matthew Auld 2026-10-13 8:51 ` Matthew Auld 2026-10-09 13:06 ` Thomas Hellström 1 sibling, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-09 13:02 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 09/10/2026 12:38, Christian König wrote: > On 10/9/26 12:29, Matthew Auld wrote: >> On 09/10/2026 10:26, Christian König wrote: >>> On 10/9/26 11:16, Matthew Auld wrote: >>>> On 08/10/2026 12:17, Matthew Auld wrote: >>>>> On 08/10/2026 10:55, Christian König wrote: >>>>>> On 10/8/26 11:46, Matthew Auld wrote: >>>>>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>>>>>>> If p2p is allowed on the CPU (like on AMD), we currently permit p2p >>>>>>>>> for >>>>>>>>> all exported VRAM buffers. However, in some multi-GPU setups, like >>>>>>>>> with >>>>>>>>> prime rendering on a dgpu and scanout/compositor using the igpu, it >>>>>>>>> looks like the exported dma-buf window buffer can in some cases end >>>>>>>>> up >>>>>>>>> using p2p. >>>>>>>>> >>>>>>>>> However, user is reporting a kernel regression where performance here >>>>>>>>> massively tanks across many games/workloads, where it looks like on >>>>>>>>> each >>>>>>>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>>>>>>> window >>>>>>>>> buffer, since it uses p2p. p2p is currently more intended for dgpu <- >>>>>>>>>> >>>>>>>>> dgpu, and not so much display buffers with igpu <-> dgpu, at least >>>>>>>>> for >>>>>>>>> now. >>>>>>>>> >>>>>>>>> We don't currently have any hints to say "please use p2p for this >>>>>>>>> buffer, or please don't", however we can use some heuristics to try >>>>>>>>> to >>>>>>>>> determine if this is likely a display buffer being exported to an >>>>>>>>> igpu, >>>>>>>>> which probably never intended for p2p: >>>>>>>>> >>>>>>>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers usually get >>>>>>>>> tagged with this, but not always. >>>>>>>>> >>>>>>>>> 2. Importer being a PCI device with display, which is not xe driver. >>>>>>>>> >>>>>>>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>>>>>>> happening on xe igpu. Currently p2p is only enabled for server CPUs, >>>>>>>>> on >>>>>>>>> Intel. We also limit the above to dual placement buffers. >>>>>>>>> >>>>>>>>> The original regression was bisected to commit 609fc8766d68 ("dma- >>>>>>>>> buf: >>>>>>>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>>>>>>> already present when enabling that knob, so this goes back even >>>>>>>>> further. >>>>>>>>> >>>>>>>>> User has confirmed that along with the sticky dma-buf TT placement >>>>>>>>> fix, >>>>>>>>> to prevent ping-ponging, the performance is back to normal. >>>>>>>>> >>>>>>>> >>>>>>>> Actually, I think this needs to be fixed on the importer side. Until we >>>>>>>> have some some user-space hints, I don't think the exporter should >>>>>>>> second-guess. Most sane solution IMO is if igfx importers could just >>>>>>>> not advertise pcie p2p support? >>>>>>> >>>>>>> Yeah, not sure tbh. I'm not a huge fan of this patch either. But you could argue that the SCANOUT flag is already a good implicit hint/ signal on the exporter side, but unfortunately there were some games where this did not reliably work. The other implicit hint/signal is perhaps the dual placement angle. For example, in compute-runtime, it might be that they use VRAM-only if they want p2p? AFAICT mesa for us always uses a dual placement or just system memory for these types of buffers. >>>>>>> >>>>>>> Christian, do you have any thoughts here? We have a kernel perf regression after DMABUF_MOVE_NOTIFY was made the default after updating to a new kernel. The gist is that this then turned on the p2p paths, with a multi-GPU setup with an AMD igpu/APU and Xe dgpu doing prime rendering. From the user bug report it sounds like the dgpu linear window buffer exported to the compositor (igpu) is dual placement VRAM + TT, and so this now attempts to keep that memory in VRAM instead of moving it. Compositor then uses p2p to access it. It works but performance falls off a massive cliff here. >>>>>>> >>>>>>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >>>>>>> >>>>>>> Some questions: >>>>>>> >>>>>>> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >>>>>>> >>>>>>> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? >>>>>> >>>>>> If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. >>>>> >>>>> Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? >>>> >>>> For example: https://gitlab.freedesktop.org/mwa/kernel/-/commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c >>>> >>>> Is that something you would accept? >>> >>> No, you misunderstood me. Enabling P2P is perfectly fine for APUs and mandatory for a couple of customer projects. >>> >>> The buffer should just be rejected from scanout in the display code and on the AMDGPU side we already do that if I'm not completely mistaken. >> >> Reporter confirmed seeing the issue in windowed mode also. So I don't think it's just a case of rejecting from scanout? > > Then I don't understand what the problem is. > > When the dGPU renders and the iGPU displays you have only two possibilities, either the dGPU pushes the frames over the bus or the iGPU pulls the frames over the bus. Both should be equaly slow or fast, so what exactly is the expectation here? I assume the AMD APU/iGPU is sampling/reading from the p2p window buffer every frame. Expectation is that buffer rather gets moved from VRAM to system memory when mapping the buffer with dma-buf, where it stays, and never negotiated to use p2p in the first place. I don't think p2p is really valid here. What happened is that user updated their kernel and due to the MOVE_NOTIFY config getting removed and enabled by default on newer kernels this suddenly started negotiating p2p for things like the window buffer AFAICT, instead of moving it to system memory and keeping it there. On previous kernel versions the buffer would just get moved and pinned in system memory. From user pov this is a kernel regression, given the perf they are now getting. On AMD CPUs you allow p2p for everything. On Intel iGPUs we currently ban p2p. Maybe there is bigger question here for what is the story for p2p and iGPU? Should it really be blanket allowed like this? Should it not be opt-in with some userspace hints or something? The other important part is that even when you move to system memory, it's no longer pinned (MOVE_NOTIFY) so can you now can also get violent ping-ponging, which is addressed in the first patch in this series. > > The only thing you need to avoid is that the iGPU starts to scan out over the PCIe bus directly, cause then the frame is pulled over the bus multiple times (if that actually works correctly). > > Do you have a link to the original bug report? I was only added later to the discussion so I don't have that. https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 > > Regards, > Christian. > >> >>> >>> Userspace has fallback path that when a DRM atomic requests fails for an imported buffer it falls back to copying the buffer first. >>> >>> Regards, >>> Christian. >>>>> >>>>>> >>>>>> At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. >>>>>> >>>>>> And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. >>>>> >>>>> AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. >>>>> >>>>>> >>>>>> Regards, >>>>>> Christian. >>>>>> >>>>>>> >>>>>>>> >>>>>>>> Thanks, >>>>>>>> Thomas >>>>>>>> >>>>>>>>> Assisted-by: LLM >>>>>>>>> Closes: >>>>>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>>>>>> GPUs") >>>>>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>>>>> --- >>>>>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++++++ +------ >>>>>>>>> -- >>>>>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>>>>> >>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>>>>>> void *data, >>>>>>>>> * Display scanout is always non-coherent with the CPU >>>>>>>>> cache. >>>>>>>>> */ >>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>>>>> XE_BO_FLAG_SCANOUT; >>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>>>>> *file_priv, >>>>>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>>>>> XE_BO_FLAG_FORCE_WC | >>>>>>>>> + XE_BO_FLAG_SCANOUT | >>>>>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>>>>> if (IS_ERR(bo)) >>>>>>>>> return PTR_ERR(bo); >>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>> @@ -56,6 +56,7 @@ >>>>>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>>>>> /* this one is trigger internally only */ >>>>>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>>>>> _ddebug *desc, bool *flag) >>>>>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>>>>> #endif >>>>>>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>>> *attach) >>>>>>>>> +{ >>>>>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>> + >>>>>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>> +} >>>>>>>>> + >>>>>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>> + .allow_peer2peer = true, >>>>>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>> +}; >>>>>>>>> + >>>>>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>>>>>> +{ >>>>>>>>> + while (dev) { >>>>>>>>> + if (dev_is_pci(dev)) >>>>>>>>> + return to_pci_dev(dev); >>>>>>>>> + dev = dev->parent; >>>>>>>>> + } >>>>>>>>> + return NULL; >>>>>>>>> +} >>>>>>>>> + >>>>>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>>>>>> *attach) >>>>>>>>> +{ >>>>>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>>>>> +} >>>>>>>>> + >>>>>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>>>>> struct dma_buf_attachment *attach) >>>>>>>>> { >>>>>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>>>>> *dmabuf, >>>>>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>>>>>> dev, verbose_p2p_dist) < 0) >>>>>>>>> attach->peer2peer = false; >>>>>>>>> - if (!attach->peer2peer && >>>>>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>>>>> + struct pci_dev *client = >>>>>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>>>>> + >>>>>>>>> + /* >>>>>>>>> + * There are perf issues with using p2p with display >>>>>>>>> buffers, >>>>>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>>>>> p2p window >>>>>>>>> + * buffer). Probably we need some actual userspace >>>>>>>>> hints. We >>>>>>>>> + * shouldn't really assume anything. Also, note that >>>>>>>>> some of >>>>>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>>>>> + */ >>>>>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>>>>> + attach->peer2peer = false; >>>>>>>>> + else if (client && pci_is_display(client) && >>>>>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>>>>> + attach->peer2peer = false; >>>>>>>>> + } >>>>>>>>> + >>>>>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>>>>> return -EOPNOTSUPP; >>>>>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>>>>>> struct dma_buf *dma_buf) >>>>>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>>>>> } >>>>>>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>>> *attach) >>>>>>>>> -{ >>>>>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>> - >>>>>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>> -} >>>>>>>>> - >>>>>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>> - .allow_peer2peer = true, >>>>>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>> -}; >>>>>>>>> - >>>>>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>>>>> struct dma_buf_test_params { >>>>>>> >>>>>> >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-09 13:02 ` Matthew Auld @ 2026-10-13 8:51 ` Matthew Auld 2026-10-13 9:10 ` Christian König 0 siblings, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-13 8:51 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 09/10/2026 14:02, Matthew Auld wrote: > On 09/10/2026 12:38, Christian König wrote: >> On 10/9/26 12:29, Matthew Auld wrote: >>> On 09/10/2026 10:26, Christian König wrote: >>>> On 10/9/26 11:16, Matthew Auld wrote: >>>>> On 08/10/2026 12:17, Matthew Auld wrote: >>>>>> On 08/10/2026 10:55, Christian König wrote: >>>>>>> On 10/8/26 11:46, Matthew Auld wrote: >>>>>>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>>>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: >>>>>>>>>> If p2p is allowed on the CPU (like on AMD), we currently >>>>>>>>>> permit p2p >>>>>>>>>> for >>>>>>>>>> all exported VRAM buffers. However, in some multi-GPU setups, >>>>>>>>>> like >>>>>>>>>> with >>>>>>>>>> prime rendering on a dgpu and scanout/compositor using the >>>>>>>>>> igpu, it >>>>>>>>>> looks like the exported dma-buf window buffer can in some >>>>>>>>>> cases end >>>>>>>>>> up >>>>>>>>>> using p2p. >>>>>>>>>> >>>>>>>>>> However, user is reporting a kernel regression where >>>>>>>>>> performance here >>>>>>>>>> massively tanks across many games/workloads, where it looks >>>>>>>>>> like on >>>>>>>>>> each >>>>>>>>>> frame, the igpu is reading/sampling from VRAM over pcie for the >>>>>>>>>> window >>>>>>>>>> buffer, since it uses p2p. p2p is currently more intended for >>>>>>>>>> dgpu <- >>>>>>>>>>> >>>>>>>>>> dgpu, and not so much display buffers with igpu <-> dgpu, at >>>>>>>>>> least >>>>>>>>>> for >>>>>>>>>> now. >>>>>>>>>> >>>>>>>>>> We don't currently have any hints to say "please use p2p for this >>>>>>>>>> buffer, or please don't", however we can use some heuristics >>>>>>>>>> to try >>>>>>>>>> to >>>>>>>>>> determine if this is likely a display buffer being exported to an >>>>>>>>>> igpu, >>>>>>>>>> which probably never intended for p2p: >>>>>>>>>> >>>>>>>>>> 1. FLAG_SCANOUT being set on the buffer. Window buffers >>>>>>>>>> usually get >>>>>>>>>> tagged with this, but not always. >>>>>>>>>> >>>>>>>>>> 2. Importer being a PCI device with display, which is not >>>>>>>>>> xe driver. >>>>>>>>>> >>>>>>>>>> We also rely on the existing p2p Intel CPU whitelist to stop this >>>>>>>>>> happening on xe igpu. Currently p2p is only enabled for server >>>>>>>>>> CPUs, >>>>>>>>>> on >>>>>>>>>> Intel. We also limit the above to dual placement buffers. >>>>>>>>>> >>>>>>>>>> The original regression was bisected to commit 609fc8766d68 >>>>>>>>>> ("dma- >>>>>>>>>> buf: >>>>>>>>>> Always build with DMABUF_MOVE_NOTIFY"), however the behaviour was >>>>>>>>>> already present when enabling that knob, so this goes back even >>>>>>>>>> further. >>>>>>>>>> >>>>>>>>>> User has confirmed that along with the sticky dma-buf TT >>>>>>>>>> placement >>>>>>>>>> fix, >>>>>>>>>> to prevent ping-ponging, the performance is back to normal. >>>>>>>>>> >>>>>>>>> >>>>>>>>> Actually, I think this needs to be fixed on the importer side. >>>>>>>>> Until we >>>>>>>>> have some some user-space hints, I don't think the exporter should >>>>>>>>> second-guess. Most sane solution IMO is if igfx importers could >>>>>>>>> just >>>>>>>>> not advertise pcie p2p support? >>>>>>>> >>>>>>>> Yeah, not sure tbh. I'm not a huge fan of this patch either. But >>>>>>>> you could argue that the SCANOUT flag is already a good implicit >>>>>>>> hint/ signal on the exporter side, but unfortunately there were >>>>>>>> some games where this did not reliably work. The other implicit >>>>>>>> hint/signal is perhaps the dual placement angle. For example, in >>>>>>>> compute-runtime, it might be that they use VRAM-only if they >>>>>>>> want p2p? AFAICT mesa for us always uses a dual placement or >>>>>>>> just system memory for these types of buffers. >>>>>>>> >>>>>>>> Christian, do you have any thoughts here? We have a kernel perf >>>>>>>> regression after DMABUF_MOVE_NOTIFY was made the default after >>>>>>>> updating to a new kernel. The gist is that this then turned on >>>>>>>> the p2p paths, with a multi-GPU setup with an AMD igpu/APU and >>>>>>>> Xe dgpu doing prime rendering. From the user bug report it >>>>>>>> sounds like the dgpu linear window buffer exported to the >>>>>>>> compositor (igpu) is dual placement VRAM + TT, and so this now >>>>>>>> attempts to keep that memory in VRAM instead of moving it. >>>>>>>> Compositor then uses p2p to access it. It works but performance >>>>>>>> falls off a massive cliff here. >>>>>>>> >>>>>>>> Perhaps Intel Mesa side should use tt-only for this type buffer >>>>>>>> (?), but since this is a kernel regression I was hoping we could >>>>>>>> fix this on the kernel side. >>>>>>>> >>>>>>>> Some questions: >>>>>>>> >>>>>>>> 1. Do you know what is the general story with p2p between igpu >>>>>>>> and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p >>>>>>>> can't be dropped? It clearly seems to work though. >>>>>>>> >>>>>>>> 2. If this is legit, do you guys then also prevent p2p with >>>>>>>> display buffers? Are you just doing that at the Mesa level, or >>>>>>>> is there some magic in the kernel also? >>>>>>> >>>>>>> If I understand that correctly that the igpu is the importer and >>>>>>> the dgpu is the exporter then yeah Thomas is right that >>>>>>> definately needs to be fixed on the importer side. >>>>>> >>>>>> Ideas on how do that? Could we do something like allow_peer2peer = >>>>>> false in amdgpu_gem_prime_import() if this is an APU device, or >>>>>> further check if the exporter side is not amdgpu, like APU + Xe? >>>>> >>>>> For example: https://gitlab.freedesktop.org/mwa/kernel/-/ >>>>> commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c >>>>> >>>>> Is that something you would accept? >>>> >>>> No, you misunderstood me. Enabling P2P is perfectly fine for APUs >>>> and mandatory for a couple of customer projects. >>>> >>>> The buffer should just be rejected from scanout in the display code >>>> and on the AMDGPU side we already do that if I'm not completely >>>> mistaken. >>> >>> Reporter confirmed seeing the issue in windowed mode also. So I don't >>> think it's just a case of rejecting from scanout? >> >> Then I don't understand what the problem is. >> >> When the dGPU renders and the iGPU displays you have only two >> possibilities, either the dGPU pushes the frames over the bus or the >> iGPU pulls the frames over the bus. Both should be equaly slow or >> fast, so what exactly is the expectation here? > > I assume the AMD APU/iGPU is sampling/reading from the p2p window buffer > every frame. > > Expectation is that buffer rather gets moved from VRAM to system memory > when mapping the buffer with dma-buf, where it stays, and never > negotiated to use p2p in the first place. I don't think p2p is really > valid here. > > What happened is that user updated their kernel and due to the > MOVE_NOTIFY config getting removed and enabled by default on newer > kernels this suddenly started negotiating p2p for things like the window > buffer AFAICT, instead of moving it to system memory and keeping it > there. On previous kernel versions the buffer would just get moved and > pinned in system memory. From user pov this is a kernel regression, > given the perf they are now getting. > > On AMD CPUs you allow p2p for everything. On Intel iGPUs we currently > ban p2p. Maybe there is bigger question here for what is the story for > p2p and iGPU? Should it really be blanket allowed like this? Should it > not be opt-in with some userspace hints or something? > > The other important part is that even when you move to system memory, > it's no longer pinned (MOVE_NOTIFY) so can you now can also get violent > ping-ponging, which is addressed in the first patch in this series. > >> >> The only thing you need to avoid is that the iGPU starts to scan out >> over the PCIe bus directly, cause then the frame is pulled over the >> bus multiple times (if that actually works correctly). >> >> Do you have a link to the original bug report? I was only added later >> to the discussion so I don't have that. > > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 Christian, did you get a chance to look at this? Any further thoughts? > >> >> Regards, >> Christian. >> >>> >>>> >>>> Userspace has fallback path that when a DRM atomic requests fails >>>> for an imported buffer it falls back to copying the buffer first. >>>> >>>> Regards, >>>> Christian. >>>>>> >>>>>>> >>>>>>> At least for AMD APUs we clearly support P2P both as exporter and >>>>>>> importer, but what isn't supported is to scan out over P2P. We >>>>>>> explicitely reject that inside DC/DAL. >>>>>>> >>>>>>> And I'm pretty sure that you should do that as well. I'm actually >>>>>>> really suprised that this works technically and doesn't cause >>>>>>> corruption or anything. >>>>>> >>>>>> AFAICT on Intel iGPUs it will currently always end up with >>>>>> peer2peer = false on attach. Not 100% sure if direct scanout was >>>>>> happening on the p2p buffer itself, for the user report. >>>>>> >>>>>>> >>>>>>> Regards, >>>>>>> Christian. >>>>>>> >>>>>>>> >>>>>>>>> >>>>>>>>> Thanks, >>>>>>>>> Thomas >>>>>>>>> >>>>>>>>>> Assisted-by: LLM >>>>>>>>>> Closes: >>>>>>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for >>>>>>>>>> Intel >>>>>>>>>> GPUs") >>>>>>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>>>>>> --- >>>>>>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++ >>>>>>>>>> ++++ +------ >>>>>>>>>> -- >>>>>>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>>>>>> >>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/ >>>>>>>>>> xe_bo.c >>>>>>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct >>>>>>>>>> drm_device *dev, >>>>>>>>>> void *data, >>>>>>>>>> * Display scanout is always non-coherent with the CPU >>>>>>>>>> cache. >>>>>>>>>> */ >>>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>>>>>> XE_BO_FLAG_SCANOUT; >>>>>>>>>> if (args->flags & >>>>>>>>>> DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>>>>>> *file_priv, >>>>>>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>>>>>> XE_BO_FLAG_FORCE_WC | >>>>>>>>>> + XE_BO_FLAG_SCANOUT | >>>>>>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>>>>>> if (IS_ERR(bo)) >>>>>>>>>> return PTR_ERR(bo); >>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/ >>>>>>>>>> xe_bo.h >>>>>>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>>> @@ -56,6 +56,7 @@ >>>>>>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>>>>>> /* this one is trigger internally only */ >>>>>>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>>>>>> _ddebug *desc, bool *flag) >>>>>>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>>>>>> #endif >>>>>>>>>> +static void xe_dma_buf_move_notify(struct >>>>>>>>>> dma_buf_attachment >>>>>>>>>> *attach) >>>>>>>>>> +{ >>>>>>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>>> + >>>>>>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>>> +} >>>>>>>>>> + >>>>>>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>>> + .allow_peer2peer = true, >>>>>>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>>> +}; >>>>>>>>>> + >>>>>>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device >>>>>>>>>> *dev) >>>>>>>>>> +{ >>>>>>>>>> + while (dev) { >>>>>>>>>> + if (dev_is_pci(dev)) >>>>>>>>>> + return to_pci_dev(dev); >>>>>>>>>> + dev = dev->parent; >>>>>>>>>> + } >>>>>>>>>> + return NULL; >>>>>>>>>> +} >>>>>>>>>> + >>>>>>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct >>>>>>>>>> dma_buf_attachment >>>>>>>>>> *attach) >>>>>>>>>> +{ >>>>>>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>>>>>> +} >>>>>>>>>> + >>>>>>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>>>>>> struct dma_buf_attachment *attach) >>>>>>>>>> { >>>>>>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>> bool verbose_p2p_dist = >>>>>>>>>> drm_debug_enabled(DRM_UT_DRIVER); >>>>>>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>>>>>> *dmabuf, >>>>>>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), >>>>>>>>>> attach- >>>>>>>>>>> dev, verbose_p2p_dist) < 0) >>>>>>>>>> attach->peer2peer = false; >>>>>>>>>> - if (!attach->peer2peer && >>>>>>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>>>>>> + struct pci_dev *client = >>>>>>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>>>>>> + >>>>>>>>>> + /* >>>>>>>>>> + * There are perf issues with using p2p with display >>>>>>>>>> buffers, >>>>>>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>>>>>> p2p window >>>>>>>>>> + * buffer). Probably we need some actual userspace >>>>>>>>>> hints. We >>>>>>>>>> + * shouldn't really assume anything. Also, note that >>>>>>>>>> some of >>>>>>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>>>>>> + */ >>>>>>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>>>>>> + attach->peer2peer = false; >>>>>>>>>> + else if (client && pci_is_display(client) && >>>>>>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>>>>>> + attach->peer2peer = false; >>>>>>>>>> + } >>>>>>>>>> + >>>>>>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>>>>>> return -EOPNOTSUPP; >>>>>>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device >>>>>>>>>> *dev, >>>>>>>>>> struct dma_buf *dma_buf) >>>>>>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>>>>>> } >>>>>>>>>> -static void xe_dma_buf_move_notify(struct >>>>>>>>>> dma_buf_attachment >>>>>>>>>> *attach) >>>>>>>>>> -{ >>>>>>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>>> - >>>>>>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>>> -} >>>>>>>>>> - >>>>>>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>>> - .allow_peer2peer = true, >>>>>>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>>> -}; >>>>>>>>>> - >>>>>>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>>>>>> struct dma_buf_test_params { >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 8:51 ` Matthew Auld @ 2026-10-13 9:10 ` Christian König 2026-10-13 9:18 ` Thomas Hellström 2026-10-13 9:40 ` Matthew Auld 0 siblings, 2 replies; 23+ messages in thread From: Christian König @ 2026-10-13 9:10 UTC (permalink / raw) To: Matthew Auld, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 10/13/26 10:51, Matthew Auld wrote: > On 09/10/2026 14:02, Matthew Auld wrote: >> On 09/10/2026 12:38, Christian König wrote: >>> On 10/9/26 12:29, Matthew Auld wrote: >>>> On 09/10/2026 10:26, Christian König wrote: >>>>> On 10/9/26 11:16, Matthew Auld wrote: >>>>>> On 08/10/2026 12:17, Matthew Auld wrote: >>>>>>> On 08/10/2026 10:55, Christian König wrote: >>>>>>>> On 10/8/26 11:46, Matthew Auld wrote: >>>>>>>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>>>>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: .... >>>>>>>>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >>>>>>>>> >>>>>>>>> Some questions: >>>>>>>>> >>>>>>>>> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >>>>>>>>> >>>>>>>>> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? >>>>>>>> >>>>>>>> If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. >>>>>>> >>>>>>> Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? >>>>>> >>>>>> For example: https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c >>>>>> >>>>>> Is that something you would accept? >>>>> >>>>> No, you misunderstood me. Enabling P2P is perfectly fine for APUs and mandatory for a couple of customer projects. >>>>> >>>>> The buffer should just be rejected from scanout in the display code and on the AMDGPU side we already do that if I'm not completely mistaken. >>>> >>>> Reporter confirmed seeing the issue in windowed mode also. So I don't think it's just a case of rejecting from scanout? >>> >>> Then I don't understand what the problem is. >>> >>> When the dGPU renders and the iGPU displays you have only two possibilities, either the dGPU pushes the frames over the bus or the iGPU pulls the frames over the bus. Both should be equaly slow or fast, so what exactly is the expectation here? >> >> I assume the AMD APU/iGPU is sampling/reading from the p2p window buffer every frame. That is not possible. The HW can't do this and because of this we should have a check for that in the DAL/DC code. It could be that this check doesn't work and because of low resolution and/or not busy PCIe it somehow works, but that would then clearly not be intended behavior. What could also be is that the frame is pulled over the PCIe bus using the SDMA multiple times, but that would then be a bug in userspace somewhere. >> Expectation is that buffer rather gets moved from VRAM to system memory when mapping the buffer with dma-buf, where it stays, and never negotiated to use p2p in the first place. I don't think p2p is really valid here. No, that is incorrect. When P2P is available it should be used, it's just that AMDs APUs can't scanout over P2P. >> What happened is that user updated their kernel and due to the MOVE_NOTIFY config getting removed and enabled by default on newer kernels this suddenly started negotiating p2p for things like the window buffer AFAICT, instead of moving it to system memory and keeping it there. On previous kernel versions the buffer would just get moved and pinned in system memory. From user pov this is a kernel regression, given the perf they are now getting. >> >> On AMD CPUs you allow p2p for everything. On Intel iGPUs we currently ban p2p. Maybe there is bigger question here for what is the story for p2p and iGPU? Should it really be blanket allowed like this? Should it not be opt-in with some userspace hints or something? Again no, P2P is mandatory for APUs. We have basically tons of customers using that with AMD dGPU and AMD iGPU and that works fine. >> >> The other important part is that even when you move to system memory, it's no longer pinned (MOVE_NOTIFY) so can you now can also get violent ping-ponging, which is addressed in the first patch in this series. Yeah that is something which we have seen before as well. That should definately be prevented by the kernel. Does that patch help with the performance? >> >>> >>> The only thing you need to avoid is that the iGPU starts to scan out over the PCIe bus directly, cause then the frame is pulled over the bus multiple times (if that actually works correctly). >>> >>> Do you have a link to the original bug report? I was only added later to the discussion so I don't have that. >> >> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 > > Christian, did you get a chance to look at this? Any further thoughts? Sorry for the delay, I'm quite busy at the moment with AMD internal things and preparing a presentation for FOSDEM. Regards, Christian. > >> >>> >>> Regards, >>> Christian. >>> >>>> >>>>> >>>>> Userspace has fallback path that when a DRM atomic requests fails for an imported buffer it falls back to copying the buffer first. >>>>> >>>>> Regards, >>>>> Christian. >>>>>>> >>>>>>>> >>>>>>>> At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. >>>>>>>> >>>>>>>> And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. >>>>>>> >>>>>>> AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. >>>>>>> >>>>>>>> >>>>>>>> Regards, >>>>>>>> Christian. >>>>>>>> >>>>>>>>> >>>>>>>>>> >>>>>>>>>> Thanks, >>>>>>>>>> Thomas >>>>>>>>>> >>>>>>>>>>> Assisted-by: LLM >>>>>>>>>>> Closes: >>>>>>>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>>>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>>>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>>>>>>>> GPUs") >>>>>>>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>>>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>>>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>>>>>>> --- >>>>>>>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>>>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>>>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++ ++++ +------ >>>>>>>>>>> -- >>>>>>>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>>>>>>> >>>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/ xe_bo.c >>>>>>>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>>>>>>>> void *data, >>>>>>>>>>> * Display scanout is always non-coherent with the CPU >>>>>>>>>>> cache. >>>>>>>>>>> */ >>>>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>>>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>>>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>>>>>>> XE_BO_FLAG_SCANOUT; >>>>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>>>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>>>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>>>>>>> *file_priv, >>>>>>>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>>>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>>>>>>> XE_BO_FLAG_FORCE_WC | >>>>>>>>>>> + XE_BO_FLAG_SCANOUT | >>>>>>>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>>>>>>> if (IS_ERR(bo)) >>>>>>>>>>> return PTR_ERR(bo); >>>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/ xe_bo.h >>>>>>>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>>>> @@ -56,6 +56,7 @@ >>>>>>>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>>>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>>>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>>>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>>>>>>> /* this one is trigger internally only */ >>>>>>>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>>>>>>> _ddebug *desc, bool *flag) >>>>>>>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>>>>>>> #endif >>>>>>>>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>>>>> *attach) >>>>>>>>>>> +{ >>>>>>>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>>>> + >>>>>>>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>>>> +} >>>>>>>>>>> + >>>>>>>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>>>> + .allow_peer2peer = true, >>>>>>>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>>>> +}; >>>>>>>>>>> + >>>>>>>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>>>>>>>> +{ >>>>>>>>>>> + while (dev) { >>>>>>>>>>> + if (dev_is_pci(dev)) >>>>>>>>>>> + return to_pci_dev(dev); >>>>>>>>>>> + dev = dev->parent; >>>>>>>>>>> + } >>>>>>>>>>> + return NULL; >>>>>>>>>>> +} >>>>>>>>>>> + >>>>>>>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>>>>>>>> *attach) >>>>>>>>>>> +{ >>>>>>>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>>>>>>> +} >>>>>>>>>>> + >>>>>>>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>>>>>>> struct dma_buf_attachment *attach) >>>>>>>>>>> { >>>>>>>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>>>>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>>>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>>>>>>> *dmabuf, >>>>>>>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>>>>>>>> dev, verbose_p2p_dist) < 0) >>>>>>>>>>> attach->peer2peer = false; >>>>>>>>>>> - if (!attach->peer2peer && >>>>>>>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>>>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>>>>>>> + struct pci_dev *client = >>>>>>>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>>>>>>> + >>>>>>>>>>> + /* >>>>>>>>>>> + * There are perf issues with using p2p with display >>>>>>>>>>> buffers, >>>>>>>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>>>>>>> p2p window >>>>>>>>>>> + * buffer). Probably we need some actual userspace >>>>>>>>>>> hints. We >>>>>>>>>>> + * shouldn't really assume anything. Also, note that >>>>>>>>>>> some of >>>>>>>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>>>>>>> + */ >>>>>>>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>>>>>>> + attach->peer2peer = false; >>>>>>>>>>> + else if (client && pci_is_display(client) && >>>>>>>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>>>>>>> + attach->peer2peer = false; >>>>>>>>>>> + } >>>>>>>>>>> + >>>>>>>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>>>>>>> return -EOPNOTSUPP; >>>>>>>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>>>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>>>>>>>> struct dma_buf *dma_buf) >>>>>>>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>>>>>>> } >>>>>>>>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>>>>> *attach) >>>>>>>>>>> -{ >>>>>>>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>>>> - >>>>>>>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>>>> -} >>>>>>>>>>> - >>>>>>>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>>>> - .allow_peer2peer = true, >>>>>>>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>>>> -}; >>>>>>>>>>> - >>>>>>>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>>>>>>> struct dma_buf_test_params { >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 9:10 ` Christian König @ 2026-10-13 9:18 ` Thomas Hellström 2026-10-13 9:55 ` Christian König 2026-10-13 9:40 ` Matthew Auld 1 sibling, 1 reply; 23+ messages in thread From: Thomas Hellström @ 2026-10-13 9:18 UTC (permalink / raw) To: Christian König, Matthew Auld, intel-xe Cc: Matthew Brost, stable, Christian König On Tue, 2026-10-13 at 11:10 +0200, Christian König wrote: > On 10/13/26 10:51, Matthew Auld wrote: > > On 09/10/2026 14:02, Matthew Auld wrote: > > > On 09/10/2026 12:38, Christian König wrote: > > > > On 10/9/26 12:29, Matthew Auld wrote: > > > > > On 09/10/2026 10:26, Christian König wrote: > > > > > > On 10/9/26 11:16, Matthew Auld wrote: > > > > > > > On 08/10/2026 12:17, Matthew Auld wrote: > > > > > > > > On 08/10/2026 10:55, Christian König wrote: > > > > > > > > > On 10/8/26 11:46, Matthew Auld wrote: > > > > > > > > > > On 07/10/2026 19:11, Thomas Hellström wrote: > > > > > > > > > > > On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld > > > > > > > > > > > wrote: > .... > > > > > > > > > > Perhaps Intel Mesa side should use tt-only for this > > > > > > > > > > type buffer (?), but since this is a kernel > > > > > > > > > > regression I was hoping we could fix this on the > > > > > > > > > > kernel side. > > > > > > > > > > > > > > > > > > > > Some questions: > > > > > > > > > > > > > > > > > > > > 1. Do you know what is the general story with p2p > > > > > > > > > > between igpu and dgpu? Is this a legit thing? For > > > > > > > > > > igpus/APUs, advertising p2p can't be dropped? It > > > > > > > > > > clearly seems to work though. > > > > > > > > > > > > > > > > > > > > 2. If this is legit, do you guys then also prevent > > > > > > > > > > p2p with display buffers? Are you just doing that > > > > > > > > > > at the Mesa level, or is there some magic in the > > > > > > > > > > kernel also? > > > > > > > > > > > > > > > > > > If I understand that correctly that the igpu is the > > > > > > > > > importer and the dgpu is the exporter then yeah > > > > > > > > > Thomas is right that definately needs to be fixed on > > > > > > > > > the importer side. > > > > > > > > > > > > > > > > Ideas on how do that? Could we do something like > > > > > > > > allow_peer2peer = false in amdgpu_gem_prime_import() if > > > > > > > > this is an APU device, or further check if the exporter > > > > > > > > side is not amdgpu, like APU + Xe? > > > > > > > > > > > > > > For example: > > > > > > > https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7ec86 > > > > > > > c68b42655b4bb85ee90af434a617e7bec8c > > > > > > > > > > > > > > Is that something you would accept? > > > > > > > > > > > > No, you misunderstood me. Enabling P2P is perfectly fine > > > > > > for APUs and mandatory for a couple of customer projects. > > > > > > > > > > > > The buffer should just be rejected from scanout in the > > > > > > display code and on the AMDGPU side we already do that if > > > > > > I'm not completely mistaken. > > > > > > > > > > Reporter confirmed seeing the issue in windowed mode also. So > > > > > I don't think it's just a case of rejecting from scanout? > > > > > > > > Then I don't understand what the problem is. > > > > > > > > When the dGPU renders and the iGPU displays you have only two > > > > possibilities, either the dGPU pushes the frames over the bus > > > > or the iGPU pulls the frames over the bus. Both should be > > > > equaly slow or fast, so what exactly is the expectation here? > > > > > > I assume the AMD APU/iGPU is sampling/reading from the p2p window > > > buffer every frame. > > That is not possible. The HW can't do this and because of this we > should have a check for that in the DAL/DC code. > Couldn't the issue be that the compositor, running on the amd iGPU blits from the dma-buf rather than scans out from it? And the scanout is not the shared buffer at all? /Thomas ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 9:18 ` Thomas Hellström @ 2026-10-13 9:55 ` Christian König 2026-10-13 10:16 ` Thomas Hellström 0 siblings, 1 reply; 23+ messages in thread From: Christian König @ 2026-10-13 9:55 UTC (permalink / raw) To: Thomas Hellström, Matthew Auld, intel-xe Cc: Matthew Brost, stable, Christian König On 10/13/26 11:18, Thomas Hellström wrote: ... >>>>>>>>> Ideas on how do that? Could we do something like >>>>>>>>> allow_peer2peer = false in amdgpu_gem_prime_import() if >>>>>>>>> this is an APU device, or further check if the exporter >>>>>>>>> side is not amdgpu, like APU + Xe? >>>>>>>> >>>>>>>> For example: >>>>>>>> https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7ec86 >>>>>>>> c68b42655b4bb85ee90af434a617e7bec8c >>>>>>>> >>>>>>>> Is that something you would accept? >>>>>>> >>>>>>> No, you misunderstood me. Enabling P2P is perfectly fine >>>>>>> for APUs and mandatory for a couple of customer projects. >>>>>>> >>>>>>> The buffer should just be rejected from scanout in the >>>>>>> display code and on the AMDGPU side we already do that if >>>>>>> I'm not completely mistaken. >>>>>> >>>>>> Reporter confirmed seeing the issue in windowed mode also. So >>>>>> I don't think it's just a case of rejecting from scanout? >>>>> >>>>> Then I don't understand what the problem is. >>>>> >>>>> When the dGPU renders and the iGPU displays you have only two >>>>> possibilities, either the dGPU pushes the frames over the bus >>>>> or the iGPU pulls the frames over the bus. Both should be >>>>> equaly slow or fast, so what exactly is the expectation here? >>>> >>>> I assume the AMD APU/iGPU is sampling/reading from the p2p window >>>> buffer every frame. >> >> That is not possible. The HW can't do this and because of this we >> should have a check for that in the DAL/DC code. >> > Couldn't the issue be that the compositor, running on the amd iGPU > blits from the dma-buf rather than scans out from it? And the scanout > is not the shared buffer at all? Yeah that is possible, but that should only happen when there is some change. When the iGPU reads the buffer only once then that isn't really problematic, it's actually exactly what should happen. It only becomes problematic when the buffer is read multiple times to construct multiple frames, either because the compositor does something unexpected or we really start to scanout from the buffer. Regards, Christian. > > /Thomas > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 9:55 ` Christian König @ 2026-10-13 10:16 ` Thomas Hellström 2026-10-13 11:45 ` Christian König 0 siblings, 1 reply; 23+ messages in thread From: Thomas Hellström @ 2026-10-13 10:16 UTC (permalink / raw) To: Christian König, Matthew Auld, intel-xe Cc: Matthew Brost, stable, Christian König On Tue, 2026-10-13 at 11:55 +0200, Christian König wrote: > On 10/13/26 11:18, Thomas Hellström wrote: > ... > > > > > > > > > > Ideas on how do that? Could we do something like > > > > > > > > > > allow_peer2peer = false in > > > > > > > > > > amdgpu_gem_prime_import() if > > > > > > > > > > this is an APU device, or further check if the > > > > > > > > > > exporter > > > > > > > > > > side is not amdgpu, like APU + Xe? > > > > > > > > > > > > > > > > > > For example: > > > > > > > > > https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7 > > > > > > > > > ec86 > > > > > > > > > c68b42655b4bb85ee90af434a617e7bec8c > > > > > > > > > > > > > > > > > > Is that something you would accept? > > > > > > > > > > > > > > > > No, you misunderstood me. Enabling P2P is perfectly > > > > > > > > fine > > > > > > > > for APUs and mandatory for a couple of customer > > > > > > > > projects. > > > > > > > > > > > > > > > > The buffer should just be rejected from scanout in the > > > > > > > > display code and on the AMDGPU side we already do that > > > > > > > > if > > > > > > > > I'm not completely mistaken. > > > > > > > > > > > > > > Reporter confirmed seeing the issue in windowed mode > > > > > > > also. So > > > > > > > I don't think it's just a case of rejecting from scanout? > > > > > > > > > > > > Then I don't understand what the problem is. > > > > > > > > > > > > When the dGPU renders and the iGPU displays you have only > > > > > > two > > > > > > possibilities, either the dGPU pushes the frames over the > > > > > > bus > > > > > > or the iGPU pulls the frames over the bus. Both should be > > > > > > equaly slow or fast, so what exactly is the expectation > > > > > > here? > > > > > > > > > > I assume the AMD APU/iGPU is sampling/reading from the p2p > > > > > window > > > > > buffer every frame. > > > > > > That is not possible. The HW can't do this and because of this we > > > should have a check for that in the DAL/DC code. > > > > > Couldn't the issue be that the compositor, running on the amd iGPU > > blits from the dma-buf rather than scans out from it? And the > > scanout > > is not the shared buffer at all? > > Yeah that is possible, but that should only happen when there is some > change. > > When the iGPU reads the buffer only once then that isn't really > problematic, it's actually exactly what should happen. > > It only becomes problematic when the buffer is read multiple times to > construct multiple frames, either because the compositor does > something unexpected or we really start to scanout from the buffer. But in windowed mode (and perhaps even fullscreen?) the compositor blits repeatedly over PCI each frame from the dma-buf? /Thomas > > Regards, > Christian. > > > > > /Thomas > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 10:16 ` Thomas Hellström @ 2026-10-13 11:45 ` Christian König 2026-10-13 12:36 ` Matthew Auld 0 siblings, 1 reply; 23+ messages in thread From: Christian König @ 2026-10-13 11:45 UTC (permalink / raw) To: Thomas Hellström, Matthew Auld, intel-xe Cc: Matthew Brost, stable, Christian König On 10/13/26 12:16, Thomas Hellström wrote: > On Tue, 2026-10-13 at 11:55 +0200, Christian König wrote: >> On 10/13/26 11:18, Thomas Hellström wrote: >> ... >>>>>>>>>>> Ideas on how do that? Could we do something like >>>>>>>>>>> allow_peer2peer = false in >>>>>>>>>>> amdgpu_gem_prime_import() if >>>>>>>>>>> this is an APU device, or further check if the >>>>>>>>>>> exporter >>>>>>>>>>> side is not amdgpu, like APU + Xe? >>>>>>>>>> >>>>>>>>>> For example: >>>>>>>>>> https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7 >>>>>>>>>> ec86 >>>>>>>>>> c68b42655b4bb85ee90af434a617e7bec8c >>>>>>>>>> >>>>>>>>>> Is that something you would accept? >>>>>>>>> >>>>>>>>> No, you misunderstood me. Enabling P2P is perfectly >>>>>>>>> fine >>>>>>>>> for APUs and mandatory for a couple of customer >>>>>>>>> projects. >>>>>>>>> >>>>>>>>> The buffer should just be rejected from scanout in the >>>>>>>>> display code and on the AMDGPU side we already do that >>>>>>>>> if >>>>>>>>> I'm not completely mistaken. >>>>>>>> >>>>>>>> Reporter confirmed seeing the issue in windowed mode >>>>>>>> also. So >>>>>>>> I don't think it's just a case of rejecting from scanout? >>>>>>> >>>>>>> Then I don't understand what the problem is. >>>>>>> >>>>>>> When the dGPU renders and the iGPU displays you have only >>>>>>> two >>>>>>> possibilities, either the dGPU pushes the frames over the >>>>>>> bus >>>>>>> or the iGPU pulls the frames over the bus. Both should be >>>>>>> equaly slow or fast, so what exactly is the expectation >>>>>>> here? >>>>>> >>>>>> I assume the AMD APU/iGPU is sampling/reading from the p2p >>>>>> window >>>>>> buffer every frame. >>>> >>>> That is not possible. The HW can't do this and because of this we >>>> should have a check for that in the DAL/DC code. >>>> >>> Couldn't the issue be that the compositor, running on the amd iGPU >>> blits from the dma-buf rather than scans out from it? And the >>> scanout >>> is not the shared buffer at all? >> >> Yeah that is possible, but that should only happen when there is some >> change. >> >> When the iGPU reads the buffer only once then that isn't really >> problematic, it's actually exactly what should happen. >> >> It only becomes problematic when the buffer is read multiple times to >> construct multiple frames, either because the compositor does >> something unexpected or we really start to scanout from the buffer. > > But in windowed mode (and perhaps even fullscreen?) the compositor > blits repeatedly over PCI each frame from the dma-buf? It usually doesn't. In fullscreen mode using the buffer directly for scanout in an atomic commit should be rejected by the kernel because the HW can't do this. The fallback path then copies the buffer content to a locally allocated buffer exactly once and then display that new buffer. I'm not 100% sure what happens in windowed mode for the specific compositor the end user is using. What could be is that when the compositor gets buffers from multiple applications that it composes multiple times over PCIe, but it is unlikely that this will cause problems. You would need something like game/test running on dGPU and glxgears without vblank sync on the iGPU. AMD uses basically exactly that setup in the Tesla infotainment system for example and it works perfectly fine there. I have not the slightest idea what's going wrong here. Regards, Christian. > > /Thomas > > > >> >> Regards, >> Christian. >> >>> >>> /Thomas >>> >>> ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 11:45 ` Christian König @ 2026-10-13 12:36 ` Matthew Auld 0 siblings, 0 replies; 23+ messages in thread From: Matthew Auld @ 2026-10-13 12:36 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 13/10/2026 12:45, Christian König wrote: > On 10/13/26 12:16, Thomas Hellström wrote: >> On Tue, 2026-10-13 at 11:55 +0200, Christian König wrote: >>> On 10/13/26 11:18, Thomas Hellström wrote: >>> ... >>>>>>>>>>>> Ideas on how do that? Could we do something like >>>>>>>>>>>> allow_peer2peer = false in >>>>>>>>>>>> amdgpu_gem_prime_import() if >>>>>>>>>>>> this is an APU device, or further check if the >>>>>>>>>>>> exporter >>>>>>>>>>>> side is not amdgpu, like APU + Xe? >>>>>>>>>>> >>>>>>>>>>> For example: >>>>>>>>>>> https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7 >>>>>>>>>>> ec86 >>>>>>>>>>> c68b42655b4bb85ee90af434a617e7bec8c >>>>>>>>>>> >>>>>>>>>>> Is that something you would accept? >>>>>>>>>> >>>>>>>>>> No, you misunderstood me. Enabling P2P is perfectly >>>>>>>>>> fine >>>>>>>>>> for APUs and mandatory for a couple of customer >>>>>>>>>> projects. >>>>>>>>>> >>>>>>>>>> The buffer should just be rejected from scanout in the >>>>>>>>>> display code and on the AMDGPU side we already do that >>>>>>>>>> if >>>>>>>>>> I'm not completely mistaken. >>>>>>>>> >>>>>>>>> Reporter confirmed seeing the issue in windowed mode >>>>>>>>> also. So >>>>>>>>> I don't think it's just a case of rejecting from scanout? >>>>>>>> >>>>>>>> Then I don't understand what the problem is. >>>>>>>> >>>>>>>> When the dGPU renders and the iGPU displays you have only >>>>>>>> two >>>>>>>> possibilities, either the dGPU pushes the frames over the >>>>>>>> bus >>>>>>>> or the iGPU pulls the frames over the bus. Both should be >>>>>>>> equaly slow or fast, so what exactly is the expectation >>>>>>>> here? >>>>>>> >>>>>>> I assume the AMD APU/iGPU is sampling/reading from the p2p >>>>>>> window >>>>>>> buffer every frame. >>>>> >>>>> That is not possible. The HW can't do this and because of this we >>>>> should have a check for that in the DAL/DC code. >>>>> >>>> Couldn't the issue be that the compositor, running on the amd iGPU >>>> blits from the dma-buf rather than scans out from it? And the >>>> scanout >>>> is not the shared buffer at all? >>> >>> Yeah that is possible, but that should only happen when there is some >>> change. >>> >>> When the iGPU reads the buffer only once then that isn't really >>> problematic, it's actually exactly what should happen. >>> >>> It only becomes problematic when the buffer is read multiple times to >>> construct multiple frames, either because the compositor does >>> something unexpected or we really start to scanout from the buffer. >> >> But in windowed mode (and perhaps even fullscreen?) the compositor >> blits repeatedly over PCI each frame from the dma-buf? > > It usually doesn't. > > In fullscreen mode using the buffer directly for scanout in an atomic commit should be rejected by the kernel because the HW can't do this. > > The fallback path then copies the buffer content to a locally allocated buffer exactly once and then display that new buffer. > > I'm not 100% sure what happens in windowed mode for the specific compositor the end user is using. What could be is that when the compositor gets buffers from multiple applications that it composes multiple times over PCIe, but it is unlikely that this will cause problems. You would need something like game/test running on dGPU and glxgears without vblank sync on the iGPU. > > AMD uses basically exactly that setup in the Tesla infotainment system for example and it works perfectly fine there. > > I have not the slightest idea what's going wrong here. My rough mental model here (could be waaaay wrong), is that dgpu is constantly rendering into a private VRAM buffer, and then copying that to a linear buffer which is the window buffer exported to the compositor. Display is plugged into the igpu. DRI_PRIME=1 is used for the game (like GravityMark) to the do rendering on the dgpu. In windowed mode, every frame the compositor needs to somehow sample the exported window/texture from a shader, like in mutter. In this flow the igpu is still using native buffer for the actual scanout. That exported window buffer is using a flexible placement, so it could technically go in VRAM or system memory. But I don't think p2p was on the bingo cards here for Intel. So if you suddenly negotiate p2p instead of moving it, it now stays in VRAM. So igpu is now constantly reading over pcie every frame, which is extremely slow it seems, for the user. Other thing is that when I tried to repro this locally, I used xe igpu + xe dgpu, and manually forcing p2p in the kernel. When I traced it with bpf I just see the private VRAM buffer sitting there, and the exported window buffer as system memory. So no p2p. There was also no ping-pong, so it must mean that the exported window buffer is NOT flexible placement in this multi-GPU setup. So it could potentially be specific to some cross-driver flow with OpenGL, like with amd + intel, where you can end up with the flexible placement for the exported window buffer on some multi-GPU setups. Not 100% sure though. > > Regards, > Christian. > > > >> >> /Thomas >> >> >> >>> >>> Regards, >>> Christian. >>> >>>> >>>> /Thomas >>>> >>>> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 9:10 ` Christian König 2026-10-13 9:18 ` Thomas Hellström @ 2026-10-13 9:40 ` Matthew Auld 2026-10-13 11:50 ` Christian König 1 sibling, 1 reply; 23+ messages in thread From: Matthew Auld @ 2026-10-13 9:40 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 13/10/2026 10:10, Christian König wrote: > On 10/13/26 10:51, Matthew Auld wrote: >> On 09/10/2026 14:02, Matthew Auld wrote: >>> On 09/10/2026 12:38, Christian König wrote: >>>> On 10/9/26 12:29, Matthew Auld wrote: >>>>> On 09/10/2026 10:26, Christian König wrote: >>>>>> On 10/9/26 11:16, Matthew Auld wrote: >>>>>>> On 08/10/2026 12:17, Matthew Auld wrote: >>>>>>>> On 08/10/2026 10:55, Christian König wrote: >>>>>>>>> On 10/8/26 11:46, Matthew Auld wrote: >>>>>>>>>> On 07/10/2026 19:11, Thomas Hellström wrote: >>>>>>>>>>> On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: > .... >>>>>>>>>> Perhaps Intel Mesa side should use tt-only for this type buffer (?), but since this is a kernel regression I was hoping we could fix this on the kernel side. >>>>>>>>>> >>>>>>>>>> Some questions: >>>>>>>>>> >>>>>>>>>> 1. Do you know what is the general story with p2p between igpu and dgpu? Is this a legit thing? For igpus/APUs, advertising p2p can't be dropped? It clearly seems to work though. >>>>>>>>>> >>>>>>>>>> 2. If this is legit, do you guys then also prevent p2p with display buffers? Are you just doing that at the Mesa level, or is there some magic in the kernel also? >>>>>>>>> >>>>>>>>> If I understand that correctly that the igpu is the importer and the dgpu is the exporter then yeah Thomas is right that definately needs to be fixed on the importer side. >>>>>>>> >>>>>>>> Ideas on how do that? Could we do something like allow_peer2peer = false in amdgpu_gem_prime_import() if this is an APU device, or further check if the exporter side is not amdgpu, like APU + Xe? >>>>>>> >>>>>>> For example: https://gitlab.freedesktop.org/mwa/kernel/-/ commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c >>>>>>> >>>>>>> Is that something you would accept? >>>>>> >>>>>> No, you misunderstood me. Enabling P2P is perfectly fine for APUs and mandatory for a couple of customer projects. >>>>>> >>>>>> The buffer should just be rejected from scanout in the display code and on the AMDGPU side we already do that if I'm not completely mistaken. >>>>> >>>>> Reporter confirmed seeing the issue in windowed mode also. So I don't think it's just a case of rejecting from scanout? >>>> >>>> Then I don't understand what the problem is. >>>> >>>> When the dGPU renders and the iGPU displays you have only two possibilities, either the dGPU pushes the frames over the bus or the iGPU pulls the frames over the bus. Both should be equaly slow or fast, so what exactly is the expectation here? >>> >>> I assume the AMD APU/iGPU is sampling/reading from the p2p window buffer every frame. > > That is not possible. The HW can't do this and because of this we should have a check for that in the DAL/DC code. Yeah, I don't doubt it, but this was also without full-screen, so scanout buffer will be native on amdgpu AFAIK. I assume here mutter or so samples the window/texture from a shader over pcie. > > It could be that this check doesn't work and because of low resolution and/or not busy PCIe it somehow works, but that would then clearly not be intended behavior. > > What could also be is that the frame is pulled over the PCIe bus using the SDMA multiple times, but that would then be a bug in userspace somewhere. > >>> Expectation is that buffer rather gets moved from VRAM to system memory when mapping the buffer with dma-buf, where it stays, and never negotiated to use p2p in the first place. I don't think p2p is really valid here. > > No, that is incorrect. When P2P is available it should be used, it's just that AMDs APUs can't scanout over P2P. > >>> What happened is that user updated their kernel and due to the MOVE_NOTIFY config getting removed and enabled by default on newer kernels this suddenly started negotiating p2p for things like the window buffer AFAICT, instead of moving it to system memory and keeping it there. On previous kernel versions the buffer would just get moved and pinned in system memory. From user pov this is a kernel regression, given the perf they are now getting. >>> >>> On AMD CPUs you allow p2p for everything. On Intel iGPUs we currently ban p2p. Maybe there is bigger question here for what is the story for p2p and iGPU? Should it really be blanket allowed like this? Should it not be opt-in with some userspace hints or something? > > Again no, P2P is mandatory for APUs. We have basically tons of customers using that with AMD dGPU and AMD iGPU and that works fine. > >>> >>> The other important part is that even when you move to system memory, it's no longer pinned (MOVE_NOTIFY) so can you now can also get violent ping-ponging, which is addressed in the first patch in this series. > > Yeah that is something which we have seen before as well. That should definately be prevented by the kernel. > > Does that patch help with the performance? Mostly it's p2p that is the big killer. Series gives much as 10x FPS jump, if we stop negotiating p2p for the window buffer and prevent ping-ponging. For example, in GravityMark, we go from ~18 FPS -> ~180 FPS, which matches the previous performance the user was getting. The ping-ponging hurts, but from the reports it looked like it was only in some games and not as severe as p2p. User confirmed this with CONFIG_PCI_P2PDMA=n, and that fixed most of the workloads. Adding the ping-pong fix then fixes everything. > >>> >>>> >>>> The only thing you need to avoid is that the iGPU starts to scan out over the PCIe bus directly, cause then the frame is pulled over the bus multiple times (if that actually works correctly). >>>> >>>> Do you have a link to the original bug report? I was only added later to the discussion so I don't have that. >>> >>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >> >> Christian, did you get a chance to look at this? Any further thoughts? > > Sorry for the delay, I'm quite busy at the moment with AMD internal things and preparing a presentation for FOSDEM. No worries. > > Regards, > Christian. > >> >>> >>>> >>>> Regards, >>>> Christian. >>>> >>>>> >>>>>> >>>>>> Userspace has fallback path that when a DRM atomic requests fails for an imported buffer it falls back to copying the buffer first. >>>>>> >>>>>> Regards, >>>>>> Christian. >>>>>>>> >>>>>>>>> >>>>>>>>> At least for AMD APUs we clearly support P2P both as exporter and importer, but what isn't supported is to scan out over P2P. We explicitely reject that inside DC/DAL. >>>>>>>>> >>>>>>>>> And I'm pretty sure that you should do that as well. I'm actually really suprised that this works technically and doesn't cause corruption or anything. >>>>>>>> >>>>>>>> AFAICT on Intel iGPUs it will currently always end up with peer2peer = false on attach. Not 100% sure if direct scanout was happening on the p2p buffer itself, for the user report. >>>>>>>> >>>>>>>>> >>>>>>>>> Regards, >>>>>>>>> Christian. >>>>>>>>> >>>>>>>>>> >>>>>>>>>>> >>>>>>>>>>> Thanks, >>>>>>>>>>> Thomas >>>>>>>>>>> >>>>>>>>>>>> Assisted-by: LLM >>>>>>>>>>>> Closes: >>>>>>>>>>>> https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 >>>>>>>>>>>> Signed-off-by: Matthew Auld <matthew.auld@intel.com> >>>>>>>>>>>> Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel >>>>>>>>>>>> GPUs") >>>>>>>>>>>> Cc: Thomas Hellström <thomas.hellstrom@linux.intel.com> >>>>>>>>>>>> Cc: Matthew Brost <matthew.brost@intel.com> >>>>>>>>>>>> Cc: <stable@vger.kernel.org> # v6.12 >>>>>>>>>>>> --- >>>>>>>>>>>> drivers/gpu/drm/xe/xe_bo.c | 3 +- >>>>>>>>>>>> drivers/gpu/drm/xe/xe_bo.h | 1 + >>>>>>>>>>>> drivers/gpu/drm/xe/xe_dma_buf.c | 63 ++++++++++++++++++++ ++++ +------ >>>>>>>>>>>> -- >>>>>>>>>>>> 3 files changed, 51 insertions(+), 16 deletions(-) >>>>>>>>>>>> >>>>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/ xe_bo.c >>>>>>>>>>>> index 5065a7bac39c..e63437a261cb 100644 >>>>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.c >>>>>>>>>>>> @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct drm_device *dev, >>>>>>>>>>>> void *data, >>>>>>>>>>>> * Display scanout is always non-coherent with the CPU >>>>>>>>>>>> cache. >>>>>>>>>>>> */ >>>>>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_SCANOUT) >>>>>>>>>>>> - bo_flags |= XE_BO_FLAG_FORCE_WC; >>>>>>>>>>>> + bo_flags |= XE_BO_FLAG_FORCE_WC | >>>>>>>>>>>> XE_BO_FLAG_SCANOUT; >>>>>>>>>>>> if (args->flags & DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { >>>>>>>>>>>> if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < 20)) >>>>>>>>>>>> @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct drm_file >>>>>>>>>>>> *file_priv, >>>>>>>>>>>> DRM_XE_GEM_CPU_CACHING_WC, >>>>>>>>>>>> XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) | >>>>>>>>>>>> XE_BO_FLAG_FORCE_WC | >>>>>>>>>>>> + XE_BO_FLAG_SCANOUT | >>>>>>>>>>>> XE_BO_FLAG_NEEDS_CPU_ACCESS, NULL); >>>>>>>>>>>> if (IS_ERR(bo)) >>>>>>>>>>>> return PTR_ERR(bo); >>>>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/ xe_bo.h >>>>>>>>>>>> index 1ddf3b6d4a9e..6965c2655731 100644 >>>>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_bo.h >>>>>>>>>>>> @@ -56,6 +56,7 @@ >>>>>>>>>>>> #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) >>>>>>>>>>>> #define XE_BO_FLAG_NO_COMPRESSION BIT(26) >>>>>>>>>>>> #define XE_BO_FLAG_NEEDS_1G BIT(27) >>>>>>>>>>>> +#define XE_BO_FLAG_SCANOUT BIT(28) >>>>>>>>>>>> /* this one is trigger internally only */ >>>>>>>>>>>> #define XE_BO_FLAG_INTERNAL_TEST BIT(30) >>>>>>>>>>>> diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>>> b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>>> index b973579a69b8..0a347f09cee4 100644 >>>>>>>>>>>> --- a/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>>> +++ b/drivers/gpu/drm/xe/xe_dma_buf.c >>>>>>>>>>>> @@ -36,10 +36,40 @@ static void xe_ddebug_flip_bool(const struct >>>>>>>>>>>> _ddebug *desc, bool *flag) >>>>>>>>>>>> #define xe_dma_buf_p2p_debug_flip(flag) do { } while (0) >>>>>>>>>>>> #endif >>>>>>>>>>>> +static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>>>>>> *attach) >>>>>>>>>>>> +{ >>>>>>>>>>>> + struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>>>> + struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>>>>> + >>>>>>>>>>>> + XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>>>>> +} >>>>>>>>>>>> + >>>>>>>>>>>> +static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>>>>> + .allow_peer2peer = true, >>>>>>>>>>>> + .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>>>>> +}; >>>>>>>>>>>> + >>>>>>>>>>>> +static struct pci_dev *xe_find_parent_pci_dev(struct device *dev) >>>>>>>>>>>> +{ >>>>>>>>>>>> + while (dev) { >>>>>>>>>>>> + if (dev_is_pci(dev)) >>>>>>>>>>>> + return to_pci_dev(dev); >>>>>>>>>>>> + dev = dev->parent; >>>>>>>>>>>> + } >>>>>>>>>>>> + return NULL; >>>>>>>>>>>> +} >>>>>>>>>>>> + >>>>>>>>>>>> +static bool xe_dma_buf_attach_is_xe_device(struct dma_buf_attachment >>>>>>>>>>>> *attach) >>>>>>>>>>>> +{ >>>>>>>>>>>> + return attach->importer_ops == &xe_dma_buf_attach_ops; >>>>>>>>>>>> +} >>>>>>>>>>>> + >>>>>>>>>>>> static int xe_dma_buf_attach(struct dma_buf *dmabuf, >>>>>>>>>>>> struct dma_buf_attachment *attach) >>>>>>>>>>>> { >>>>>>>>>>>> struct drm_gem_object *obj = attach->dmabuf->priv; >>>>>>>>>>>> + struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>>>> bool verbose_p2p_dist = drm_debug_enabled(DRM_UT_DRIVER); >>>>>>>>>>>> xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); >>>>>>>>>>>> @@ -48,7 +78,24 @@ static int xe_dma_buf_attach(struct dma_buf >>>>>>>>>>>> *dmabuf, >>>>>>>>>>>> pci_p2pdma_distance(to_pci_dev(obj->dev->dev), attach- >>>>>>>>>>>>> dev, verbose_p2p_dist) < 0) >>>>>>>>>>>> attach->peer2peer = false; >>>>>>>>>>>> - if (!attach->peer2peer && >>>>>>>>>>>> !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) >>>>>>>>>>>> + if (attach->peer2peer && xe_bo_can_migrate(bo, XE_PL_TT)) { >>>>>>>>>>>> + struct pci_dev *client = >>>>>>>>>>>> xe_find_parent_pci_dev(attach->dev); >>>>>>>>>>>> + >>>>>>>>>>>> + /* >>>>>>>>>>>> + * There are perf issues with using p2p with display >>>>>>>>>>>> buffers, >>>>>>>>>>>> + * like with igpu (compositor) <-> dgpu (exported >>>>>>>>>>>> p2p window >>>>>>>>>>>> + * buffer). Probably we need some actual userspace >>>>>>>>>>>> hints. We >>>>>>>>>>>> + * shouldn't really assume anything. Also, note that >>>>>>>>>>>> some of >>>>>>>>>>>> + * these buffers are not always tagged with SCANOUT. >>>>>>>>>>>> + */ >>>>>>>>>>>> + if (bo->flags & XE_BO_FLAG_SCANOUT) >>>>>>>>>>>> + attach->peer2peer = false; >>>>>>>>>>>> + else if (client && pci_is_display(client) && >>>>>>>>>>>> + !xe_dma_buf_attach_is_xe_device(attach)) >>>>>>>>>>>> + attach->peer2peer = false; >>>>>>>>>>>> + } >>>>>>>>>>>> + >>>>>>>>>>>> + if (!attach->peer2peer && !xe_bo_can_migrate(bo, XE_PL_TT)) >>>>>>>>>>>> return -EOPNOTSUPP; >>>>>>>>>>>> xe_pm_runtime_get(to_xe_device(obj->dev)); >>>>>>>>>>>> @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct drm_device *dev, >>>>>>>>>>>> struct dma_buf *dma_buf) >>>>>>>>>>>> return ret ? ERR_PTR(ret) : &bo->ttm.base; >>>>>>>>>>>> } >>>>>>>>>>>> -static void xe_dma_buf_move_notify(struct dma_buf_attachment >>>>>>>>>>>> *attach) >>>>>>>>>>>> -{ >>>>>>>>>>>> - struct drm_gem_object *obj = attach->importer_priv; >>>>>>>>>>>> - struct xe_bo *bo = gem_to_xe_bo(obj); >>>>>>>>>>>> - struct drm_exec *exec = XE_VALIDATION_UNSUPPORTED; >>>>>>>>>>>> - >>>>>>>>>>>> - XE_WARN_ON(xe_bo_evict(bo, exec)); >>>>>>>>>>>> -} >>>>>>>>>>>> - >>>>>>>>>>>> -static const struct dma_buf_attach_ops xe_dma_buf_attach_ops = { >>>>>>>>>>>> - .allow_peer2peer = true, >>>>>>>>>>>> - .invalidate_mappings = xe_dma_buf_move_notify >>>>>>>>>>>> -}; >>>>>>>>>>>> - >>>>>>>>>>>> #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) >>>>>>>>>>>> struct dma_buf_test_params { >>>>>>>>>> >>>>>>>>> >>>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 9:40 ` Matthew Auld @ 2026-10-13 11:50 ` Christian König 2026-10-13 13:15 ` Matthew Auld 0 siblings, 1 reply; 23+ messages in thread From: Christian König @ 2026-10-13 11:50 UTC (permalink / raw) To: Matthew Auld, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 10/13/26 11:40, Matthew Auld wrote: ...>> >> It could be that this check doesn't work and because of low resolution and/or not busy PCIe it somehow works, but that would then clearly not be intended behavior. >> >> What could also be is that the frame is pulled over the PCIe bus using the SDMA multiple times, but that would then be a bug in userspace somewhere. >> >>>> Expectation is that buffer rather gets moved from VRAM to system memory when mapping the buffer with dma-buf, where it stays, and never negotiated to use p2p in the first place. I don't think p2p is really valid here. >> >> No, that is incorrect. When P2P is available it should be used, it's just that AMDs APUs can't scanout over P2P. >> >>>> What happened is that user updated their kernel and due to the MOVE_NOTIFY config getting removed and enabled by default on newer kernels this suddenly started negotiating p2p for things like the window buffer AFAICT, instead of moving it to system memory and keeping it there. On previous kernel versions the buffer would just get moved and pinned in system memory. From user pov this is a kernel regression, given the perf they are now getting. >>>> >>>> On AMD CPUs you allow p2p for everything. On Intel iGPUs we currently ban p2p. Maybe there is bigger question here for what is the story for p2p and iGPU? Should it really be blanket allowed like this? Should it not be opt-in with some userspace hints or something? >> >> Again no, P2P is mandatory for APUs. We have basically tons of customers using that with AMD dGPU and AMD iGPU and that works fine. >> >>>> >>>> The other important part is that even when you move to system memory, it's no longer pinned (MOVE_NOTIFY) so can you now can also get violent ping-ponging, which is addressed in the first patch in this series. >> >> Yeah that is something which we have seen before as well. That should definately be prevented by the kernel. >> >> Does that patch help with the performance? > > Mostly it's p2p that is the big killer. Series gives much as 10x FPS jump, if we stop negotiating p2p for the window buffer and prevent ping-ponging. > > For example, in GravityMark, we go from ~18 FPS -> ~180 FPS, which matches the previous performance the user was getting. The ping-ponging hurts, but from the reports it looked like it was only in some games and not as severe as p2p. User confirmed this with CONFIG_PCI_P2PDMA=n, and that fixed most of the workloads. Adding the ping-pong fix then fixes everything. In that case I have not the slightest idea what going wrong here. The only difference with P2P enabled I can see is that the buffer is pulled over the PCIe bus by the iGPU instead of pushed by the dGPU. As I wrote in Thomas reply as well AMD uses exactly that setup with an dGPU and iGPU in the infotainment system found in Teslas and that works rather well as far as I know. Regards, Christian. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-13 11:50 ` Christian König @ 2026-10-13 13:15 ` Matthew Auld 0 siblings, 0 replies; 23+ messages in thread From: Matthew Auld @ 2026-10-13 13:15 UTC (permalink / raw) To: Christian König, Thomas Hellström, intel-xe Cc: Matthew Brost, stable, Christian König On 13/10/2026 12:50, Christian König wrote: > On 10/13/26 11:40, Matthew Auld wrote: > ...>> >>> It could be that this check doesn't work and because of low resolution and/or not busy PCIe it somehow works, but that would then clearly not be intended behavior. >>> >>> What could also be is that the frame is pulled over the PCIe bus using the SDMA multiple times, but that would then be a bug in userspace somewhere. >>> >>>>> Expectation is that buffer rather gets moved from VRAM to system memory when mapping the buffer with dma-buf, where it stays, and never negotiated to use p2p in the first place. I don't think p2p is really valid here. >>> >>> No, that is incorrect. When P2P is available it should be used, it's just that AMDs APUs can't scanout over P2P. >>> >>>>> What happened is that user updated their kernel and due to the MOVE_NOTIFY config getting removed and enabled by default on newer kernels this suddenly started negotiating p2p for things like the window buffer AFAICT, instead of moving it to system memory and keeping it there. On previous kernel versions the buffer would just get moved and pinned in system memory. From user pov this is a kernel regression, given the perf they are now getting. >>>>> >>>>> On AMD CPUs you allow p2p for everything. On Intel iGPUs we currently ban p2p. Maybe there is bigger question here for what is the story for p2p and iGPU? Should it really be blanket allowed like this? Should it not be opt-in with some userspace hints or something? >>> >>> Again no, P2P is mandatory for APUs. We have basically tons of customers using that with AMD dGPU and AMD iGPU and that works fine. >>> >>>>> >>>>> The other important part is that even when you move to system memory, it's no longer pinned (MOVE_NOTIFY) so can you now can also get violent ping-ponging, which is addressed in the first patch in this series. >>> >>> Yeah that is something which we have seen before as well. That should definately be prevented by the kernel. >>> >>> Does that patch help with the performance? >> >> Mostly it's p2p that is the big killer. Series gives much as 10x FPS jump, if we stop negotiating p2p for the window buffer and prevent ping-ponging. >> >> For example, in GravityMark, we go from ~18 FPS -> ~180 FPS, which matches the previous performance the user was getting. The ping-ponging hurts, but from the reports it looked like it was only in some games and not as severe as p2p. User confirmed this with CONFIG_PCI_P2PDMA=n, and that fixed most of the workloads. Adding the ping-pong fix then fixes everything. > > In that case I have not the slightest idea what going wrong here. > > The only difference with P2P enabled I can see is that the buffer is pulled over the PCIe bus by the iGPU instead of pushed by the dGPU. As Thomas way saying, reads can be very slow, but writes are much faster, at least for pcie on this hw? > > As I wrote in Thomas reply as well AMD uses exactly that setup with an dGPU and iGPU in the infotainment system found in Teslas and that works rather well as far as I know. So do you use p2p for stuff like window buffers, where you constantly read over pcie every frame? And is it just on this setup, or is this more generally with any amd igpu + amd dgpu combo with upstream? Like with DRI_PRIME=1 with a game on the dgpu? There is no perf concern if you use p2p for this on your hw? If not, then I guess this is specific to our hw. > > Regards, > Christian. ^ permalink raw reply [flat|nested] 23+ messages in thread
* Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers 2026-10-09 11:38 ` Christian König 2026-10-09 13:02 ` Matthew Auld @ 2026-10-09 13:06 ` Thomas Hellström 1 sibling, 0 replies; 23+ messages in thread From: Thomas Hellström @ 2026-10-09 13:06 UTC (permalink / raw) To: Christian König, Matthew Auld, intel-xe Cc: Matthew Brost, stable, Christian König On Fri, 2026-10-09 at 13:38 +0200, Christian König wrote: > On 10/9/26 12:29, Matthew Auld wrote: > > On 09/10/2026 10:26, Christian König wrote: > > > On 10/9/26 11:16, Matthew Auld wrote: > > > > On 08/10/2026 12:17, Matthew Auld wrote: > > > > > On 08/10/2026 10:55, Christian König wrote: > > > > > > On 10/8/26 11:46, Matthew Auld wrote: > > > > > > > On 07/10/2026 19:11, Thomas Hellström wrote: > > > > > > > > On Wed, 2026-10-07 at 16:36 +0100, Matthew Auld wrote: > > > > > > > > > If p2p is allowed on the CPU (like on AMD), we > > > > > > > > > currently permit p2p > > > > > > > > > for > > > > > > > > > all exported VRAM buffers. However, in some multi-GPU > > > > > > > > > setups, like > > > > > > > > > with > > > > > > > > > prime rendering on a dgpu and scanout/compositor > > > > > > > > > using the igpu, it > > > > > > > > > looks like the exported dma-buf window buffer can in > > > > > > > > > some cases end > > > > > > > > > up > > > > > > > > > using p2p. > > > > > > > > > > > > > > > > > > However, user is reporting a kernel regression where > > > > > > > > > performance here > > > > > > > > > massively tanks across many games/workloads, where it > > > > > > > > > looks like on > > > > > > > > > each > > > > > > > > > frame, the igpu is reading/sampling from VRAM over > > > > > > > > > pcie for the > > > > > > > > > window > > > > > > > > > buffer, since it uses p2p. p2p is currently more > > > > > > > > > intended for dgpu <- > > > > > > > > > > > > > > > > > > > dgpu, and not so much display buffers with igpu <-> > > > > > > > > > dgpu, at least > > > > > > > > > for > > > > > > > > > now. > > > > > > > > > > > > > > > > > > We don't currently have any hints to say "please use > > > > > > > > > p2p for this > > > > > > > > > buffer, or please don't", however we can use some > > > > > > > > > heuristics to try > > > > > > > > > to > > > > > > > > > determine if this is likely a display buffer being > > > > > > > > > exported to an > > > > > > > > > igpu, > > > > > > > > > which probably never intended for p2p: > > > > > > > > > > > > > > > > > > 1. FLAG_SCANOUT being set on the buffer. Window > > > > > > > > > buffers usually get > > > > > > > > > tagged with this, but not always. > > > > > > > > > > > > > > > > > > 2. Importer being a PCI device with display, > > > > > > > > > which is not xe driver. > > > > > > > > > > > > > > > > > > We also rely on the existing p2p Intel CPU whitelist > > > > > > > > > to stop this > > > > > > > > > happening on xe igpu. Currently p2p is only enabled > > > > > > > > > for server CPUs, > > > > > > > > > on > > > > > > > > > Intel. We also limit the above to dual placement > > > > > > > > > buffers. > > > > > > > > > > > > > > > > > > The original regression was bisected to commit > > > > > > > > > 609fc8766d68 ("dma- > > > > > > > > > buf: > > > > > > > > > Always build with DMABUF_MOVE_NOTIFY"), however the > > > > > > > > > behaviour was > > > > > > > > > already present when enabling that knob, so this goes > > > > > > > > > back even > > > > > > > > > further. > > > > > > > > > > > > > > > > > > User has confirmed that along with the sticky dma-buf > > > > > > > > > TT placement > > > > > > > > > fix, > > > > > > > > > to prevent ping-ponging, the performance is back to > > > > > > > > > normal. > > > > > > > > > > > > > > > > > > > > > > > > > Actually, I think this needs to be fixed on the > > > > > > > > importer side. Until we > > > > > > > > have some some user-space hints, I don't think the > > > > > > > > exporter should > > > > > > > > second-guess. Most sane solution IMO is if igfx > > > > > > > > importers could just > > > > > > > > not advertise pcie p2p support? > > > > > > > > > > > > > > Yeah, not sure tbh. I'm not a huge fan of this patch > > > > > > > either. But you could argue that the SCANOUT flag is > > > > > > > already a good implicit hint/ signal on the exporter > > > > > > > side, but unfortunately there were some games where this > > > > > > > did not reliably work. The other implicit hint/signal is > > > > > > > perhaps the dual placement angle. For example, in > > > > > > > compute-runtime, it might be that they use VRAM-only if > > > > > > > they want p2p? AFAICT mesa for us always uses a dual > > > > > > > placement or just system memory for these types of > > > > > > > buffers. > > > > > > > > > > > > > > Christian, do you have any thoughts here? We have a > > > > > > > kernel perf regression after DMABUF_MOVE_NOTIFY was made > > > > > > > the default after updating to a new kernel. The gist is > > > > > > > that this then turned on the p2p paths, with a multi-GPU > > > > > > > setup with an AMD igpu/APU and Xe dgpu doing prime > > > > > > > rendering. From the user bug report it sounds like the > > > > > > > dgpu linear window buffer exported to the compositor > > > > > > > (igpu) is dual placement VRAM + TT, and so this now > > > > > > > attempts to keep that memory in VRAM instead of moving > > > > > > > it. Compositor then uses p2p to access it. It works but > > > > > > > performance falls off a massive cliff here. > > > > > > > > > > > > > > Perhaps Intel Mesa side should use tt-only for this type > > > > > > > buffer (?), but since this is a kernel regression I was > > > > > > > hoping we could fix this on the kernel side. > > > > > > > > > > > > > > Some questions: > > > > > > > > > > > > > > 1. Do you know what is the general story with p2p between > > > > > > > igpu and dgpu? Is this a legit thing? For igpus/APUs, > > > > > > > advertising p2p can't be dropped? It clearly seems to > > > > > > > work though. > > > > > > > > > > > > > > 2. If this is legit, do you guys then also prevent p2p > > > > > > > with display buffers? Are you just doing that at the Mesa > > > > > > > level, or is there some magic in the kernel also? > > > > > > > > > > > > If I understand that correctly that the igpu is the > > > > > > importer and the dgpu is the exporter then yeah Thomas is > > > > > > right that definately needs to be fixed on the importer > > > > > > side. > > > > > > > > > > Ideas on how do that? Could we do something like > > > > > allow_peer2peer = false in amdgpu_gem_prime_import() if this > > > > > is an APU device, or further check if the exporter side is > > > > > not amdgpu, like APU + Xe? > > > > > > > > For example: > > > > https://gitlab.freedesktop.org/mwa/kernel/-/commit/7ec86c68b42655b4bb85ee90af434a617e7bec8c > > > > > > > > Is that something you would accept? > > > > > > No, you misunderstood me. Enabling P2P is perfectly fine for APUs > > > and mandatory for a couple of customer projects. > > > > > > The buffer should just be rejected from scanout in the display > > > code and on the AMDGPU side we already do that if I'm not > > > completely mistaken. > > > > Reporter confirmed seeing the issue in windowed mode also. So I > > don't think it's just a case of rejecting from scanout? > > Then I don't understand what the problem is. > > When the dGPU renders and the iGPU displays you have only two > possibilities, either the dGPU pushes the frames over the bus or the > iGPU pulls the frames over the bus. Both should be equaly slow or > fast, so what exactly is the expectation here? I think PCIe reads, even p2p can be agonizingly slow. From my understanding writes are posted whereas reads are not. The reading /Thomas > > The only thing you need to avoid is that the iGPU starts to scan out > over the PCIe bus directly, cause then the frame is pulled over the > bus multiple times (if that actually works correctly). > > Do you have a link to the original bug report? I was only added later > to the discussion so I don't have that. > > Regards, > Christian. > > > > > > > > > Userspace has fallback path that when a DRM atomic requests fails > > > for an imported buffer it falls back to copying the buffer first. > > > > > > Regards, > > > Christian. > > > > > > > > > > > > > > > > > At least for AMD APUs we clearly support P2P both as > > > > > > exporter and importer, but what isn't supported is to scan > > > > > > out over P2P. We explicitely reject that inside DC/DAL. > > > > > > > > > > > > And I'm pretty sure that you should do that as well. I'm > > > > > > actually really suprised that this works technically and > > > > > > doesn't cause corruption or anything. > > > > > > > > > > AFAICT on Intel iGPUs it will currently always end up with > > > > > peer2peer = false on attach. Not 100% sure if direct scanout > > > > > was happening on the p2p buffer itself, for the user report. > > > > > > > > > > > > > > > > > Regards, > > > > > > Christian. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Thanks, > > > > > > > > Thomas > > > > > > > > > > > > > > > > > Assisted-by: LLM > > > > > > > > > Closes: > > > > > > > > > https://gitlab.freedesktop.org/drm/xe/kernel/-/work_items/9415 > > > > > > > > > Signed-off-by: Matthew Auld <matthew.auld@intel.com> > > > > > > > > > Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM > > > > > > > > > driver for Intel > > > > > > > > > GPUs") > > > > > > > > > Cc: Thomas Hellström > > > > > > > > > <thomas.hellstrom@linux.intel.com> > > > > > > > > > Cc: Matthew Brost <matthew.brost@intel.com> > > > > > > > > > Cc: <stable@vger.kernel.org> # v6.12 > > > > > > > > > --- > > > > > > > > > drivers/gpu/drm/xe/xe_bo.c | 3 +- > > > > > > > > > drivers/gpu/drm/xe/xe_bo.h | 1 + > > > > > > > > > drivers/gpu/drm/xe/xe_dma_buf.c | 63 > > > > > > > > > ++++++++++++++++++++++++ +------ > > > > > > > > > -- > > > > > > > > > 3 files changed, 51 insertions(+), 16 deletions(- > > > > > > > > > ) > > > > > > > > > > > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_bo.c > > > > > > > > > b/drivers/gpu/drm/xe/xe_bo.c > > > > > > > > > index 5065a7bac39c..e63437a261cb 100644 > > > > > > > > > --- a/drivers/gpu/drm/xe/xe_bo.c > > > > > > > > > +++ b/drivers/gpu/drm/xe/xe_bo.c > > > > > > > > > @@ -3658,7 +3658,7 @@ int xe_gem_create_ioctl(struct > > > > > > > > > drm_device *dev, > > > > > > > > > void *data, > > > > > > > > > * Display scanout is always non-coherent > > > > > > > > > with the CPU > > > > > > > > > cache. > > > > > > > > > */ > > > > > > > > > if (args->flags & > > > > > > > > > DRM_XE_GEM_CREATE_FLAG_SCANOUT) > > > > > > > > > - bo_flags |= XE_BO_FLAG_FORCE_WC; > > > > > > > > > + bo_flags |= XE_BO_FLAG_FORCE_WC | > > > > > > > > > XE_BO_FLAG_SCANOUT; > > > > > > > > > if (args->flags & > > > > > > > > > DRM_XE_GEM_CREATE_FLAG_NO_COMPRESSION) { > > > > > > > > > if (XE_IOCTL_DBG(xe, GRAPHICS_VER(xe) < > > > > > > > > > 20)) > > > > > > > > > @@ -4228,6 +4228,7 @@ int xe_bo_dumb_create(struct > > > > > > > > > drm_file > > > > > > > > > *file_priv, > > > > > > > > > DRM_XE_GEM_CPU_CACHING_WC, > > > > > > > > > XE_BO_FLAG_VRAM_IF_DGFX(xe_device_get_root_tile(xe)) > > > > > > > > > | > > > > > > > > > XE_BO_FLAG_FORCE_WC | > > > > > > > > > + XE_BO_FLAG_SCANOUT | > > > > > > > > > XE_BO_FLAG_NEEDS_CPU_ACCESS, > > > > > > > > > NULL); > > > > > > > > > if (IS_ERR(bo)) > > > > > > > > > return PTR_ERR(bo); > > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_bo.h > > > > > > > > > b/drivers/gpu/drm/xe/xe_bo.h > > > > > > > > > index 1ddf3b6d4a9e..6965c2655731 100644 > > > > > > > > > --- a/drivers/gpu/drm/xe/xe_bo.h > > > > > > > > > +++ b/drivers/gpu/drm/xe/xe_bo.h > > > > > > > > > @@ -56,6 +56,7 @@ > > > > > > > > > #define XE_BO_FLAG_FORCE_USER_VRAM BIT(25) > > > > > > > > > #define XE_BO_FLAG_NO_COMPRESSION BIT(26) > > > > > > > > > #define XE_BO_FLAG_NEEDS_1G BIT(27) > > > > > > > > > +#define XE_BO_FLAG_SCANOUT BIT(28) > > > > > > > > > /* this one is trigger internally only */ > > > > > > > > > #define XE_BO_FLAG_INTERNAL_TEST BIT(30) > > > > > > > > > diff --git a/drivers/gpu/drm/xe/xe_dma_buf.c > > > > > > > > > b/drivers/gpu/drm/xe/xe_dma_buf.c > > > > > > > > > index b973579a69b8..0a347f09cee4 100644 > > > > > > > > > --- a/drivers/gpu/drm/xe/xe_dma_buf.c > > > > > > > > > +++ b/drivers/gpu/drm/xe/xe_dma_buf.c > > > > > > > > > @@ -36,10 +36,40 @@ static void > > > > > > > > > xe_ddebug_flip_bool(const struct > > > > > > > > > _ddebug *desc, bool *flag) > > > > > > > > > #define xe_dma_buf_p2p_debug_flip(flag) do { } > > > > > > > > > while (0) > > > > > > > > > #endif > > > > > > > > > +static void xe_dma_buf_move_notify(struct > > > > > > > > > dma_buf_attachment > > > > > > > > > *attach) > > > > > > > > > +{ > > > > > > > > > + struct drm_gem_object *obj = attach- > > > > > > > > > >importer_priv; > > > > > > > > > + struct xe_bo *bo = gem_to_xe_bo(obj); > > > > > > > > > + struct drm_exec *exec = > > > > > > > > > XE_VALIDATION_UNSUPPORTED; > > > > > > > > > + > > > > > > > > > + XE_WARN_ON(xe_bo_evict(bo, exec)); > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +static const struct dma_buf_attach_ops > > > > > > > > > xe_dma_buf_attach_ops = { > > > > > > > > > + .allow_peer2peer = true, > > > > > > > > > + .invalidate_mappings = xe_dma_buf_move_notify > > > > > > > > > +}; > > > > > > > > > + > > > > > > > > > +static struct pci_dev *xe_find_parent_pci_dev(struct > > > > > > > > > device *dev) > > > > > > > > > +{ > > > > > > > > > + while (dev) { > > > > > > > > > + if (dev_is_pci(dev)) > > > > > > > > > + return to_pci_dev(dev); > > > > > > > > > + dev = dev->parent; > > > > > > > > > + } > > > > > > > > > + return NULL; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > +static bool xe_dma_buf_attach_is_xe_device(struct > > > > > > > > > dma_buf_attachment > > > > > > > > > *attach) > > > > > > > > > +{ > > > > > > > > > + return attach->importer_ops == > > > > > > > > > &xe_dma_buf_attach_ops; > > > > > > > > > +} > > > > > > > > > + > > > > > > > > > static int xe_dma_buf_attach(struct dma_buf > > > > > > > > > *dmabuf, > > > > > > > > > struct dma_buf_attachment > > > > > > > > > *attach) > > > > > > > > > { > > > > > > > > > struct drm_gem_object *obj = attach->dmabuf- > > > > > > > > > >priv; > > > > > > > > > + struct xe_bo *bo = gem_to_xe_bo(obj); > > > > > > > > > bool verbose_p2p_dist = > > > > > > > > > drm_debug_enabled(DRM_UT_DRIVER); > > > > > > > > > > > > > > > > > > xe_dma_buf_p2p_debug_flip(&verbose_p2p_dist); > > > > > > > > > @@ -48,7 +78,24 @@ static int > > > > > > > > > xe_dma_buf_attach(struct dma_buf > > > > > > > > > *dmabuf, > > > > > > > > > pci_p2pdma_distance(to_pci_dev(obj->dev- > > > > > > > > > >dev), attach- > > > > > > > > > > dev, verbose_p2p_dist) < 0) > > > > > > > > > attach->peer2peer = false; > > > > > > > > > - if (!attach->peer2peer && > > > > > > > > > !xe_bo_can_migrate(gem_to_xe_bo(obj), XE_PL_TT)) > > > > > > > > > + if (attach->peer2peer && xe_bo_can_migrate(bo, > > > > > > > > > XE_PL_TT)) { > > > > > > > > > + struct pci_dev *client = > > > > > > > > > xe_find_parent_pci_dev(attach->dev); > > > > > > > > > + > > > > > > > > > + /* > > > > > > > > > + * There are perf issues with using p2p with > > > > > > > > > display > > > > > > > > > buffers, > > > > > > > > > + * like with igpu (compositor) <-> dgpu > > > > > > > > > (exported > > > > > > > > > p2p window > > > > > > > > > + * buffer). Probably we need some actual > > > > > > > > > userspace > > > > > > > > > hints. We > > > > > > > > > + * shouldn't really assume anything. Also, > > > > > > > > > note that > > > > > > > > > some of > > > > > > > > > + * these buffers are not always tagged with > > > > > > > > > SCANOUT. > > > > > > > > > + */ > > > > > > > > > + if (bo->flags & XE_BO_FLAG_SCANOUT) > > > > > > > > > + attach->peer2peer = false; > > > > > > > > > + else if (client && pci_is_display(client) && > > > > > > > > > + > > > > > > > > > !xe_dma_buf_attach_is_xe_device(attach)) > > > > > > > > > + attach->peer2peer = false; > > > > > > > > > + } > > > > > > > > > + > > > > > > > > > + if (!attach->peer2peer && !xe_bo_can_migrate(bo, > > > > > > > > > XE_PL_TT)) > > > > > > > > > return -EOPNOTSUPP; > > > > > > > > > xe_pm_runtime_get(to_xe_device(obj->dev)); > > > > > > > > > @@ -349,20 +396,6 @@ xe_dma_buf_create_obj(struct > > > > > > > > > drm_device *dev, > > > > > > > > > struct dma_buf *dma_buf) > > > > > > > > > return ret ? ERR_PTR(ret) : &bo->ttm.base; > > > > > > > > > } > > > > > > > > > -static void xe_dma_buf_move_notify(struct > > > > > > > > > dma_buf_attachment > > > > > > > > > *attach) > > > > > > > > > -{ > > > > > > > > > - struct drm_gem_object *obj = attach- > > > > > > > > > >importer_priv; > > > > > > > > > - struct xe_bo *bo = gem_to_xe_bo(obj); > > > > > > > > > - struct drm_exec *exec = > > > > > > > > > XE_VALIDATION_UNSUPPORTED; > > > > > > > > > - > > > > > > > > > - XE_WARN_ON(xe_bo_evict(bo, exec)); > > > > > > > > > -} > > > > > > > > > - > > > > > > > > > -static const struct dma_buf_attach_ops > > > > > > > > > xe_dma_buf_attach_ops = { > > > > > > > > > - .allow_peer2peer = true, > > > > > > > > > - .invalidate_mappings = xe_dma_buf_move_notify > > > > > > > > > -}; > > > > > > > > > - > > > > > > > > > #if IS_ENABLED(CONFIG_DRM_XE_KUNIT_TEST) > > > > > > > > > struct dma_buf_test_params { > > > > > > > > > > > > > > > > > > > > > > > > > > > ^ permalink raw reply [flat|nested] 23+ messages in thread
end of thread, other threads:[~2026-10-13 13:15 UTC | newest]
Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20261007153605.1743626-5-matthew.auld@intel.com>
2026-10-07 15:36 ` [PATCH v2 1/3] drm/xe/dma-buf: keep non-p2p imported buffers in system memory Matthew Auld
2026-10-07 15:36 ` [PATCH v2 2/3] drm/xe/vm: make PREFETCH to TT sticky Matthew Auld
2026-10-07 15:36 ` [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers Matthew Auld
2026-10-07 18:11 ` Thomas Hellström
2026-10-08 9:46 ` Matthew Auld
2026-10-08 9:55 ` Christian König
2026-10-08 11:17 ` Matthew Auld
2026-10-09 9:16 ` Matthew Auld
2026-10-09 9:26 ` Christian König
2026-10-09 10:29 ` Matthew Auld
2026-10-09 11:38 ` Christian König
2026-10-09 13:02 ` Matthew Auld
2026-10-13 8:51 ` Matthew Auld
2026-10-13 9:10 ` Christian König
2026-10-13 9:18 ` Thomas Hellström
2026-10-13 9:55 ` Christian König
2026-10-13 10:16 ` Thomas Hellström
2026-10-13 11:45 ` Christian König
2026-10-13 12:36 ` Matthew Auld
2026-10-13 9:40 ` Matthew Auld
2026-10-13 11:50 ` Christian König
2026-10-13 13:15 ` Matthew Auld
2026-10-09 13:06 ` Thomas Hellström
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox