dri-devel Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended
@ 2026-09-09  2:08 Mike Lothian
  2026-09-09  2:23 ` sashiko-bot
  2026-09-09  9:46 ` [PATCH v2] " Mike Lothian
  0 siblings, 2 replies; 9+ messages in thread
From: Mike Lothian @ 2026-09-09  2:08 UTC (permalink / raw)
  To: amd-gfx
  Cc: alexander.deucher, christian.koenig, kevinyang.wang, dri-devel,
	stable, Mike Lothian

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

Taking a reference here would deadlock: the reservation is held across
these callbacks and is also taken during resume, which is why
commit 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference
when we attach a buffer" v2") removed it.  GTT stays accessible while
the GPU is powered down, so leaving the buffer there is safe.

Only request VRAM when the device can perform the move.

Fixes: 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference when we attach a buffer" v2")
Cc: stable@vger.kernel.org
Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
Assisted-by: Claude:Opus-5 [Claude Code]
---

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.  With the patch the map stays in GTT
(ttm_bo_validate() returns 0, mem_type TTM_PL_TT) and the dGPU keeps
autosuspending.

 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index b33c300e26e2..d1dae5f3fee5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
@@ -191,7 +191,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
 		unsigned int domains = AMDGPU_GEM_DOMAIN_GTT;
 
 		if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM &&
-		    attach->peer2peer) {
+		    attach->peer2peer &&
+		    adev->mman.buffer_funcs_enabled) {
 			bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
 			domains |= AMDGPU_GEM_DOMAIN_VRAM;
 		}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended
  2026-09-09  2:08 [PATCH] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended Mike Lothian
@ 2026-09-09  2:23 ` sashiko-bot
  2026-09-09  9:46 ` [PATCH v2] " Mike Lothian
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-09  2:23 UTC (permalink / raw)
  To: Mike Lothian; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The check for `adev->mman.buffer_funcs_enabled` is vulnerable to a Time-Of-Check to Time-Of-Use (TOCTOU) race with concurrent runtime PM suspend, potentially leading to a PCIe Master Abort system crash or scheduler corruption.
--

commit 7e4c0d8b57d6cf1fda7d356466ca2334810cad8a
Author: Mike Lothian <mike@fireburn.co.uk>

drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended

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. This patch only requests VRAM when the
device can perform the move.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index b33c300e26e2e..d1dae5f3fee5e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> @@ -191,7 +191,8 @@ static struct sg_table *amdgpu_dma_buf_map(struct dma_buf_attachment *attach,
>  		unsigned int domains = AMDGPU_GEM_DOMAIN_GTT;
>  
>  		if (bo->preferred_domains & AMDGPU_GEM_DOMAIN_VRAM &&
> -		    attach->peer2peer) {
> +		    attach->peer2peer &&
> +		    adev->mman.buffer_funcs_enabled) {

[Severity: High]
In drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c at line 194 in
amdgpu_dma_buf_map(), does this lockless check introduce a time-of-check to
time-of-use race with the runtime PM suspend path?

When userspace (e.g., a compositor) issues buffer validations on an exported
dGPU buffer in a multi-GPU environment, the exporter's BO dma_resv lock is
held.

However, if the BO is in GTT, amdgpu_device_suspend() does not block on this
lock since it only evicts VRAM.

This means amdgpu_device_suspend() can proceed concurrently, call
amdgpu_ttm_disable_buffer_funcs() to set buffer_funcs_enabled = false,
destroy the scheduler entities, and transition the device to D3cold.

When the subsequent ttm_bo_validate() reaches amdgpu_bo_move(), could it see
buffer_funcs_enabled as false and fall back to ttm_bo_move_memcpy(), causing
a PCIe Master Abort crash on a powered-down device?

Alternatively, could it see buffer_funcs_enabled as true but submit a job to
an already destroyed drm_sched_entity?

>  			bo->flags |= AMDGPU_GEM_CREATE_CPU_ACCESS_REQUIRED;
>  			domains |= AMDGPU_GEM_DOMAIN_VRAM;
>  		}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260909020854.58462-1-mike@fireburn.co.uk?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v2] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended
  2026-09-09  2:08 [PATCH] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended Mike Lothian
  2026-09-09  2:23 ` sashiko-bot
@ 2026-09-09  9:46 ` Mike Lothian
  2026-09-09 12:46   ` Christian König
  1 sibling, 1 reply; 9+ messages in thread
From: Mike Lothian @ 2026-09-09  9:46 UTC (permalink / raw)
  To: amd-gfx
  Cc: alexander.deucher, christian.koenig, kevinyang.wang, dri-devel,
	stable, Mike Lothian

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.

Fixes: 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference when we attach a buffer" v2")
Cc: stable@vger.kernel.org
Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
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.

 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.
+			 */
+			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);
 	}
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended
  2026-09-09  9:46 ` [PATCH v2] " Mike Lothian
@ 2026-09-09 12:46   ` Christian König
  2026-09-10  0:15     ` Mike Lothian
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2026-09-09 12:46 UTC (permalink / raw)
  To: Mike Lothian, amd-gfx
  Cc: alexander.deucher, kevinyang.wang, dri-devel, stable

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: stable@vger.kernel.org
> Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
> 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.

So question is here why is kwin still having that imported DMA-buf as necessary resource for the rendering?

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.

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.

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);
>  	}


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended
  2026-09-09 12:46   ` Christian König
@ 2026-09-10  0:15     ` Mike Lothian
  2026-09-10  9:27       ` Christian König
  0 siblings, 1 reply; 9+ messages in thread
From: Mike Lothian @ 2026-09-10  0:15 UTC (permalink / raw)
  To: Christian König
  Cc: amd-gfx, alexander.deucher, kevinyang.wang, dri-devel, stable

On Wed, 9 Sept 2026 at 13:46, Christian König <christian.koenig@amd.com> 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: stable@vger.kernel.org
> > Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
> > 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);
> >       }
>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH v2] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended
  2026-09-10  0:15     ` Mike Lothian
@ 2026-09-10  9:27       ` Christian König
  2026-09-11 18:38         ` [PATCH v3] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments Mike Lothian
  0 siblings, 1 reply; 9+ messages in thread
From: Christian König @ 2026-09-10  9:27 UTC (permalink / raw)
  To: Mike Lothian
  Cc: amd-gfx, alexander.deucher, kevinyang.wang, dri-devel, stable

On 9/10/26 02:15, Mike Lothian wrote:
> On Wed, 9 Sept 2026 at 13:46, Christian König <christian.koenig@amd.com> wrote:
...
>> 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

Ah, yes that starts to make more sense now. I was really wondering how this was reproduced.

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

Good point as well, yes. That is for optimizing placements for BOs which have both VRAM|GTT set in their preferred domains.

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

Oh, well that is a really good question.

> 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

Ideally we would want to grab the PM reference during operations like pin, map, etc.. *and* keep it alive as long as those data access paths can't be reverted by an invalidation notification.

But what makes it additionally complicated is that we hold locks in those operations which are also needed during suspend/resume, so we can't wait for resume to finish because that would deadlock.

So in practice that is most likely horrible complicate and error prone. And my educated guess is that it is also probably overkill.

For now I think we should use this instead:

In amdgpu_dma_buf_attach() when pm_runtime_get_if_active() fails we just set attach->peer2peer = false.

And then add a matching amdgpu_dma_buf_detach() to drop the reference again when attach->peer2peer is true.

Regards,
Christian.


 

> 
> 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);
>>>       }
>>


^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v3] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments
  2026-09-10  9:27       ` Christian König
@ 2026-09-11 18:38         ` Mike Lothian
  2026-09-11 18:46           ` sashiko-bot
  2026-09-11 23:29           ` [PATCH v4] " Mike Lothian
  0 siblings, 2 replies; 9+ messages in thread
From: Mike Lothian @ 2026-09-11 18:38 UTC (permalink / raw)
  To: amd-gfx
  Cc: christian.koenig, alexander.deucher, kevinyang.wang, dri-devel,
	stable, Mike Lothian

amdgpu_dma_buf_map() adds VRAM to the allowed domains for a peer2peer
attachment.  GTT is only a fallback placement when VRAM is preferred, so
ttm_bo_validate() migrates 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

Pinning a dma-buf into VRAM has the same requirement, which
commit 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference
when we attach a buffer" v2") called out as the one case that would
need the reference back.

Take it in attach and drop it in detach.  pm_runtime_get_if_active()
never resumes the device, so it cannot deadlock against the reservation
taken during resume, which is why the old pm_runtime_get_sync() had to
go.  If the device is not active, clear peer2peer instead: the buffer
then stays in GTT, which remains accessible while the GPU is powered
down.

Fixes: 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference when we attach a buffer" v2")
Cc: stable@vger.kernel.org
Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
Assisted-by: Claude:Opus-5 [Claude Code]
---

v3: take the reference in attach and drop it in detach, clearing
    peer2peer when the device is not active, as suggested by Christian.
    v2 only covered amdgpu_dma_buf_map() and left VRAM pinning exposed.
v2: use pm_runtime_get_if_active() instead of testing
    adev->mman.buffer_funcs_enabled, which was racy against a
    concurrent suspend.  Reported by Sashiko AI review.

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.

Tested with v3: Chromium rendering on the dGPU and composited by kwin
6.7.5 for five minutes, then closed.  The dGPU stayed active while the
window was on screen and suspended six seconds after Chromium exited,
with no fallback errors or runtime PM usage count underflows.  After
an hour of yuzu render offload the dGPU also suspended once yuzu
exited.

 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 39 ++++++++++++++++++++-
 1 file changed, 38 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index b33c300e26e2..fae695c3e531 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;
 
@@ -100,15 +101,50 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
 	    pci_p2pdma_distance(adev->pdev, attach->dev, false) < 0)
 		attach->peer2peer = false;
 
+	/*
+	 * P2P access needs the exporter awake for the lifetime of the
+	 * attachment.  pm_runtime_get_if_active() never resumes the device,
+	 * so it cannot deadlock against the reservation taken during resume.
+	 * A negative return means runtime PM is disabled and the device
+	 * cannot suspend, in which case the put in detach is a no-op.
+	 */
+	if (attach->peer2peer &&
+	    !pm_runtime_get_if_active(adev_to_drm(adev)->dev))
+		attach->peer2peer = false;
+
 	r = dma_resv_lock(bo->tbo.base.resv, NULL);
 	if (r)
-		return r;
+		goto err_pm_put;
 
 	amdgpu_vm_bo_update_shared(bo);
 
 	dma_resv_unlock(bo->tbo.base.resv);
 
 	return 0;
+
+err_pm_put:
+	if (attach->peer2peer)
+		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
+	return r;
+}
+
+/**
+ * amdgpu_dma_buf_detach - &dma_buf_ops.detach implementation
+ *
+ * @dmabuf: DMA-buf where we remove the attachment from
+ * @attach: the attachment to remove
+ *
+ * Drop the runtime PM reference taken in amdgpu_dma_buf_attach().
+ */
+static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf,
+				  struct dma_buf_attachment *attach)
+{
+	struct drm_gem_object *obj = dmabuf->priv;
+	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+	if (attach->peer2peer)
+		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
 }
 
 /**
@@ -350,6 +386,7 @@ static void amdgpu_dma_buf_vunmap(struct dma_buf *dma_buf, struct iosys_map *map
 
 const struct dma_buf_ops amdgpu_dmabuf_ops = {
 	.attach = amdgpu_dma_buf_attach,
+	.detach = amdgpu_dma_buf_detach,
 	.pin = amdgpu_dma_buf_pin,
 	.unpin = amdgpu_dma_buf_unpin,
 	.map_dma_buf = amdgpu_dma_buf_map,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH v3] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments
  2026-09-11 18:38         ` [PATCH v3] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments Mike Lothian
@ 2026-09-11 18:46           ` sashiko-bot
  2026-09-11 23:29           ` [PATCH v4] " Mike Lothian
  1 sibling, 0 replies; 9+ messages in thread
From: sashiko-bot @ 2026-09-11 18:46 UTC (permalink / raw)
  To: Mike Lothian; +Cc: dri-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Incorrect handling of negative return values from pm_runtime_get_if_active() causes a runtime PM usage count underflow.
--

commit ea3c97ff59ae3ca0f3d8bdf4b2a1ca3b95cda6f8
Author: Mike Lothian <mike@fireburn.co.uk>
drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments

This commit takes a runtime PM reference in amdgpu_dma_buf_attach() and drops
it in amdgpu_dma_buf_detach() to prevent P2P dma-buf attachments from falling
back to GTT when the exporting device is suspended. It uses
pm_runtime_get_if_active() to avoid deadlocks against the resume path.

> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> index b33c300e26e2e..fae695c3e5315 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
[ ... ]
> @@ -100,15 +101,50 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
>  	    pci_p2pdma_distance(adev->pdev, attach->dev, false) < 0)
>  		attach->peer2peer = false;
>  
> +	/*
> +	 * P2P access needs the exporter awake for the lifetime of the
> +	 * attachment.  pm_runtime_get_if_active() never resumes the device,
> +	 * so it cannot deadlock against the reservation taken during resume.
> +	 * A negative return means runtime PM is disabled and the device
> +	 * cannot suspend, in which case the put in detach is a no-op.
> +	 */
> +	if (attach->peer2peer &&
> +	    !pm_runtime_get_if_active(adev_to_drm(adev)->dev))
> +		attach->peer2peer = false;

[Severity: High]
The comment states that the put in detach is a no-op, but is this accurate?

pm_runtime_get_if_active() can return a negative error code (such as -EINVAL
when runtime PM is disabled). In that case, !pm_runtime_get_if_active()
evaluates to false, bypassing the attach->peer2peer = false assignment.

Since the reference was not actually acquired, but attach->peer2peer remains
true, wouldn't the unbalanced pm_runtime_put_autosuspend() calls in
err_pm_put and amdgpu_dma_buf_detach() cause a PM usage count underflow
rather than acting as a no-op?

> +
>  	r = dma_resv_lock(bo->tbo.base.resv, NULL);
>  	if (r)
> -		return r;
> +		goto err_pm_put;
>  
>  	amdgpu_vm_bo_update_shared(bo);
>  
>  	dma_resv_unlock(bo->tbo.base.resv);
>  
>  	return 0;
> +
> +err_pm_put:
> +	if (attach->peer2peer)
> +		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
> +	return r;
> +}
> +
> +/**
> + * amdgpu_dma_buf_detach - &dma_buf_ops.detach implementation
> + *
> + * @dmabuf: DMA-buf where we remove the attachment from
> + * @attach: the attachment to remove
> + *
> + * Drop the runtime PM reference taken in amdgpu_dma_buf_attach().
> + */
> +static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf,
> +				  struct dma_buf_attachment *attach)
> +{
> +	struct drm_gem_object *obj = dmabuf->priv;
> +	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
> +	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
> +
> +	if (attach->peer2peer)
> +		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260911183825.935140-1-mike@fireburn.co.uk?part=1

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH v4] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments
  2026-09-11 18:38         ` [PATCH v3] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments Mike Lothian
  2026-09-11 18:46           ` sashiko-bot
@ 2026-09-11 23:29           ` Mike Lothian
  1 sibling, 0 replies; 9+ messages in thread
From: Mike Lothian @ 2026-09-11 23:29 UTC (permalink / raw)
  To: amd-gfx
  Cc: christian.koenig, alexander.deucher, kevinyang.wang, dri-devel,
	stable, Mike Lothian

amdgpu_dma_buf_map() adds VRAM to the allowed domains for a peer2peer
attachment.  GTT is only a fallback placement when VRAM is preferred, so
ttm_bo_validate() migrates 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

Pinning a dma-buf into VRAM has the same requirement, which
commit 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference
when we attach a buffer" v2") called out as the one case that would
need the reference back.

Take it in attach and drop it in detach.  pm_runtime_get_if_active()
never resumes the device, so it cannot deadlock against the reservation
taken during resume, which is why the old pm_runtime_get_sync() had to
go.  If the device is not active, clear peer2peer instead: the buffer
then stays in GTT, which remains accessible while the GPU is powered
down.  If runtime PM is disabled, take a plain reference so the put in
detach stays balanced.

Fixes: 030631e97b20 ("drm/amdgpu: revert "take runtime pm reference when we attach a buffer" v2")
Cc: stable@vger.kernel.org
Suggested-by: Christian König <christian.koenig@amd.com>
Signed-off-by: Mike Lothian <mike@fireburn.co.uk>
Assisted-by: Claude:Opus-5 [Claude Code]
---

v4: take a plain reference when runtime PM is disabled.
    pm_runtime_get_if_active() returns -EINVAL then, and v3 left
    peer2peer set without a reference, so detach dropped one it never
    took.  Reported by Sashiko AI review.
v3: take the reference in attach and drop it in detach, clearing
    peer2peer when the device is not active, as suggested by Christian.
    v2 only covered amdgpu_dma_buf_map() and left VRAM pinning exposed.
v2: use pm_runtime_get_if_active() instead of testing
    adev->mman.buffer_funcs_enabled, which was racy against a
    concurrent suspend.  Reported by Sashiko AI review.

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.

Tested with v3: Chromium rendering on the dGPU and composited by kwin
6.7.5 for five minutes, then closed.  The dGPU stayed active while the
window was on screen and suspended six seconds after Chromium exited,
with no fallback errors or runtime PM usage count underflows.  After
an hour of yuzu render offload the dGPU also suspended once yuzu
exited.  v4 only changes the runtime PM disabled path, which this
machine does not hit.

 drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c | 43 ++++++++++++++++++++-
 1 file changed, 42 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_dma_buf.c
index b33c300e26e2..9adf3eed8822 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;
 
@@ -100,15 +101,54 @@ static int amdgpu_dma_buf_attach(struct dma_buf *dmabuf,
 	    pci_p2pdma_distance(adev->pdev, attach->dev, false) < 0)
 		attach->peer2peer = false;
 
+	/*
+	 * Only allow P2P while the exporter is active, and keep it active
+	 * until detach.  With runtime PM disabled take a plain reference so
+	 * the put in detach stays balanced.
+	 */
+	if (attach->peer2peer) {
+		struct device *dev = adev_to_drm(adev)->dev;
+		int ret = pm_runtime_get_if_active(dev);
+
+		if (!ret)
+			attach->peer2peer = false;
+		else if (ret < 0)
+			pm_runtime_get_noresume(dev);
+	}
+
 	r = dma_resv_lock(bo->tbo.base.resv, NULL);
 	if (r)
-		return r;
+		goto err_pm_put;
 
 	amdgpu_vm_bo_update_shared(bo);
 
 	dma_resv_unlock(bo->tbo.base.resv);
 
 	return 0;
+
+err_pm_put:
+	if (attach->peer2peer)
+		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
+	return r;
+}
+
+/**
+ * amdgpu_dma_buf_detach - &dma_buf_ops.detach implementation
+ *
+ * @dmabuf: DMA-buf where we remove the attachment from
+ * @attach: the attachment to remove
+ *
+ * Drop the runtime PM reference taken in amdgpu_dma_buf_attach().
+ */
+static void amdgpu_dma_buf_detach(struct dma_buf *dmabuf,
+				  struct dma_buf_attachment *attach)
+{
+	struct drm_gem_object *obj = dmabuf->priv;
+	struct amdgpu_bo *bo = gem_to_amdgpu_bo(obj);
+	struct amdgpu_device *adev = amdgpu_ttm_adev(bo->tbo.bdev);
+
+	if (attach->peer2peer)
+		pm_runtime_put_autosuspend(adev_to_drm(adev)->dev);
 }
 
 /**
@@ -350,6 +390,7 @@ static void amdgpu_dma_buf_vunmap(struct dma_buf *dma_buf, struct iosys_map *map
 
 const struct dma_buf_ops amdgpu_dmabuf_ops = {
 	.attach = amdgpu_dma_buf_attach,
+	.detach = amdgpu_dma_buf_detach,
 	.pin = amdgpu_dma_buf_pin,
 	.unpin = amdgpu_dma_buf_unpin,
 	.map_dma_buf = amdgpu_dma_buf_map,
-- 
2.55.0


^ permalink raw reply related	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2026-09-11 23:30 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-09  2:08 [PATCH] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended Mike Lothian
2026-09-09  2:23 ` sashiko-bot
2026-09-09  9:46 ` [PATCH v2] " Mike Lothian
2026-09-09 12:46   ` Christian König
2026-09-10  0:15     ` Mike Lothian
2026-09-10  9:27       ` Christian König
2026-09-11 18:38         ` [PATCH v3] drm/amdgpu: hold a runtime PM reference for P2P dma-buf attachments Mike Lothian
2026-09-11 18:46           ` sashiko-bot
2026-09-11 23:29           ` [PATCH v4] " Mike Lothian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox