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

[Daniel Vetter]

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 at collabora.com>

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

Acked-by: Daniel Vetter <daniel.vetter at 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
> 

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch