[PATCHv4 05/36] drm/gem-fb-helper: Add generic afbc size checks

Liviu Dudau liviu.dudau at arm.com
Mon Dec 16 17:19:03 UTC 2019


Hi Andrzej,

On Fri, Dec 13, 2019 at 04:58:36PM +0100, Andrzej Pietrasiewicz wrote:
> Extend the size-checking special function to handle afbc.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p at collabora.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c                 | 10 +++-
>  drivers/gpu/drm/drm_framebuffer.c            |  3 +
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 60 ++++++++++++++++++--
>  include/drm/drm_gem_framebuffer_helper.h     | 16 ++++++
>  4 files changed, 82 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index d14dd7c86020..9ac2175c5bee 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -323,8 +323,14 @@ drm_get_format_info(struct drm_device *dev,
>  {
>  	const struct drm_format_info *info = NULL;
>  
> -	if (dev->mode_config.funcs->get_format_info)
> -		info = dev->mode_config.funcs->get_format_info(mode_cmd);
> +	/* bypass driver callback if afbc */
> +	if (!drm_is_afbc(mode_cmd->modifier[0]))
> +		if (dev->mode_config.funcs->get_format_info) {
> +			const struct drm_mode_config_funcs *funcs;
> +
> +			funcs = dev->mode_config.funcs;
> +			info = funcs->get_format_info(mode_cmd);
> +		}

What has this change to do with the rest of the patch? Also, I think this goes
against the idea that an AFBC-aware driver might return better data about the format
info than the drm_format_info() code.

As a bikeshed, I know it is useful for debugging to turn the oneliner into 3, but it
feels like not necessary here.

Best regards,
Liviu

>  
>  	if (!info)
>  		info = drm_format_info(mode_cmd->pixel_format);
> diff --git a/drivers/gpu/drm/drm_framebuffer.c b/drivers/gpu/drm/drm_framebuffer.c
> index 57564318ceea..33b741cc73e8 100644
> --- a/drivers/gpu/drm/drm_framebuffer.c
> +++ b/drivers/gpu/drm/drm_framebuffer.c
> @@ -204,6 +204,9 @@ static int framebuffer_check(struct drm_device *dev,
>  		unsigned int block_size = info->char_per_block[i];
>  		u64 min_pitch = drm_format_info_min_pitch(info, i, width);
>  
> +		if (drm_is_afbc(r->modifier[i]))
> +			block_size = 0;
> +
>  		if (!block_size && (r->modifier[i] == DRM_FORMAT_MOD_LINEAR)) {
>  			DRM_DEBUG_KMS("Format requires non-linear modifier for plane %d\n", i);
>  			return -EINVAL;
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 4201dc1f32a5..e20f4d00b0a5 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -21,6 +21,11 @@
>  #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_SUPERBLOCK_PIXELS		256
> +#define AFBC_SUPERBLOCK_ALIGNMENT	128
> +
>  /**
>   * DOC: overview
>   *
> @@ -200,6 +205,40 @@ int drm_gem_fb_lookup(struct drm_device *dev,
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
>  
> +static int drm_gem_afbc_min_size(struct drm_device *dev,
> +				 const struct drm_mode_fb_cmd2 *mode_cmd,
> +				 struct drm_afbc *afbc)
> +{
> +	u32 n_blocks;
> +
> +	if (!drm_afbc_get_superblock_wh(mode_cmd->modifier[0],
> +					&afbc->block_width,
> +					&afbc->block_height)) {
> +		return -EINVAL;
> +	}
> +
> +	/* tiled header afbc */
> +	if (mode_cmd->modifier[0] & AFBC_FORMAT_MOD_TILED) {
> +		afbc->block_width *= AFBC_TH_LAYOUT_ALIGNMENT;
> +		afbc->block_height *= AFBC_TH_LAYOUT_ALIGNMENT;
> +	}
> +
> +	afbc->aligned_width = ALIGN(mode_cmd->width, afbc->block_width);
> +	afbc->aligned_height = ALIGN(mode_cmd->height, afbc->block_height);
> +	afbc->offset = mode_cmd->offsets[0];
> +
> +	n_blocks = (afbc->aligned_width * afbc->aligned_height)
> +		 / AFBC_SUPERBLOCK_PIXELS;
> +	afbc->offset_payload = ALIGN(n_blocks * AFBC_HEADER_SIZE,
> +				     afbc->alignment_header);
> +
> +	afbc->afbc_size = afbc->offset_payload + n_blocks *
> +			  ALIGN(afbc->bpp * AFBC_SUPERBLOCK_PIXELS / 8,
> +				AFBC_SUPERBLOCK_ALIGNMENT);
> +
> +	return 0;
> +}
> +
>  /**
>   * drm_gem_fb_size_check_special() - Helper function for use in
>   *				     &drm_mode_config_funcs.fb_create
> @@ -218,6 +257,7 @@ int drm_gem_fb_size_check_special(struct drm_device *dev,
>  				  const struct drm_size_check *check,
>  				  struct drm_gem_object **objs)
>  {
> +#define CHECK_HAS(field) (check && check->field)
>  	const struct drm_format_info *info;
>  	int i;
>  
> @@ -231,24 +271,34 @@ int drm_gem_fb_size_check_special(struct drm_device *dev,
>  		unsigned int min_size;
>  		u32 pitch = mode_cmd->pitches[i];
>  
> -		if (check && check->use_pitch_multiplier)
> +		if (CHECK_HAS(use_pitch_multiplier))
>  			if ((pitch * check->pitch_multiplier[i]) %
>  			    check->pitch_modulo)
>  				return -EINVAL;
>  
> -		if (check && check->use_min_size)
> +		if (CHECK_HAS(use_min_size)) {
>  			min_size = check->min_size[i];
> -		else
> +		} else if (CHECK_HAS(data) &&
> +				drm_is_afbc(mode_cmd->modifier[0])) {
> +			struct drm_afbc *afbc;
> +			int ret;
> +
> +			afbc = check->data;
> +			ret = drm_gem_afbc_min_size(dev, mode_cmd, afbc);
> +			if (ret < 0)
> +				return ret;
> +			min_size = ret;
> +		} else {
>  			min_size = (height - 1) * pitch
>  				 + drm_format_info_min_pitch(info, i, width)
>  				 + mode_cmd->offsets[i];
> -
> +		}
>  		if (objs[i]->size < min_size)
>  			return -EINVAL;
>  	}
>  
>  	return 0;
> -
> +#undef CHECK_HAS
>  }
>  EXPORT_SYMBOL_GPL(drm_gem_fb_size_check_special);
>  
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index 74304a268694..3d6015194b3c 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -22,6 +22,22 @@ struct drm_size_check {
>  	u32 pitch_multiplier[4];
>  	u32 pitch_modulo;
>  	bool use_pitch_multiplier;
> +	void *data;
> +};
> +
> +/**
> + * struct drm_afbc - AFBC-specific data.
> + */
> +struct drm_afbc {
> +	u32 block_width;
> +	u32 block_height;
> +	u32 aligned_width;
> +	u32 aligned_height;
> +	u32 offset;
> +	u32 alignment_header;
> +	u32 afbc_size;
> +	u32 offset_payload;
> +	u32 bpp;
>  };
>  
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
> -- 
> 2.17.1
> 

-- 
====================
| I would like to |
| fix the world,  |
| but they're not |
| giving me the   |
 \ source code!  /
  ---------------
    ¯\_(ツ)_/¯


More information about the dri-devel mailing list