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

[Abhinav Kumar <quic_abhinavk@quicinc.com>]

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@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.

<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?