Re: [PATCH RFC 0/6] amdgpu: Avoid powering on the dGPU on vkEnumeratePhysicalDevices()

[Philipp Zabel <p.zabel@pengutronix.de>]

On Mi, 2025-08-06 at 10:58 +0200, Christian König wrote:
> On 31.07.25 07:36, Philipp Zabel wrote:
> > This is an attempt at fixing amd#2295 [1]:
> > 
> >   On an AMD Rembrandt laptop with 680M iGPU and 6700S dGPU, calling
> >   vkEnumeratePhysicalDevices() wakes up the sleeping dGPU, even if all
> >   the application wants is to find and use the iGPU. This causes a delay
> >   of about 2 seconds on this system, followed by a few seconds of
> >   increased power draw until runtime PM turns the dGPU back off again.
> > 
> > [1] https://gitlab.freedesktop.org/drm/amd/-/issues/2295
> > 
> > Patch 1 avoids power up on some ioctls that don't need it.
> > Patch 2 avoids power up on open() by postponing fpriv initialization to
> > the first ioctl() that wakes up the dGPU.
> > Patches 3 and 4 add AMDGPU_INFO to the list of non-waking ioctls,
> > returning cached values for some queries.
> > Patch 5 works around an explicit register access from libdrm.
> > Patch 6 shorts out the syncobj ioctls while fpriv is still
> > uninitialized. This avoids waking up the dGPU during Vulkan syncobj
> > feature detection.
> 
> This idea came up multiple times now but was never completed.
> 
> IIRC Pierre-Eric last worked on it, it would probably be a good idea to dig up his patches from the mailing list.

Thank you, I wasn't aware of those patches [1]. Pierre-Eric did mention
them in https://gitlab.freedesktop.org/mesa/mesa/-/issues/13001, but I
didn't pick up on that back then.

[1] https://lore.kernel.org/all/20240618153003.146168-1-pierre-eric.pelloux-prayer@amd.com/

Is that the latest version? It looks to me like the review stalled out
on a disagreement whether the GB_ADDR_CONFIG query should be a separate
ioctl or whether it should be added to drm_amdgpu_info_device. The
discussion was later continued at
https://gitlab.freedesktop.org/mesa/libdrm/-/merge_requests/368,
seemingly coming to the conclusion that keeping the register read (but
cached) is the way to go? I didn't find a newer series with that
implemented.

> > 
> > regards
> > Philipp
> > 
> > Signed-off-by: Philipp Zabel <p.zabel@pengutronix.de>
> > ---
> > Alex Deucher (1):
> >       drm/amdgpu: don't wake up the GPU for some IOCTLs
> > 
> > Philipp Zabel (5):
> >       drm/amdgpu: don't wake up the GPU when opening the device
> >       drm/amdgpu: don't query xclk in AMDGPU_INFO_DEV_INFO
> >       drm/amdgpu: don't wake up the GPU for some AMDGPU_INFO queries
> >       drm/amdgpu: don't wake up the GPU for mmGB_ADDR_CONFIG register read
> 
> That is both unnecessary an insufficient. Unnecessary because we already have a mechanism to cache register values and insufficient because IIRC you need to add a bunch of more registers to the cached list.

This series was (just barely) sufficient for my purpose, which was only
to make vkEnumeratePhysicalDevices() not wake the dGPU on my Laptop.
I didn't realize there already was a caching mechanism in the lower
layers.

> See Pierre-Erics latest patch set, I think we already solved that but I'm not 100% sure.

If I found the correct version, it seems Sima's suggestion of pushing
runtime pm handling down from amdgpu_drm_ioctl into the amdgpu ioctl
callbacks [2] would be the best first next step?

[2] https://lore.kernel.org/amd-gfx/ZnvJHwnNAvDrRMVG@phenom.ffwll.local/

regards
Philipp