Re: [PATCH 3/6] drm/msm/dpu: add support for SM8550

[Neil Armstrong]

On 04/01/2023 10:45, Dmitry Baryshkov wrote:
> On 04/01/2023 11:08, Neil Armstrong wrote:
> > Add definitions for the display hardware used on Qualcomm SM8550
> > platform.
> >
> > Signed-off-by: Neil Armstrong <neil.armstrong@xxxxxxxxxx>
> > ---
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 197 +++++++++++++++++++++++++
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h |   1 +
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h    |   2 +
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c        |   1 +
> >   4 files changed, 201 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > index b4ca123d8e69..adf5e25269dc 100644
> > --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c
> > +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c

<snip>

> > @@ -776,6 +821,45 @@ static const struct dpu_ctl_cfg sm8450_ctl[] = {
> >       },
> >   };
> > +static const struct dpu_ctl_cfg sm8550_ctl[] = {
> > +    {
> > +    .name = "ctl_0", .id = CTL_0,
> > +    .base = 0x15000, .len = 0x290,?
> > +    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_SPLIT_DISPLAY) | BIT(DPU_CTL_FETCH_ACTIVE),
>
> CTL_SC7280_MASK | BIT(DPU_CTL_SPLIT_DISPLAY) ?

Indeed DPU_CTL_VM_CFG is missing, will switch to that.

> > +    .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 9),
> > +    },
> > +    {
> > +    .name = "ctl_1", .id = CTL_1,
> > +    .base = 0x16000, .len = 0x290,
> > +    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_SPLIT_DISPLAY) | BIT(DPU_CTL_FETCH_ACTIVE),
> > +    .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 10),
> > +    },
> > +    {
> > +    .name = "ctl_2", .id = CTL_2,
> > +    .base = 0x17000, .len = 0x290,
> > +    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
>
> CTL_SC7280_MASK?

Ack

> > +    .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 11),
> > +    },
> > +    {
> > +    .name = "ctl_3", .id = CTL_3,
> > +    .base = 0x18000, .len = 0x290,
> > +    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
> > +    .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 12),
> > +    },
> > +    {
> > +    .name = "ctl_4", .id = CTL_4,
> > +    .base = 0x19000, .len = 0x290,
> > +    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
> > +    .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 13),
> > +    },
> > +    {
> > +    .name = "ctl_5", .id = CTL_5,
> > +    .base = 0x1a000, .len = 0x290,
> > +    .features = BIT(DPU_CTL_ACTIVE_CFG) | BIT(DPU_CTL_FETCH_ACTIVE),
> > +    .intr_start = DPU_IRQ_IDX(MDP_SSPP_TOP0_INTR2, 23),
> > +    },
> > +};
> > +
> >   static const struct dpu_ctl_cfg sc7280_ctl[] = {
> >       {
> >       .name = "ctl_0", .id = CTL_0,

<snip>

> > @@ -1268,6 +1386,16 @@ static const struct dpu_pingpong_sub_blks sc7280_pp_sblk = {
> >       .len = 0x20, .version = 0x20000},
> >   };
> > +#define PP_BLK_DIPHER(_name, _id, _base, _merge_3d, _sblk, _done, _rdptr) \
> > +    {\
> > +    .name = _name, .id = _id, \
> > +    .base = _base, .len = 0, \
>
> len = 0 looks incorrect. Any particular reason why can't we use plain PP_BLK here?

The TE block has been moved to the DSI INTF blocks since SM8350 I think, or earlier.

This removes the DPU_PINGPONG_DITHER feature used downstream to enable the PP TE callbacks.

Since there's only the DIPHER sub-block remaining, this is why I set len to 0.

> > +    .features = BIT(DPU_PINGPONG_DITHER), \
> > +    .merge_3d = _merge_3d, \
> > +    .sblk = &_sblk, \
> > +    .intr_done = _done, \
> > +    .intr_rdptr = _rdptr, \
> > +    }
> >   #define PP_BLK_TE(_name, _id, _base, _merge_3d, _sblk, _done, _rdptr) \
> >       {\
> >       .name = _name, .id = _id, \

<snip>