Re: [PATCH v2 08/11] drm/msm: adreno: request for maximum bus bandwidth usage

[Akhil P Oommen <quic_akhilpo@quicinc.com>]

On 11/25/2024 1:44 PM, Neil Armstrong wrote:
> On 23/11/2024 23:59, Akhil P Oommen wrote:
>> On Tue, Nov 19, 2024 at 06:56:43PM +0100, Neil Armstrong wrote:
>>> When requesting a DDR bandwidth level along a GPU frequency
>>> level via the GMU, we can also specify the bus bandwidth usage in a
>>> 16bit
>>> quantitized value.
>>>
>>> For now simply request the maximum bus usage.
>>
>> Why? You don't care about power efficiency?
>> Lets drop this patch. We don't care about AB vote yet.
> 
> I care about functionality, without this AB vote the spillall use
> case that fails on SM8650 HDK fails on the SM8650 QRD.

This should have been documented as a comment so that someone doesn't
remove it in future.

> 
> AB is a quantitized value of the BW voted, so yes I expect we can have
> 100% of the BW voted, but since we scale the BW it's perfectly fine.

Ah! no. MAX AB vote here is equal to the Max IB vote value in the hfi
table. This is why I was asking about including all BW levels from the
DT in the hfi table in the other patch.

So you are always voting for Max DDR Freq which is probably helping (or
masking?) the spill all issue. We can just add a quirk to vote for MAX
IB probably.

-Akhil

> 
> Neil
> 
>>
>> -Akhil
>>
>>>
>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org>
>>> ---
>>>   drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 11 +++++++++++
>>>   drivers/gpu/drm/msm/adreno/a6xx_hfi.h |  5 +++++
>>>   2 files changed, 16 insertions(+)
>>>
>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/
>>> msm/adreno/a6xx_gmu.c
>>> index
>>> dc2d0035544e7848e5c4ea27f1ea9a191f9c4991..36c0f67fd8e109aabf09a0804bacbed3593c39d7 100644
>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
>>> @@ -134,6 +134,17 @@ void a6xx_gmu_set_freq(struct msm_gpu *gpu,
>>> struct dev_pm_opp *opp,
>>>               if (bw == gmu->gpu_bw_table[bw_index])
>>>                   break;
>>>           }
>>> +
>>> +        if (bw_index) {
>>> +            /*
>>> +             * Append AB vote to the maximum bus usage.
>>> +             * AB represents a quantitized 16bit value of the
>>> +             * max ddr bandwidth we could use, let's simply
>>> +             * request the maximum for now.
>>> +             */
>>> +            bw_index |= AB_VOTE(MAX_AB_VOTE);
>>> +            bw_index |= AB_VOTE_ENABLE;
>>> +        }
>>>       }
>>>         gmu->current_perf_index = perf_index;
>>> diff --git a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h b/drivers/gpu/drm/
>>> msm/adreno/a6xx_hfi.h
>>> index
>>> 528110169398f69f16443a29a1594d19c36fb595..52ba4a07d7b9a709289acd244a751ace9bdaab5d 100644
>>> --- a/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
>>> +++ b/drivers/gpu/drm/msm/adreno/a6xx_hfi.h
>>> @@ -173,6 +173,11 @@ struct a6xx_hfi_gx_bw_perf_vote_cmd {
>>>       u32 bw;
>>>   };
>>>   +#define AB_VOTE_MASK        GENMASK(31, 16)
>>> +#define MAX_AB_VOTE        (FIELD_MAX(AB_VOTE_MASK) - 1)
>>> +#define AB_VOTE(vote)        FIELD_PREP(AB_VOTE_MASK, (vote))
>>> +#define AB_VOTE_ENABLE        BIT(8)
>>> +
>>>   #define HFI_H2F_MSG_PREPARE_SLUMBER 33
>>>     struct a6xx_hfi_prep_slumber_cmd {
>>>
>>> -- 
>>> 2.34.1
>>>
>