| Message ID | 20231202214016.1257621-5-dmitry.baryshkov@linaro.org |
|---|---|
| State | Superseded |
| Headers | show |
| Series | drm/msm: fold dpu_format into mdp_formats database | expand |
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@linaro.org> > --- > 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 > + 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?
On Sat, 20 Apr 2024 at 00:06, Abhinav Kumar <quic_abhinavk@quicinc.com> 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@linaro.org> > > --- > > 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.
On 4/19/2024 2:21 PM, Dmitry Baryshkov wrote: > On Sat, 20 Apr 2024 at 00:06, Abhinav Kumar <quic_abhinavk@quicinc.com> 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@linaro.org> >>> --- >>> 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. > Ack. >> >> > + 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. > Ok, I dont expect them to change tbh as it has not happened last few years now. Anyway, that part is fine. Hence Reviewed-by: Abhinav Kumar <quic_abhinavk@quicinc.com>
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; diff --git a/drivers/gpu/drm/msm/disp/mdp_format.c b/drivers/gpu/drm/msm/disp/mdp_format.c index 025595336f26..69ab5bcff1a9 100644 --- a/drivers/gpu/drm/msm/disp/mdp_format.c +++ b/drivers/gpu/drm/msm/disp/mdp_format.c @@ -80,10 +80,6 @@ static struct csc_cfg csc_convert[CSC_MAX] = { #define BPC0A 0 -/* - * Note: Keep RGB formats 1st, followed by YUV formats to avoid breaking - * mdp_get_rgb_formats()'s implementation. - */ static const struct mdp_format formats[] = { /* name a r g b e0 e1 e2 e3 alpha tight cpp cnt ... */ FMT(ARGB8888, 8, 8, 8, 8, 1, 0, 2, 3, true, true, 4, 4, @@ -138,30 +134,6 @@ static const struct mdp_format formats[] = { MDP_PLANE_PLANAR, CHROMA_420, true), }; -/* - * Note: - * @rgb_only must be set to true, when requesting - * supported formats for RGB pipes. - */ -uint32_t mdp_get_formats(uint32_t *pixel_formats, uint32_t max_formats, - bool rgb_only) -{ - uint32_t i; - for (i = 0; i < ARRAY_SIZE(formats); i++) { - const struct mdp_format *f = &formats[i]; - - if (i == max_formats) - break; - - if (rgb_only && MDP_FORMAT_IS_YUV(f)) - break; - - pixel_formats[i] = f->base.pixel_format; - } - - return i; -} - const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format, uint64_t modifier) { diff --git a/drivers/gpu/drm/msm/disp/mdp_kms.h b/drivers/gpu/drm/msm/disp/mdp_kms.h index b0286d5d5130..d0718c16de3e 100644 --- a/drivers/gpu/drm/msm/disp/mdp_kms.h +++ b/drivers/gpu/drm/msm/disp/mdp_kms.h @@ -91,7 +91,6 @@ struct mdp_format { #define to_mdp_format(x) container_of(x, struct mdp_format, base) #define MDP_FORMAT_IS_YUV(mdp_format) ((mdp_format)->is_yuv) -uint32_t mdp_get_formats(uint32_t *formats, uint32_t max_formats, bool rgb_only); const struct msm_format *mdp_get_format(struct msm_kms *kms, uint32_t format, uint64_t modifier); /* MDP capabilities */
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@linaro.org> --- 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(-)