[PATCH 04/12] drm/msm: add arrays listing formats supported by MDP4/MDP5 hardware
Abhinav Kumar
quic_abhinavk at quicinc.com
Fri Apr 19 21:57:00 UTC 2024
On 4/19/2024 2:21 PM, Dmitry Baryshkov wrote:
> On Sat, 20 Apr 2024 at 00:06, Abhinav Kumar <quic_abhinavk at 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 at 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 at quicinc.com>
More information about the dri-devel
mailing list