Hi Andrzej:
Good work.
It's a real useful patch, with it seems most vendor-specific fb_create
can be simplified by these helper funcs.
On Fri, Dec 13, 2019 at 04:58:34PM +0100, Andrzej Pietrasiewicz wrote:
> Prepare tools for drivers which need to allocate a struct drm_framebuffer
> (or a container of struct drm_framebuffer) explicitly, before calling
> helpers. In such a case we need new helpers which omit allocating the
> struct drm_framebuffer and this patch provides them. Consequently, they
> are used also inside the helpers themselves.
>
> The interested drivers will likely need to be able to perform object
> lookups and size checks in separate invocations and this patch provides
> that as well. Helpers themselves are updated, too.
>
> Signed-off-by: Andrzej Pietrasiewicz <andrze...@collabora.com>
> ---
> drivers/gpu/drm/drm_gem_framebuffer_helper.c | 184 ++++++++++++++-----
> include/drm/drm_gem_framebuffer_helper.h | 17 ++
> 2 files changed, 153 insertions(+), 48 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index b9bcd310ca2d..787edb9a916b 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -54,6 +54,44 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct
> drm_framebuffer *fb,
> }
> EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
>
> +int drm_gem_fb_init_with_funcs(struct drm_framebuffer *fb,
> + struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **obj,
> + unsigned int num_planes,
> + const struct drm_framebuffer_funcs *funcs)
> +{
> + int ret, i;
> +
> + drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> +
> + for (i = 0; i < num_planes; i++)
> + fb->obj[i] = obj[i];
> +
> + ret = drm_framebuffer_init(dev, fb, funcs);
> + if (ret)
> + DRM_DEV_ERROR(dev->dev, "Failed to init framebuffer: %d\n",
> + ret);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
> +
> +static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
> + .destroy = drm_gem_fb_destroy,
> + .create_handle = drm_gem_fb_create_handle,
> +};
> +
> +int drm_gem_fb_init(struct drm_framebuffer *fb,
> + struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **obj, unsigned int num_planes)
> +{
> + return drm_gem_fb_init_with_funcs(fb, dev, mode_cmd, obj, num_planes,
> + &drm_gem_fb_funcs);
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_init);
> +
> static struct drm_framebuffer *
> drm_gem_fb_alloc(struct drm_device *dev,
> const struct drm_mode_fb_cmd2 *mode_cmd,
> @@ -61,21 +99,15 @@ drm_gem_fb_alloc(struct drm_device *dev,
> const struct drm_framebuffer_funcs *funcs)
> {
> struct drm_framebuffer *fb;
> - int ret, i;
> + int ret;
>
> fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> if (!fb)
> return ERR_PTR(-ENOMEM);
>
> - drm_helper_mode_fill_fb_struct(dev, fb, mode_cmd);
> -
> - for (i = 0; i < num_planes; i++)
> - fb->obj[i] = obj[i];
> -
> - ret = drm_framebuffer_init(dev, fb, funcs);
> + ret = drm_gem_fb_init_with_funcs(fb, dev, mode_cmd, obj, num_planes,
> + funcs);
> if (ret) {
> - DRM_DEV_ERROR(dev->dev, "Failed to init framebuffer: %d\n",
> - ret);
> kfree(fb);
> return ERR_PTR(ret);
> }
> @@ -124,79 +156,135 @@ int drm_gem_fb_create_handle(struct drm_framebuffer
> *fb, struct drm_file *file,
> EXPORT_SYMBOL(drm_gem_fb_create_handle);
>
> /**
> - * drm_gem_fb_create_with_funcs() - Helper function for the
> - * &drm_mode_config_funcs.fb_create
> - * callback
> + * drm_gem_fb_lookup() - Helper function for use in
> + * &drm_mode_config_funcs.fb_create implementations
> * @dev: DRM device
> * @file: DRM file that holds the GEM handle(s) backing the framebuffer
> * @mode_cmd: Metadata from the userspace framebuffer creation request
> - * @funcs: vtable to be used for the new framebuffer object
> *
> - * This function can be used to set &drm_framebuffer_funcs for drivers that
> need
> - * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
> - * change &drm_framebuffer_funcs. The function does buffer size validation.
> + * This function can be used to look up the objects for all planes.
> + * In case an error is returned all the objects are put by the
> + * function before returning.
> *
> * Returns:
> - * Pointer to a &drm_framebuffer on success or an error pointer on failure.
> + * Number of planes on success or a negative error code on failure.
> */
> -struct drm_framebuffer *
> -drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> - const struct drm_mode_fb_cmd2 *mode_cmd,
> - const struct drm_framebuffer_funcs *funcs)
> +int drm_gem_fb_lookup(struct drm_device *dev,
[nit-pick] Maybe name it to drm_gem_fb_objs_lookup()
> + struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **objs)
> {
> const struct drm_format_info *info;
> - struct drm_gem_object *objs[4];
> - struct drm_framebuffer *fb;
> int ret, i;
>
> info = drm_get_format_info(dev, mode_cmd);
> if (!info)
> - return ERR_PTR(-EINVAL);
> + return -EINVAL;
>
> for (i = 0; i < info->num_planes; i++) {
> - unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> - unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> - unsigned int min_size;
> -
> objs[i] = drm_gem_object_lookup(file, mode_cmd->handles[i]);
> if (!objs[i]) {
> DRM_DEBUG_KMS("Failed to lookup GEM object\n");
> ret = -ENOENT;
> goto err_gem_object_put;
> }
> + }
> +
> + return i;
> +
> +err_gem_object_put:
> + for (i--; i >= 0; i--)
> + drm_gem_object_put_unlocked(objs[i]);
> +
> + return ret;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
> +
> +/**
> + * drm_gem_fb_size_check() - Helper function for use in
> + * &drm_mode_config_funcs.fb_create implementations
> + * @dev: DRM device
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + *
> + * This function can be used to verify buffer sizes for all planes.
> + * It is caller's responsibility to put the objects on failure.
> + *
> + * Returns:
> + * Zero on success or a negative error code on failure.
> + */
> +int drm_gem_fb_size_check(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **objs)
> +{
> + const struct drm_format_info *info;
> + int i;
> +
> + info = drm_get_format_info(dev, mode_cmd);
> + if (!info)
> + return -EINVAL;
> +
> + for (i = 0; i < info->num_planes; i++) {
> + unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
> + unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
> + unsigned int min_size;
>
> min_size = (height - 1) * mode_cmd->pitches[i]
> + drm_format_info_min_pitch(info, i, width)
> + mode_cmd->offsets[i];
>
> - if (objs[i]->size < min_size) {
> - drm_gem_object_put_unlocked(objs[i]);
> - ret = -EINVAL;
> - goto err_gem_object_put;
> - }
> + if (objs[i]->size < min_size)
> + return -EINVAL;
> }
>
> - fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
> - if (IS_ERR(fb)) {
> - ret = PTR_ERR(fb);
> - goto err_gem_object_put;
> - }
> + return 0;
>
> - return fb;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_size_check);
>
> -err_gem_object_put:
> - for (i--; i >= 0; i--)
> - drm_gem_object_put_unlocked(objs[i]);
> +/**
> + * drm_gem_fb_create_with_funcs() - Helper function for the
> + * &drm_mode_config_funcs.fb_create
> + * callback
> + * @dev: DRM device
> + * @file: DRM file that holds the GEM handle(s) backing the framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @funcs: vtable to be used for the new framebuffer object
> + *
> + * This function can be used to set &drm_framebuffer_funcs for drivers that
> need
> + * custom framebuffer callbacks. Use drm_gem_fb_create() if you don't need to
> + * change &drm_framebuffer_funcs. The function does buffer size validation.
> + *
> + * Returns:
> + * Pointer to a &drm_framebuffer on success or an error pointer on failure.
> + */
> +struct drm_framebuffer *
> +drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + const struct drm_framebuffer_funcs *funcs)
> +{
> + struct drm_gem_object *objs[4];
> + struct drm_framebuffer *fb;
> + int ret, num_planes;
> +
> + ret = drm_gem_fb_lookup(dev, file, mode_cmd, objs);
> + if (ret < 0)
> + return ERR_PTR(ret);
> + num_planes = ret;
> +
> + ret = drm_gem_fb_size_check(dev, mode_cmd, objs);
> + if (ret)
> + fb = ERR_PTR(ret);
> + else
> + fb = drm_gem_fb_alloc(dev, mode_cmd, objs, num_planes, funcs);
>
> - return ERR_PTR(ret);
> + if (IS_ERR(fb))
> + for (num_planes--; num_planes >= 0; num_planes--)
> + drm_gem_object_put_unlocked(objs[num_planes]);
> +
> + return fb;
> }
> EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
>
> -static const struct drm_framebuffer_funcs drm_gem_fb_funcs = {
> - .destroy = drm_gem_fb_destroy,
> - .create_handle = drm_gem_fb_create_handle,
> -};
> -
> /**
> * drm_gem_fb_create() - Helper function for the
> * &drm_mode_config_funcs.fb_create callback
> diff --git a/include/drm/drm_gem_framebuffer_helper.h
> b/include/drm/drm_gem_framebuffer_helper.h
> index d9f13fd25b0a..c85d4b152e91 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -14,10 +14,27 @@ struct drm_simple_display_pipe;
>
> struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> unsigned int plane);
> +int drm_gem_fb_init_with_funcs(struct drm_framebuffer *fb,
> + struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **obj,
> + unsigned int num_planes,
> + const struct drm_framebuffer_funcs *funcs);
> +int drm_gem_fb_init(struct drm_framebuffer *fb,
> + struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **obj, unsigned int num_planes);
> void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> int drm_gem_fb_create_handle(struct drm_framebuffer *fb, struct drm_file
> *file,
> unsigned int *handle);
>
> +int drm_gem_fb_lookup(struct drm_device *dev,
> + struct drm_file *file,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **objs);
> +int drm_gem_fb_size_check(struct drm_device *dev,
> + const struct drm_mode_fb_cmd2 *mode_cmd,
> + struct drm_gem_object **objs);
> struct drm_framebuffer *
> drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
> const struct drm_mode_fb_cmd2 *mode_cmd,
Reviewed-by: James Qian Wang (Arm Technology China) <james.qian.w...@arm.com>
James.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
