Message-ID: <17afba8b-dae5-d724-4c8c-8b4c79fcfb84@codeaurora.org>
Date:   Sat, 11 Jul 2020 02:33:40 +0530
From:   Akhil P Oommen <akhilpo@...eaurora.org>
To:     Rob Clark <robdclark@...il.com>
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 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 


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