Re: [PATCHv4,04/36] drm/gem-fb-helper: Add special version of drm_gem_fb_size_check

["james qian wang (Arm Technology China)" <james.qian.wang@arm.com>]

Hi Andrzej:

Really a good idea for introducing this custom size check, it's very
useful for some Komeda/malidp format, espcially pitch_multiplier, maybe
in future we can add it into into the drm_format_info.    

On Fri, Dec 13, 2019 at 04:58:35PM +0100, Andrzej Pietrasiewicz wrote:
> The new version accepts a struct describing deviations from standard way of
> doing the size checks. The caller must provide the respective values.
> 
> Signed-off-by: Andrzej Pietrasiewicz <andrzej.p@collabora.com>
> ---
>  drivers/gpu/drm/drm_gem_framebuffer_helper.c | 47 ++++++++++++++++----
>  include/drm/drm_gem_framebuffer_helper.h     | 16 +++++++
>  2 files changed, 55 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/gpu/drm/drm_gem_framebuffer_helper.c b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> index 787edb9a916b..4201dc1f32a5 100644
> --- a/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> +++ b/drivers/gpu/drm/drm_gem_framebuffer_helper.c
> @@ -201,8 +201,9 @@ int drm_gem_fb_lookup(struct drm_device *dev,
>  EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
>  
>  /**
> - * drm_gem_fb_size_check() - Helper function for use in
> - *			     &drm_mode_config_funcs.fb_create implementations
> + * drm_gem_fb_size_check_special() - Helper function for use in
> + *				     &drm_mode_config_funcs.fb_create
> + *				     implementations
>   * @dev: DRM device
>   * @mode_cmd: Metadata from the userspace framebuffer creation request
>   *
> @@ -212,9 +213,10 @@ EXPORT_SYMBOL_GPL(drm_gem_fb_lookup);
>   * Returns:
>   * Zero on success or a negative error code on failure.
>   */
> -int drm_gem_fb_size_check(struct drm_device *dev,
> -			  const struct drm_mode_fb_cmd2 *mode_cmd,
> -			  struct drm_gem_object **objs)
> +int drm_gem_fb_size_check_special(struct drm_device *dev,

How about name it to drm_gem_fb_custom_size_check()

> +				  const struct drm_mode_fb_cmd2 *mode_cmd,
> +				  const struct drm_size_check *check,
> +				  struct drm_gem_object **objs)
>  {
>  	const struct drm_format_info *info;
>  	int i;
> @@ -227,10 +229,19 @@ int drm_gem_fb_size_check(struct drm_device *dev,
>  		unsigned int width = mode_cmd->width / (i ? info->hsub : 1);
>  		unsigned int height = mode_cmd->height / (i ? info->vsub : 1);
>  		unsigned int min_size;
> +		u32 pitch = mode_cmd->pitches[i];
> +
> +		if (check && check->use_pitch_multiplier)
> +			if ((pitch * check->pitch_multiplier[i]) %
> +			    check->pitch_modulo)
> +				return -EINVAL;
>  
> -		min_size = (height - 1) * mode_cmd->pitches[i]
> -			 + drm_format_info_min_pitch(info, i, width)
> -			 + mode_cmd->offsets[i];
> +		if (check && check->use_min_size)
> +			min_size = check->min_size[i];
> +		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;
> @@ -239,6 +250,26 @@ int drm_gem_fb_size_check(struct drm_device *dev,
>  	return 0;
>  
>  }
> +EXPORT_SYMBOL_GPL(drm_gem_fb_size_check_special);
> +
> +/**
> + * drm_gem_fb_size_check() - Helper function for use in
> + *			     &drm_mode_config_funcs.fb_create implementations
> + * @dev: DRM device
> + * @mode_cmd: Metadata from the userspace framebuffer creation request
> + *
> + * This function can be used to verify buffer sizes for all planes.
> + * It is caller's responsibility to put the objects on failure.
> + *
> + * Returns:
> + * Zero on success or a negative error code on failure.
> + */
> +int drm_gem_fb_size_check(struct drm_device *dev,
> +			  const struct drm_mode_fb_cmd2 *mode_cmd,
> +			  struct drm_gem_object **objs)
> +{
> +	return drm_gem_fb_size_check_special(dev, mode_cmd, NULL, objs);
> +}
>  EXPORT_SYMBOL_GPL(drm_gem_fb_size_check);
>  
>  /**
> diff --git a/include/drm/drm_gem_framebuffer_helper.h b/include/drm/drm_gem_framebuffer_helper.h
> index c85d4b152e91..74304a268694 100644
> --- a/include/drm/drm_gem_framebuffer_helper.h
> +++ b/include/drm/drm_gem_framebuffer_helper.h
> @@ -11,6 +11,18 @@ struct drm_mode_fb_cmd2;
>  struct drm_plane;
>  struct drm_plane_state;
>  struct drm_simple_display_pipe;
> +struct drm_size_check;
> +
> +/**
> + * struct drm_size_check - Description of special requirements for size checks.
> + */
> +struct drm_size_check {
> +	unsigned int min_size[4];
> +	bool use_min_size;
> +	u32 pitch_multiplier[4];
> +	u32 pitch_modulo;
> +	bool use_pitch_multiplier;
> +};
>  
>  struct drm_gem_object *drm_gem_fb_get_obj(struct drm_framebuffer *fb,
>  					  unsigned int plane);
> @@ -32,6 +44,10 @@ int drm_gem_fb_lookup(struct drm_device *dev,
>  		      struct drm_file *file,
>  		      const struct drm_mode_fb_cmd2 *mode_cmd,
>  		      struct drm_gem_object **objs);
> +int drm_gem_fb_size_check_special(struct drm_device *dev,
> +				  const struct drm_mode_fb_cmd2 *mode_cmd,
> +				  const struct drm_size_check *check,
> +				  struct drm_gem_object **objs);
>  int drm_gem_fb_size_check(struct drm_device *dev,
>  			  const struct drm_mode_fb_cmd2 *mode_cmd,
>  			  struct drm_gem_object **objs);

For this common case can we just define it as a MACRO, or a inline
func here in this header.

Thanks
James
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel