diff mbox series

[PATCHv7,4/6] drm/arm/malidp: Allocate an afbc-specific drm_framebuffer

Message ID 20200311145541.29186-5-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 
Prepare for using generic afbc helpers.

Use an existing helper which allows allocating a struct drm_framebuffer
in the driver.

afbc-specific checks should go after drm_gem_fb_init_with_funcs().

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 drivers/gpu/drm/arm/malidp_drv.c | 50 +++++++++++++++++++++++++-------
 1 file changed, 39 insertions(+), 11 deletions(-)

Comments

Emil Velikov March 16, 2020, 2:05 p.m. UTC | #1 
On Wed, 11 Mar 2020 at 14:56, Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Prepare for using generic afbc helpers.
>
> Use an existing helper which allows allocating a struct drm_framebuffer
> in the driver.
>
> afbc-specific checks should go after drm_gem_fb_init_with_funcs().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/arm/malidp_drv.c | 50 +++++++++++++++++++++++++-------
>  1 file changed, 39 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
> index cafbd81e4c1e..b9715b19af94 100644
> --- a/drivers/gpu/drm/arm/malidp_drv.c
> +++ b/drivers/gpu/drm/arm/malidp_drv.c
> @@ -269,13 +269,36 @@ static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
>         .atomic_commit_tail = malidp_atomic_commit_tail,
>  };
>
> +static const struct drm_framebuffer_funcs malidp_fb_funcs = {
> +       .destroy        = drm_gem_fb_destroy,
> +       .create_handle  = drm_gem_fb_create_handle,
> +};
> +
>  static struct drm_framebuffer *
>  malidp_fb_create(struct drm_device *dev, struct drm_file *file,
>                  const struct drm_mode_fb_cmd2 *mode_cmd)
>  {
> +       struct drm_afbc_framebuffer *afbc_fb;
> +       const struct drm_format_info *info;
> +       int ret, i;
> +
> +       info = drm_get_format_info(dev, mode_cmd);
> +       if (!info)
> +               return ERR_PTR(-ENOMEM);
> +
The underlying implementation in drm_get_format_info() will throw a
WARN_ON() if we return NULL.
Returning ENOMEM here is misleading and EINVAL sounds better IMHO.

Regardless, of the above 1-5 are:
Reviewed-by: Emil Velikov <emil.velikov@collabora.com>

I don't know much about Rockchip HW to review 6/6.

-Emil
diff mbox series

Patch

diff --git a/drivers/gpu/drm/arm/malidp_drv.c b/drivers/gpu/drm/arm/malidp_drv.c
index cafbd81e4c1e..b9715b19af94 100644
--- a/drivers/gpu/drm/arm/malidp_drv.c
+++ b/drivers/gpu/drm/arm/malidp_drv.c
@@ -269,13 +269,36 @@  static const struct drm_mode_config_helper_funcs malidp_mode_config_helpers = {
 	.atomic_commit_tail = malidp_atomic_commit_tail,
 };
 
+static const struct drm_framebuffer_funcs malidp_fb_funcs = {
+	.destroy	= drm_gem_fb_destroy,
+	.create_handle	= drm_gem_fb_create_handle,
+};
+
 static struct drm_framebuffer *
 malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 		 const struct drm_mode_fb_cmd2 *mode_cmd)
 {
+	struct drm_afbc_framebuffer *afbc_fb;
+	const struct drm_format_info *info;
+	int ret, i;
+
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return ERR_PTR(-ENOMEM);
+
+	afbc_fb = kzalloc(sizeof(*afbc_fb), GFP_KERNEL);
+	if (!afbc_fb)
+		return ERR_PTR(-ENOMEM);
+
+	ret = drm_gem_fb_init_with_funcs(dev, &afbc_fb->base, file, mode_cmd,
+					 &malidp_fb_funcs);
+	if (ret) {
+		kfree(afbc_fb);
+		return ERR_PTR(-ENOMEM);
+	}
+
 	if (mode_cmd->modifier[0]) {
 		int n_superblocks = 0;
-		const struct drm_format_info *info;
 		struct drm_gem_object *objs = NULL;
 		u32 afbc_superblock_size = 0, afbc_superblock_height = 0;
 		u32 afbc_superblock_width = 0, afbc_size = 0;
@@ -283,23 +306,23 @@  malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 
 		if (malidp_format_mod_supported(dev, mode_cmd->pixel_format,
 						mode_cmd->modifier[0]) == false)
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 
 		if (mode_cmd->offsets[0] != 0) {
 			DRM_DEBUG_KMS("AFBC buffer plane offset should be 0\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
 		case AFBC_SIZE_16X16:
 			if ((mode_cmd->width % 16) || (mode_cmd->height % 16)) {
 				DRM_DEBUG_KMS("AFBC buffers must be aligned to 16 pixels\n");
-				return ERR_PTR(-EINVAL);
+				goto free_fb;
 			}
 			break;
 		default:
 			DRM_DEBUG_KMS("Unsupported AFBC block size\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		switch (mode_cmd->modifier[0] & AFBC_SIZE_MASK) {
@@ -309,10 +332,9 @@  malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 			break;
 		default:
 			DRM_DEBUG_KMS("AFBC superblock size not supported\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
-		info = drm_get_format_info(dev, mode_cmd);
 
 		n_superblocks = (mode_cmd->width / afbc_superblock_width) *
 			(mode_cmd->height / afbc_superblock_height);
@@ -333,26 +355,32 @@  malidp_fb_create(struct drm_device *dev, struct drm_file *file,
 			DRM_DEBUG_KMS("Invalid value of (pitch * BITS_PER_BYTE) (=%u) should be same as width (=%u) * bpp (=%u)\n",
 				      (mode_cmd->pitches[0] * BITS_PER_BYTE),
 				      mode_cmd->width, bpp);
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		objs = drm_gem_object_lookup(file, mode_cmd->handles[0]);
 		if (!objs) {
 			DRM_DEBUG_KMS("Failed to lookup GEM object\n");
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		if (objs->size < afbc_size) {
 			DRM_DEBUG_KMS("buffer size %zu/%u too small for AFBC\n",
 				      objs->size, afbc_size);
 			drm_gem_object_put_unlocked(objs);
-			return ERR_PTR(-EINVAL);
+			goto free_fb;
 		}
 
 		drm_gem_object_put_unlocked(objs);
 	}
 
-	return drm_gem_fb_create(dev, file, mode_cmd);
+	return &afbc_fb->base;
+
+free_fb:
+	for (i = 0; i < info->num_planes; ++i)
+		drm_gem_object_put_unlocked(afbc_fb->base.obj[i]);
+	kfree(afbc_fb);
+	return ERR_PTR(-EINVAL);
 }
 
 static const struct drm_mode_config_funcs malidp_mode_config_funcs = {