Re: [PATCH v2 1/4] drm: Add Y210, Y212, Y216 format definitions and fourcc

[Juha-Pekka Heikkila <juhapekka.heikkila@xxxxxxxxx>]

On 12.09.2018 13:32, Swati Sharma wrote:
> From: Vidya Srinivas <vidya.srinivas@xxxxxxxxx>
>
> The following pixel formats are packed format that follows 4:2:2
> chroma sampling. For memory represenation each component is
> allocated 16 bits each. Thus each pixel occupies a DWORD.

Just to be clear I wouldn't use 'DWORD' here but if it's 64 bits bits 
call it so. Not a big issue as its just in the patch comment but still.

> Y210:Valid data occupies MSB 10 bits.
> LSB 6 bits are filled with zeroes.
> Y212:Valid data occupies MSB 12 bits.
> LSB 4 bits are filled with zeroes.
> Y216:Valid data occupies 16 bits,
> doesn't require any padding bits.
>
> First 16 bits stores the Y value and the next 16 bits stores one
> of the chroma samples alternatively. The first luma sample will
> be accompanied by first U sample and second luma sample is
> accompanied by the first V sample.
>
> v2: is_yuv setted to true (mahesh)
>      different order of yuv samples (mahesh): still update from
>      hardware team pending
>      change in comment (alexandru)
>
> Signed-off-by: Swati Sharma <swati2.sharma@xxxxxxxxx>
> Signed-off-by: Vidya Srinivas <vidya.srinivas@xxxxxxxxx>
> ---
>   drivers/gpu/drm/drm_fourcc.c  | 3 +++
>   include/uapi/drm/drm_fourcc.h | 8 ++++++++
>   2 files changed, 11 insertions(+)
>
> diff --git a/drivers/gpu/drm/drm_fourcc.c b/drivers/gpu/drm/drm_fourcc.c
> index be1d6aa..143c26a 100644
> --- a/drivers/gpu/drm/drm_fourcc.c
> +++ b/drivers/gpu/drm/drm_fourcc.c
> @@ -190,6 +190,9 @@ const struct drm_format_info *__drm_format_info(u32 format)
>   		{ .format = DRM_FORMAT_UYVY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
>   		{ .format = DRM_FORMAT_VYUY,		.depth = 0,  .num_planes = 1, .cpp = { 2, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
>   		{ .format = DRM_FORMAT_AYUV,		.depth = 0,  .num_planes = 1, .cpp = { 4, 0, 0 }, .hsub = 1, .vsub = 1, .has_alpha = true, .is_yuv = true },
> +		{ .format = DRM_FORMAT_Y210,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> +		{ .format = DRM_FORMAT_Y212,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
> +		{ .format = DRM_FORMAT_Y216,            .depth = 0,  .num_planes = 1, .cpp = { 8, 0, 0 }, .hsub = 2, .vsub = 1, .is_yuv = true },
>   	};
>   
>   	unsigned int i;
> diff --git a/include/uapi/drm/drm_fourcc.h b/include/uapi/drm/drm_fourcc.h
> index 139632b..95118cc 100644
> --- a/include/uapi/drm/drm_fourcc.h
> +++ b/include/uapi/drm/drm_fourcc.h
> @@ -153,6 +153,14 @@
>   #define DRM_FORMAT_AYUV		fourcc_code('A', 'Y', 'U', 'V') /* [31:0] A:Y:Cb:Cr 8:8:8:8 little endian */
>   
>   /*
> + * Y2xx indicate for each component, xx valid data occupy msb
> + * 16-xx padding occupy lsb
> + */
> +#define DRM_FORMAT_Y210         fourcc_code('Y', '2', '1', '0') /* [63:0] Y0:Cb0:Y1:Cr1 10:10:10:10 little endian */
> +#define DRM_FORMAT_Y212         fourcc_code('Y', '2', '1', '2') /* [63:0] Y0:Cb0:Y1:Cr1 12:12:12:12 little endian */
> +#define DRM_FORMAT_Y216         fourcc_code('Y', '2', '1', '6') /* [63:0] Y0:Cb0:Y1:Cr1 16:16:16:16 little endian */

I think what Alexandru-Cosmin Gheorghe meant with earlier review comment 
is these defines should be better explained here in part which will be 
visible in header file. Looking how other formats have been commented 
this would probably be something along the lines of

#define DRM_FORMAT_Y210         fourcc_code('Y', '2', '1', '0') /* 
[63:0] Y0:x:Cb0:x:Y1:x:Cr1:x 10:6:10:6:10:6:10:6 little endian */

or something.

/Juha-Pekka

> +
> +/*
>    * 2 plane RGB + A
>    * index 0 = RGB plane, same format as the corresponding non _A8 format has
>    * index 1 = A plane, [7:0] A

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