[PATCH v2 1/5] drm: Define DRM_FORMAT_MAX_PLANES
Thomas Zimmermann
tzimmermann at suse.de
Mon Jul 26 18:03:17 UTC 2021
Hi
Am 25.07.21 um 21:49 schrieb Sam Ravnborg:
> Hi Thomas,
>
> On Sun, Jul 25, 2021 at 07:44:34PM +0200, Thomas Zimmermann wrote:
>> DRM uses a magic number of 4 for the maximum number of planes per color
>> format. Declare this constant via DRM_FORMAT_MAX_PLANES and update the
>> related code. Some code depends on the length of arrays that are now
>> declared with DRM_FORMAT_MAX_PLANES. Convert it from '4' to ARRAY_SIZE.
>>
>> v2:
>> * mention usage of ARRAY_SIZE() in the commit message (Maxime)
>> * also fix error handling in drm_gem_fb_init_with_funcs()
>> (kernel test robot)
>> * include <drm/drm_fourcc.h> for DRM_FORMAT_MAX_PLANES
>>
>> Signed-off-by: Thomas Zimmermann <tzimmermann at suse.de>
>
> One nit below.
> Reviewed-by: Sam Ravnborg <sam at ravnborg.org>
Thanks a lot for your reviews.
>
>> ---
>> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 19 +++++++++++--------
>> include/drm/drm_fourcc.h | 13 +++++++++----
>> include/drm/drm_framebuffer.h | 8 ++++----
>> include/drm/drm_gem_atomic_helper.h | 3 ++-
>> 4 files changed, 26 insertions(+), 17 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> index 67bc9edc1d98..421e029a6b3e 100644
>> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
>> @@ -48,7 +48,7 @@
>> struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>> unsigned int plane)
>> {
>> - if (plane >= 4)
>> + if (plane >= ARRAY_SIZE(fb->obj))
>> return NULL;
>>
>> return fb->obj[plane];
>> @@ -62,7 +62,8 @@ drm_gem_fb_init(struct drm_device *dev,
>> struct drm_gem_object **obj, unsigned int num_planes,
>> const struct drm_framebuffer_funcs *funcs)
>> {
>> - int ret, i;
>> + unsigned int i;
>> + int ret;
>>
>> drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
>>
> This change looks accidental/unrelated.
> But I guess it is to be consistent in use of unsigned int when
> iterating the array.
Indeed. The current code uses a mixture of signed and unsigned types in
several places. DRM_FORMAT_MAX_PLANES is defined as unsigned, so I
updated all the related code accordingly.
Best regards
Thomas
>
>> @@ -86,9 +87,9 @@ drm_gem_fb_init(struct drm_device *dev,
>> */
>> void drm_gem_fb_destroy(struct drm_framebuffer *fb)
>> {
>> - int i;
>> + size_t i;
>>
>> - for (i = 0; i < 4; i++)
>> + for (i = 0; i < ARRAY_SIZE(fb->obj); i++)
>> drm_gem_object_put(fb->obj[i]);
>>
>> drm_framebuffer_cleanup(fb);
>> @@ -145,8 +146,9 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>> const struct drm_framebuffer_funcs *funcs)
>> {
>> const struct drm_format_info *info;
>> - struct drm_gem_object *objs[4];
>> - int ret, i;
>> + struct drm_gem_object *objs[DRM_FORMAT_MAX_PLANES];
>> + unsigned int i;
>> + int ret;
>>
>> info = drm_get_format_info(dev, mode_cmd);
>> if (!info) {
>> @@ -187,9 +189,10 @@ int drm_gem_fb_init_with_funcs(struct drm_device *dev,
>> return 0;
>>
>> err_gem_object_put:
>> - for (i--; i >= 0; i--)
>> + while (i > 0) {
>> + --i;
>> drm_gem_object_put(objs[i]);
>> -
>> + }
>> return ret;
>> }
>> EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
>> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
>> index 3b138d4ae67e..22aa64d07c79 100644
>> --- a/include/drm/drm_fourcc.h
>> +++ b/include/drm/drm_fourcc.h
>> @@ -25,6 +25,11 @@
>> #include <linux/types.h>
>> #include <uapi/drm/drm_fourcc.h>
>>
>> +/**
>> + * DRM_FORMAT_MAX_PLANES - maximum number of planes a DRM format can have
>> + */
>> +#define DRM_FORMAT_MAX_PLANES 4u
>> +
>> /*
>> * DRM formats are little endian. Define host endian variants for the
>> * most common formats here, to reduce the #ifdefs needed in drivers.
>> @@ -78,7 +83,7 @@ struct drm_format_info {
>> * triplet @char_per_block, @block_w, @block_h for better
>> * describing the pixel format.
>> */
>> - u8 cpp[4];
>> + u8 cpp[DRM_FORMAT_MAX_PLANES];
>>
>> /**
>> * @char_per_block:
>> @@ -104,7 +109,7 @@ struct drm_format_info {
>> * information from their drm_mode_config.get_format_info hook
>> * if they want the core to be validating the pitch.
>> */
>> - u8 char_per_block[4];
>> + u8 char_per_block[DRM_FORMAT_MAX_PLANES];
>> };
>>
>> /**
>> @@ -113,7 +118,7 @@ struct drm_format_info {
>> * Block width in pixels, this is intended to be accessed through
>> * drm_format_info_block_width()
>> */
>> - u8 block_w[4];
>> + u8 block_w[DRM_FORMAT_MAX_PLANES];
>>
>> /**
>> * @block_h:
>> @@ -121,7 +126,7 @@ struct drm_format_info {
>> * Block height in pixels, this is intended to be accessed through
>> * drm_format_info_block_height()
>> */
>> - u8 block_h[4];
>> + u8 block_h[DRM_FORMAT_MAX_PLANES];
>>
>> /** @hsub: Horizontal chroma subsampling factor */
>> u8 hsub;
>> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
>> index be658ebbec72..f67c5b7bcb68 100644
>> --- a/include/drm/drm_framebuffer.h
>> +++ b/include/drm/drm_framebuffer.h
>> @@ -27,12 +27,12 @@
>> #include <linux/list.h>
>> #include <linux/sched.h>
>>
>> +#include <drm/drm_fourcc.h>
>> #include <drm/drm_mode_object.h>
>>
>> struct drm_clip_rect;
>> struct drm_device;
>> struct drm_file;
>> -struct drm_format_info;
>> struct drm_framebuffer;
>> struct drm_gem_object;
>>
>> @@ -147,7 +147,7 @@ struct drm_framebuffer {
>> * @pitches: Line stride per buffer. For userspace created object this
>> * is copied from drm_mode_fb_cmd2.
>> */
>> - unsigned int pitches[4];
>> + unsigned int pitches[DRM_FORMAT_MAX_PLANES];
>> /**
>> * @offsets: Offset from buffer start to the actual pixel data in bytes,
>> * per buffer. For userspace created object this is copied from
>> @@ -165,7 +165,7 @@ struct drm_framebuffer {
>> * data (even for linear buffers). Specifying an x/y pixel offset is
>> * instead done through the source rectangle in &struct drm_plane_state.
>> */
>> - unsigned int offsets[4];
>> + unsigned int offsets[DRM_FORMAT_MAX_PLANES];
>> /**
>> * @modifier: Data layout modifier. This is used to describe
>> * tiling, or also special layouts (like compression) of auxiliary
>> @@ -210,7 +210,7 @@ struct drm_framebuffer {
>> * This is used by the GEM framebuffer helpers, see e.g.
>> * drm_gem_fb_create().
>> */
>> - struct drm_gem_object *obj[4];
>> + struct drm_gem_object *obj[DRM_FORMAT_MAX_PLANES];
>> };
>>
>> #define obj_to_fb(x) container_of(x, struct drm_framebuffer, base)
>> diff --git a/include/drm/drm_gem_atomic_helper.h b/include/drm/drm_gem_atomic_helper.h
>> index d82c23622156..f9f8b6f0494a 100644
>> --- a/include/drm/drm_gem_atomic_helper.h
>> +++ b/include/drm/drm_gem_atomic_helper.h
>> @@ -5,6 +5,7 @@
>>
>> #include <linux/dma-buf-map.h>
>>
>> +#include <drm/drm_fourcc.h>
>> #include <drm/drm_plane.h>
>>
>> struct drm_simple_display_pipe;
>> @@ -40,7 +41,7 @@ struct drm_shadow_plane_state {
>> * The memory mappings stored in map should be established in the plane's
>> * prepare_fb callback and removed in the cleanup_fb callback.
>> */
>> - struct dma_buf_map map[4];
>> + struct dma_buf_map map[DRM_FORMAT_MAX_PLANES];
>> };
>>
>> /**
>> --
>> 2.32.0
--
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Felix Imendörffer
-------------- next part --------------
A non-text attachment was scrubbed...
Name: OpenPGP_signature
Type: application/pgp-signature
Size: 840 bytes
Desc: OpenPGP digital signature
URL: <https://lists.freedesktop.org/archives/dri-devel/attachments/20210726/553d076a/attachment-0001.sig>
More information about the dri-devel
mailing list