Re: [PATCH 5/9] drm/vc4: hdmi: Rework the CSC matrices organization

[Thomas Zimmermann <tzimmermann@suse.de>]

[-- Attachment #1.1: Type: text/plain, Size: 7248 bytes --]

Hi

Am 07.12.22 um 17:07 schrieb Maxime Ripard:
> From: Dave Stevenson <dave.stevenson@raspberrypi.com>
> 
> The CSC matrices were stored as separate matrix for each colorspace, and
> if we wanted a limited or full RGB output.
> 
> This created some gaps in our support and we would not always pick the
> relevant matrix.
> 
> Let's rework our data structure to store one per colorspace, and then a
> matrix for limited range and one for full range. This makes us add a new
> matrix to support full range BT709 YUV output, and drops the redundant
> (but somehow different) BT709 YUV444 vs YUV422 matrix.

The final sentence is confusing and I didn't understand how two 
different matrices can now be just one.

Best regards
Thomas

> 
> Signed-off-by: Dave Stevenson <dave.stevenson@raspberrypi.com>
> Signed-off-by: Maxime Ripard <maxime@cerno.tech>
> ---
>   drivers/gpu/drm/vc4/vc4_hdmi.c | 124 +++++++++++++++++++++--------------------
>   1 file changed, 63 insertions(+), 61 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
> index 51469939a8b4..299a8fe7a2ae 100644
> --- a/drivers/gpu/drm/vc4/vc4_hdmi.c
> +++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
> @@ -1178,68 +1178,72 @@ static void vc4_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,
>   }
>   
>   /*
> - * If we need to output Full Range RGB, then use the unity matrix
> + * Matrices for (internal) RGB to RGB output.
>    *
> - * [ 1      0      0      0]
> - * [ 0      1      0      0]
> - * [ 0      0      1      0]
> - *
> - * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
> - */
> -static const u16 vc5_hdmi_csc_full_rgb_unity[3][4] = {
> -	{ 0x2000, 0x0000, 0x0000, 0x0000 },
> -	{ 0x0000, 0x2000, 0x0000, 0x0000 },
> -	{ 0x0000, 0x0000, 0x2000, 0x0000 },
> -};
> -
> -/*
> - * CEA VICs other than #1 require limited range RGB output unless
> - * overridden by an AVI infoframe. Apply a colorspace conversion to
> - * squash 0-255 down to 16-235. The matrix here is:
> - *
> - * [ 0.8594 0      0      16]
> - * [ 0      0.8594 0      16]
> - * [ 0      0      0.8594 16]
> - *
> - * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
> + * Matrices are signed 2p13 fixed point, with signed 9p6 offsets
>    */
> -static const u16 vc5_hdmi_csc_full_rgb_to_limited_rgb[3][4] = {
> -	{ 0x1b80, 0x0000, 0x0000, 0x0400 },
> -	{ 0x0000, 0x1b80, 0x0000, 0x0400 },
> -	{ 0x0000, 0x0000, 0x1b80, 0x0400 },
> +static const u16 vc5_hdmi_csc_full_rgb_to_rgb[2][3][4] = {
> +	{
> +		/*
> +		 * Full range - unity
> +		 *
> +		 * [ 1      0      0      0]
> +		 * [ 0      1      0      0]
> +		 * [ 0      0      1      0]
> +		 */
> +		{ 0x2000, 0x0000, 0x0000, 0x0000 },
> +		{ 0x0000, 0x2000, 0x0000, 0x0000 },
> +		{ 0x0000, 0x0000, 0x2000, 0x0000 },
> +	},
> +	{
> +		/*
> +		 * Limited range
> +		 *
> +		 * CEA VICs other than #1 require limited range RGB
> +		 * output unless overridden by an AVI infoframe. Apply a
> +		 * colorspace conversion to squash 0-255 down to 16-235.
> +		 * The matrix here is:
> +		 *
> +		 * [ 0.8594 0      0      16]
> +		 * [ 0      0.8594 0      16]
> +		 * [ 0      0      0.8594 16]
> +		 */
> +		{ 0x1b80, 0x0000, 0x0000, 0x0400 },
> +		{ 0x0000, 0x1b80, 0x0000, 0x0400 },
> +		{ 0x0000, 0x0000, 0x1b80, 0x0400 },
> +	},
>   };
>   
>   /*
> - * Conversion between Full Range RGB and Full Range YUV422 using the
> - * BT.709 Colorspace
> - *
> - *
> - * [  0.181906  0.611804  0.061758  16  ]
> - * [ -0.100268 -0.337232  0.437500  128 ]
> - * [  0.437500 -0.397386 -0.040114  128 ]
> - *
> - * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
> - */
> -static const u16 vc5_hdmi_csc_full_rgb_to_limited_yuv422_bt709[3][4] = {
> -	{ 0x05d2, 0x1394, 0x01fa, 0x0400 },
> -	{ 0xfccc, 0xf536, 0x0e00, 0x2000 },
> -	{ 0x0e00, 0xf34a, 0xfeb8, 0x2000 },
> -};
> -
> -/*
> - * Conversion between Full Range RGB and Full Range YUV444 using the
> - * BT.709 Colorspace
> - *
> - * [ -0.100268 -0.337232  0.437500  128 ]
> - * [  0.437500 -0.397386 -0.040114  128 ]
> - * [  0.181906  0.611804  0.061758  16  ]
> + * Conversion between Full Range RGB and YUV using the BT.709 Colorspace
>    *
> - * Matrix is signed 2p13 fixed point, with signed 9p6 offsets
> + * Matrices are signed 2p13 fixed point, with signed 9p6 offsets
>    */
> -static const u16 vc5_hdmi_csc_full_rgb_to_limited_yuv444_bt709[3][4] = {
> -	{ 0xfccc, 0xf536, 0x0e00, 0x2000 },
> -	{ 0x0e00, 0xf34a, 0xfeb8, 0x2000 },
> -	{ 0x05d2, 0x1394, 0x01fa, 0x0400 },
> +static const u16 vc5_hdmi_csc_full_rgb_to_yuv_bt709[2][3][4] = {
> +	{
> +		/*
> +		 * Full Range
> +		 *
> +		 * [  0.212600  0.715200  0.072200  0   ]
> +		 * [ -0.114572 -0.385428  0.500000  128 ]
> +		 * [  0.500000 -0.454153 -0.045847  128 ]
> +		 */
> +		{ 0x06ce, 0x16e3, 0x024f, 0x0000 },
> +		{ 0xfc56, 0xf3ac, 0x1000, 0x2000 },
> +		{ 0x1000, 0xf179, 0xfe89, 0x2000 },
> +	},
> +	{
> +		/*
> +		 * Limited Range
> +		 *
> +		 * [  0.181906  0.611804  0.061758  16  ]
> +		 * [ -0.100268 -0.337232  0.437500  128 ]
> +		 * [  0.437500 -0.397386 -0.040114  128 ]
> +		 */
> +		{ 0x05d2, 0x1394, 0x01fa, 0x0400 },
> +		{ 0xfccc, 0xf536, 0x0e00, 0x2000 },
> +		{ 0x0e00, 0xf34a, 0xfeb8, 0x2000 },
> +	},
>   };
>   
>   static void vc5_hdmi_set_csc_coeffs(struct vc4_hdmi *vc4_hdmi,
> @@ -1262,6 +1266,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,
>   	struct drm_device *drm = vc4_hdmi->connector.dev;
>   	struct vc4_hdmi_connector_state *vc4_state =
>   		conn_state_to_vc4_hdmi_conn_state(state);
> +	unsigned int lim_range = vc4_hdmi_is_full_range(vc4_hdmi, mode) ? 0 : 1;
>   	unsigned long flags;
>   	u32 if_cfg = 0;
>   	u32 if_xbar = 0x543210;
> @@ -1277,7 +1282,7 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,
>   
>   	switch (vc4_state->output_format) {
>   	case VC4_HDMI_OUTPUT_YUV444:
> -		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_limited_yuv444_bt709);
> +		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_yuv_bt709[lim_range]);
>   		break;
>   
>   	case VC4_HDMI_OUTPUT_YUV422:
> @@ -1292,16 +1297,13 @@ static void vc5_hdmi_csc_setup(struct vc4_hdmi *vc4_hdmi,
>   		if_cfg |= VC4_SET_FIELD(VC5_DVP_HT_VEC_INTERFACE_CFG_SEL_422_FORMAT_422_LEGACY,
>   					VC5_DVP_HT_VEC_INTERFACE_CFG_SEL_422);
>   
> -		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_limited_yuv422_bt709);
> +		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_yuv_bt709[lim_range]);
>   		break;
>   
>   	case VC4_HDMI_OUTPUT_RGB:
>   		if_xbar = 0x354021;
>   
> -		if (!vc4_hdmi_is_full_range(vc4_hdmi, mode))
> -			vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_limited_rgb);
> -		else
> -			vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_unity);
> +		vc5_hdmi_set_csc_coeffs(vc4_hdmi, vc5_hdmi_csc_full_rgb_to_rgb[lim_range]);
>   		break;
>   
>   	default:
> 

-- 
Thomas Zimmermann
Graphics Driver Developer
SUSE Software Solutions Germany GmbH
Maxfeldstr. 5, 90409 Nürnberg, Germany
(HRB 36809, AG Nürnberg)
Geschäftsführer: Ivo Totev

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 840 bytes --]