Re: [PATCH 1/3] drm: drm_fourcc: Add scaling and padding factor to drm_format_info

[Laurent Pinchart <laurent.pinchart@ideasonboard.com>]

Hi Hyun,

Thank you for the patch.

On Tuesday, 28 November 2017 04:27:31 EET Hyun Kwon wrote:
> From: Satish Kumar Nagireddy <satish.nagireddy.nagireddy@xilinx.com>
> 
> 'cpp_scale' can be used as a multiplying factor to calculate
> bytes per component based on color format.
> For eg. scaling factor of YUV420 8 bit format is 1
>         so multiplying factor is 1 (8/8)
>         scaling factor of YUV420 10 bit format is 1.25 (10/8)
> 
> 'padding_scale' can be used as a multiplying factor to calculate
> actual width of video according to color format.
> For eg. padding factor of YUV420 8 bit format: 8 bits per 1 component
>         no padding bits here, so multiplying factor is 1
>         padding factor of YUV422 10 bit format: 32 bits per 3 components
>         each component is 10 bit and the factor is 32/30
> 
> Signed-off-by: Satish Kumar Nagireddy <satishna@xilinx.com>
> Signed-off-by: Hyun Kwon <hyun.kwon@xilinx.com>
> ---
>  drivers/gpu/drm/drm_fourcc.c | 136 ++++++++++++++++++----------------------
>  include/drm/drm_fourcc.h     |   9 +++
>  2 files changed, 77 insertions(+), 68 deletions(-)

[snip]

> diff --git a/include/drm/drm_fourcc.h b/include/drm/drm_fourcc.h
> index 6942e84..0202d19 100644
> --- a/include/drm/drm_fourcc.h
> +++ b/include/drm/drm_fourcc.h
> @@ -36,16 +36,25 @@ struct drm_mode_fb_cmd2;
>   *	use in new code and set to 0 for new formats.
>   * @num_planes: Number of color planes (1 to 3)
>   * @cpp: Number of bytes per pixel (per plane)
> + * @cpp_scale: { numerator, denominator }. Scaling factor for
> + *	non 8bit aligned formats. For instance, { 10, 8 } can be used for
> + *	10 bit component / pixel formats.

Stupid question, wouldn't it be better to replace cpp with a number of bits 
per pixel then ? Or possibly supplement it if we need both. A scaling factor 
seems difficult to use.

>   * @hsub: Horizontal chroma subsampling factor
>   * @vsub: Vertical chroma subsampling factor
> + * @padding_scale: { numerator, denominator }. Scaling factor for
> + *	padding. This can be used for formats with padding bits after
> + *	multiple pixels / components. For instance, if there are 2 bit
> + *	padding after 3 10bit components, the value should be { 32, 30 }.

Similarly, why don't you instead specify the number of padding bits directly ?

>   */
>  struct drm_format_info {
>  	u32 format;
>  	u8 depth;
>  	u8 num_planes;
>  	u8 cpp[3];
> +	u8 cpp_scale[2];
>  	u8 hsub;
>  	u8 vsub;
> +	u8 padding_scale[2];
>  };
> 
>  /**


-- 
Regards,

Laurent Pinchart

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