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

[Dmitry Baryshkov]

On 02/12/2023 03:36, Abhinav Kumar wrote:


On 7/7/2023 6:03 PM, Dmitry Baryshkov wrote:
MDP4 and MDP5 drivers enumerate supported formats each time the plane is
created. As the list of supported image formats is constant, create
corresponding data arrays to be used by MDP4 and MDP5 drivers.

Signed-off-by: Dmitry Baryshkov <dmitry.barysh...@linaro.org>
---
  drivers/gpu/drm/msm/disp/mdp_format.c | 49 +++++++++++++++++++++++++--
  drivers/gpu/drm/msm/disp/mdp_kms.h    |  5 +++
  2 files changed, 52 insertions(+), 2 deletions(-)


After going through the patch series, as commented in patch 17 I am 
totally fine with migrating to drmm-managed APIs but I dont like to 
maintain 3 format arrays.

Can we keep the existing format mechanism to avoid having two more arrays?

For DPU we have exactly the same idea: single formats data array 
describing and per-usecase arrays, like plane RGB arrays, plane YUV+RGB 
array, WB arrays.

Anyway. formats was one of the topics where we had a lot of duplication 
between mdp4/mdp5 and dpu anyway. I think I'm going to back up the 
patches 1, 10, 16, 17 (plane patches depend on the format arrays), push 
the rest of the patches to msm-next-lumag and send a series reworking 
all the formats handling.



diff --git a/drivers/gpu/drm/msm/disp/mdp_format.c 
b/drivers/gpu/drm/msm/disp/mdp_format.c
index 025595336f26..ba9abe8b3acc 100644
--- a/drivers/gpu/drm/msm/disp/mdp_format.c
+++ b/drivers/gpu/drm/msm/disp/mdp_format.c
@@ -81,8 +81,8 @@ 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.
+ * Note: Keep mdp_rgb_formats and mdp_rgb_yuv_formats in sync when 
adding
+ * entries to this array.
   */
  static const struct mdp_format formats[] = {
      /*  name      a  r  g  b   e0 e1 e2 e3  alpha   tight  cpp cnt 
... */
@@ -138,6 +138,51 @@ static const struct mdp_format formats[] = {
              MDP_PLANE_PLANAR, CHROMA_420, true),
  };
+const uint32_t mdp_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,
+};
+
+size_t mdp_rgb_num_formats = ARRAY_SIZE(mdp_rgb_formats);
+
+const uint32_t mdp_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,
+};
+
+size_t mdp_rgb_yuv_num_formats = ARRAY_SIZE(mdp_rgb_yuv_formats);
+
  /*
   * Note:
   * @rgb_only must be set to true, when requesting
diff --git a/drivers/gpu/drm/msm/disp/mdp_kms.h 
b/drivers/gpu/drm/msm/disp/mdp_kms.h
index b0286d5d5130..11402a859574 100644
--- a/drivers/gpu/drm/msm/disp/mdp_kms.h
+++ b/drivers/gpu/drm/msm/disp/mdp_kms.h
@@ -94,6 +94,11 @@ struct mdp_format {
  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);
+extern const uint32_t mdp_rgb_formats[];
+extern size_t mdp_rgb_num_formats;
+extern const uint32_t mdp_rgb_yuv_formats[];
+extern size_t mdp_rgb_yuv_num_formats;
+
  /* MDP capabilities */
  #define MDP_CAP_SMP        BIT(0)    /* Shared Memory 
Pool                 */
  #define MDP_CAP_DSC        BIT(1)    /* VESA Display Stream 
Compression    */

--
With best wishes
Dmitry