When IFPC is supported, devfreq idling is redundant and adds
unnecessary pm suspend/wake latency. So skip it when IFPC is
supported.
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.1On 7/20/25 2:16 PM, 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. > > 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; Do we want a static_branch here instead? Konrad
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? > > 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
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. 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
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. -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
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. 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 >
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 >>
Hi Akhil,
kernel test robot noticed the following build errors:
[auto build test ERROR on 88bf743cabe5793d24f831ef8240a0bf90e5fd44]
url: https://github.com/intel-lab-lkp/linux/commits/Akhil-P-Oommen/drm-msm-Update-GMU-register-xml/20250720-201844
base: 88bf743cabe5793d24f831ef8240a0bf90e5fd44
patch link: https://lore.kernel.org/r/20250720-ifpc-support-v1-12-9347aa5bcbd6%40oss.qualcomm.com
patch subject: [PATCH 12/17] drm/msm: Skip devfreq IDLE when possible
config: sparc-allmodconfig (https://download.01.org/0day-ci/archive/20250721/202507211133.d0ChrtTQ-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 15.1.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250721/202507211133.d0ChrtTQ-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202507211133.d0ChrtTQ-lkp@intel.com/
All errors (new ones prefixed by >>):
In file included from drivers/gpu/drm/msm/msm_gpu_devfreq.c:7:
>> drivers/gpu/drm/msm/adreno/adreno_gpu.h:17:10: fatal error: adreno_common.xml.h: No such file or directory
17 | #include "adreno_common.xml.h"
| ^~~~~~~~~~~~~~~~~~~~~
compilation terminated.
vim +17 drivers/gpu/drm/msm/adreno/adreno_gpu.h
7198e6b03155f6 Rob Clark 2013-07-19 16
7198e6b03155f6 Rob Clark 2013-07-19 @17 #include "adreno_common.xml.h"
7198e6b03155f6 Rob Clark 2013-07-19 18 #include "adreno_pm4.xml.h"
7198e6b03155f6 Rob Clark 2013-07-19 19
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
© 2016 - 2025 Red Hat, Inc.