[PATCH 2/6] drm/msm/mdss: generate MDSS data for MDP5 platforms

[Dmitry Baryshkov]

On Thu, 7 Sept 2023 at 01:10, Stephen Boyd <swboyd at chromium.org> wrote:
>
> Quoting Dmitry Baryshkov (2023-09-05 10:43:49)
> > diff --git a/drivers/gpu/drm/msm/msm_mdss.c b/drivers/gpu/drm/msm/msm_mdss.c
> > index 348c66b14683..fb6ee93b5abc 100644
> > --- a/drivers/gpu/drm/msm/msm_mdss.c
> > +++ b/drivers/gpu/drm/msm/msm_mdss.c
> > @@ -222,6 +222,36 @@ static void msm_mdss_setup_ubwc_dec_40(struct msm_mdss *msm_mdss)
> >         }
> >  }
> >
> > +static struct msm_mdss_data *msm_mdss_generate_mdp5_mdss_data(struct msm_mdss *mdss)
>
> const mdss?

Ack

>
> > +{
> > +       struct msm_mdss_data *data;
> > +       u32 hw_rev;
> > +
> > +       data = devm_kzalloc(mdss->dev, sizeof(*data), GFP_KERNEL);
> > +       if (!data)
> > +               return NULL;
> > +
> > +       hw_rev = readl_relaxed(mdss->mmio + HW_REV) >> 16;
>
> Or like this
>
>         hw_rev = upper_16_bits(readl_relaxed(...))

Ugh. That looks ugly, I'd say. >> 16 is more common.

>
> > +
> > +       if (hw_rev == 0x1007 || /* msm8996 */
> > +           hw_rev == 0x100e || /* msm8937 */
> > +           hw_rev == 0x1010 || /* msm8953 */
> > +           hw_rev == 0x3000 || /* msm8998 */
> > +           hw_rev == 0x3002 || /* sdm660 */
> > +           hw_rev == 0x3003) { /* sdm630 */
>
> Can we have #defines for hw_revs and drop the comments?
>
> > +               data->ubwc_dec_version = UBWC_1_0;
> > +               data->ubwc_enc_version = UBWC_1_0;
> > +       }
> > +
> > +       if (hw_rev == 0x1007 || /* msm8996 */
> > +           hw_rev == 0x3000) /* msm8998 */
>
> Then we don't have to worry about these two having typos.

Ack

>
> > +               data->highest_bank_bit = 2;
> > +       else
> > +               data->highest_bank_bit = 1;
> >



-- 
With best wishes
Dmitry