Re: [PATCH v2] drm/amdgpu: don't migrate a dma-buf into VRAM while runtime suspended

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

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