Re: [PATCH v4 03/11] drm/vkms: Add KUnit test for YCbCr to RGB conversion matrices

TBH this test feels a bit redundant to me, given that it just repeats 
the exact values, which again just get memcpy'd. I suggest to drop it.

On 22.07.26 15:45, Harry Wentland wrote:
> The existing yuv_u16_to_argb_u16 test performs a full RGB->YCbCr->RGB
> round trip with a tolerance of 0x1ff. That tolerance is required because
> fully saturated primaries clamp their chroma at encode time and cannot
> round-trip exactly, but it also makes the test unable to detect small
> scaling errors in the conversion matrices (such as using a 2^n instead
> of a 2^n - 1 full-range maximum for limited range, an error of ~129 at
> gray - below the tolerance).
>
> Add a parameterised test that checks the exact S31.32 fixed-point
> coefficients and luma offset returned by
> get_conversion_matrix_to_argb_u16() for each encoding and range against
> reference values. This directly pins the matrices and catches
> sub-tolerance regressions that the round-trip test cannot.
>
> Assisted-by: Copilot:claude-opus-4.8
> Signed-off-by: Harry Wentland <[email protected]>
> ---
>   drivers/gpu/drm/vkms/tests/vkms_format_test.c | 122 ++++++++++++++++++
>   1 file changed, 122 insertions(+)
>
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_format_test.c b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> index 789c59d07ffb..e746b18bf37b 100644
> --- a/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> +++ b/drivers/gpu/drm/vkms/tests/vkms_format_test.c
> @@ -263,8 +263,130 @@ KUNIT_ARRAY_PARAM(yuv_u16_to_argb_u16, yuv_u16_to_argb_u16_cases,
>   		  vkms_format_test_yuv_u16_to_argb_u16_case_desc
>   );
>   
> +/*
> + * struct conversion_matrix_case - Reference matrix to test the YUV to RGB
> + * conversion matrices returned by get_conversion_matrix_to_argb_u16()
> + *
> + * @encoding: Encoding of the conversion matrix under test
> + * @range: Range of the conversion matrix under test
> + * @expected: Expected S31.32 fixed-point matrix and luma offset
> + *
> + * The limited-range coefficients use the studio-range scaling mandated by the
> + * DRM UAPI and IGT's igt_ycbcr_to_rgb_matrix(): the narrow range is expanded
> + * relative to a full-range maximum of 2^n - 1 (255 for 8-bit), i.e. luma by
> + * 255/(235 - 16) and chroma by 255/(240 - 128). Generating them with the
> + * common 2^n normalisation (as colour.matrix_YCbCr(is_legal=True) does) is off
> + * by a factor of 256/255. See the DRM_COLOROP_FM_YCBCR*_LIMITED_RGB
> + * documentation in <drm/drm_colorop.h>.
> + */
> +struct conversion_matrix_case {
> +	enum drm_color_encoding encoding;
> +	enum drm_color_range range;
> +	struct conversion_matrix expected;
> +};
> +
> +static struct conversion_matrix_case conversion_matrix_cases[] = {
> +	{
> +		.encoding = DRM_COLOR_YCBCR_BT601,
> +		.range = DRM_COLOR_YCBCR_FULL_RANGE,
> +		.expected = { .matrix = {
> +			{ 4294967296, 0,           6021544149 },
> +			{ 4294967296, -1478054095, -3067191994 },
> +			{ 4294967296, 7610682049,  0 },
> +		}, .y_offset = 0 },
> +	},
> +	{
> +		.encoding = DRM_COLOR_YCBCR_BT601,
> +		.range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> +		.expected = { .matrix = {
> +			{ 5000989317, 0,           6854882848 },
> +			{ 5000989317, -1682606224, -3491669458 },
> +			{ 5000989317, 8663946082,  0 },
> +		}, .y_offset = 16 },
> +	},
> +	{
> +		.encoding = DRM_COLOR_YCBCR_BT709,
> +		.range = DRM_COLOR_YCBCR_FULL_RANGE,
> +		.expected = { .matrix = {
> +			{ 4294967296, 0,          6763714498 },
> +			{ 4294967296, -804551626, -2010578443 },
> +			{ 4294967296, 7969741314, 0 },
> +		}, .y_offset = 0 },
> +	},
> +	{
> +		.encoding = DRM_COLOR_YCBCR_BT709,
> +		.range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> +		.expected = { .matrix = {
> +			{ 5000989317, 0,          7699764272 },
> +			{ 5000989317, -915895824, -2288828138 },
> +			{ 5000989317, 9072696586, 0 },
> +		}, .y_offset = 16 },
> +	},
> +	{
> +		.encoding = DRM_COLOR_YCBCR_BT2020,
> +		.range = DRM_COLOR_YCBCR_FULL_RANGE,
> +		.expected = { .matrix = {
> +			{ 4294967296, 0,          6333358775 },
> +			{ 4294967296, -706750298, -2453942994 },
> +			{ 4294967296, 8080551471, 0 },
> +		}, .y_offset = 0 },
> +	},
> +	{
> +		.encoding = DRM_COLOR_YCBCR_BT2020,
> +		.range = DRM_COLOR_YCBCR_LIMITED_RANGE,
> +		.expected = { .matrix = {
> +			{ 5000989317, 0,          7209850391 },
> +			{ 5000989317, -804559491, -2793551177 },
> +			{ 5000989317, 9198842076, 0 },
> +		}, .y_offset = 16 },
> +	},
> +};
> +
> +/*
> + * vkms_format_test_conversion_matrix - Verify the YUV to RGB conversion matrices
> + *
> + * This test checks that get_conversion_matrix_to_argb_u16() returns the exact
> + * fixed-point coefficients expected for each encoding and range. Unlike the
> + * round-trip test above, it is sensitive to small (sub-tolerance) scaling
> + * errors such as using a 2^n instead of a 2^n - 1 full-range maximum for the
> + * limited-range matrices.
> + */
> +static void vkms_format_test_conversion_matrix(struct kunit *test)
> +{
> +	const struct conversion_matrix_case *param = test->param_value;
> +	struct conversion_matrix matrix;
> +
> +	get_conversion_matrix_to_argb_u16(DRM_FORMAT_NV12, param->encoding,
> +					  param->range, false, &matrix);
> +
> +	for (size_t i = 0; i < 3; i++) {
> +		for (size_t j = 0; j < 3; j++) {
> +			KUNIT_EXPECT_EQ_MSG(test, matrix.matrix[i][j],
> +					    param->expected.matrix[i][j],
> +					    "matrix[%zu][%zu] mismatch for %s - %s",
> +					    i, j,
> +					    drm_get_color_encoding_name(param->encoding),
> +					    drm_get_color_range_name(param->range));
> +		}
> +	}
> +
> +	KUNIT_EXPECT_EQ(test, matrix.y_offset, param->expected.y_offset);
> +}
> +
> +static void vkms_format_test_conversion_matrix_case_desc(struct conversion_matrix_case *t,
> +							 char *desc)
> +{
> +	snprintf(desc, KUNIT_PARAM_DESC_SIZE, "%s - %s",
> +		 drm_get_color_encoding_name(t->encoding), drm_get_color_range_name(t->range));
> +}
> +
> +KUNIT_ARRAY_PARAM(conversion_matrix, conversion_matrix_cases,
> +		  vkms_format_test_conversion_matrix_case_desc
> +);
> +
>   static struct kunit_case vkms_format_test_cases[] = {
>   	KUNIT_CASE_PARAM(vkms_format_test_yuv_u16_to_argb_u16, yuv_u16_to_argb_u16_gen_params),
> +	KUNIT_CASE_PARAM(vkms_format_test_conversion_matrix, conversion_matrix_gen_params),
>   	{}
>   };
>   

-- 
Robert Mader
Consultant Software Developer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718