Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



+ more folks

On 5/18/2020 9:55 PM, Rob Clark wrote:
On Mon, May 18, 2020 at 7:23 AM Jordan Crouse <jcrouse@xxxxxxxxxxxxxx> wrote:
On Thu, May 14, 2020 at 04:24:18PM +0530, Sharat Masetty wrote:
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.

Signed-off-by: Sharat Masetty <smasetty@xxxxxxxxxxxxxx>
---
  drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 6 +-----
  1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index 2d8124b..79433d3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -141,11 +141,7 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, struct dev_pm_opp *opp)

       gmu->freq = gmu->gpu_freqs[perf_index];

-     /*
-      * 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);
  }
This adds an implicit requirement that all targets need bandwidth settings
defined in the OPP or they won't get a bus vote at all. I would prefer that
there be an default escape valve but if not you'll need to add
bandwidth values for the sdm845 OPP that target doesn't regress.

it looks like we could maybe do something like:

   ret = dev_pm_opp_set_bw(...);
   if (ret) {
       dev_warn_once(dev, "no bandwidth settings");
       icc_set_bw(...);
   }

?

BR,
-R

There is a bit of an issue here - Looks like its not possible to two icc handles to the same path.  Its causing double enumeration of the paths in the icc core and messing up path votes. With [1] Since opp/core already gets a handle to the icc path as part of table add,  drm/msm could do either

a) Conditionally enumerate gpu->icc_path handle only when pm/opp core has not got the icc path handle. I could use something like [2] to determine if should initialize gpu->icc_path*

b) Add peak-opp-configs in 845 dt and mandate all future versions to use this bindings. With this, I can remove gpu->icc_path from msm/drm completely and only rely on opp/core for bw voting.

[1] - https://lore.kernel.org/patchwork/cover/1240687/

[2] - https://patchwork.kernel.org/patch/11527573/

Let me know your thoughts

Sharat


Jordan

  unsigned long a6xx_gmu_get_freq(struct msm_gpu *gpu)
--
2.7.4

--
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
_______________________________________________
Freedreno mailing list
Freedreno@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/freedreno
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel




[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux