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

[Andrzej Pietrasiewicz]

Hi James,

Thank you for the review.

Did you intentionally review patches from the v4 series or you simply
didn't notice the v5? There are some differences, the most notable one
is using proper way of subclassing a drm_framebuffer.
The v5 series was sent on 17th December 2019.

Andrzej

W dniu 17.02.2020 o 09:16, james qian wang (Arm Technology China) pisze:
> 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 at 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
>