Re: [Freedreno] [PATCH v4 4/7] drm: msm: a6xx: use dev_pm_opp_set_bw to scale DDR

[Akhil P Oommen]

On 7/11/2020 1:11 AM, Rob Clark wrote:
> On Thu, Jul 9, 2020 at 1:01 PM Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx> wrote:
> > From: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
> >
> > This patches replaces the previously used static DDR vote and uses
> > dev_pm_opp_set_bw() to scale GPU->DDR bandwidth along with scaling
> > GPU frequency. Also since the icc path voting is handled completely
> > in the opp driver, remove the icc_path handle and its usage in the
> > drm driver.
> >
> > Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
> > Signed-off-by: Akhil P Oommen <akhilpo@xxxxxxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 25 +++++++++++++++++--------
> >   1 file changed, 17 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index b547339..6fbfd7d 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -123,7 +123,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
> >
> >          if (!gmu->legacy) {
> >                  a6xx_hfi_set_freq(gmu, gmu->current_perf_index);
> > -               icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> > +               dev_pm_opp_set_bw(&gpu->pdev->dev, opp);
> What is the status of the patch to add dev_pm_opp_set_bw()?  If it is
> ready to go, and I get an ack-by from the OPP maintainer, I suppose I
> could merge it via drm/msm.
>
> Otherwise should we consider pulling in a private copy of it into
> drm/msm (and then drop it to use the helper in, hopefully, the next
> cycle)?
>
> I'm pulling the patches preceding this one into msm-next-staging to do
> some testing.  And the dt patches following this one would normally
> get merged via Bjorn.  At the moment, I'm not sure what to do with
> this one.
>
> BR,
> -R
I see Sibi's patch is already picked in opp/linux-next branch.
https://kernel.googlesource.com/pub/scm/linux/kernel/git/vireshk/pm/+/b466542f331e221a3628c1cfe5ccff307d7d787f 


Thanks,
-Akhil

> >                  return;
> >          }
> >
> > @@ -149,11 +149,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
> >          if (ret)
> >                  dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
> >
> > -       /*
> > -        * Eventually we will want to scale the path vote with the frequency but
> > -        * for now leave it at max so that the performance is nominal.
> > -        */
> > -       icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> > +       dev_pm_opp_set_bw(&gpu->pdev->dev, opp);
> >   }
> >
> >   unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
> > @@ -840,6 +836,19 @@ static void a6xx_gmu_set_initial_freq(struct msm_gpu *gpu, struct a6xx_gmu *gmu)
> >          dev_pm_opp_put(gpu_opp);
> >   }
> >
> > +static void a6xx_gmu_set_initial_bw(struct msm_gpu *gpu, struct a6xx_gmu *gmu)
> > +{
> > +       struct dev_pm_opp *gpu_opp;
> > +       unsigned long gpu_freq = gmu->gpu_freqs[gmu->current_perf_index];
> > +
> > +       gpu_opp = dev_pm_opp_find_freq_exact(&gpu->pdev->dev, gpu_freq, true);
> > +       if (IS_ERR_OR_NULL(gpu_opp))
> > +               return;
> > +
> > +       dev_pm_opp_set_bw(&gpu->pdev->dev, gpu_opp);
> > +       dev_pm_opp_put(gpu_opp);
> > +}
> > +
> >   int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
> >   {
> >          struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> > @@ -864,7 +873,7 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
> >          }
> >
> >          /* Set the bus quota to a reasonable value for boot */
> > -       icc_set_bw(gpu->icc_path, 0, MBps_to_icc(3072));
> > +       a6xx_gmu_set_initial_bw(gpu, gmu);
> >
> >          /* Enable the GMU interrupt */
> >          gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0);
> > @@ -1040,7 +1049,7 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
> >                  a6xx_gmu_shutdown(gmu);
> >
> >          /* Remove the bus vote */
> > -       icc_set_bw(gpu->icc_path, 0, 0);
> > +       dev_pm_opp_set_bw(&gpu->pdev->dev, NULL);
> >
> >          /*
> >           * Make sure the GX domain is off before turning off the GMU (CX)
> > --
> > 2.7.4
> >
> > _______________________________________________
> > Freedreno mailing list
> > Freedreno@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.freedesktop.org/mailman/listinfo/freedreno