Re: [PATCH 19/20] lib: image-formats: Add more functions

[Sakari Ailus]

Hi Maxime,

On Wed, Apr 17, 2019 at 09:54:45AM +0200, Maxime Ripard wrote:
> V4L2 drivers typically need a few more helpers compared to DRM drivers, so
> let's add them.
> 
> Signed-off-by: Maxime Ripard <maxime.ripard@xxxxxxxxxxx>
> ---
>  include/linux/image-formats.h | 42 ++++++++++++++++++++++++++++++++++++-
>  1 file changed, 42 insertions(+)
> 
> diff --git a/include/linux/image-formats.h b/include/linux/image-formats.h
> index b78b8e861fc9..a2cf3528bd31 100644
> --- a/include/linux/image-formats.h
> +++ b/include/linux/image-formats.h
> @@ -388,6 +388,48 @@ uint64_t image_format_info_min_pitch(const struct image_format_info *info,
>  			    image_format_info_block_height(info, plane));
>  }
>  
> +/**
> + * image_format_info_plane_stride - determine the stride value
> + * @format: pointer to the image_format_info
> + * @width: plane width
> + * @plane: plane index
> + *
> + * Returns:
> + * The bytes per pixel value for the specified plane.

In V4L2 the bytesperline (stride) value is user configurable and is also
subject to hardware capabilities. So effectively this makes what the
function returns a minimum value for a given format. I think you could add
this to the documentation. This is also not the number of bytes per pixel,
as suggested above. So, e.g.

	Returns the minimum number of bytes between two consecutive lines.

> + */
> +static inline
> +unsigned int image_format_info_plane_stride(const struct image_format_info *format,
> +					    unsigned int width, int plane)
> +{
> +	if (!format || plane >= format->num_planes)
> +		return 0;
> +
> +	return image_format_info_plane_width(format, width, plane) *
> +		image_format_info_plane_cpp(format, plane);
> +}
> +
> +/**
> + * image_format_info_plane_size - determine the size value
> + * @format: pointer to the image_format_info
> + * @width: plane width
> + * @height: plane width
> + * @plane: plane index
> + *
> + * Returns:
> + * The size of the plane buffer.

Similarly:

	Returns the minimum size of the plane buffer.

With these,

Acked-by: Sakari Ailus <sakari.ailus@xxxxxxxxxxxxxxx>

> + */
> +static inline
> +unsigned int image_format_info_plane_size(const struct image_format_info *format,
> +					  unsigned int width, unsigned int height,
> +					  int plane)
> +{
> +	if (!format || plane >= format->num_planes)
> +		return 0;
> +
> +	return image_format_info_plane_stride(format, width, plane) *
> +		image_format_info_plane_height(format, height, plane);
> +}
> +
>  const struct image_format_info *__image_format_drm_lookup(u32 drm);
>  const struct image_format_info *__image_format_v4l2_lookup(u32 v4l2);
>  const struct image_format_info *image_format_drm_lookup(u32 drm);

-- 
Sakari Ailus
sakari.ailus@xxxxxxxxxxxxxxx