| Message ID | 20210715180133.3675-2-tzimmermann@suse.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm: Provide framebuffer vmap helpers | expand |
Hi, On Thu, Jul 15, 2021 at 08:01:29PM +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. > > Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> > --- > drivers/gpu/drm/drm_gem_framebuffer_helper.c | 14 ++++++++------ > include/drm/drm_fourcc.h | 13 +++++++++---- > include/drm/drm_framebuffer.h | 8 ++++---- > include/drm/drm_gem_atomic_helper.h | 2 +- > 4 files changed, 22 insertions(+), 15 deletions(-) > > diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c > index e2c68822e05c..975a3df0561e 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; This doesn't look related to what's mentionned in the commit log though? > 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); > > @@ -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]); Ditto Both these changes look fine though, but I guess you should just mention it Maxime
Hi Am 16.07.21 um 09:30 schrieb Maxime Ripard: > Hi, > > On Thu, Jul 15, 2021 at 08:01:29PM +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. >> >> Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> >> --- >> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 14 ++++++++------ >> include/drm/drm_fourcc.h | 13 +++++++++---- >> include/drm/drm_framebuffer.h | 8 ++++---- >> include/drm/drm_gem_atomic_helper.h | 2 +- >> 4 files changed, 22 insertions(+), 15 deletions(-) >> >> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c >> index e2c68822e05c..975a3df0561e 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; > > This doesn't look related to what's mentionned in the commit log though? > >> 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); >> >> @@ -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]); > > Ditto > > Both these changes look fine though, but I guess you should just mention it Well, good point. I thought it would be cleaner than using FORMAT_MAX_PLANES here. I'll leave a note in the commit log. Best regards Thomas > > Maxime >
Hi Thomas,
I love your patch! Perhaps something to improve:
[auto build test WARNING on 4d00e2309398147acdbfefbe1deb4b0e78868466]
url: https://github.com/0day-ci/linux/commits/Thomas-Zimmermann/drm-Provide-framebuffer-vmap-helpers/20210716-020508
base: 4d00e2309398147acdbfefbe1deb4b0e78868466
config: x86_64-randconfig-m001-20210718 (attached as .config)
compiler: gcc-10 (Ubuntu 10.3.0-1ubuntu1~20.04) 10.3.0
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
smatch warnings:
drivers/gpu/drm/drm_gem_framebuffer_helper.c:192 drm_gem_fb_init_with_funcs() warn: always true condition '(i >= 0) => (0-u32max >= 0)'
drivers/gpu/drm/drm_gem_framebuffer_helper.c:192 drm_gem_fb_init_with_funcs() warn: always true condition '(i >= 0) => (0-u32max >= 0)'
vim +192 drivers/gpu/drm/drm_gem_framebuffer_helper.c
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 119
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 120 /**
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 121 * drm_gem_fb_init_with_funcs() - Helper function for implementing
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 122 * &drm_mode_config_funcs.fb_create
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 123 * callback in cases when the driver
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 124 * allocates a subclass of
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 125 * struct drm_framebuffer
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 126 * @dev: DRM device
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 127 * @fb: framebuffer object
2e187b2099034a Noralf Trønnes 2017-09-22 128 * @file: DRM file that holds the GEM handle(s) backing the framebuffer
2e187b2099034a Noralf Trønnes 2017-09-22 129 * @mode_cmd: Metadata from the userspace framebuffer creation request
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 130 * @funcs: vtable to be used for the new framebuffer object
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 131 *
dbd62e16fd53d3 Noralf Trønnes 2019-01-15 132 * This function can be used to set &drm_framebuffer_funcs for drivers that need
dbd62e16fd53d3 Noralf Trønnes 2019-01-15 133 * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
dbd62e16fd53d3 Noralf Trønnes 2019-01-15 134 * change &drm_framebuffer_funcs. The function does buffer size validation.
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 135 * The buffer size validation is for a general case, though, so users should
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 136 * pay attention to the checks being appropriate for them or, at least,
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 137 * non-conflicting.
2e187b2099034a Noralf Trønnes 2017-09-22 138 *
2e187b2099034a Noralf Trønnes 2017-09-22 139 * Returns:
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 140 * Zero or a negative error code.
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 141 */
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 142 int drm_gem_fb_init_with_funcs(struct drm_device *dev,
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 143 struct drm_framebuffer *fb,
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 144 struct drm_file *file,
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 145 const struct drm_mode_fb_cmd2 *mode_cmd,
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 146 const struct drm_framebuffer_funcs *funcs)
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 147 {
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 148 const struct drm_format_info *info;
6065e7036e073e Thomas Zimmermann 2021-07-15 149 struct drm_gem_object *objs[DRM_FORMAT_MAX_PLANES];
6065e7036e073e Thomas Zimmermann 2021-07-15 150 unsigned int i;
6065e7036e073e Thomas Zimmermann 2021-07-15 151 int ret;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 152
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 153 info = drm_get_format_info(dev, mode_cmd);
f7f525030854b1 Simon Ser 2021-05-03 154 if (!info) {
f7f525030854b1 Simon Ser 2021-05-03 155 drm_dbg_kms(dev, "Failed to get FB format info\n");
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 156 return -EINVAL;
f7f525030854b1 Simon Ser 2021-05-03 157 }
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 158
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 159 for (i = 0; i < info->num_planes; i++) {
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 160 unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 161 unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 162 unsigned int min_size;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 163
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 164 objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 165 if (!objs[i]) {
24f03be4aa7922 Jani Nikula 2019-12-10 166 drm_dbg_kms(dev, "Failed to lookup GEM object\n");
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 167 ret = -ENOENT;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 168 goto err_gem_object_put;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 169 }
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 170
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 171 min_size = (height - 1) * mode_cmd->pitches[i]
042bf753842ddb Alexandru Gheorghe 2018-11-01 172 + drm_format_info_min_pitch(info, i, width)
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 173 + mode_cmd->offsets[i];
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 174
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 175 if (objs[i]->size < min_size) {
f7f525030854b1 Simon Ser 2021-05-03 176 drm_dbg_kms(dev,
f7f525030854b1 Simon Ser 2021-05-03 177 "GEM object size (%zu) smaller than minimum size (%u) for plane %d\n",
f7f525030854b1 Simon Ser 2021-05-03 178 objs[i]->size, min_size, i);
be6ee102341bc4 Emil Velikov 2020-05-15 179 drm_gem_object_put(objs[i]);
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 180 ret = -EINVAL;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 181 goto err_gem_object_put;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 182 }
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 183 }
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 184
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 185 ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 186 if (ret)
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 187 goto err_gem_object_put;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 188
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 189 return 0;
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 190
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 191 err_gem_object_put:
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 @192 for (i--; i >= 0; i--)
be6ee102341bc4 Emil Velikov 2020-05-15 193 drm_gem_object_put(objs[i]);
4c3dbb2c312c9f Noralf Trønnes 2017-08-13 194
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 195 return ret;
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 196 }
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 197 EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
f2b816d78a9431 Andrzej Pietrasiewicz 2020-03-11 198
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c index e2c68822e05c..975a3df0561e 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); @@ -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) { 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..b2b441361051 100644 --- a/include/drm/drm_gem_atomic_helper.h +++ b/include/drm/drm_gem_atomic_helper.h @@ -40,7 +40,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]; }; /**
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. Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de> --- drivers/gpu/drm/drm_gem_framebuffer_helper.c | 14 ++++++++------ include/drm/drm_fourcc.h | 13 +++++++++---- include/drm/drm_framebuffer.h | 8 ++++---- include/drm/drm_gem_atomic_helper.h | 2 +- 4 files changed, 22 insertions(+), 15 deletions(-)