[PATCH 11/12] drm/msm: merge dpu format database to MDP formats

[Dmitry Baryshkov]

On Fri, 12 Apr 2024 at 22:47, Abhinav Kumar <quic_abhinavk at quicinc.com> wrote:
>
>
>
> On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote:
> > Finally remove duplication between DPU and generic MDP code by merging
> > DPU format lists to the MDP format database.
> >
> > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov at linaro.org>
> > ---
> >   .../drm/msm/disp/dpu1/dpu_encoder_phys_vid.c  |   2 +-
> >   .../drm/msm/disp/dpu1/dpu_encoder_phys_wb.c   |   4 +-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c   | 602 ------------------
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h   |  23 -
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_hw_mdss.h   |  10 -
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_kms.c       |   2 +-
> >   drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c     |   3 +-
> >   drivers/gpu/drm/msm/disp/mdp_format.c         | 595 +++++++++++++++--
> >   drivers/gpu/drm/msm/disp/mdp_kms.h            |   2 -
> >   drivers/gpu/drm/msm/msm_drv.h                 |  12 +
> >   10 files changed, 549 insertions(+), 706 deletions(-)
> >
>
> I cross-checked a few macros visually (not each one) and it LGTM in
> terms of just moving it from dpu_formats.c to mdp_format.c
>
> Even in this change I had the same concern about whether to use MDP for
> dpu formats.
>
> But I think even if we make it MSM_*** then we will have to keep them in
> some msm_** header and not mdp_format.c
>
> So lets go ahead with the MDP naming which you have. If we see its not
> working out later on, please be open to a mass renaming that time.

Ack.

>
> <snip>
>
> > index dea6d47854fe..e7651a0e878c 100644
> > --- a/drivers/gpu/drm/msm/msm_drv.h
> > +++ b/drivers/gpu/drm/msm/msm_drv.h
> > @@ -267,6 +267,16 @@ enum msm_format_flags {
> >   #define MSM_FORMAT_FLAG_UNPACK_ALIGN_MSB BIT(MSM_FORMAT_FLAG_UNPACK_ALIGN_MSB_BIT)
> >   #define MSM_FORMAT_FLAG_ALPHA_ENABLE        BIT(MSM_FORMAT_FLAG_ALPHA_ENABLE_BIT)
> >
> > +/**
> > + * DPU HW,Component order color map
> > + */
> > +enum {
> > +     C0_G_Y = 0,
> > +     C1_B_Cb = 1,
> > +     C2_R_Cr = 2,
> > +     C3_ALPHA = 3
> > +};
> > +
> >   /**
> >    * struct msm_format: defines the format configuration
> >    * @pixel_format: format fourcc
> > @@ -305,6 +315,8 @@ struct msm_format {
> >       (((X)->fetch_mode == MDP_FETCH_UBWC) && \
> >        ((X)->flags & MSM_FORMAT_FLAG_COMPRESSED))
> >
> > +const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format, uint64_t modifier);
> > +
> >   struct msm_pending_timer;
> >
> >   int msm_atomic_init_pending_timer(struct msm_pending_timer *timer,
>
> I am now thinking that do you think it makes sense to move all
> MDP_FORMAT macros to a new mdp_formats.h including the RGB/YUV bitfield
> macros (even though I already acked that change).
>
> Instead of bloating msm_drv.h even more?

Sounds like a good idea, yes. Thank you!

-- 
With best wishes
Dmitry