diff mbox series

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

Message ID 20200311145541.29186-2-andrzej.p@collabora.com (mailing list archive)
State New, archived
Headers show
Series Add AFBC support for Rockchip | expand

Commit Message

Andrzej Pietrasiewicz March 11, 2020, 2:55 p.m. UTC 
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@collabora.com>
---
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 87 ++++++++++++++------
 include/drm/drm_gem_framebuffer_helper.h     |  5 ++
 2 files changed, 67 insertions(+), 25 deletions(-)

Comments

James Qian Wang March 17, 2020, 3:08 a.m. UTC | #1 
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@collabora.com>

Looks good to me. :)

Reviewed-by: James Qian Wang <james.qian.wang@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
diff mbox series

Patch

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,