On 12/13/2024 10:10 PM, neil.armstrong@xxxxxxxxxx wrote: > On 13/12/2024 17:31, Konrad Dybcio wrote: >> On 13.12.2024 5:28 PM, neil.armstrong@xxxxxxxxxx wrote: >>> On 13/12/2024 16:37, Konrad Dybcio wrote: >>>> On 13.12.2024 2:12 PM, Akhil P Oommen wrote: >>>>> On 12/13/2024 3:07 AM, Neil Armstrong wrote: >>>>>> On 12/12/2024 21:21, Konrad Dybcio wrote: >>>>>>> On 11.12.2024 9:29 AM, Neil Armstrong wrote: >>>>>>>> The Adreno GPU Management Unit (GMU) can also scale the DDR >>>>>>>> Bandwidth >>>>>>>> along the Frequency and Power Domain level, until now we left >>>>>>>> the OPP >>>>>>>> core scale the OPP bandwidth via the interconnect path. >>>>>>>> >>>>>>>> In order to enable bandwidth voting via the GPU Management >>>>>>>> Unit (GMU), when an opp is set by devfreq we also look for >>>>>>>> the corresponding bandwidth index in the previously generated >>>>>>>> bw_table and pass this value along the frequency index to the GMU. >>>>>>>> >>>>>>>> The GMU also takes another vote called AB which is a 16bit >>>>>>>> quantized >>>>>>>> value of the floor bandwidth against the maximum supported >>>>>>>> bandwidth. >>>>>>>> >>>>>>>> The AB is calculated with a default 25% of the bandwidth like the >>>>>>>> downstream implementation too inform the GMU firmware the minimal >>>>>>>> quantity of bandwidth we require for this OPP. >>>>>>>> >>>>>>>> Since we now vote for all resources via the GMU, setting the OPP >>>>>>>> is no more needed, so we can completely skip calling >>>>>>>> dev_pm_opp_set_opp() in this situation. >>>>>>>> >>>>>>>> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> >>>>>>>> Reviewed-by: Akhil P Oommen <quic_akhilpo@xxxxxxxxxxx> >>>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx> >>>>>>>> --- >>>>>>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 39 +++++++++++++++++ >>>>>>>> +++++++ >>>>>>>> +++++++++-- >>>>>>>> drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 +- >>>>>>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.c | 6 +++--- >>>>>>>> drivers/gpu/drm/msm/adreno/a6xx_hfi.h | 5 +++++ >>>>>>>> 4 files changed, 46 insertions(+), 6 deletions(-) >>>>>>>> >>>>>>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/ >>>>>>>> gpu/drm/ >>>>>>>> msm/adreno/a6xx_gmu.c >>>>>>>> index >>>>>>>> 36696d372a42a27b26a018b19e73bc6d8a4a5235..46ae0ec7a16a41d55755ce04fb32404cdba087be 100644 >>>>>>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >>>>>>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c >>>>>>>> @@ -110,9 +110,11 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu, >>>>>>>> struct dev_pm_opp *opp, >>>>>>>> bool suspended) >>>>>>>> { >>>>>>>> struct adreno_gpu *adreno_gpu = to_adreno_gpu(gpu); >>>>>>>> + const struct a6xx_info *info = adreno_gpu->info->a6xx; >>>>>>>> struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu); >>>>>>>> struct a6xx_gmu *gmu = &a6xx_gpu->gmu; >>>>>>>> u32 perf_index; >>>>>>>> + u32 bw_index = 0; >>>>>>>> unsigned long gpu_freq; >>>>>>>> int ret = 0; >>>>>>>> @@ -125,6 +127,37 @@ void a6xx_gmu_set_freq(struct msm_gpu >>>>>>>> *gpu, >>>>>>>> struct dev_pm_opp *opp, >>>>>>>> if (gpu_freq == gmu->gpu_freqs[perf_index]) >>>>>>>> break; >>>>>>>> + /* If enabled, find the corresponding DDR bandwidth >>>>>>>> index */ >>>>>>>> + if (info->bcms && gmu->nr_gpu_bws > 1) { >>>>>>> >>>>>>> if (gmu->nr_gpu_bws) >>>>>> >>>>>> gmu->nr_gpu_bws == 1 means there's not BW in the OPPs (index 0 is the >>>>>> "off" state) >>>>>> >>>>>>> >>>>>>>> + unsigned int bw = dev_pm_opp_get_bw(opp, true, 0); >>>>>>>> + >>>>>>>> + for (bw_index = 0; bw_index < gmu->nr_gpu_bws - 1; >>>>>>>> bw_index+ >>>>>>>> +) { >>>>>>>> + if (bw == gmu->gpu_bw_table[bw_index]) >>>>>>>> + break; >>>>>>>> + } >>>>>>>> + >>>>>>>> + /* Vote AB as a fraction of the max bandwidth */ >>>>>>>> + if (bw) { >>>>>>> >>>>>>> This seems to only be introduced with certain a7xx too.. you should >>>>>>> ping the GMU with HFI_VALUE_GMU_AB_VOTE to check if it's supported >>>>>> >>>>>> Good point >>>>> >>>>> No no. Doing this will trigger some assert in pre-A750 gmu >>>>> firmwares. We >>>>> learned it the hard way. No improvisation please. :) >>>> >>>> We shouldn't be sending that AB data to firmware that doesn't expect >>>> it either too, though.. >>> >>> Well we don't ! >> >> The code in the scope that I quoted above does that > > No it doesn't, if the proper bcms are not declared in the gpu_info, it > won't I think what Konrad meant was that IB voting is supported from a650+, but AB voting is support only from a750+. So we can add bcm nodes to enable IB voting, but how do we ensure AB voting via GMU is done only on a750+. -Akhil > > Neil > >> >> see the full explanation here >> >> https://git.codelinaro.org/clo/le/platform/vendor/qcom/opensource/ >> graphics-kernel/-/commit/6026c31a54444b712f7ea36ac1aafaaeef07fa4e >> >> Konrad >