Le 20/06/2024 à 15:36, Christian König a écrit :
Am 20.06.24 um 15:06 schrieb Pierre-Eric Pelloux-Prayer:
Le 19/06/2024 à 11:26, Christian König a écrit :
Am 18.06.24 um 17:23 schrieb Pierre-Eric Pelloux-Prayer:
Waking up a device can take multiple seconds, so if it's not
going to be used we might as well not resume it.
The safest default behavior for all ioctls is to resume the GPU,
so this change allows specific ioctls to opt-out of generic
runtime pm.
I'm really wondering if we shouldn't put that into the IOCTL
description.
See amdgpu_ioctls_kms and DRM_IOCTL_DEF_DRV() for what I mean.
Are you suggesting to add a new entry in enum drm_ioctl_flags to
indicate ioctls which need the device to be awake?
Something like: "DRM_NO_DEVICE = BIT(6)" and then use it for both
core and amdgpu ioctls?
Yeah something like that. Maybe name that DRM_SW_ONLY or something like
that.
+ dri-devel to gauge interest in adding such a flag in shared code.
Pierre-Eric
Christian.
Pierre-Eric
Regards,
Christian.
Signed-off-by: Pierre-Eric Pelloux-Prayer
<pierre-eric.pelloux-prayer@xxxxxxx>
---
drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c | 25
++++++++++++++++++++-----
1 file changed, 20 insertions(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
index 60d5758939ae..a9831b243bfc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_drv.c
@@ -2855,18 +2855,33 @@ long amdgpu_drm_ioctl(struct file *filp,
{
struct drm_file *file_priv = filp->private_data;
struct drm_device *dev;
+ bool needs_device;
long ret;
dev = file_priv->minor->dev;
- ret = pm_runtime_get_sync(dev->dev);
- if (ret < 0)
- goto out;
+
+ /* Some ioctl can opt-out of powermanagement handling
+ * if they don't require the device to be resumed.
+ */
+ switch (cmd) {
+ default:
+ needs_device = true;
+ }
+
+ if (needs_device) {
+ 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);
out:
- pm_runtime_put_autosuspend(dev->dev);
+ if (needs_device) {
+ pm_runtime_mark_last_busy(dev->dev);
+ pm_runtime_put_autosuspend(dev->dev);
+ }
+
return ret;
}
