Re: [PATCH v3 2/6] drm: msm: a6xx: send opp instead of a frequency

[Matthias Kaehlcke]

Hi,

On Thu, Jun 18, 2020 at 10:52:09AM -0700, Rob Clark wrote:
> On Fri, Jun 5, 2020 at 9:26 PM Sharat Masetty <smasetty@xxxxxxxxxxxxxx> wrote:
> >
> > This patch changes the plumbing to send the devfreq recommended opp rather
> > than the frequency. Also consolidate and rearrange the code in a6xx to set
> > the GPU frequency and the icc vote in preparation for the upcoming
> > changes for GPU->DDR scaling votes.
> >
> > Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
> > ---
> >  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 62 +++++++++++++++++++----------------
> >  drivers/gpu/drm/msm/adreno/a6xx_gpu.h |  2 +-
> >  drivers/gpu/drm/msm/msm_gpu.c         |  3 +-
> >  drivers/gpu/drm/msm/msm_gpu.h         |  3 +-
> >  4 files changed, 38 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > index 748cd37..2d8124b 100644
> > --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
> > @@ -100,17 +100,30 @@ bool a6xx_gmu_gx_is_on(struct a6xx_gmu *gmu)
> >                 A6XX_GMU_SPTPRAC_PWR_CLK_STATUS_GX_HM_CLK_OFF));
> >  }
> >
> > -static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
> > +void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)
> >  {
> > -       struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
> > -       struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
> > -       struct msm_gpu *gpu = &adreno_gpu->base;
> > -       int ret;
> > +       struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> > +       struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> > +       struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
> > +       u32 perf_index;
> > +       unsigned long gpu_freq;
> > +       int ret = 0;
> > +
> > +       gpu_freq = dev_pm_opp_get_freq(opp);
> > +
> > +       if (gpu_freq == gmu->freq)
> > +               return;
> > +
> > +       for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
> > +               if (gpu_freq == gmu->gpu_freqs[perf_index])
> > +                       break;
> > +
> > +       gmu->current_perf_index = perf_index;
> >
> >         gmu_write(gmu, REG_A6XX_GMU_DCVS_ACK_OPTION, 0);
> >
> >         gmu_write(gmu, REG_A6XX_GMU_DCVS_PERF_SETTING,
> > -               ((3 & 0xf) << 28) | index);
> > +                       ((3 & 0xf) << 28) | perf_index);
> >
> >         /*
> >          * Send an invalid index as a vote for the bus bandwidth and let the
> > @@ -126,7 +139,7 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
> >         if (ret)
> >                 dev_err(gmu->dev, "GMU set GPU frequency error: %d\n", ret);
> >
> > -       gmu->freq = gmu->gpu_freqs[index];
> > +       gmu->freq = gmu->gpu_freqs[perf_index];
> >
> >         /*
> >          * Eventually we will want to scale the path vote with the frequency but
> > @@ -135,25 +148,6 @@ static void __a6xx_gmu_set_freq(struct a6xx_gmu *gmu, int index)
> >         icc_set_bw(gpu->icc_path, 0, MBps_to_icc(7216));
> >  }
> >
> > -void a6xx_gmu_set_freq(struct msm_gpu *gpu, unsigned long freq)
> > -{
> > -       struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu);
> > -       struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
> > -       struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
> > -       u32 perf_index = 0;
> > -
> > -       if (freq == gmu->freq)
> > -               return;
> > -
> > -       for (perf_index = 0; perf_index < gmu->nr_gpu_freqs - 1; perf_index++)
> > -               if (freq == gmu->gpu_freqs[perf_index])
> > -                       break;
> > -
> > -       gmu->current_perf_index = perf_index;
> > -
> > -       __a6xx_gmu_set_freq(gmu, perf_index);
> > -}
> 
> this does end up conflicting a bit with some of the newer stuff that
> landed this cycle, in particular "drm/msm/a6xx: HFI v2 for A640 and
> A650"
> 
> Adding Jonathan on CC since I think he will want to test this on
> a650/a640 as well..

Sharat, please send an updated version that is rebased on the latest drm-msm.

Thanks

Matthias