Message-ID: <CAF6AEGtramec7yTE1mzLO=Vtic63LOgM1pEuejUS3OrT3_vGng@mail.gmail.com>
Date:   Fri, 10 Jul 2020 15:32:48 -0700
From:   Rob Clark <robdclark@...il.com>
To:     Akhil P Oommen <akhilpo@...eaurora.org>
Cc:     freedreno <freedreno@...ts.freedesktop.org>,
        "open list:OPEN FIRMWARE AND FLATTENED DEVICE TREE BINDINGS" 
        <devicetree@...r.kernel.org>, Jonathan <jonathan@...ek.ca>,
        saravanak@...gle.com,
        linux-arm-msm <linux-arm-msm@...r.kernel.org>,
        Sharat Masetty <smasetty@...eaurora.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jordan Crouse <jcrouse@...eaurora.org>,
        Matthias Kaehlcke <mka@...omium.org>,
        dri-devel@...edesktop.org, Viresh Kumar <viresh.kumar@...aro.org>,
        Sibi Sankar <sibis@...eaurora.org>
Subject: Re: [Freedreno] [PATCH v4 4/7] drm: msm: a6xx: use dev_pm_opp_set_bw
 to scale DDR

On Fri, Jul 10, 2020 at 2:03 PM Akhil P Oommen <akhilpo@...eaurora.org> wrote:
>
>
> On 7/11/2020 1:11 AM, Rob Clark wrote:
> > On Thu, Jul 9, 2020 at 1:01 PM Akhil P Oommen <akhilpo@...eaurora.org> wrote:
> >> From: Sharat Masetty <smasetty@...eaurora.org>
> >>
> >> 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@...eaurora.org>
> >> Signed-off-by: Akhil P Oommen <akhilpo@...eaurora.org>
> >> ---
> >>   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
>

ok, I guess we can try and do a 2nd late pull-req for msm-next, after
the opp pull-req lands..

BR,
-R

>
> 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@...ts.freedesktop.org
> >> https://lists.freedesktop.org/mailman/listinfo/freedreno

Powered by blists - more mailing lists