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@suse.de
One nit below. Reviewed-by: Sam Ravnborg sam@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;
- } 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