Messages in this thread | | | Subject | Re: [Freedreno] [PATCH 5/6] drm: msm: a6xx: use dev_pm_opp_set_bw to set DDR bandwidth | From | Sharat Masetty <> | Date | Thu, 28 May 2020 16:32:57 +0530 |
| |
On 5/27/2020 9:08 PM, Rob Clark wrote: > On Wed, May 27, 2020 at 1:47 AM Sharat Masetty <smasetty@codeaurora.org> wrote: >> + more folks >> >> On 5/18/2020 9:55 PM, Rob Clark wrote: >>> On Mon, May 18, 2020 at 7:23 AM Jordan Crouse <jcrouse@codeaurora.org> 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@codeaurora.org> >>>>> --- >>>>> 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. > The main thing is that we want to make sure newer dtb always works on > an older kernel without regression.. but, hmm.. I guess the > interconnects/interconnects-names properties haven't landed yet in > sdm845.dtsi? Maybe that lets us go with the simpler approach (b). > Looks like we haven't wired up interconnect for 8916 or 8996 either, > so probably we can just mandate this for all of them?
I checked all three 845, 820 and 8916 and none of them have the interconnect configs for GPU. So, I think we are good here. I'll go with option (b) and re-spin v3. Adding interconnects and opp-peak-kBps configs for previous chips can be taken up as a separate activity.
Sharat
> If we have landed the interconnect dts hookup for gpu somewhere that > I'm overlooking, I guess we would have to go with (a) and keep the > existing interconnects/interconnects-names properties. > > BR, > -R > >> [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@lists.freedesktop.org >>>> https://lists.freedesktop.org/mailman/listinfo/freedreno
| |