On Sat, 20 Apr 2024 at 00:06, Abhinav Kumar <quic_abhinavk@xxxxxxxxxxx> wrote: > > > > On 12/2/2023 1:40 PM, Dmitry Baryshkov wrote: > > MDP4 and MDP5 drivers enumerate supported formats each time the plane is > > created. In preparation to merger of MDP DPU format databases, define > > precise formats list, so that changes to the database do not cause the > > driver to add unsupported format to the list. > > > > Signed-off-by: Dmitry Baryshkov <dmitry.baryshkov@xxxxxxxxxx> > > --- > > drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c | 57 ++++++++++++++++++++-- > > drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c | 36 +++++++++++--- > > drivers/gpu/drm/msm/disp/mdp_format.c | 28 ----------- > > drivers/gpu/drm/msm/disp/mdp_kms.h | 1 - > > 4 files changed, 80 insertions(+), 42 deletions(-) > > > > diff --git a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c > > index b689b618da78..cebe20c82a54 100644 > > --- a/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c > > +++ b/drivers/gpu/drm/msm/disp/mdp4/mdp4_plane.c > > @@ -371,6 +371,47 @@ static const uint64_t supported_format_modifiers[] = { > > DRM_FORMAT_MOD_INVALID > > }; > > > > +const uint32_t mdp4_rgb_formats[] = { > > + DRM_FORMAT_ARGB8888, > > + DRM_FORMAT_ABGR8888, > > + DRM_FORMAT_RGBA8888, > > + DRM_FORMAT_BGRA8888, > > + DRM_FORMAT_XRGB8888, > > + DRM_FORMAT_XBGR8888, > > + DRM_FORMAT_RGBX8888, > > + DRM_FORMAT_BGRX8888, > > + DRM_FORMAT_RGB888, > > + DRM_FORMAT_BGR888, > > + DRM_FORMAT_RGB565, > > + DRM_FORMAT_BGR565, > > +}; > > + > > +const uint32_t mdp4_rgb_yuv_formats[] = { > > + DRM_FORMAT_ARGB8888, > > + DRM_FORMAT_ABGR8888, > > + DRM_FORMAT_RGBA8888, > > + DRM_FORMAT_BGRA8888, > > + DRM_FORMAT_XRGB8888, > > + DRM_FORMAT_XBGR8888, > > + DRM_FORMAT_RGBX8888, > > + DRM_FORMAT_BGRX8888, > > + DRM_FORMAT_RGB888, > > + DRM_FORMAT_BGR888, > > + DRM_FORMAT_RGB565, > > + DRM_FORMAT_BGR565, > > + > > + DRM_FORMAT_NV12, > > + DRM_FORMAT_NV21, > > + DRM_FORMAT_NV16, > > + DRM_FORMAT_NV61, > > + DRM_FORMAT_VYUY, > > + DRM_FORMAT_UYVY, > > + DRM_FORMAT_YUYV, > > + DRM_FORMAT_YVYU, > > + DRM_FORMAT_YUV420, > > + DRM_FORMAT_YVU420, > > +}; > > + > > /* initialize plane */ > > struct drm_plane *mdp4_plane_init(struct drm_device *dev, > > enum mdp4_pipe pipe_id, bool private_plane) > > @@ -379,6 +420,8 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev, > > struct mdp4_plane *mdp4_plane; > > int ret; > > enum drm_plane_type type; > > + const uint32_t *formats; > > + unsigned int nformats; > > > > mdp4_plane = kzalloc(sizeof(*mdp4_plane), GFP_KERNEL); > > if (!mdp4_plane) { > > @@ -392,13 +435,17 @@ struct drm_plane *mdp4_plane_init(struct drm_device *dev, > > mdp4_plane->name = pipe_names[pipe_id]; > > mdp4_plane->caps = mdp4_pipe_caps(pipe_id); > > > > - mdp4_plane->nformats = mdp_get_formats(mdp4_plane->formats, > > - ARRAY_SIZE(mdp4_plane->formats), > > - !pipe_supports_yuv(mdp4_plane->caps)); > > - > > type = private_plane ? DRM_PLANE_TYPE_PRIMARY : DRM_PLANE_TYPE_OVERLAY; > > + > > + if (pipe_supports_yuv(mdp4_plane->caps)) { > > + formats = mdp4_rgb_yuv_formats; > > + nformats = ARRAY_SIZE(mdp4_rgb_yuv_formats); > > + } else { > > + formats = mdp4_rgb_formats; > > + nformats = ARRAY_SIZE(mdp4_rgb_formats); > > + } > > ret = drm_universal_plane_init(dev, plane, 0xff, &mdp4_plane_funcs, > > - mdp4_plane->formats, mdp4_plane->nformats, > > + formats, nformats, > > supported_format_modifiers, type, NULL); > > if (ret) > > goto fail; > > diff --git a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > index 0d5ff03cb091..aa8342d93393 100644 > > --- a/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > +++ b/drivers/gpu/drm/msm/disp/mdp5/mdp5_plane.c > > @@ -17,9 +17,6 @@ > > > > struct mdp5_plane { > > struct drm_plane base; > > - > > - uint32_t nformats; > > - uint32_t formats[32]; > > }; > > #define to_mdp5_plane(x) container_of(x, struct mdp5_plane, base) > > > > @@ -1007,6 +1004,32 @@ uint32_t mdp5_plane_get_flush(struct drm_plane *plane) > > return mask; > > } > > > > +const uint32_t mdp5_plane_formats[] = { > > + DRM_FORMAT_ARGB8888, > > + DRM_FORMAT_ABGR8888, > > + DRM_FORMAT_RGBA8888, > > + DRM_FORMAT_BGRA8888, > > + DRM_FORMAT_XRGB8888, > > + DRM_FORMAT_XBGR8888, > > + DRM_FORMAT_RGBX8888, > > + DRM_FORMAT_BGRX8888, > > + DRM_FORMAT_RGB888, > > + DRM_FORMAT_BGR888, > > + DRM_FORMAT_RGB565, > > + DRM_FORMAT_BGR565, > > + > > + DRM_FORMAT_NV12, > > + DRM_FORMAT_NV21, > > + DRM_FORMAT_NV16, > > + DRM_FORMAT_NV61, > > + DRM_FORMAT_VYUY, > > + DRM_FORMAT_UYVY, > > + DRM_FORMAT_YUYV, > > + DRM_FORMAT_YVYU, > > + DRM_FORMAT_YUV420, > > + DRM_FORMAT_YVU420, > > +}; > > + > > /* initialize plane */ > > struct drm_plane *mdp5_plane_init(struct drm_device *dev, > > enum drm_plane_type type) > > @@ -1023,12 +1046,9 @@ struct drm_plane *mdp5_plane_init(struct drm_device *dev, > > > > plane = &mdp5_plane->base; > > > > - mdp5_plane->nformats = mdp_get_formats(mdp5_plane->formats, > > - ARRAY_SIZE(mdp5_plane->formats), false); > > - > > ret = drm_universal_plane_init(dev, plane, 0xff, &mdp5_plane_funcs, > > - mdp5_plane->formats, mdp5_plane->nformats, > > - NULL, type, NULL); > > + mdp5_plane_formats, ARRAY_SIZE(mdp5_plane_formats), > > + NULL, type, NULL); > > if (ret) > > goto fail; > > > > Did you accidentally drop checking for YUV format cap before adding the > formats for the plane similar to No. MDP5 driver asks RGB+YUV planes see the mdp_get_formats() arguments. > > > + if (pipe_supports_yuv(mdp4_plane->caps)) { > > + formats = mdp4_rgb_yuv_formats; > > + nformats = ARRAY_SIZE(mdp4_rgb_yuv_formats); > > + } else { > > + formats = mdp4_rgb_formats; > > + nformats = ARRAY_SIZE(mdp4_rgb_formats); > > + } > > > Also, from what I checked the format table is identical between mdp4 and > mdp5. Even if we cannot unify mdp4/5 and dpu tables, we can atleast have > mdp_4_5_rgb and mdp_4_5_rgb_yuv? I'd rather not do that. If we are to change mdp4 or mdp5 planes at some point, I don't want to think about the second driver. The amount of duplication is minimal compared to the burden of thinking about the second driver. -- With best wishes Dmitry