[PATCHv7,2/6] drm/core: Add drm_afbc_framebuffer and a corresponding helper
diff mbox series

Message ID 20200311145541.29186-3-andrzej.p@collabora.com
State New
Headers show
Series
  • Add AFBC support for Rockchip
Related show

Commit Message

Andrzej Pietrasiewicz March 11, 2020, 2:55 p.m. UTC
The new struct contains afbc-specific data.

The new function can be used by drivers which support afbc to complete
the preparation of struct drm_afbc_framebuffer. It must be called after
allocating the said struct and calling drm_gem_fb_init_with_funcs().

Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
---
 Documentation/gpu/todo.rst                   |  15 +++
 drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
 include/drm/drm_framebuffer.h                |  45 ++++++++
 include/drm/drm_gem_framebuffer_helper.h     |  10 ++
 4 files changed, 178 insertions(+)

Comments

james qian wang (Arm Technology China) March 17, 2020, 3:15 a.m. UTC | #1
On Wed, Mar 11, 2020 at 10:55:37PM +0800, Andrzej Pietrasiewicz wrote:
> The new struct contains afbc-specific data.
> 
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
> 
> 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
> ---
>  Documentation/gpu/todo.rst                   |  15 +++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
>  include/drm/drm_framebuffer.h                |  45 ++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  10 ++
>  4 files changed, 178 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ 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 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_simple_kms_helper.h>
>  
> +#define AFBC_HEADER_SIZE		16
> +#define AFBC_TH_LAYOUT_ALIGNMENT	8
> +#define AFBC_HDR_ALIGN			64
> +#define AFBC_SUPERBLOCK_PIXELS		256
> +#define AFBC_SUPERBLOCK_ALIGNMENT	128
> +#define AFBC_TH_BODY_START_ALIGNMENT	4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +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;
> +
> +	switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> +		afbc_fb->block_width = 16;
> +		afbc_fb->block_height = 16;
> +		break;
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> +		afbc_fb->block_width = 32;
> +		afbc_fb->block_height = 8;
> +		break;
> +	/* no user exists yet - fall through */
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> +	default:
> +		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +			      mode_cmd->modifier[0]
> +			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +		return -EINVAL;
> +	}
> +
> +	/* tiled header afbc */
> +	w_alignment = afbc_fb->block_width;
> +	h_alignment = afbc_fb->block_height;
> +	hdr_alignment = AFBC_HDR_ALIGN;
> +	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +		w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> +	}
> +
> +	afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> +	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;
> +
> +	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> +		   / AFBC_SUPERBLOCK_PIXELS;
> +	afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> +	afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +					       AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +	return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *			    fill and validate all the afbc-specific
> + *			    struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb)
> +{
> +	const struct drm_format_info *info;
> +	struct drm_gem_object **objs;
> +	int ret;
> +
> +	objs = afbc_fb->base.obj;
> +	info = drm_get_format_info(dev, mode_cmd);
> +	if (!info)
> +		return -EINVAL;
> +
> +	ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (objs[0]->size < afbc_fb->afbc_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
>   * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
>  int drm_framebuffer_plane_height(int height,
>  				 const struct drm_framebuffer *fb, int plane);
>  
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> +	/**
> +	 * @base: base framebuffer structure.
> +	 */
> +	struct drm_framebuffer base;
> +	/**
> +	 * @block_widht: width of a single afbc block
> +	 */
> +	u32 block_width;
> +	/**
> +	 * @block_widht: height of a single afbc block
> +	 */
> +	u32 block_height;
> +	/**
> +	 * @aligned_width: aligned frame buffer width
> +	 */
> +	u32 aligned_width;
> +	/**
> +	 * @aligned_height: aligned frame buffer height
> +	 */
> +	u32 aligned_height;
> +	/**
> +	 * @offset: offset of the first afbc header
> +	 */
> +	u32 offset;
> +	/**
> +	 * @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)
> +
>  #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>  
> +struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
>  
> +#define AFBC_VENDOR_AND_TYPE_MASK	GENMASK_ULL(63, 52)
> +
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  					  unsigned int plane);
>  void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  			     const struct drm_mode_fb_cmd2 *mode_cmd);
>  
> +#define drm_is_afbc(modifier) \
> +	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb);
> +
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state);
>  int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> -- 
> 2.17.1
Daniel Vetter March 17, 2020, 10:16 a.m. UTC | #2
On Wed, Mar 11, 2020 at 03:55:37PM +0100, Andrzej Pietrasiewicz wrote:
> The new struct contains afbc-specific data.
> 
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>

Yeah I think this now looks like a fairly clean interface for drivers. On
the first two core patches:

Acked-by: Daniel Vetter <daniel.vetter@ffwll.ch>

Thanks for sticking around through all the fairly major revisions here!

Cheers, Daniel

> ---
>  Documentation/gpu/todo.rst                   |  15 +++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
>  include/drm/drm_framebuffer.h                |  45 ++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  10 ++
>  4 files changed, 178 insertions(+)
> 
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ 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 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_simple_kms_helper.h>
>  
> +#define AFBC_HEADER_SIZE		16
> +#define AFBC_TH_LAYOUT_ALIGNMENT	8
> +#define AFBC_HDR_ALIGN			64
> +#define AFBC_SUPERBLOCK_PIXELS		256
> +#define AFBC_SUPERBLOCK_ALIGNMENT	128
> +#define AFBC_TH_BODY_START_ALIGNMENT	4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>  
> +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;
> +
> +	switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> +		afbc_fb->block_width = 16;
> +		afbc_fb->block_height = 16;
> +		break;
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> +		afbc_fb->block_width = 32;
> +		afbc_fb->block_height = 8;
> +		break;
> +	/* no user exists yet - fall through */
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> +	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> +	default:
> +		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +			      mode_cmd->modifier[0]
> +			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +		return -EINVAL;
> +	}
> +
> +	/* tiled header afbc */
> +	w_alignment = afbc_fb->block_width;
> +	h_alignment = afbc_fb->block_height;
> +	hdr_alignment = AFBC_HDR_ALIGN;
> +	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +		w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> +	}
> +
> +	afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> +	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;
> +
> +	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> +		   / AFBC_SUPERBLOCK_PIXELS;
> +	afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> +	afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +					       AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +	return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *			    fill and validate all the afbc-specific
> + *			    struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb)
> +{
> +	const struct drm_format_info *info;
> +	struct drm_gem_object **objs;
> +	int ret;
> +
> +	objs = afbc_fb->base.obj;
> +	info = drm_get_format_info(dev, mode_cmd);
> +	if (!info)
> +		return -EINVAL;
> +
> +	ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> +	if (ret < 0)
> +		return ret;
> +
> +	if (objs[0]->size < afbc_fb->afbc_size)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
>   * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
>  int drm_framebuffer_plane_height(int height,
>  				 const struct drm_framebuffer *fb, int plane);
>  
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> +	/**
> +	 * @base: base framebuffer structure.
> +	 */
> +	struct drm_framebuffer base;
> +	/**
> +	 * @block_widht: width of a single afbc block
> +	 */
> +	u32 block_width;
> +	/**
> +	 * @block_widht: height of a single afbc block
> +	 */
> +	u32 block_height;
> +	/**
> +	 * @aligned_width: aligned frame buffer width
> +	 */
> +	u32 aligned_width;
> +	/**
> +	 * @aligned_height: aligned frame buffer height
> +	 */
> +	u32 aligned_height;
> +	/**
> +	 * @offset: offset of the first afbc header
> +	 */
> +	u32 offset;
> +	/**
> +	 * @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)
> +
>  #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>  
> +struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
>  
> +#define AFBC_VENDOR_AND_TYPE_MASK	GENMASK_ULL(63, 52)
> +
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  					  unsigned int plane);
>  void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  			     const struct drm_mode_fb_cmd2 *mode_cmd);
>  
> +#define drm_is_afbc(modifier) \
> +	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +			 const struct drm_mode_fb_cmd2 *mode_cmd,
> +			 struct drm_afbc_framebuffer *afbc_fb);
> +
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>  			  struct drm_plane_state *state);
>  int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> -- 
> 2.17.1
>
Daniel Vetter March 30, 2020, 5:01 p.m. UTC | #3
On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> The new struct contains afbc-specific data.
>
> The new function can be used by drivers which support afbc to complete
> the preparation of struct drm_afbc_framebuffer. It must be called after
> allocating the said struct and calling drm_gem_fb_init_with_funcs().
>
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  Documentation/gpu/todo.rst                   |  15 +++
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 108 +++++++++++++++++++
>  include/drm/drm_framebuffer.h                |  45 ++++++++
>  include/drm/drm_gem_framebuffer_helper.h     |  10 ++
>  4 files changed, 178 insertions(+)
>
> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> index 439656f55c5d..37a3a023c114 100644
> --- a/Documentation/gpu/todo.rst
> +++ b/Documentation/gpu/todo.rst
> @@ -404,6 +404,21 @@ 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

Just stumbled over this todo, which is really surprising. Also
definitely not something I wanted to ack, earlier versions at least
didn't have this.

Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
which has a pointer to drm_format_info, which we're always setting
(the core does that for all drivers, both for addfb and addfb2). Why
is that not good enough to get at cpp for everyone?

Cheers, Daniel

> +
>  Core refactorings
>  =================
>
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 86c1907c579a..7e3982c36baa 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,13 @@
>  #include <drm/drm_modeset_helper.h>
>  #include <drm/drm_simple_kms_helper.h>
>
> +#define AFBC_HEADER_SIZE               16
> +#define AFBC_TH_LAYOUT_ALIGNMENT       8
> +#define AFBC_HDR_ALIGN                 64
> +#define AFBC_SUPERBLOCK_PIXELS         256
> +#define AFBC_SUPERBLOCK_ALIGNMENT      128
> +#define AFBC_TH_BODY_START_ALIGNMENT   4096
> +
>  /**
>   * DOC: overview
>   *
> @@ -302,6 +309,107 @@ drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
>
> +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;
> +
> +       switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
> +               afbc_fb->block_width = 16;
> +               afbc_fb->block_height = 16;
> +               break;
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
> +               afbc_fb->block_width = 32;
> +               afbc_fb->block_height = 8;
> +               break;
> +       /* no user exists yet - fall through */
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
> +       case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
> +       default:
> +               DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
> +                             mode_cmd->modifier[0]
> +                             & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
> +               return -EINVAL;
> +       }
> +
> +       /* tiled header afbc */
> +       w_alignment = afbc_fb->block_width;
> +       h_alignment = afbc_fb->block_height;
> +       hdr_alignment = AFBC_HDR_ALIGN;
> +       if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +               w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +               h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
> +               hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
> +       }
> +
> +       afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
> +       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;
> +
> +       n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
> +                  / AFBC_SUPERBLOCK_PIXELS;
> +       afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
> +       afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +                                              AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +       return 0;
> +}
> +
> +/**
> + * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
> + *                         fill and validate all the afbc-specific
> + *                         struct drm_afbc_framebuffer members
> + *
> + * @dev: DRM device
> + * @afbc_fb: afbc-specific framebuffer
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + * @afbc_fb: afbc framebuffer
> + *
> + * This function can be used by drivers which support afbc to complete
> + * the preparation of struct drm_afbc_framebuffer. It must be called after
> + * allocating the said struct and calling drm_gem_fb_init_with_funcs().
> + * It is caller's responsibility to put afbc_fb->base.obj objects in case
> + * the call is unsuccessful.
> + *
> + * Returns:
> + * Zero on success or a negative error value on failure.
> + */
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +                        const struct drm_mode_fb_cmd2 *mode_cmd,
> +                        struct drm_afbc_framebuffer *afbc_fb)
> +{
> +       const struct drm_format_info *info;
> +       struct drm_gem_object **objs;
> +       int ret;
> +
> +       objs = afbc_fb->base.obj;
> +       info = drm_get_format_info(dev, mode_cmd);
> +       if (!info)
> +               return -EINVAL;
> +
> +       ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
> +       if (ret < 0)
> +               return ret;
> +
> +       if (objs[0]->size < afbc_fb->afbc_size)
> +               return -EINVAL;
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
> +
>  /**
>   * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
>   * @plane: Plane
> diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
> index c0e0256e3e98..e9f1b0e2968d 100644
> --- a/include/drm/drm_framebuffer.h
> +++ b/include/drm/drm_framebuffer.h
> @@ -297,4 +297,49 @@ int drm_framebuffer_plane_width(int width,
>  int drm_framebuffer_plane_height(int height,
>                                  const struct drm_framebuffer *fb, int plane);
>
> +/**
> + * struct drm_afbc_framebuffer - a special afbc frame buffer object
> + *
> + * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
> + */
> +struct drm_afbc_framebuffer {
> +       /**
> +        * @base: base framebuffer structure.
> +        */
> +       struct drm_framebuffer base;
> +       /**
> +        * @block_widht: width of a single afbc block
> +        */
> +       u32 block_width;
> +       /**
> +        * @block_widht: height of a single afbc block
> +        */
> +       u32 block_height;
> +       /**
> +        * @aligned_width: aligned frame buffer width
> +        */
> +       u32 aligned_width;
> +       /**
> +        * @aligned_height: aligned frame buffer height
> +        */
> +       u32 aligned_height;
> +       /**
> +        * @offset: offset of the first afbc header
> +        */
> +       u32 offset;
> +       /**
> +        * @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)
> +
>  #endif
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c029c1618661..6b013154911d 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -1,6 +1,7 @@
>  #ifndef __DRM_GEM_FB_HELPER_H__
>  #define __DRM_GEM_FB_HELPER_H__
>
> +struct drm_afbc_framebuffer;
>  struct drm_device;
>  struct drm_fb_helper_surface_size;
>  struct drm_file;
> @@ -12,6 +13,8 @@ struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
>
> +#define AFBC_VENDOR_AND_TYPE_MASK      GENMASK_ULL(63, 52)
> +
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>                                           unsigned int plane);
>  void drm_gem_fb_destroy(struct drm_framebuffer *fb);
> @@ -34,6 +37,13 @@ struct drm_framebuffer *
>  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
>                              const struct drm_mode_fb_cmd2 *mode_cmd);
>
> +#define drm_is_afbc(modifier) \
> +       (((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
> +
> +int drm_gem_fb_afbc_init(struct drm_device *dev,
> +                        const struct drm_mode_fb_cmd2 *mode_cmd,
> +                        struct drm_afbc_framebuffer *afbc_fb);
> +
>  int drm_gem_fb_prepare_fb(struct drm_plane *plane,
>                           struct drm_plane_state *state);
>  int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,
> --
> 2.17.1
>


--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
Andrzej Pietrasiewicz March 30, 2020, 5:44 p.m. UTC | #4
Hi Daniel,

W dniu 30.03.2020 o 19:01, Daniel Vetter pisze:
> On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
> <andrzej.p@collabora.com> wrote:
>>
>> The new struct contains afbc-specific data.

<snip>

>> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
>> index 439656f55c5d..37a3a023c114 100644
>> --- a/Documentation/gpu/todo.rst
>> +++ b/Documentation/gpu/todo.rst
>> @@ -404,6 +404,21 @@ 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
> 
> Just stumbled over this todo, which is really surprising. Also
> definitely not something I wanted to ack, earlier versions at least
> didn't have this.
> 
> Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
> which has a pointer to drm_format_info, which we're always setting
> (the core does that for all drivers, both for addfb and addfb2). Why
> is that not good enough to get at cpp for everyone?
> 
> Cheers, Daniel
> 

Let me quote James https://patchwork.freedesktop.org/patch/345603/#comment_653081:

"Seems we can remove this bpp or no need to define it as a pass in argument
for size check, maybe the komeda/malidp get_afbc_bpp() function mislead
you that afbc formats may have vendor specific bpp.

But the story is:

for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set
nothing in drm_format_info, neither cpp nor block_size, so both malidp
or komeda introduce a get_bpp(), but actually the two funcs basically
are same.

So my suggestion is we can temporary use the get_afbc_bpp() in malidp
or komeda. and eventually I think we'd better set the block size
for these formats, then we can defines a common get_bpp() like pitch".

Andrzej
Daniel Vetter March 30, 2020, 6:24 p.m. UTC | #5
On Mon, Mar 30, 2020 at 7:44 PM Andrzej Pietrasiewicz
<andrzej.p@collabora.com> wrote:
>
> Hi Daniel,
>
> W dniu 30.03.2020 o 19:01, Daniel Vetter pisze:
> > On Wed, Mar 11, 2020 at 3:55 PM Andrzej Pietrasiewicz
> > <andrzej.p@collabora.com> wrote:
> >>
> >> The new struct contains afbc-specific data.
>
> <snip>
>
> >> diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
> >> index 439656f55c5d..37a3a023c114 100644
> >> --- a/Documentation/gpu/todo.rst
> >> +++ b/Documentation/gpu/todo.rst
> >> @@ -404,6 +404,21 @@ 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
> >
> > Just stumbled over this todo, which is really surprising. Also
> > definitely not something I wanted to ack, earlier versions at least
> > didn't have this.
> >
> > Why is this needed? drm_afbc_framebuffer contains a drm_framebuffer,
> > which has a pointer to drm_format_info, which we're always setting
> > (the core does that for all drivers, both for addfb and addfb2). Why
> > is that not good enough to get at cpp for everyone?
> >
> > Cheers, Daniel
> >
>
> Let me quote James https://patchwork.freedesktop.org/patch/345603/#comment_653081:
>
> "Seems we can remove this bpp or no need to define it as a pass in argument
> for size check, maybe the komeda/malidp get_afbc_bpp() function mislead
> you that afbc formats may have vendor specific bpp.
>
> But the story is:
>
> for afbc only formats like DRM_FORMAT_YUV420_8BIT/10BIT, we have set
> nothing in drm_format_info, neither cpp nor block_size, so both malidp
> or komeda introduce a get_bpp(), but actually the two funcs basically
> are same.
>
> So my suggestion is we can temporary use the get_afbc_bpp() in malidp
> or komeda. and eventually I think we'd better set the block size
> for these formats, then we can defines a common get_bpp() like pitch".

Hm iirc we had some good reasons not to set the block size for these,
but maybe with these afbc helpers that's changed. We could also
compute cpp/bpp in the helper, starting from the format_info/fourcc
code, for these special cases. Essentially move the exact computation
that komeda/malidp do right now to set afbc->bpp and move it into the
helper. Just kinda feels like unfinished work that we still leave this
to drivers, that's some very quirky api.
-Daniel

Patch
diff mbox series

diff --git a/Documentation/gpu/todo.rst b/Documentation/gpu/todo.rst
index 439656f55c5d..37a3a023c114 100644
--- a/Documentation/gpu/todo.rst
+++ b/Documentation/gpu/todo.rst
@@ -404,6 +404,21 @@  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 86c1907c579a..7e3982c36baa 100644
--- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
+++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
@@ -21,6 +21,13 @@ 
 #include <drm/drm_modeset_helper.h>
 #include <drm/drm_simple_kms_helper.h>
 
+#define AFBC_HEADER_SIZE		16
+#define AFBC_TH_LAYOUT_ALIGNMENT	8
+#define AFBC_HDR_ALIGN			64
+#define AFBC_SUPERBLOCK_PIXELS		256
+#define AFBC_SUPERBLOCK_ALIGNMENT	128
+#define AFBC_TH_BODY_START_ALIGNMENT	4096
+
 /**
  * DOC: overview
  *
@@ -302,6 +309,107 @@  drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 }
 EXPORT_SYMBOL_GPL(drm_gem_fb_create_with_dirty);
 
+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;
+
+	switch (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK) {
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_16x16:
+		afbc_fb->block_width = 16;
+		afbc_fb->block_height = 16;
+		break;
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8:
+		afbc_fb->block_width = 32;
+		afbc_fb->block_height = 8;
+		break;
+	/* no user exists yet - fall through */
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_64x4:
+	case AFBC_FORMAT_MOD_BLOCK_SIZE_32x8_64x4:
+	default:
+		DRM_DEBUG_KMS("Invalid AFBC_FORMAT_MOD_BLOCK_SIZE: %lld.\n",
+			      mode_cmd->modifier[0]
+			      & AFBC_FORMAT_MOD_BLOCK_SIZE_MASK);
+		return -EINVAL;
+	}
+
+	/* tiled header afbc */
+	w_alignment = afbc_fb->block_width;
+	h_alignment = afbc_fb->block_height;
+	hdr_alignment = AFBC_HDR_ALIGN;
+	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
+		w_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
+		h_alignment *= AFBC_TH_LAYOUT_ALIGNMENT;
+		hdr_alignment = AFBC_TH_BODY_START_ALIGNMENT;
+	}
+
+	afbc_fb->aligned_width = ALIGN(mode_cmd->width, w_alignment);
+	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;
+
+	n_blocks = (afbc_fb->aligned_width * afbc_fb->aligned_height)
+		   / AFBC_SUPERBLOCK_PIXELS;
+	afbc_fb->afbc_size = ALIGN(n_blocks * AFBC_HEADER_SIZE, hdr_alignment);
+	afbc_fb->afbc_size += n_blocks * ALIGN(bpp * AFBC_SUPERBLOCK_PIXELS / 8,
+					       AFBC_SUPERBLOCK_ALIGNMENT);
+
+	return 0;
+}
+
+/**
+ * drm_gem_fb_afbc_init() - Helper function for drivers using afbc to
+ *			    fill and validate all the afbc-specific
+ *			    struct drm_afbc_framebuffer members
+ *
+ * @dev: DRM device
+ * @afbc_fb: afbc-specific framebuffer
+ * @mode_cmd: Metadata from the userspace framebuffer creation request
+ * @afbc_fb: afbc framebuffer
+ *
+ * This function can be used by drivers which support afbc to complete
+ * the preparation of struct drm_afbc_framebuffer. It must be called after
+ * allocating the said struct and calling drm_gem_fb_init_with_funcs().
+ * It is caller's responsibility to put afbc_fb->base.obj objects in case
+ * the call is unsuccessful.
+ *
+ * Returns:
+ * Zero on success or a negative error value on failure.
+ */
+int drm_gem_fb_afbc_init(struct drm_device *dev,
+			 const struct drm_mode_fb_cmd2 *mode_cmd,
+			 struct drm_afbc_framebuffer *afbc_fb)
+{
+	const struct drm_format_info *info;
+	struct drm_gem_object **objs;
+	int ret;
+
+	objs = afbc_fb->base.obj;
+	info = drm_get_format_info(dev, mode_cmd);
+	if (!info)
+		return -EINVAL;
+
+	ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc_fb);
+	if (ret < 0)
+		return ret;
+
+	if (objs[0]->size < afbc_fb->afbc_size)
+		return -EINVAL;
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(drm_gem_fb_afbc_init);
+
 /**
  * drm_gem_fb_prepare_fb() - Prepare a GEM backed framebuffer
  * @plane: Plane
diff --git a/include/drm/drm_framebuffer.h b/include/drm/drm_framebuffer.h
index c0e0256e3e98..e9f1b0e2968d 100644
--- a/include/drm/drm_framebuffer.h
+++ b/include/drm/drm_framebuffer.h
@@ -297,4 +297,49 @@  int drm_framebuffer_plane_width(int width,
 int drm_framebuffer_plane_height(int height,
 				 const struct drm_framebuffer *fb, int plane);
 
+/**
+ * struct drm_afbc_framebuffer - a special afbc frame buffer object
+ *
+ * A derived class of struct drm_framebuffer, dedicated for afbc use cases.
+ */
+struct drm_afbc_framebuffer {
+	/**
+	 * @base: base framebuffer structure.
+	 */
+	struct drm_framebuffer base;
+	/**
+	 * @block_widht: width of a single afbc block
+	 */
+	u32 block_width;
+	/**
+	 * @block_widht: height of a single afbc block
+	 */
+	u32 block_height;
+	/**
+	 * @aligned_width: aligned frame buffer width
+	 */
+	u32 aligned_width;
+	/**
+	 * @aligned_height: aligned frame buffer height
+	 */
+	u32 aligned_height;
+	/**
+	 * @offset: offset of the first afbc header
+	 */
+	u32 offset;
+	/**
+	 * @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)
+
 #endif
diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
index c029c1618661..6b013154911d 100644
--- a/include/drm/drm_gem_framebuffer_helper.h
+++ b/include/drm/drm_gem_framebuffer_helper.h
@@ -1,6 +1,7 @@ 
 #ifndef __DRM_GEM_FB_HELPER_H__
 #define __DRM_GEM_FB_HELPER_H__
 
+struct drm_afbc_framebuffer;
 struct drm_device;
 struct drm_fb_helper_surface_size;
 struct drm_file;
@@ -12,6 +13,8 @@  struct drm_plane;
 struct drm_plane_state;
 struct drm_simple_display_pipe;
 
+#define AFBC_VENDOR_AND_TYPE_MASK	GENMASK_ULL(63, 52)
+
 struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
 					  unsigned int plane);
 void drm_gem_fb_destroy(struct drm_framebuffer *fb);
@@ -34,6 +37,13 @@  struct drm_framebuffer *
 drm_gem_fb_create_with_dirty(struct drm_device *dev, struct drm_file *file,
 			     const struct drm_mode_fb_cmd2 *mode_cmd);
 
+#define drm_is_afbc(modifier) \
+	(((modifier) & AFBC_VENDOR_AND_TYPE_MASK) == DRM_FORMAT_MOD_ARM_AFBC(0))
+
+int drm_gem_fb_afbc_init(struct drm_device *dev,
+			 const struct drm_mode_fb_cmd2 *mode_cmd,
+			 struct drm_afbc_framebuffer *afbc_fb);
+
 int drm_gem_fb_prepare_fb(struct drm_plane *plane,
 			  struct drm_plane_state *state);
 int drm_gem_fb_simple_display_pipe_prepare_fb(struct drm_simple_display_pipe *pipe,