On Fri, 12 Apr 2024 at 22:47, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> 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@xxxxxxxxxx>
> > ---
> > .../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
