| Message ID | 20241119-topic-sm8x50-gpu-bw-vote-v2-8-4deb87be2498@linaro.org |
|---|---|
| State | New |
| Headers | show |
| Series | drm/msm: adreno: add support for DDR bandwidth scaling via GMU | expand |
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 >>> >
On 27/11/2024 16:46, Akhil P Oommen wrote: > 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. Oh, indeed I've been re-reading gen7_bus_ab_quantize() and it seems I should calculate the AB vote when building the bw_table. Thanks, Neil > > -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 >>>> >> >
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 {
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. 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(+)