Re: [PATCH 04/12] drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux DRI Users]     [Linux Intel Graphics]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [XFree86]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux