On Thu, Jul 6, 2023 at 7:54 PM Dmitry Baryshkov
<dmitry.baryshkov@xxxxxxxxxx> wrote:
>
> On 07/07/2023 00:10, Rob Clark wrote:
> > From: Rob Clark <robdclark@xxxxxxxxxxxx>
> >
> > This simplifies the code.
> >
> > Signed-off-by: Rob Clark <robdclark@xxxxxxxxxxxx>
> > ---
> > drivers/gpu/drm/msm/adreno/a6xx_gpu.c | 171 ++-------------------
> > drivers/gpu/drm/msm/adreno/adreno_device.c | 51 ++++++
> > drivers/gpu/drm/msm/adreno/adreno_gpu.h | 25 +++
> > 3 files changed, 92 insertions(+), 155 deletions(-)
>
>
> Interesting hack, I'd say.
>
> Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx>
>
> Minor nit below.
>
> >
>
> > diff --git a/drivers/gpu/drm/msm/adreno/adreno_gpu.h b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > index d5335b99c64c..994ac26ce731 100644
> > --- a/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > +++ b/drivers/gpu/drm/msm/adreno/adreno_gpu.h
> > @@ -72,8 +72,33 @@ struct adreno_info {
> > u32 inactive_period;
> > const struct adreno_reglist *hwcg;
> > u64 address_space_size;
> > + /**
> > + * @speedbins: Optional table of fuse to speedbin mappings
> > + *
> > + * Consists of pairs of fuse, index mappings, terminated with
> > + * UINT_MAX sentinal.
> > + */
> > + uint32_t *speedbins;
>
> Would it be better to explicitly list this as pairs of uint32_t? And
> then use braces in ADRENO_SPEEDBIN initialisation.
It would mean the sentinel would take 8 bytes instead of 4.. maybe
that is over-thinking it, but it was the reason I just stuck with a
flat table
BR,
-R
> > };
> >
> > +/*
> > + * Helper to build a speedbin table, ie. the table:
> > + * fuse | speedbin
> > + * -----+---------
> > + * 0 | 0
> > + * 169 | 1
> > + * 174 | 2
> > + *
> > + * would be declared as:
> > + *
> > + * .speedbins = ADRENO_SPEEDBINS(
> > + * 0, 0,
> > + * 169, 1,
> > + * 174, 2
> > + * ),
> > + */
> > +#define ADRENO_SPEEDBINS(tbl...) (uint32_t[]) { tbl, UINT_MAX }
> > +
> > const struct adreno_info *adreno_info(struct adreno_rev rev);
> >
> > struct adreno_gpu {
>
> --
> With best wishes
> Dmitry
>
