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

[Christian König]

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.

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