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

On 2026-07-30 10:19, Pekka Paalanen wrote:
> On Tue, 28 Jul 2026 17:17:34 +0200
> Robert Mader <[email protected]> wrote:
> 
>> 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.
> 
> I agree.
> 

Agreed. The goal was to create specific tests that fail with the old
conversion matrices and pass with the new ones but there's little
value in tests that are so specific.

Harry

> If these reference matrices were produced by completely different means
> instead of being checked in as "binaries", e.g. by floating-point
> calculations from first principles at test time, they might have some
> use.
> 
> What would actually be useful is to record the script used to produce
> these "binary" numbers in the first place. It's much easier to review
> such a script, than try to come up with one's own in order to check if
> the "binary" numbers are correct (like I did when reviewing these
> patches).
> 
> I've been wondering, what is the source code - the preferential format
> for modification - the numbers recorded here, or the scripts producing
> them.
> 
> 
> Thanks,
> pq
> 
>> 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),
>>>   	{}
>>>   };
>>>     
>>
>