drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 + drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 3 + drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 + drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 80 +++++++++++++++- drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 3 + drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 137 +++++++++++++++++++++------- 6 files changed, 194 insertions(+), 36 deletions(-)
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.
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
drm/amdgpu: don't wake up the GPU for syncobj feature detection
drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 +
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 +
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 80 +++++++++++++++-
drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 3 +
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 137 +++++++++++++++++++++-------
6 files changed, 194 insertions(+), 36 deletions(-)
---
base-commit: 6ac55eab4fc41e0ea80f9064945e4340f13d8b5c
change-id: 20250730-b4-dont-wake-next-17fc02114331
Best regards,
--
Philipp Zabel <philipp.zabel@gmail.com>
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. > > 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. See Pierre-Erics latest patch set, I think we already solved that but I'm not 100% sure. Regards, Christian. > drm/amdgpu: don't wake up the GPU for syncobj feature detection > > drivers/gpu/drm/amd/amdgpu/amdgpu.h | 5 + > drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c | 3 + > drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 + > drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 80 +++++++++++++++- > drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c | 3 + > drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 137 +++++++++++++++++++++------- > 6 files changed, 194 insertions(+), 36 deletions(-) > --- > base-commit: 6ac55eab4fc41e0ea80f9064945e4340f13d8b5c > change-id: 20250730-b4-dont-wake-next-17fc02114331 > > Best regards, > -- > Philipp Zabel <philipp.zabel@gmail.com> >
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
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-prayer@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.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
[resend because the previous email didn't make it to most recipients] Hi, Le 06/08/2025 à 15:17, Christian König a écrit : > 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-prayer@amd.com/ >> >> Is that the latest version? > > I honestly don't know. @Pierre-Eric? https://lore.kernel.org/all/ZnvJHwnNAvDrRMVG@phenom.ffwll.local/ killed the approach taken by this patchset. After that I've reworked the series, and sent https://lists.freedesktop.org/archives/amd-gfx/2024-September/114417.html to do fine grain runtime pm in drm/amd/pm as a first step. I also have a local branch that I never sent that implements Sima's suggestion: pushing rpm handling down into the ioctl implementation. I'll try to rebase it and push it out on gitlab soon. 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.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
© 2016 - 2025 Red Hat, Inc.