On 7/23/2025 1:43 AM, Rob Clark wrote: > On Tue, Jul 22, 2025 at 12:23 PM Akhil P Oommen > <akhi...@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.barysh...@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 <akhi...@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 <robdcl...@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 >>
