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

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

[Philipp Zabel]

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>

[PATCH RFC 1/6] drm/amdgpu: don't wake up the GPU for some IOCTLs

[Philipp Zabel]

From: Alex Deucher <alexander.deucher@amd.com>

Don't wake the GPU if the IOCTL doesn't need to power
up the GPU.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
Signed-off-by: Alex Deucher <alexander.deucher@amd.com>
Tested-by: Philipp Zabel <philipp.zabel@gmail.com>
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 60 ++++++++++++++++++++++++++++++---
 1 file changed, 55 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 3bb9b25cd1219..34b9d63a86227 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2926,23 +2926,73 @@ static int amdgpu_drm_release(struct inode *inode, struct file *filp)
 	return drm_release(inode, filp);
 }
 
+static const unsigned int no_wake_ioctl_list[] = {
+	DRM_IOCTL_VERSION,
+	DRM_IOCTL_GET_UNIQUE,
+	DRM_IOCTL_GET_MAGIC,
+	DRM_IOCTL_GET_CLIENT,
+	DRM_IOCTL_GET_STATS,
+	DRM_IOCTL_GET_CAP,
+	DRM_IOCTL_SET_CLIENT_CAP,
+	DRM_IOCTL_SET_VERSION,
+	DRM_IOCTL_SET_UNIQUE,
+	DRM_IOCTL_BLOCK,
+	DRM_IOCTL_UNBLOCK,
+	DRM_IOCTL_AUTH_MAGIC,
+	DRM_IOCTL_SET_MASTER,
+	DRM_IOCTL_ADD_DRAW,
+	DRM_IOCTL_RM_DRAW,
+	DRM_IOCTL_FINISH,
+	DRM_IOCTL_UPDATE_DRAW,
+	DRM_IOCTL_MODE_GETRESOURCES,
+	DRM_IOCTL_MODE_GETPLANERESOURCES,
+	DRM_IOCTL_MODE_GETCRTC,
+	DRM_IOCTL_MODE_GETPLANE,
+	DRM_IOCTL_MODE_GETGAMMA,
+	DRM_IOCTL_MODE_GETENCODER,
+	DRM_IOCTL_MODE_ATTACHMODE,
+	DRM_IOCTL_MODE_DETACHMODE,
+	DRM_IOCTL_MODE_GETPROPERTY,
+	DRM_IOCTL_MODE_GETPROPBLOB,
+	DRM_IOCTL_MODE_OBJ_GETPROPERTIES,
+	DRM_IOCTL_MODE_CREATEPROPBLOB,
+	DRM_IOCTL_MODE_DESTROYPROPBLOB,
+	DRM_IOCTL_MODE_CREATE_LEASE,
+	DRM_IOCTL_MODE_LIST_LESSEES,
+	DRM_IOCTL_MODE_GET_LEASE,
+	DRM_IOCTL_MODE_REVOKE_LEASE,
+};
+
 long amdgpu_drm_ioctl(struct file *filp,
 		      unsigned int cmd, unsigned long arg)
 {
 	struct drm_file *file_priv = filp->private_data;
 	struct drm_device *dev;
 	long ret;
+	bool wake = true;
+	int i;
+
+	for (i = 0; i < ARRAY_SIZE(no_wake_ioctl_list); i++) {
+		if (cmd == no_wake_ioctl_list[i]) {
+			wake = false;
+			break;
+		}
+	}
 
 	dev = file_priv->minor->dev;
-	ret = pm_runtime_get_sync(dev->dev);
-	if (ret < 0)
-		goto out;
+	if (wake) {
+		ret = pm_runtime_get_sync(dev->dev);
+		if (ret < 0)
+			goto out;
+	}
 
 	ret = drm_ioctl(filp, cmd, arg);
 
-	pm_runtime_mark_last_busy(dev->dev);
+	if (wake)
+		pm_runtime_mark_last_busy(dev->dev);
 out:
-	pm_runtime_put_autosuspend(dev->dev);
+	if (wake)
+		pm_runtime_put_autosuspend(dev->dev);
 	return ret;
 }
 

-- 
2.50.1

[PATCH RFC 2/6] drm/amdgpu: don't wake up the GPU when opening the device

[Philipp Zabel]

Don't wake the GPU when opening the device. Delay amdgpu_fpriv (and
with it VM) initialization until the first IOCTL that wakes the GPU
anyway, unless it is already active.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h         |  4 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c     | 11 +++++
 drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c  |  3 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c     | 66 ++++++++++++++++++++---------
 5 files changed, 68 insertions(+), 19 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index a1737556a77eb..e33c90c44697e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -508,6 +508,8 @@ struct amdgpu_fpriv {
 
 	/** GPU partition selection */
 	uint32_t		xcp_id;
+	struct mutex		init_lock;
+	bool			initialized;
 };
 
 int amdgpu_file_to_fpriv(struct file *filp, struct amdgpu_fpriv **fpriv);
@@ -1617,6 +1619,8 @@ extern const int amdgpu_max_kms_ioctl;
 int amdgpu_driver_load_kms(struct amdgpu_device *adev, unsigned long flags);
 void amdgpu_driver_unload_kms(struct drm_device *dev);
 int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv);
+int amdgpu_driver_init_fpriv(struct drm_device *dev, struct drm_file *file_priv,
+			     struct amdgpu_fpriv *fpriv);
 void amdgpu_driver_postclose_kms(struct drm_device *dev,
 				 struct drm_file *file_priv);
 void amdgpu_driver_release_kms(struct drm_device *dev);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index 0e6e2e2acf5b5..2b6cb3b1ca276 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1784,6 +1784,9 @@ static int amdgpu_debugfs_vm_info_show(struct seq_file *m, void *unused)
 		struct amdgpu_vm *vm = &fpriv->vm;
 		struct amdgpu_task_info *ti;
 
+		if (!fpriv->initialized)
+			continue;
+
 		ti = amdgpu_vm_get_task_info_vm(vm);
 		if (ti) {
 			seq_printf(m, "pid:%d\tProcess:%s ----------\n", ti->task.pid, ti->process_name);
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 34b9d63a86227..732f398922da5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2967,6 +2967,7 @@ long amdgpu_drm_ioctl(struct file *filp,
 		      unsigned int cmd, unsigned long arg)
 {
 	struct drm_file *file_priv = filp->private_data;
+	struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
 	struct drm_device *dev;
 	long ret;
 	bool wake = true;
@@ -2984,10 +2985,17 @@ long amdgpu_drm_ioctl(struct file *filp,
 		ret = pm_runtime_get_sync(dev->dev);
 		if (ret < 0)
 			goto out;
+
+		if (unlikely(!fpriv->initialized)) {
+			ret = amdgpu_driver_init_fpriv(dev, file_priv, fpriv);
+			if (ret < 0)
+				goto out_suspend;
+		}
 	}
 
 	ret = drm_ioctl(filp, cmd, arg);
 
+out_suspend:
 	if (wake)
 		pm_runtime_mark_last_busy(dev->dev);
 out:
@@ -3017,6 +3025,9 @@ static int amdgpu_flush(struct file *f, fl_owner_t id)
 	struct amdgpu_fpriv *fpriv = file_priv->driver_priv;
 	long timeout = MAX_WAIT_SCHED_ENTITY_Q_EMPTY;
 
+	if (!fpriv->initialized)
+		return 0;
+
 	timeout = amdgpu_ctx_mgr_entity_flush(&fpriv->ctx_mgr, timeout);
 	timeout = amdgpu_vm_wait_idle(&fpriv->vm, timeout);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
index 91d638098889d..e13024bcd8bd1 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fdinfo.c
@@ -73,6 +73,9 @@ void amdgpu_show_fdinfo(struct drm_printer *p, struct drm_file *file)
 	};
 	unsigned int hw_ip, i;
 
+	if (!fpriv->initialized)
+		return;
+
 	amdgpu_vm_get_memory(vm, stats);
 	amdgpu_ctx_mgr_usage(&fpriv->ctx_mgr, usage);
 
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 8a76960803c65..60f36e03def2a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -1382,7 +1382,7 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 {
 	struct amdgpu_device *adev = drm_to_adev(dev);
 	struct amdgpu_fpriv *fpriv;
-	int r, pasid;
+	int r;
 
 	/* Ensure IB tests are run on ring */
 	flush_delayed_work(&adev->delayed_init_work);
@@ -1395,16 +1395,45 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 
 	file_priv->driver_priv = NULL;
 
-	r = pm_runtime_get_sync(dev->dev);
+	fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
+	if (unlikely(!fpriv))
+		return -ENOMEM;
+
+	mutex_init(&fpriv->init_lock);
+
+	r = pm_runtime_get_if_active(dev->dev);
 	if (r < 0)
-		goto pm_put;
+		goto error_free;
 
-	fpriv = kzalloc(sizeof(*fpriv), GFP_KERNEL);
-	if (unlikely(!fpriv)) {
-		r = -ENOMEM;
-		goto out_suspend;
+	if (r == 1) {
+		r = amdgpu_driver_init_fpriv(dev, file_priv, fpriv);
+
+		pm_runtime_mark_last_busy(dev->dev);
+		pm_runtime_put_autosuspend(dev->dev);
+
+		if (r < 0)
+			goto error_free;
 	}
 
+	file_priv->driver_priv = fpriv;
+	return 0;
+
+error_free:
+	kfree(fpriv);
+
+	return r;
+}
+
+int amdgpu_driver_init_fpriv(struct drm_device *dev, struct drm_file *file_priv,
+			     struct amdgpu_fpriv *fpriv)
+{
+	struct amdgpu_device *adev = drm_to_adev(dev);
+	int r, pasid;
+
+	mutex_lock(&fpriv->init_lock);
+	if (fpriv->initialized)
+		goto out_unlock;
+
 	pasid = amdgpu_pasid_alloc(16);
 	if (pasid < 0) {
 		dev_warn(adev->dev, "No more PASIDs available!");
@@ -1457,8 +1486,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 
 	amdgpu_ctx_mgr_init(&fpriv->ctx_mgr, adev);
 
-	file_priv->driver_priv = fpriv;
-	goto out_suspend;
+	fpriv->initialized = true;
+	goto out_unlock;
 
 error_vm:
 	amdgpu_vm_fini(adev, &fpriv->vm);
@@ -1469,13 +1498,8 @@ int amdgpu_driver_open_kms(struct drm_device *dev, struct drm_file *file_priv)
 		amdgpu_vm_set_pasid(adev, &fpriv->vm, 0);
 	}
 
-	kfree(fpriv);
-
-out_suspend:
-	pm_runtime_mark_last_busy(dev->dev);
-pm_put:
-	pm_runtime_put_autosuspend(dev->dev);
-
+out_unlock:
+	mutex_unlock(&fpriv->init_lock);
 	return r;
 }
 
@@ -1500,6 +1524,9 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
 	if (!fpriv)
 		return;
 
+	if (!fpriv->initialized)
+		goto out_free;
+
 	pm_runtime_get_sync(dev->dev);
 
 	if (amdgpu_device_ip_get_ip_block(adev, AMD_IP_BLOCK_TYPE_UVD) != NULL)
@@ -1537,11 +1564,12 @@ void amdgpu_driver_postclose_kms(struct drm_device *dev,
 	idr_destroy(&fpriv->bo_list_handles);
 	mutex_destroy(&fpriv->bo_list_lock);
 
-	kfree(fpriv);
-	file_priv->driver_priv = NULL;
-
 	pm_runtime_mark_last_busy(dev->dev);
 	pm_runtime_put_autosuspend(dev->dev);
+
+out_free:
+	kfree(fpriv);
+	file_priv->driver_priv = NULL;
 }
 
 

-- 
2.50.1

[PATCH RFC 3/6] drm/amdgpu: don't query xclk in AMDGPU_INFO_DEV_INFO

[Philipp Zabel]

Cache the xclk rate during amdgpu_device_init() and return the cached
value in the AMDGPU_INFO_DEV_INFO query.
This will allow to avoid waking up the GPU for this query later.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu.h        | 1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c | 2 ++
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c    | 2 +-
 3 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index e33c90c44697e..35827a0cca780 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -431,6 +431,7 @@ struct amdgpu_clock {
 	struct amdgpu_pll spll;
 	struct amdgpu_pll mpll;
 	/* 10 Khz units */
+	u32 xclk;
 	uint32_t default_mclk;
 	uint32_t default_sclk;
 	uint32_t default_dispclk;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index 79d0ff0bda290..44a8fad60512e 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -4818,6 +4818,8 @@ int amdgpu_device_init(struct amdgpu_device *adev,
 	if (r)
 		goto failed;
 
+	adev->clock.xclk = amdgpu_asic_get_xclk(adev);
+
 	return 0;
 
 release_ras_con:
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 60f36e03def2a..190602bacbcdf 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -911,7 +911,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		dev_info->num_shader_engines = adev->gfx.config.max_shader_engines;
 		dev_info->num_shader_arrays_per_engine = adev->gfx.config.max_sh_per_se;
 		/* return all clocks in KHz */
-		dev_info->gpu_counter_freq = amdgpu_asic_get_xclk(adev) * 10;
+		dev_info->gpu_counter_freq = adev->clock.xclk * 10;
 		if (adev->pm.dpm_enabled) {
 			dev_info->max_engine_clock = amdgpu_dpm_get_sclk(adev, false) * 10;
 			dev_info->max_memory_clock = amdgpu_dpm_get_mclk(adev, false) * 10;

-- 
2.50.1

[PATCH RFC 4/6] drm/amdgpu: don't wake up the GPU for some AMDGPU_INFO queries

[Philipp Zabel]

Don't wake the GPU if the AMDGPU_INFO query doesn't need to power up the
GPU.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c |  1 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 59 +++++++++++++++++++++++++++------
 2 files changed, 49 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 732f398922da5..79d31ac6a7b37 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2961,6 +2961,7 @@ static const unsigned int no_wake_ioctl_list[] = {
 	DRM_IOCTL_MODE_LIST_LESSEES,
 	DRM_IOCTL_MODE_GET_LEASE,
 	DRM_IOCTL_MODE_REVOKE_LEASE,
+	DRM_IOCTL_AMDGPU_INFO,
 };
 
 long amdgpu_drm_ioctl(struct file *filp,
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index 190602bacbcdf..fe1347a4075c4 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -732,7 +732,17 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		return copy_to_user(out, &count, min(size, 4u)) ? -EFAULT : 0;
 	}
 	case AMDGPU_INFO_TIMESTAMP:
+		ret = pm_runtime_get_sync(dev->dev);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(dev->dev);
+			return ret;
+		}
+
 		ui64 = amdgpu_gfx_get_gpu_clock_counter(adev);
+
+		pm_runtime_mark_last_busy(dev->dev);
+		pm_runtime_put_autosuspend(dev->dev);
+
 		return copy_to_user(out, &ui64, min(size, 8u)) ? -EFAULT : 0;
 	case AMDGPU_INFO_FW_VERSION: {
 		struct drm_amdgpu_info_firmware fw_info;
@@ -873,6 +883,12 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 
 		alloc_size = info->read_mmr_reg.count * sizeof(*regs);
 
+		ret = pm_runtime_get_sync(dev->dev);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(dev->dev);
+			goto out;
+		}
+
 		amdgpu_gfx_off_ctrl(adev, false);
 		for (i = 0; i < info->read_mmr_reg.count; i++) {
 			if (amdgpu_asic_read_register(adev, se_num, sh_num,
@@ -882,11 +898,16 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 					      info->read_mmr_reg.dword_offset + i);
 				kfree(regs);
 				amdgpu_gfx_off_ctrl(adev, true);
+				pm_runtime_mark_last_busy(dev->dev);
+				pm_runtime_put_autosuspend(dev->dev);
 				ret = -EFAULT;
 				goto out;
 			}
 		}
 		amdgpu_gfx_off_ctrl(adev, true);
+		pm_runtime_mark_last_busy(dev->dev);
+		pm_runtime_put_autosuspend(dev->dev);
+
 		n = copy_to_user(out, regs, min(size, alloc_size));
 		kfree(regs);
 		ret = (n ? -EFAULT : 0);
@@ -912,7 +933,8 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		dev_info->num_shader_arrays_per_engine = adev->gfx.config.max_sh_per_se;
 		/* return all clocks in KHz */
 		dev_info->gpu_counter_freq = adev->clock.xclk * 10;
-		if (adev->pm.dpm_enabled) {
+		ret = pm_runtime_get_if_active(dev->dev);
+		if (ret == 1 && adev->pm.dpm_enabled) {
 			dev_info->max_engine_clock = amdgpu_dpm_get_sclk(adev, false) * 10;
 			dev_info->max_memory_clock = amdgpu_dpm_get_mclk(adev, false) * 10;
 			dev_info->min_engine_clock = amdgpu_dpm_get_sclk(adev, true) * 10;
@@ -925,6 +947,10 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 				dev_info->min_memory_clock =
 					adev->clock.default_mclk * 10;
 		}
+		if (ret == 1) {
+			pm_runtime_mark_last_busy(dev->dev);
+			pm_runtime_put_autosuspend(dev->dev);
+		}
 		dev_info->enabled_rb_pipes_mask = adev->gfx.config.backend_enable_mask;
 		dev_info->num_rb_pipes = adev->gfx.config.max_backends_per_se *
 			adev->gfx.config.max_shader_engines;
@@ -1125,13 +1151,19 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		if (!adev->pm.dpm_enabled)
 			return -ENOENT;
 
+		ret = pm_runtime_get_sync(dev->dev);
+		if (ret < 0) {
+			pm_runtime_put_autosuspend(dev->dev);
+			return ret;
+		}
+
 		switch (info->sensor_info.type) {
 		case AMDGPU_INFO_SENSOR_GFX_SCLK:
 			/* get sclk in Mhz */
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_GFX_SCLK,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			ui32 /= 100;
 			break;
@@ -1140,7 +1172,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_GFX_MCLK,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			ui32 /= 100;
 			break;
@@ -1149,7 +1181,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_GPU_TEMP,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			break;
 		case AMDGPU_INFO_SENSOR_GPU_LOAD:
@@ -1157,7 +1189,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_GPU_LOAD,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			break;
 		case AMDGPU_INFO_SENSOR_GPU_AVG_POWER:
@@ -1169,7 +1201,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 				if (amdgpu_dpm_read_sensor(adev,
 							   AMDGPU_PP_SENSOR_GPU_INPUT_POWER,
 							   (void *)&ui32, &ui32_size)) {
-					return -EINVAL;
+					ret = -EINVAL;
 				}
 			}
 			ui32 >>= 8;
@@ -1188,7 +1220,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_VDDNB,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			break;
 		case AMDGPU_INFO_SENSOR_VDDGFX:
@@ -1196,7 +1228,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_VDDGFX,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			break;
 		case AMDGPU_INFO_SENSOR_STABLE_PSTATE_GFX_SCLK:
@@ -1204,7 +1236,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_STABLE_PSTATE_SCLK,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			ui32 /= 100;
 			break;
@@ -1213,7 +1245,7 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 			if (amdgpu_dpm_read_sensor(adev,
 						   AMDGPU_PP_SENSOR_STABLE_PSTATE_MCLK,
 						   (void *)&ui32, &ui32_size)) {
-				return -EINVAL;
+				ret = -EINVAL;
 			}
 			ui32 /= 100;
 			break;
@@ -1238,8 +1270,13 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 		default:
 			DRM_DEBUG_KMS("Invalid request %d\n",
 				      info->sensor_info.type);
-			return -EINVAL;
+			ret = -EINVAL;
+			break;
 		}
+		pm_runtime_mark_last_busy(dev->dev);
+		pm_runtime_put_autosuspend(dev->dev);
+		if (ret < 0)
+			return ret;
 		return copy_to_user(out, &ui32, min(size, 4u)) ? -EFAULT : 0;
 	}
 	case AMDGPU_INFO_VRAM_LOST_COUNTER:

-- 
2.50.1

[PATCH RFC 5/6] drm/amdgpu: don't wake up the GPU for mmGB_ADDR_CONFIG register read

[Philipp Zabel]

Don't wake the GPU if libdrm queries the mmGB_ADDR_CONFIG register
value during amdgpu_query_gpu_info_init(). Instead, return the already
cached value adev->gfx.config.gb_addr_config.

Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
index fe1347a4075c4..ed4d7d72f2065 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
@@ -883,6 +883,16 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
 
 		alloc_size = info->read_mmr_reg.count * sizeof(*regs);
 
+		if (info->read_mmr_reg.dword_offset == 0x263e &&
+		    info->read_mmr_reg.count == 1) {
+			/* Return cached value of mmGB_ADDR_CONFIG */
+			regs[0] = adev->gfx.config.gb_addr_config;
+
+			n = copy_to_user(out, regs, min(size, alloc_size));
+			kfree(regs);
+			return n ? -EFAULT : 0;
+		}
+
 		ret = pm_runtime_get_sync(dev->dev);
 		if (ret < 0) {
 			pm_runtime_put_autosuspend(dev->dev);

-- 
2.50.1

Re: [PATCH RFC 5/6] drm/amdgpu: don't wake up the GPU for mmGB_ADDR_CONFIG register read

[Alex Deucher]

On Thu, Jul 31, 2025 at 3:33 AM Philipp Zabel <philipp.zabel@gmail.com> wrote:
>
> Don't wake the GPU if libdrm queries the mmGB_ADDR_CONFIG register
> value during amdgpu_query_gpu_info_init(). Instead, return the already
> cached value adev->gfx.config.gb_addr_config.
>
> Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
> Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> index fe1347a4075c4..ed4d7d72f2065 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
> @@ -883,6 +883,16 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>
>                 alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>
> +               if (info->read_mmr_reg.dword_offset == 0x263e &&

I think the offset of this register varies across chip families.
You'll need some way to determine what the offset is for each family.

Alex

> +                   info->read_mmr_reg.count == 1) {
> +                       /* Return cached value of mmGB_ADDR_CONFIG */
> +                       regs[0] = adev->gfx.config.gb_addr_config;
> +
> +                       n = copy_to_user(out, regs, min(size, alloc_size));
> +                       kfree(regs);
> +                       return n ? -EFAULT : 0;
> +               }
> +
>                 ret = pm_runtime_get_sync(dev->dev);
>                 if (ret < 0) {
>                         pm_runtime_put_autosuspend(dev->dev);
>
> --
> 2.50.1
>

Re: [PATCH RFC 5/6] drm/amdgpu: don't wake up the GPU for mmGB_ADDR_CONFIG register read

[Alex Deucher]

On Fri, Aug 1, 2025 at 2:11 AM Philipp Zabel <philipp.zabel@gmail.com> wrote:
>
> On Thu, Jul 31, 2025 at 9:38 PM Alex Deucher <alexdeucher@gmail.com> wrote:
>>
>> On Thu, Jul 31, 2025 at 3:33 AM Philipp Zabel <philipp.zabel@gmail.com> wrote:
>> >
>> > Don't wake the GPU if libdrm queries the mmGB_ADDR_CONFIG register
>> > value during amdgpu_query_gpu_info_init(). Instead, return the already
>> > cached value adev->gfx.config.gb_addr_config.
>> >
>> > Link: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
>> > Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
>> > ---
>> >  drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 10 ++++++++++
>> >  1 file changed, 10 insertions(+)
>> >
>> > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > index fe1347a4075c4..ed4d7d72f2065 100644
>> > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c
>> > @@ -883,6 +883,16 @@ int amdgpu_info_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>> >
>> >                 alloc_size = info->read_mmr_reg.count * sizeof(*regs);
>> >
>> > +               if (info->read_mmr_reg.dword_offset == 0x263e &&
>>
>> I think the offset of this register varies across chip families.
>> You'll need some way to determine what the offset is for each family.
>
>
> Thank you. This workaround was specifically intended for the following call in libdrm [1]:
>
>         r = amdgpu_read_mm_registers(dev, 0x263e, 1, 0xffffffff, 0,
>                                              &dev->info.gb_addr_cfg);
>
> [1] https://gitlab.freedesktop.org/mesa/libdrm/-/blob/9ea8a8e93d542fe61d82716d1a721e8d1d257405/amdgpu/amdgpu_gpu_info.c#L215-216
>
> which also seem to hard-code the dword_offset?
>
> The same is now copied into Mesa [2] as:
>
>    r = ac_drm_read_mm_registers(dev, 0x263e, 1, 0xffffffff, 0, &info->gb_addr_cfg);
>
> [2] https://gitlab.freedesktop.org/mesa/mesa/-/blob/c64c6a0c31f9cb1339bc700d236932171f7444a3/src/amd/common/ac_linux_drm.c#L722

Nevermind, it's at the same absolute offset on all chips right now.
The relative offsets are just different.

Alex

[PATCH RFC 6/6] drm/amdgpu: don't wake up the GPU for syncobj feature detection

[Philipp Zabel]

Don't wake the GPU if the SYNCOBJ_CREATE/DESTROY/WAIT ioctls are used
to detect syncobj features before the GPU is powered up.

Closes: https://gitlab.freedesktop.org/drm/amd/-/issues/2295
Signed-off-by: Philipp Zabel <philipp.zabel@gmail.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 79d31ac6a7b37..b12342e917193 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2981,6 +2981,14 @@ long amdgpu_drm_ioctl(struct file *filp,
 		}
 	}
 
+	if (wake && unlikely(!fpriv->initialized)) {
+		/* Allow syncobj feature detection before GPU wakeup */
+		if (cmd == DRM_IOCTL_SYNCOBJ_CREATE ||
+		    cmd == DRM_IOCTL_SYNCOBJ_DESTROY ||
+		    cmd == DRM_IOCTL_SYNCOBJ_WAIT)
+			wake = false;
+	}
+
 	dev = file_priv->minor->dev;
 	if (wake) {
 		ret = pm_runtime_get_sync(dev->dev);

-- 
2.50.1

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

[Christian König]

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>
> 

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

[Philipp Zabel]

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

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

[Christian König]

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

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

[Pierre-Eric Pelloux-Prayer]

[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