[PATCHv7 1/6] drm/core: Allow drivers allocate a subclass of struct drm_framebuffer

james qian wang (Arm Technology China) james.qian.wang at arm.com
Tue Mar 17 03:08:39 UTC 2020


On Wed, Mar 11, 2020 at 10:55:36PM +0800, Andrzej Pietrasiewicz wrote:
> Allow allocating a specialized version of struct drm_framebuffer
> by moving the actual fb allocation out of drm_gem_fb_create_with_funcs();
> the respective functions names are adjusted to reflect that fact.
> Please note, though, that standard size checks are performed on buffers,
> so the drm_gem_fb_init_with_funcs() is useful for cases where those
> standard size checks are appropriate or at least don't conflict the
> checks to be performed in the specialized case.
> 
> Thanks to this change the drivers can call drm_gem_fb_init_with_funcs()
> having allocated their special version of struct drm_framebuffer, exactly
> the way the new version of drm_gem_fb_create_with_funcs() does.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p at collabora.com>

Looks good to me. :)

Reviewed-by: James Qian Wang <james.qian.wang at arm.com>

Thanks
James

> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 87 ++++++++++++++------
>  include/drm/drm_gem_framebuffer_helper.h     |  5 ++
>  2 files changed, 67 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 3a7ace19a902..86c1907c579a 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -54,19 +54,15 @@ struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_get_obj);
>  
> -static struct drm_framebuffer *
> -drm_gem_fb_alloc(struct drm_device *dev,
> +static int
> +drm_gem_fb_init(struct drm_device *dev,
> +		 struct drm_framebuffer *fb,
>  		 const struct drm_mode_fb_cmd2 *mode_cmd,
>  		 struct drm_gem_object **obj, unsigned int num_planes,
>  		 const struct drm_framebuffer_funcs *funcs)
>  {
> -	struct drm_framebuffer *fb;
>  	int ret, i;
>  
> -	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++)
> @@ -76,10 +72,9 @@ drm_gem_fb_alloc(struct drm_device *dev,
>  	if (ret) {
>  		drm_err(dev, "Failed to init framebuffer: %d\n", ret);
>  		kfree(fb);
> -		return ERR_PTR(ret);
>  	}
>  
> -	return fb;
> +	return ret;
>  }
>  
>  /**
> @@ -123,10 +118,13 @@ 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_init_with_funcs() - Helper function for implementing
> + *				  &drm_mode_config_funcs.fb_create
> + *				  callback in cases when the driver
> + *				  allocates a subclass of
> + *				  struct drm_framebuffer
>   * @dev: DRM device
> + * @fb: framebuffer object
>   * @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
> @@ -134,23 +132,26 @@ EXPORT_SYMBOL(drm_gem_fb_create_handle);
>   * 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.
> + * The buffer size validation is for a general case, though, so users should
> + * pay attention to the checks being appropriate for them or, at least,
> + * non-conflicting.
>   *
>   * Returns:
> - * Pointer to a &drm_framebuffer on success or an error pointer on failure.
> + * Zero or a negative error code.
>   */
> -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_init_with_funcs(struct drm_device *dev,
> +			       struct drm_framebuffer *fb,
> +			       struct drm_file *file,
> +			       const struct drm_mode_fb_cmd2 *mode_cmd,
> +			       const struct drm_framebuffer_funcs *funcs)
>  {
>  	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);
> @@ -175,19 +176,55 @@ drm_gem_fb_create_with_funcs(struct drm_device *dev, struct drm_file *file,
>  		}
>  	}
>  
> -	fb = drm_gem_fb_alloc(dev, mode_cmd, objs, i, funcs);
> -	if (IS_ERR(fb)) {
> -		ret = PTR_ERR(fb);
> +	ret = drm_gem_fb_init(dev, fb, mode_cmd, objs, i, funcs);
> +	if (ret)
>  		goto err_gem_object_put;
> -	}
>  
> -	return fb;
> +	return 0;
>  
>  err_gem_object_put:
>  	for (i--; i >= 0; i--)
>  		drm_gem_object_put_unlocked(objs[i]);
>  
> -	return ERR_PTR(ret);
> +	return ret;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_init_with_funcs);
> +
> +/**
> + * 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_framebuffer *fb;
> +	int ret;
> +
> +	fb = kzalloc(sizeof(*fb), GFP_KERNEL);
> +	if (!fb)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ret = drm_gem_fb_init_with_funcs(dev, fb, file, mode_cmd, funcs);
> +	if (ret) {
> +		kfree(fb);
> +		return ERR_PTR(ret);
> +	}
> +
> +	return fb;
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_funcs);
>  
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index d9f13fd25b0a..c029c1618661 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -18,6 +18,11 @@ 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_init_with_funcs(struct drm_device *dev,
> +			       struct drm_framebuffer *fb,
> +			       struct drm_file *file,
> +			       const struct drm_mode_fb_cmd2 *mode_cmd,
> +			       const struct drm_framebuffer_funcs *funcs);
>  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,
> -- 
> 2.17.1


More information about the dri-devel mailing list