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
