Re: [PATCH v2 3/3] drm/xe: skip p2p for suspected display buffers

["Christian König" <christian.koenig@amd.com>]

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 {
>>>>>>
>>>>>
>>>>
>>>
>>
>