On Wed, 9 Sept 2026 at 13:46, Christian König <[email protected]> wrote:
>
> On 9/9/26 11:46, Mike Lothian wrote:
> > amdgpu_dma_buf_map() adds VRAM to the allowed domains for a peer2peer
> > attachment, so ttm_bo_validate() can migrate the buffer from GTT into
> > VRAM. While the exporting device is runtime suspended its SDMA rings
> > are down and the move fails:
> >
> > amdgpu: Move buffer fallback to memcpy unavailable
> >
> > An importer on a second GPU reaches this holding no runtime PM
> > reference on the exporter, e.g. a compositor on the APU submitting a
> > frame that references a buffer exported by an idle dGPU:
> >
> > amdgpu_cs_ioctl -> amdgpu_cs_parser_bos -> amdgpu_cs_bo_validate
> > -> ttm_bo_validate -> amdgpu_bo_move -> dma_buf_map_attachment
> > -> amdgpu_dma_buf_map -> ttm_bo_validate -> amdgpu_bo_move
> >
> > Only migrate into VRAM while holding the exporter awake.
> > pm_runtime_get_if_active() takes a reference only when the device is
> > already active and never resumes it, so it cannot deadlock against the
> > reservation held across these callbacks. That deadlock is why
> > commit 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference
> > when we attach a buffer" v2") removed the pm_runtime_get_sync() from
> > the attach callback.
> >
> > If the device is suspended or suspending the buffer stays in GTT, which
> > remains accessible while the GPU is powered down. A negative return
> > means runtime PM is disabled, so the device cannot suspend and VRAM
> > stays usable.
>
> That is a good catch, but your bug explanation as well as the solution still
> look a bit questionable to me.
> > Fixes: 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference when we
> > attach a buffer" v2")
> > Cc: [email protected]
> > Signed-off-by: Mike Lothian <[email protected]>
> > Assisted-by: Claude:Opus-5 [Claude Code]
> > ---
> >
> > v2: hold the exporter with pm_runtime_get_if_active() across the
> > validate instead of testing adev->mman.buffer_funcs_enabled. The
> > v1 check was racy - the device could suspend between the test and
> > ttm_bo_validate(), so amdgpu_bo_move() could still see the rings
> > torn down. Reported by Sashiko AI review.
> >
> > Is this the failure that commit c52feb436539 ("drm/amdgpu: Disable
> > runtime PM for externally attached dGPUs") was working around? That
> > commit explains how to detect external attachment but not what breaks,
> > so I can't tell which.
> >
> > If it is the same thing, could the pci_is_thunderbolt_attached() ||
> > dev_is_removable() check there be narrowed or dropped on top of this,
> > so eGPU users keep runtime PM?
> >
> > I can't test that here: this box hits the bug by missing that check.
> > The dGPU is on an oculink port off a native AMD root port, so
> > pci_is_thunderbolt_attached() is false, dev_is_removable() is empty,
> > and runtime PM stays enabled.
> >
> > Reproduced on a HawkPoint APU [1002:1900] driving the display with a
> > Navi 48 [Radeon AI PRO R9700] [1002:7551] on oculink for render
> > offload. Without the patch kwin_wayland hits the call chain above
> > within a minute of the dGPU autosuspending and the desktop stops
> > repainting until it resumes.
>
> That sounds like there is also a bug in kwin as well.
>
> The GPU can only go into suspend when the rendering application closes it
> driver connection and that usually only happens when it terminates
Not any more. amdgpu_driver_open_kms() does pm_runtime_get_sync() on
entry and pm_runtime_put_autosuspend() at the pm_put: label on every
path including success, so an open fd holds no reference.
amdgpu_driver_postclose_kms() is the same shape. I see the dGPU
autosuspend with the client's render node still open
> So question is here why is kwin still having that imported DMA-buf as
> necessary resource for the rendering?
Because it is the content of a mapped window. kwin composites on the
APU and samples the buffer the client rendered on the dGPU. In 6.7.5
EglDisplay::importBufferAsImage() (src/opengl/egldisplay.cpp:395)
caches the EGLImage per GraphicsBuffer and drops it when the buffer is
destroyed, so the attach happens once, not per frame. The client can
then idle for minutes with the window still on screen
> But we still need to fix this properly in the kernel anyway to avoid having a
> deny of service.
>
> >
> > drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 16 ++++++++++++++--
> > 1 file changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > index b33c300e26e2..c89846f266d3 100644
> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> > @@ -43,6 +43,7 @@
> > #include <linux/dma-buf.h>
> > #include <linux/dma-fence-array.h>
> > #include <linux/pci-p2pdma.h>
> > +#include <linux/pm_runtime.h>
> >
> > static const struct dma_buf_attach_ops amdgpu_dma_buf_attach_ops;
> >
> > @@ -189,14 +190,25 @@ static struct sg_table *amdgpu_dma_buf_map(struct
> > dma_buf_attachment *attach,
> > /* move buffer into GTT or VRAM */
> > struct ttm_operation_ctx ctx = { false, false };
> > unsigned int domains = AMDGPU_GEM_DOMAIN_GTT;
> > + int pm_ref = 0;
> >
> > if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM &&
> > attach->peer2peer) {
> > - bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> > - domains |= AMDGPU_GEM_DOMAIN_VRAM;
> > + /*
> > + * Only migrate into VRAM while the exporter is held
> > + * awake. A negative return means runtime PM is
> > + * disabled, so it cannot suspend either.
> > + */
>
> Setting AMDGPU_GEM_DOMAIN_VRAM doesn't automatically migrate the BO, it just
> sets this as possible placement.
Not on this path. amdgpu_bo_placement_from_domain() marks GTT
TTM_PL_FLAG_FALLBACK when preferred_domains has VRAM and we are not an
APU, which is exactly when amdgpu_dma_buf_map() adds VRAM.
ttm_resource_compatible() skips fallback placements when not evicting,
so a BO in GTT is not compatible and ttm_bo_validate() migrates it
A WARN_ONCE on the failing branch gives old=TTM_PL_TT new=TTM_PL_VRAM
with amdgpu_dma_buf_map -> ttm_bo_validate -> amdgpu_bo_move in the
backtrace
> BO migration is only triggered if the BO is swapped out or similar. What most
> likely happens instead is that we suspend while something is still ongoing.
>
> But anyway the problem goes deeper than just the amdgpu_dma_buf_map()
> callback.
>
> We have picked up pinning DMA-buf to VRAM for RDMA without ODP (e.g. exactly
> the feature I mention in the commit message of c52feb436539), but failed to
> correctly fix the PM handling.
>
> So we really need to call pm_runtime_get_if_active() in
> amdgpu_dma_buf_attach() and fail to let some other driver attach if the
> device is already suspended.
Happy to do that. Which behaviour do you want when it returns 0?
Failing the attach breaks render offload. The attachment lives as long
as the buffer, so a client allocating a new one while the dGPU is idle
gets a failed import and kwin has no texture for that window
Holding the reference until detach keeps offload working, but pins the
dGPU awake for as long as any of its buffers are imported, which in
practice is the whole session
Note the importer here is amdgpu on both ends, so "some other driver"
would not cover this case
Cheers
Mike
> Regards,
> Christian.
>
> > + pm_ref =
> > pm_runtime_get_if_active(adev_to_drm(adev)->dev);
> > + if (pm_ref) {
> > + bo->flags |=
> > AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
> > + domains |= AMDGPU_GEM_DOMAIN_VRAM;
> > + }
> > }
> > amdgpu_bo_placement_from_domain(bo, domains);
> > r = ttm_bo_validate(&bo->tbo, &bo->placement, &ctx);
> > + if (pm_ref > 0)
> > + pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> > if (r)
> > return ERR_PTR(r);
> > }
>
