Re: [PATCH 12/17] drm/msm: Skip devfreq IDLE when possible

[Akhil P Oommen <akhilpo@oss.qualcomm.com>]

On 7/23/2025 1:43 AM, Rob Clark wrote:
> On Tue, Jul 22, 2025 at 12:23 PM Akhil P Oommen
> <akhilpo@oss.qualcomm.com> wrote:
>>
>> On 7/22/2025 9:08 PM, Rob Clark wrote:
>>> On Tue, Jul 22, 2025 at 6:50 AM Dmitry Baryshkov
>>> <dmitry.baryshkov@oss.qualcomm.com> wrote:
>>>>
>>>> On Sun, Jul 20, 2025 at 05:46:13PM +0530, Akhil P Oommen wrote:
>>>>> When IFPC is supported, devfreq idling is redundant and adds
>>>>> unnecessary pm suspend/wake latency. So skip it when IFPC is
>>>>> supported.
>>>>
>>>> With this in place we have a dummy devfreq instance which does nothing.
>>>> Wouldn't it be better to skip registering devfreq if IFPC is supported
>>>> on the platform?
>>>
>>> devfreq is still scaling the freq.  What is being bypassed is
>>> essentially a CPU based version of IFPC (because on sc7180 we didn't
>>> have IFPC
>>>
>>> Currently only a618 and 7c3 enable gpu_clamp_to_idle.. if at some
>>> point those grew IFPC support we could remove the trickery to drop GPU
>>> to min freq when it is idle altogether.
>>
>> There are 2 functionalities here:
>> 1. Clamp-to-idle: enabled only on a618/7c3
>> 2. boost-after-idle: Enabled for all GPUs.
>>
>> With this patch, we are skipping both when IFPC is supported. In the
>> absence of latency due to clamp-to-idle, do you think a618 (relatively
>> weaker GPU) would require boost-after-idle to keep with the
>> UI/composition workload for its typical configuration (1080p@60hz)? If
>> yes, I should probably create a quirk to disable boost-after-idle for
>> premium tier GPUs, instead of tying it to IFPC feature.
> 
> Hmm, yeah.. I suppose _this_ patch should only skip clamp-to-idle.  It
> is a different topic, boost-after-idle.

I think we can just drop this patch. Also, Boost-after-idle is not super
bad as it is only doubling the gpu freq. We can try to optimize it
separately.

-Akhil.

> 
> BR,
> -R
> 
>> -Akhil.
>>
>>>
>>> BR,
>>> -R
>>>
>>>>>
>>>>> Signed-off-by: Akhil P Oommen <akhilpo@oss.qualcomm.com>
>>>>> ---
>>>>>  drivers/gpu/drm/msm/msm_gpu_devfreq.c | 6 ++++++
>>>>>  1 file changed, 6 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
>>>>> index 2e1d5c3432728cde15d91f69da22bb915588fe86..53ef2add5047e7d6b6371af949cab36ce8409372 100644
>>>>> --- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
>>>>> +++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
>>>>> @@ -4,6 +4,7 @@
>>>>>   * Author: Rob Clark <robdclark@gmail.com>
>>>>>   */
>>>>>
>>>>> +#include "adreno/adreno_gpu.h"
>>>>>  #include "msm_gpu.h"
>>>>>  #include "msm_gpu_trace.h"
>>>>>
>>>>> @@ -300,6 +301,8 @@ void msm_devfreq_active(struct msm_gpu *gpu)
>>>>>       if (!has_devfreq(gpu))
>>>>>               return;
>>>>>
>>>>> +     if (to_adreno_gpu(gpu)->info->quirks & ADRENO_QUIRK_IFPC)
>>>>> +             return;
>>>>>       /*
>>>>>        * Cancel any pending transition to idle frequency:
>>>>>        */
>>>>> @@ -370,6 +373,9 @@ void msm_devfreq_idle(struct msm_gpu *gpu)
>>>>>       if (!has_devfreq(gpu))
>>>>>               return;
>>>>>
>>>>> +     if (to_adreno_gpu(gpu)->info->quirks & ADRENO_QUIRK_IFPC)
>>>>> +             return;
>>>>> +
>>>>>       msm_hrtimer_queue_work(&df->idle_work, ms_to_ktime(1),
>>>>>                              HRTIMER_MODE_REL);
>>>>>  }
>>>>>
>>>>> --
>>>>> 2.50.1
>>>>>
>>>>
>>>> --
>>>> With best wishes
>>>> Dmitry
>>