Re: [PATCH 2/5] drm/msm/dpu: Use simple list for plane format init

On Tue, Dec 11, 2018 at 10:42:21AM -0800, Fritz Koenig wrote:

Simplify the initilization of a list of formats by passing the list in directly instead of copying it from one structure to another.

I have notes on at least a few other instances of passing a struct of parameters instead of just passing stuff directly. Definitely a nice change that simplifies things for casual readers like us. Thanks!

Reviewed-by: Sean Paul sean@poorly.run

Signed-off-by: Fritz Koenig frkoenig@google.com

drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c | 33 ---- drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h | 14 -- .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c | 4 + .../gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h | 19 +-- .../drm/msm/disp/dpu1/dpu_hw_catalog_format.h | 151 +++++++++--------- drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h | 1 - drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c | 25 ++- 7 files changed, 93 insertions(+), 154 deletions(-)

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c index 0874f0a53bf9..d53abc8ce670 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.c @@ -1137,36 +1137,3 @@ const struct msm_format *dpu_get_msm_format( return &fmt->base; return NULL; }

-uint32_t dpu_populate_formats(

• const struct dpu_format_extended *format_list,
  

• uint32_t *pixel_formats,
  

• uint64_t *pixel_modifiers,
  

• uint32_t pixel_formats_max)
  

-{

• uint32_t i, fourcc_format;
• if (!format_list || !pixel_formats)

• return 0;
  

• for (i = 0, fourcc_format = 0;

• 	format_list->fourcc_format && i < pixel_formats_max;
  

• 	++format_list) {
  

• /* verify if listed format is in dpu_format_map? */
  

• /* optionally return modified formats */
  

• if (pixel_modifiers) {
  

• 	/* assume same modifier for all fb planes */
  

• 	pixel_formats[i] = format_list->fourcc_format;
  

• 	pixel_modifiers[i++] = format_list->modifier;
  

• } else {
  

• 	/* assume base formats grouped together */
  

• 	if (fourcc_format != format_list->fourcc_format) {
  

• 		fourcc_format = format_list->fourcc_format;
  

• 		pixel_formats[i++] = fourcc_format;
  

• 	}
  

• }
  

• }
• return i;

-} diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h index a54451d8d011..c02c81e7a667 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_formats.h @@ -40,20 +40,6 @@ const struct msm_format *dpu_get_msm_format( const uint32_t format, const uint64_t modifiers);

-/**

• • dpu_populate_formats - populate the given array with fourcc codes supported
• • @format_list: pointer to list of possible formats
• • @pixel_formats: array to populate with fourcc codes
• • @pixel_modifiers: array to populate with drm modifiers, can be NULL
• • @pixel_formats_max: length of pixel formats array
• • Return: number of elements populated
• */

-uint32_t dpu_populate_formats(

• const struct dpu_format_extended *format_list,
  

• uint32_t *pixel_formats,
  

• uint64_t *pixel_modifiers,
  

• uint32_t pixel_formats_max);
  

/**

• dpu_format_check_modified_format - validate format and buffers for

•               dpu non-standard, i.e. modified format
  

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c index 512ac0834d2b..df6852cc98b9 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.c @@ -151,7 +151,9 @@ static const struct dpu_sspp_blks_common sdm845_sspp_common = { .id = DPU_SSPP_CSC_10BIT, \ .base = 0x1a00, .len = 0x100,}, \ .format_list = plane_formats_yuv, \

• .num_formats = ARRAY_SIZE(plane_formats_yuv), \ .virt_format_list = plane_formats, \
• .virt_num_formats = ARRAY_SIZE(plane_formats), \ }

#define _DMA_SBLK(num, sdma_pri) \ @@ -163,7 +165,9 @@ static const struct dpu_sspp_blks_common sdm845_sspp_common = { .src_blk = {.name = STRCAT("sspp_src_", num), \ .id = DPU_SSPP_SRC, .base = 0x00, .len = 0x150,}, \ .format_list = plane_formats, \

• .num_formats = ARRAY_SIZE(plane_formats), \ .virt_format_list = plane_formats, \
• .virt_num_formats = ARRAY_SIZE(plane_formats), \ }

static const struct dpu_sspp_sub_blks sdm845_vig_sblk_0 = _VIG_SBLK("0", 5); diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h index 144358a3d0fb..a55653b2e466 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog.h @@ -251,17 +251,6 @@ struct dpu_pp_blk { u32 version; };

-/**

• • struct dpu_format_extended - define dpu specific pixel format+modifier
• • @fourcc_format: Base FOURCC pixel format code
• • @modifier: 64-bit drm format modifier, same modifier must be applied to all

• •        framebuffer planes
    

• */

-struct dpu_format_extended {

• uint32_t fourcc_format;
• uint64_t modifier;

-};

/**

• enum dpu_qos_lut_usage - define QoS LUT use cases

*/ @@ -348,7 +337,9 @@ struct dpu_sspp_blks_common {

• @pcc_blk:
• @igc_blk:
• @format_list: Pointer to list of supported formats

• • @num_formats: Number of supported formats
  • @virt_format_list: Pointer to list of supported formats for virtual planes
• • @virt_num_formats: Number of supported formats for virtual planes
  */

struct dpu_sspp_sub_blks { const struct dpu_sspp_blks_common *common; @@ -366,8 +357,10 @@ struct dpu_sspp_sub_blks { struct dpu_pp_blk pcc_blk; struct dpu_pp_blk igc_blk;

• const struct dpu_format_extended *format_list;
• const struct dpu_format_extended *virt_format_list;

• const u32 *format_list;
• u32 num_formats;
• const u32 *virt_format_list;
• u32 virt_num_formats;

};

/** diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h index c37ba38ac83d..d09730985951 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_catalog_format.h @@ -12,84 +12,81 @@

#include "dpu_hw_mdss.h"

-static const struct dpu_format_extended plane_formats[] = {

• {DRM_FORMAT_ARGB8888, 0},
• {DRM_FORMAT_ABGR8888, 0},
• {DRM_FORMAT_RGBA8888, 0},
• {DRM_FORMAT_ABGR8888, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_BGRA8888, 0},
• {DRM_FORMAT_XRGB8888, 0},
• {DRM_FORMAT_RGBX8888, 0},
• {DRM_FORMAT_BGRX8888, 0},
• {DRM_FORMAT_XBGR8888, 0},
• {DRM_FORMAT_XBGR8888, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_RGB888, 0},
• {DRM_FORMAT_BGR888, 0},
• {DRM_FORMAT_RGB565, 0},
• {DRM_FORMAT_BGR565, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_BGR565, 0},
• {DRM_FORMAT_ARGB1555, 0},
• {DRM_FORMAT_ABGR1555, 0},
• {DRM_FORMAT_RGBA5551, 0},
• {DRM_FORMAT_BGRA5551, 0},
• {DRM_FORMAT_XRGB1555, 0},
• {DRM_FORMAT_XBGR1555, 0},
• {DRM_FORMAT_RGBX5551, 0},
• {DRM_FORMAT_BGRX5551, 0},
• {DRM_FORMAT_ARGB4444, 0},
• {DRM_FORMAT_ABGR4444, 0},
• {DRM_FORMAT_RGBA4444, 0},
• {DRM_FORMAT_BGRA4444, 0},
• {DRM_FORMAT_XRGB4444, 0},
• {DRM_FORMAT_XBGR4444, 0},
• {DRM_FORMAT_RGBX4444, 0},
• {DRM_FORMAT_BGRX4444, 0},
• {0, 0},

+static const uint32_t qcom_compressed_supported_formats[] = {

• DRM_FORMAT_ABGR8888,
• DRM_FORMAT_XBGR8888,
• DRM_FORMAT_BGR565,

};

-static const struct dpu_format_extended plane_formats_yuv[] = {

• {DRM_FORMAT_ARGB8888, 0},
• {DRM_FORMAT_ABGR8888, 0},
• {DRM_FORMAT_RGBA8888, 0},
• {DRM_FORMAT_BGRX8888, 0},
• {DRM_FORMAT_ABGR8888, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_BGRA8888, 0},
• {DRM_FORMAT_XRGB8888, 0},
• {DRM_FORMAT_XBGR8888, 0},
• {DRM_FORMAT_RGBX8888, 0},
• {DRM_FORMAT_XBGR8888, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_RGB888, 0},
• {DRM_FORMAT_BGR888, 0},
• {DRM_FORMAT_RGB565, 0},
• {DRM_FORMAT_BGR565, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_BGR565, 0},
• {DRM_FORMAT_ARGB1555, 0},
• {DRM_FORMAT_ABGR1555, 0},
• {DRM_FORMAT_RGBA5551, 0},
• {DRM_FORMAT_BGRA5551, 0},
• {DRM_FORMAT_XRGB1555, 0},
• {DRM_FORMAT_XBGR1555, 0},
• {DRM_FORMAT_RGBX5551, 0},
• {DRM_FORMAT_BGRX5551, 0},
• {DRM_FORMAT_ARGB4444, 0},
• {DRM_FORMAT_ABGR4444, 0},
• {DRM_FORMAT_RGBA4444, 0},
• {DRM_FORMAT_BGRA4444, 0},
• {DRM_FORMAT_XRGB4444, 0},
• {DRM_FORMAT_XBGR4444, 0},
• {DRM_FORMAT_RGBX4444, 0},
• {DRM_FORMAT_BGRX4444, 0},

+static const uint32_t plane_formats[] = {

• DRM_FORMAT_ARGB8888,
• DRM_FORMAT_ABGR8888,
• DRM_FORMAT_RGBA8888,
• DRM_FORMAT_BGRA8888,
• DRM_FORMAT_XRGB8888,
• DRM_FORMAT_RGBX8888,
• DRM_FORMAT_BGRX8888,
• DRM_FORMAT_XBGR8888,
• DRM_FORMAT_RGB888,
• DRM_FORMAT_BGR888,
• DRM_FORMAT_RGB565,
• DRM_FORMAT_BGR565,
• DRM_FORMAT_ARGB1555,
• DRM_FORMAT_ABGR1555,
• DRM_FORMAT_RGBA5551,
• DRM_FORMAT_BGRA5551,
• DRM_FORMAT_XRGB1555,
• DRM_FORMAT_XBGR1555,
• DRM_FORMAT_RGBX5551,
• DRM_FORMAT_BGRX5551,
• DRM_FORMAT_ARGB4444,
• DRM_FORMAT_ABGR4444,
• DRM_FORMAT_RGBA4444,
• DRM_FORMAT_BGRA4444,
• DRM_FORMAT_XRGB4444,
• DRM_FORMAT_XBGR4444,
• DRM_FORMAT_RGBX4444,
• DRM_FORMAT_BGRX4444,

+};

+static const uint32_t plane_formats_yuv[] = {

• DRM_FORMAT_ARGB8888,
• DRM_FORMAT_ABGR8888,
• DRM_FORMAT_RGBA8888,
• DRM_FORMAT_BGRX8888,
• DRM_FORMAT_BGRA8888,
• DRM_FORMAT_XRGB8888,
• DRM_FORMAT_XBGR8888,
• DRM_FORMAT_RGBX8888,
• DRM_FORMAT_RGB888,
• DRM_FORMAT_BGR888,
• DRM_FORMAT_RGB565,
• DRM_FORMAT_BGR565,
• DRM_FORMAT_ARGB1555,
• DRM_FORMAT_ABGR1555,
• DRM_FORMAT_RGBA5551,
• DRM_FORMAT_BGRA5551,
• DRM_FORMAT_XRGB1555,
• DRM_FORMAT_XBGR1555,
• DRM_FORMAT_RGBX5551,
• DRM_FORMAT_BGRX5551,
• DRM_FORMAT_ARGB4444,
• DRM_FORMAT_ABGR4444,
• DRM_FORMAT_RGBA4444,
• DRM_FORMAT_BGRA4444,
• DRM_FORMAT_XRGB4444,
• DRM_FORMAT_XBGR4444,
• DRM_FORMAT_RGBX4444,
• DRM_FORMAT_BGRX4444,

• {DRM_FORMAT_NV12, 0},
• {DRM_FORMAT_NV12, DRM_FORMAT_MOD_QCOM_COMPRESSED},
• {DRM_FORMAT_NV21, 0},
• {DRM_FORMAT_NV16, 0},
• {DRM_FORMAT_NV61, 0},
• {DRM_FORMAT_VYUY, 0},
• {DRM_FORMAT_UYVY, 0},
• {DRM_FORMAT_YUYV, 0},
• {DRM_FORMAT_YVYU, 0},
• {DRM_FORMAT_YUV420, 0},
• {DRM_FORMAT_YVU420, 0},
• {0, 0},

• 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,

}; diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h index 321fc64ddd0e..efe70c508ee0 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_hw_util.h @@ -18,7 +18,6 @@ #include "dpu_hw_mdss.h"

#define REG_MASK(n) ((BIT(n)) - 1) -struct dpu_format_extended;

/*

• This is the common struct maintained by each sub block

diff --git a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c index 688233dbd597..6a49c253bbdc 100644 --- a/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c +++ b/drivers/gpu/drm/msm/disp/dpu1/dpu_plane.c @@ -95,8 +95,6 @@ struct dpu_plane {

enum dpu_sspp pipe; uint32_t features; /* capabilities from catalog */

• uint32_t nformats;

• uint32_t formats[64];

  struct dpu_hw_pipe *pipe_hw; struct dpu_hw_pipe_cfg pipe_cfg;

@@ -1446,11 +1444,12 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, unsigned long possible_crtcs, u32 master_plane_id) { struct drm_plane *plane = NULL, *master_plane = NULL;

• const struct dpu_format_extended *format_list;

• const uint32_t *format_list; struct dpu_plane *pdpu; struct msm_drm_private *priv = dev->dev_private; struct dpu_kms *kms = to_dpu_kms(priv->kms); int zpos_max = DPU_ZPOS_MAX;

• uint32_t num_formats; int ret = -EINVAL;

  /* create and zero local structure */

@@ -1493,23 +1492,17 @@ struct drm_plane *dpu_plane_init(struct drm_device *dev, goto clean_sspp; }

• if (!master_plane_id)

• format_list = pdpu->pipe_sblk->format_list;
  

• else

• if (pdpu->is_virtual) { format_list = pdpu->pipe_sblk->virt_format_list;

• pdpu->nformats = dpu_populate_formats(format_list,

• 		pdpu->formats,
  

• 		0,
  

• 		ARRAY_SIZE(pdpu->formats));
  

• if (!pdpu->nformats) {

• DPU_ERROR("[%u]no valid formats for plane\n", pipe);
  

• goto clean_sspp;
  

• num_formats = pdpu->pipe_sblk->virt_num_formats;
  

• }

• else {

• format_list = pdpu->pipe_sblk->format_list;
  

• num_formats = pdpu->pipe_sblk->num_formats;
  

  }

  ret = drm_universal_plane_init(dev, plane, 0xff, &dpu_plane_funcs,

• 		pdpu->formats, pdpu->nformats,
  

• 		format_list, num_formats,
  	NULL, type, NULL);
  

  if (ret) goto clean_sspp;

-- 2.20.0.rc2.403.gdbc3b29805-goog