On Wed, Nov 20, 2024 at 01:37:16PM +0100, Neil Armstrong wrote:
> On 20/11/2024 12:42, Dmitry Baryshkov wrote:
> > On Tue, Nov 19, 2024 at 06:56:40PM +0100, Neil Armstrong wrote:
> > > The Adreno GMU Management Unit (GMU) can also scale DDR Bandwidth along
> > > the Frequency and Power Domain level, but by default we leave the
> > > OPP core scale the interconnect ddr path.
> > >
> > > In order to calculate vote values used by the GPU Management
> > > Unit (GMU), we need to parse all the possible OPP Bandwidths and
> > > create a vote value to be sent to the appropriate Bus Control
> > > Modules (BCMs) declared in the GPU info struct.
> > >
> > > The vote array will then be used to dynamically generate the GMU
> > > bw_table sent during the GMU power-up.
> > >
> > > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> >
> > LGTM, two minor nits below.
> >
[...]
> > > @@ -82,6 +92,10 @@ struct a6xx_gmu {
> > > unsigned long gpu_freqs[16];
> > > u32 gx_arc_votes[16];
> > > + int nr_gpu_bws;
> > > + unsigned long gpu_bw_table[16];
> > > + u32 gpu_bw_votes[16][GMU_MAX_BCMS];
> >
> > We still have magic 16 here. GPU_MAX_FREQUENCIES? GPU_FREQ_TABLE_SIZE?
>
> Yeah it would be max frequencies.
>
> I tried to replace this 16, but it's used as ARRAY_SIZE(gmu->gpu_bw_table)
> like the other gx_arc_votes & gpu_freqs.
>
> So I don't know what do define and where, and how, a separate patch ?
>
> The define would only be used in the struct, not in the code, or whould I
> drop the ARRAY_SIZE and use this define ?
I'd say, define in struct, ARRAY_SIZE in code. And yes a separate patch
changing gpu_freqs / gx_arc_votes would be beneficial.
>
> >
> > > +
> > > int nr_gmu_freqs;
> > > unsigned long gmu_freqs[4];
> > > u32 cx_arc_votes[4];
> > > diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> > > index 4aceffb6aae89c781facc2a6e4a82b20b341b6cb..5b80919e595fa1ba0a3afcca55feb89e60870cb1 100644
> > > --- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> > > +++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
> > > @@ -44,6 +44,7 @@ struct a6xx_info {
> > > u32 gmu_chipid;
> > > u32 gmu_cgc_mode;
> > > u32 prim_fifo_threshold;
> > > + const struct a6xx_bcm bcm[GMU_MAX_BCMS];
> > > };
> > > struct a6xx_gpu {
> > >
> > > --
> > > 2.34.1
> > >
> >
>
> Thanks,
> Neil
>
--
With best wishes
Dmitry
