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

[Pierre-Eric Pelloux-Prayer]

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