Re: [PATCH v1 4/5] media: Add HEXTILE compressed format

[Nicolas Dufresne <nicolas@ndufresne.ca>]

Hello Marvin,

Le jeudi 12 mai 2022 à 15:27 +0800, Marvin Lin a écrit :
> Add HEXTILE compressed format. This format is defined in Remote
> Framebuffer Protocol (RFC 6143) and is used by VNC features.
> 
> Signed-off-by: Marvin Lin <kflin@nuvoton.com>
> ---
>  drivers/media/v4l2-core/v4l2-ioctl.c | 1 +
>  include/uapi/linux/videodev2.h       | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/media/v4l2-core/v4l2-ioctl.c b/drivers/media/v4l2-core/v4l2-ioctl.c
> index 96e307fe3aab..9365519d65bb 100644
> --- a/drivers/media/v4l2-core/v4l2-ioctl.c
> +++ b/drivers/media/v4l2-core/v4l2-ioctl.c
> @@ -1441,6 +1441,7 @@ static void v4l_fill_fmtdesc(struct v4l2_fmtdesc *fmt)
>  		case V4L2_PIX_FMT_SE401:	descr = "GSPCA SE401"; break;
>  		case V4L2_PIX_FMT_S5C_UYVY_JPG:	descr = "S5C73MX interleaved UYVY/JPEG"; break;
>  		case V4L2_PIX_FMT_MT21C:	descr = "Mediatek Compressed Format"; break;
> +		case V4L2_PIX_FMT_HEXTILE:	descr = "Hextile Compressed Format"; break;
>  		default:
>  			if (fmt->description[0])
>  				return;
> diff --git a/include/uapi/linux/videodev2.h b/include/uapi/linux/videodev2.h
> index 3768a0a80830..e0e69f71d700 100644
> --- a/include/uapi/linux/videodev2.h
> +++ b/include/uapi/linux/videodev2.h
> @@ -710,6 +710,7 @@ struct v4l2_pix_format {
>  #define V4L2_PIX_FMT_FWHT     v4l2_fourcc('F', 'W', 'H', 'T') /* Fast Walsh Hadamard Transform (vicodec) */
>  #define V4L2_PIX_FMT_FWHT_STATELESS     v4l2_fourcc('S', 'F', 'W', 'H') /* Stateless FWHT (vicodec) */
>  #define V4L2_PIX_FMT_H264_SLICE v4l2_fourcc('S', '2', '6', '4') /* H264 parsed slices */
> +#define V4L2_PIX_FMT_HEXTILE  v4l2_fourcc('H', 'X', 'T', 'L') /* Hextile */

New formats also need to be documented in one of 
Documentation/userspace-api/media/v4l/pixfmt*.rst file. Consider that formats,
specially vendor formats disappears over time, so documentation should be
written as if you are to read that in 10+ years.

I'm also wondering if that should be better located with the "vendor" formats. 
Specially that this does not look like a open standard like the other in this
section. A quick search of that format simply gives no results for me. It seems
pretty clear that this is unique to this HW vendor and unlikely to grow withing
other HW or even ever get a software implementation.
>  
>  /*  Vendor-specific formats   */
>  #define V4L2_PIX_FMT_CPIA1    v4l2_fourcc('C', 'P', 'I', 'A') /* cpia1 YUV */