On 9/10/26 02:15, Mike Lothian wrote:
> On Wed, 9 Sept 2026 at 13:46, Christian König <[email protected]>
> 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);
>>> }
>>
