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 <s...@poorly.run>


> 
> Signed-off-by: Fritz Koenig <frkoe...@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
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

Reply via email to