[PATCH] drm/msm/devfreq: Restrict idle clamping to a618 for now

[PATCH] drm/msm/devfreq: Restrict idle clamping to a618 for now

[Rob Clark]

From: Rob Clark <robdclark@chromium.org>

Until we better understand the stability issues caused by frequent
frequency changes, lets limit them to a618.

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
Caleb/John, I think this should help as a workaround for the power
instability issues on a630.. could you give it a try?

 drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 7 +++++++
 drivers/gpu/drm/msm/msm_gpu.h         | 4 ++++
 drivers/gpu/drm/msm/msm_gpu_devfreq.c | 3 ++-
 3 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index 33da25b81615..267a880811d6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -1838,6 +1838,13 @@ struct msm_gpu *a6xx_gpu_init(struct drm_device *dev)
 			adreno_cmp_rev(ADRENO_REV(6, 3, 5, ANY_ID), info->rev)))
 		adreno_gpu->base.hw_apriv = true;
 
+	/*
+	 * For now only clamp to idle freq for devices where this is known not
+	 * to cause power supply issues:
+	 */
+	if (info && (info->revn == 618))
+		gpu->clamp_to_idle = true;
+
 	a6xx_llc_slices_init(pdev, a6xx_gpu);
 
 	ret = a6xx_set_supported_hw(&pdev->dev, config->rev);
diff --git a/drivers/gpu/drm/msm/msm_gpu.h b/drivers/gpu/drm/msm/msm_gpu.h
index 59870095ea41..59cdd00b69d0 100644
--- a/drivers/gpu/drm/msm/msm_gpu.h
+++ b/drivers/gpu/drm/msm/msm_gpu.h
@@ -210,6 +210,10 @@ struct msm_gpu {
 	uint32_t suspend_count;
 
 	struct msm_gpu_state *crashstate;
+
+	/* Enable clamping to idle freq when inactive: */
+	bool clamp_to_idle;
+
 	/* True if the hardware supports expanded apriv (a650 and newer) */
 	bool hw_apriv;
 
diff --git a/drivers/gpu/drm/msm/msm_gpu_devfreq.c b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
index d32b729b4616..8b7473f69cb8 100644
--- a/drivers/gpu/drm/msm/msm_gpu_devfreq.c
+++ b/drivers/gpu/drm/msm/msm_gpu_devfreq.c
@@ -214,7 +214,8 @@ static void msm_devfreq_idle_work(struct kthread_work *work)
 
 	idle_freq = get_freq(gpu);
 
-	msm_devfreq_target(&gpu->pdev->dev, &target_freq, 0);
+	if (gpu->clamp_to_idle)
+		msm_devfreq_target(&gpu->pdev->dev, &target_freq, 0);
 
 	df->idle_time = ktime_get();
 	df->idle_freq = idle_freq;
-- 
2.31.1

Re: [PATCH] drm/msm/devfreq: Restrict idle clamping to a618 for now

[John Stultz]

On Mon, Oct 18, 2021 at 8:31 AM Rob Clark <robdclark@gmail.com> wrote:
>
> From: Rob Clark <robdclark@chromium.org>
>
> Until we better understand the stability issues caused by frequent
> frequency changes, lets limit them to a618.
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>
> ---
> Caleb/John, I think this should help as a workaround for the power
> instability issues on a630.. could you give it a try?

While I hit it fairly often, I can't reliably reproduce the crash, but
in limited testing this seems ok to me.
I've not hit the crash so far, nor seen any other negative side
effects over 5.14.

So for what that's worth:
Tested-by: John Stultz <john.stultz@linaro.org>

Caleb has better luck tripping this issue right away, so they can
hopefully provide a more assured response.

thanks
-john

Re: [PATCH] drm/msm/devfreq: Restrict idle clamping to a618 for now

[Caleb Connolly]

Hi all,

On 18/10/2021 17:42, John Stultz wrote:
> On Mon, Oct 18, 2021 at 8:31 AM Rob Clark <robdclark@gmail.com> wrote:
>>
>> From: Rob Clark <robdclark@chromium.org>
>>
>> Until we better understand the stability issues caused by frequent
>> frequency changes, lets limit them to a618.
>>
>> Signed-off-by: Rob Clark <robdclark@chromium.org>
>> ---
>> Caleb/John, I think this should help as a workaround for the power
>> instability issues on a630.. could you give it a try?
> 
> While I hit it fairly often, I can't reliably reproduce the crash, but
> in limited testing this seems ok to me.
> I've not hit the crash so far, nor seen any other negative side
> effects over 5.14.
> 
> So for what that's worth:
> Tested-by: John Stultz <john.stultz@linaro.org>
> 
> Caleb has better luck tripping this issue right away, so they can
> hopefully provide a more assured response.
This prevents the crash on the OnePlus 6 as the frequency can no longer go to zero.

I would like to find a better solution that still allows proper idling on a630, but that can wait for 5.16.

Tested-by: Caleb Connolly <caleb.connolly@linaro.org>
> 
> thanks
> -john
> 

-- 
Kind Regards,
Caleb (they/them)

Re: [PATCH] drm/msm/devfreq: Restrict idle clamping to a618 for now

[Rob Clark]

On Mon, Oct 18, 2021 at 10:33 AM Caleb Connolly
<caleb.connolly@linaro.org> wrote:
>
> Hi all,
>
> On 18/10/2021 17:42, John Stultz wrote:
> > On Mon, Oct 18, 2021 at 8:31 AM Rob Clark <robdclark@gmail.com> wrote:
> >>
> >> From: Rob Clark <robdclark@chromium.org>
> >>
> >> Until we better understand the stability issues caused by frequent
> >> frequency changes, lets limit them to a618.
> >>
> >> Signed-off-by: Rob Clark <robdclark@chromium.org>
> >> ---
> >> Caleb/John, I think this should help as a workaround for the power
> >> instability issues on a630.. could you give it a try?
> >
> > While I hit it fairly often, I can't reliably reproduce the crash, but
> > in limited testing this seems ok to me.
> > I've not hit the crash so far, nor seen any other negative side
> > effects over 5.14.
> >
> > So for what that's worth:
> > Tested-by: John Stultz <john.stultz@linaro.org>
> >
> > Caleb has better luck tripping this issue right away, so they can
> > hopefully provide a more assured response.
> This prevents the crash on the OnePlus 6 as the frequency can no longer go to zero.
>
> I would like to find a better solution that still allows proper idling on a630, but that can wait for 5.16.
>
> Tested-by: Caleb Connolly <caleb.connolly@linaro.org>

Thanks for testing, I've sent one last -fixes pull request with this patch

BR,
-R

> >
> > thanks
> > -john
> >
>
> --
> Kind Regards,
> Caleb (they/them)