diff mbox series

[2/2] drm/core: Calculate bpp in afbc helper

Message ID 20200331155308.6345-3-andrzej.p@collabora.com (mailing list archive)
State New, archived
Headers show
Series AFBC fixes | expand

Commit Message

Andrzej Pietrasiewicz March 31, 2020, 3:53 p.m. UTC 
Some drivers (komeda, malidp) don't set anything in cpp. If that is the
case the right value can be inferred from the format. Then the "bpp" member
can be eliminated from struct drm_afbc_framebuffer.

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 Documentation/gpu/todo.rst                   | 15 --------
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 39 ++++++++++++++++----
 include/drm/drm_framebuffer.h                |  7 ----
 3 files changed, 32 insertions(+), 29 deletions(-)

Comments

Daniel Vetter April 1, 2020, 9:13 a.m. UTC | #1 
On Tue, Mar 31, 2020 at 05:53:08PM +0200, Andrzej Pietrasiewicz wrote:
> Some drivers (komeda, malidp) don't set anything in cpp. If that is the
> case the right value can be inferred from the format. Then the "bpp" member
> can be eliminated from struct drm_afbc_framebuffer.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Didn't check computations, but yup this matches what I had in mind.

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> ---
>  Documentation/gpu/todo.rst                   | 15 --------
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 39 ++++++++++++++++----
>  include/drm/drm_framebuffer.h                |  7 ----
>  3 files changed, 32 insertions(+), 29 deletions(-)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 37a3a023c114..439656f55c5d 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,21 +404,6 @@ Contact: Laurent Pinchart, respective driver maintainers
>  
>  Level: Intermediate
>  
> -Encode cpp properly in malidp
> ------------------------------
> -
> -cpp (chars per pixel) is not encoded properly in malidp, zero is
> -used instead. afbc implementation needs bpp or cpp, but if it is
> -zero it needs to be provided elsewhere, and so the bpp field exists
> -in struct drm_afbc_framebuffer.
> -
> -Properly encode cpp in malidp and remove the bpp field in struct
> -drm_afbc_framebuffer.
> -
> -Contact: malidp maintainers
> -
> -Level: Intermediate
> -
>  Core refactorings
>  =================
>  
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 6fb1841fa71c..cac15294aef6 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -309,11 +309,37 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev,
> +				  const struct drm_mode_fb_cmd2 *mode_cmd)
> +{
> +	const struct drm_format_info *info;
> +
> +	info = drm_get_format_info(dev, mode_cmd);
> +
> +	/* use whatever a driver has set */
> +	if (info->cpp[0])
> +		return info->cpp[0] * 8;
> +
> +	/* guess otherwise */
> +	switch (info->format) {
> +	case DRM_FORMAT_YUV420_8BIT:
> +		return 12;
> +	case DRM_FORMAT_YUV420_10BIT:
> +		return 15;
> +	case DRM_FORMAT_VUY101010:
> +		return 30;
> +	default:
> +		break;
> +	}
> +
> +	/* all attempts failed */
> +	return 0;
> +}
> +
>  static int drm_gem_afbc_min_size(struct drm_device *dev,
>  				 const struct drm_mode_fb_cmd2 *mode_cmd,
>  				 struct drm_afbc_framebuffer *afbc_fb)
>  {
> -	const struct drm_format_info *info;
>  	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
>  	/* remove bpp when all users properly encode cpp in drm_format_info */
>  	__u32 bpp;
> @@ -351,12 +377,11 @@ static int drm_gem_afbc_min_size(struct drm_device *dev,
>  	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
>  	afbc_fb->offset = mode_cmd->offsets[0];
>  
> -	info = drm_get_format_info(dev, mode_cmd);
> -	/*
> -	 * Change to always using info->cpp[0]
> -	 * when all users properly encode it
> -	 */
> -	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
> +	bpp = drm_gem_afbc_get_bpp(dev, mode_cmd);
> +	if (!bpp) {
> +		drm_dbg_kms(dev, "Invalid AFBC bpp value: %d\n", bpp);
> +		return -EINVAL;
> +	}
>  
>  	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
>  		   / AFBC_SUPERBLOCK_PIXELS;
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index b53c0332f040..be658ebbec72 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -331,13 +331,6 @@ struct drm_afbc_framebuffer {
>  	 * @afbc_size: minimum size of afbc buffer
>  	 */
>  	u32 afbc_size;
> -	/**
> -	 * @bpp: bpp value for this afbc buffer
> -	 * To be removed when users such as malidp
> -	 * properly store the cpp in drm_format_info.
> -	 * New users should not start using this field.
> -	 */
> -	u32 bpp;
>  };
>  
>  #define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)
> -- 
> 2.17.1
>
diff mbox series

Patch

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 37a3a023c114..439656f55c5d 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -404,21 +404,6 @@  Contact: Laurent Pinchart, respective driver maintainers
 
 Level: Intermediate
 
-Encode cpp properly in malidp
------------------------------
-
-cpp (chars per pixel) is not encoded properly in malidp, zero is
-used instead. afbc implementation needs bpp or cpp, but if it is
-zero it needs to be provided elsewhere, and so the bpp field exists
-in struct drm_afbc_framebuffer.
-
-Properly encode cpp in malidp and remove the bpp field in struct
-drm_afbc_framebuffer.
-
-Contact: malidp maintainers
-
-Level: Intermediate
-
 Core refactorings
 =================
 
diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
index 6fb1841fa71c..cac15294aef6 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -309,11 +309,37 @@  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
 
+static __u32 drm_gem_afbc_get_bpp(struct drm_device *dev,
+				  const struct drm_mode_fb_cmd2 *mode_cmd)
+{
+	const struct drm_format_info *info;
+
+	info = drm_get_format_info(dev, mode_cmd);
+
+	/* use whatever a driver has set */
+	if (info->cpp[0])
+		return info->cpp[0] * 8;
+
+	/* guess otherwise */
+	switch (info->format) {
+	case DRM_FORMAT_YUV420_8BIT:
+		return 12;
+	case DRM_FORMAT_YUV420_10BIT:
+		return 15;
+	case DRM_FORMAT_VUY101010:
+		return 30;
+	default:
+		break;
+	}
+
+	/* all attempts failed */
+	return 0;
+}
+
 static int drm_gem_afbc_min_size(struct drm_device *dev,
 				 const struct drm_mode_fb_cmd2 *mode_cmd,
 				 struct drm_afbc_framebuffer *afbc_fb)
 {
-	const struct drm_format_info *info;
 	__u32 n_blocks, w_alignment, h_alignment, hdr_alignment;
 	/* remove bpp when all users properly encode cpp in drm_format_info */
 	__u32 bpp;
@@ -351,12 +377,11 @@  static int drm_gem_afbc_min_size(struct drm_device *dev,
 	afbc_fb->aligned_height = ALIGN(mode_cmd->height, h_alignment);
 	afbc_fb->offset = mode_cmd->offsets[0];
 
-	info = drm_get_format_info(dev, mode_cmd);
-	/*
-	 * Change to always using info->cpp[0]
-	 * when all users properly encode it
-	 */
-	bpp = info->cpp[0] ? info->cpp[0] * 8 : afbc_fb->bpp;
+	bpp = drm_gem_afbc_get_bpp(dev, mode_cmd);
+	if (!bpp) {
+		drm_dbg_kms(dev, "Invalid AFBC bpp value: %d\n", bpp);
+		return -EINVAL;
+	}
 
 	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
 		   / AFBC_SUPERBLOCK_PIXELS;
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index b53c0332f040..be658ebbec72 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -331,13 +331,6 @@  struct drm_afbc_framebuffer {
 	 * @afbc_size: minimum size of afbc buffer
 	 */
 	u32 afbc_size;
-	/**
-	 * @bpp: bpp value for this afbc buffer
-	 * To be removed when users such as malidp
-	 * properly store the cpp in drm_format_info.
-	 * New users should not start using this field.
-	 */
-	u32 bpp;
 };
 
 #define fb_to_afbc_fb(x) container_of(x, struct drm_afbc_framebuffer, base)