On 06.08.25 12:15, Philipp Zabel wrote: > 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-pra...@amd.com/ > > Is that the latest version?
I honestly don't know. @Pierre-Eric? > 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. Could be that Pierre-Eric dropped the work after that. But IIRC we already use a cached value for GB_ADDR_CONFIG because of GFXOFF. Regards, Christian. > >>> >>> regards >>> Philipp >>> >>> Signed-off-by: Philipp Zabel <p.za...@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
