[PATCH 1/6] drm/amdgpu: allow ioctls to opt-out of runtime pm

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

Pierre-Eric




> 
> Regards,
> Christian.
> 
>>
>> Signed-off-by: Pierre-Eric Pelloux-Prayer 
>> <pierre-eric.pelloux-prayer at amd.com>
>> ---
>>   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;
>>   }