On 2023-12-08 08:32, Pekka Paalanen wrote:
> On Wed, 8 Nov 2023 11:36:42 -0500
> Harry Wentland <harry.wentland@xxxxxxx> wrote:
>
>> A whole slew of tests for CTM handling that greatly helped in
>> debugging the CTM code. The extent of tests might seem a bit
>> silly but they're fast and might someday help save someone
>> else's day when debugging this.
>
> To be honest, this looks pretty limited testing. I guess the more
> exhaustive tests are in IGT though, would be nice to mention that in
> the commit message.
>
> There is not a single case for out of [0.0, 1.0] input nor output.
>
> The offset part is always zero.
>
It is very limited testing, mostly intended for sanity checking.
If I find time I might expand on these but it's pretty low
priority at the moment. The core of the testing is intended to
be done via IGT.
Harry
>> Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>
>> ---
>> drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 258 ++++++++++++++++++
>> drivers/gpu/drm/vkms/vkms_composer.c | 2 +-
>> 2 files changed, 259 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>> index ad4c2f72fd1e..3eaa2233afbb 100644
>> --- a/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>> +++ b/drivers/gpu/drm/vkms/tests/vkms_color_tests.c
>> @@ -3,6 +3,7 @@
>> #include <kunit/test.h>
>>
>> #include <drm/drm_fixed.h>
>> +#include <drm/drm_mode.h>
>>
>> #define TEST_LUT_SIZE 16
>>
>> @@ -80,11 +81,268 @@ static void vkms_color_srgb_inv_srgb(struct kunit *test)
>> }
>> }
>>
>> +#define FIXPT_HALF (DRM_FIXED_ONE >> 1)
>> +#define FIXPT_QUARTER (DRM_FIXED_ONE >> 2)
>> +
>> +const struct drm_color_ctm_3x4 test_matrix_3x4_50_desat = { {
>> + FIXPT_HALF, FIXPT_QUARTER, FIXPT_QUARTER, 0,
>> + FIXPT_QUARTER, FIXPT_HALF, FIXPT_QUARTER, 0,
>> + FIXPT_QUARTER, FIXPT_QUARTER, FIXPT_HALF, 0
>> +} };
>> +
>> +static void vkms_color_ctm_3x4_50_desat(struct kunit *test)
>> +{
>> + struct pixel_argb_s32 ref, out;
>> +
>> + /* full white */
>> + ref.a = 0x0;
>> + ref.r = 0xffff;
>> + ref.g = 0xffff;
>> + ref.b = 0xffff;
>> +
>> + memcpy(&out, &ref, sizeof(out));
>> + apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
>> +
>> + KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
>> +
>> + /* full black */
>> + ref.a = 0x0;
>
> This is transparent, btw.
>
>> + ref.r = 0x0;
>> + ref.g = 0x0;
>> + ref.b = 0x0;
>> +
>> + memcpy(&out, &ref, sizeof(out));
>> + apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
>> +
>> + KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
>> +
>> + /* 50% grey */
>> + ref.a = 0x0;
>> + ref.r = 0x8000;
>> + ref.g = 0x8000;
>> + ref.b = 0x8000;
>> +
>> + memcpy(&out, &ref, sizeof(out));
>> + apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
>> +
>> + KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
>> +
>> + /* full red to 50% desat */
>> + ref.a = 0x0;
>> + ref.r = 0x7fff;
>> + ref.g = 0x3fff;
>> + ref.b = 0x3fff;
>> +
>> + out.a = 0x0;
>> + out.r = 0xffff;
>> + out.g = 0x0;
>> + out.b = 0x0;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
>> +
>> + KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
>> +}
>> +
>> +const struct drm_color_ctm_3x4 test_matrix_3x4_bt709_enc = { {
>> + 0x00000000366cf400ull, 0x00000000b7175900ull, 0x0000000127bb300ull, 0,
>> + 0x800000001993b3a0ull, 0x800000005609fe80ull, 0x000000006f9db200ull, 0,
>> + 0x000000009d70a400ull, 0x800000008f011100ull, 0x800000000e6f9330ull, 0
>> +} };
>
> Would be nice to have a comment explaining where these values came
> from, like a snippet of Python printing these. The conversion from real
> numbers to this representation is the interesting part.
>
>> +
>> +static void vkms_color_ctm_3x4_bt709(struct kunit *test)
>> +{
>> + struct pixel_argb_s32 ref, out;
>> +
>> + /* full white to bt709 */
>> + ref.a = 0x0;
>> + ref.r = 0xfffe; /* off by one in 16bpc not a big deal */
>> + ref.g = 0x0;
>> + ref.b = 0x0;
>> +
>> + out.a = 0x0;
>> + out.r = 0xffff;
>> + out.g = 0xffff;
>> + out.b = 0xffff;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 255 */
>> + KUNIT_EXPECT_GT(test, out.r, 0xfe00);
>
> This allows result grossly greater than 1.0.
>
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x0100);
>
> This allows grossly negative results.
>
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x0100);
>> +
>> + /* full black to bt709 */
>> + ref.a = 0x0;
>> + ref.r = 0x0; /* off by one in 16bpc not a big deal */
>> + ref.g = 0x0;
>> + ref.b = 0x0;
>> +
>> + out.a = 0x0;
>> + out.r = 0x0;
>> + out.g = 0x0;
>> + out.b = 0x0;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 0 */
>> + KUNIT_EXPECT_LT(test, out.r, 0x100);
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x0100);
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x0100);
>> +
>> + /* gray to bt709 */
>> + ref.a = 0x0;
>> + ref.r = 0x7fff; /* off by one in 16bpc not a big deal */
>> + ref.g = 0x0;
>> + ref.b = 0x0;
>
> This ref does not seem to be used?
>
>> +
>> + out.a = 0x0;
>> + out.r = 0x7fff;
>> + out.g = 0x7fff;
>> + out.b = 0x7fff;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 127 */
>> + KUNIT_EXPECT_GT(test, out.r, 0x7e00);
>> + KUNIT_EXPECT_LT(test, out.r, 0x8000);
>
> Maybe a new macro for "expect in between", or "expect with +/-
> tolerance"?
>
>
> Thanks,
> pq
>
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x0100);
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x0100);
>> +
>> + /* == red 255 - bt709 enc == */
>> + out.a = 0x0;
>> + out.r = 0xffff;
>> + out.g = 0x0;
>> + out.b = 0x0;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 54 */
>> + KUNIT_EXPECT_GT(test, out.r, 0x3500);
>> + KUNIT_EXPECT_LT(test, out.r, 0x3700);
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x0100);
>> +
>> + /* blue 157 */
>> + KUNIT_EXPECT_GT(test, out.b, 0x9C00);
>> + KUNIT_EXPECT_LT(test, out.b, 0x9E00);
>> +
>> +
>> + /* == green 255 - bt709 enc == */
>> + out.a = 0x0;
>> + out.r = 0x0;
>> + out.g = 0xffff;
>> + out.b = 0x0;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 182 */
>> + KUNIT_EXPECT_GT(test, out.r, 0xB500);
>> + KUNIT_EXPECT_LT(test, out.r, 0xB780); /* laxed by half*/
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x0100);
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x0100);
>> +
>> + /* == blue 255 - bt709 enc == */
>> + out.a = 0x0;
>> + out.r = 0x0;
>> + out.g = 0x0;
>> + out.b = 0xffff;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 18 */
>> + KUNIT_EXPECT_GT(test, out.r, 0x1100);
>> + KUNIT_EXPECT_LT(test, out.r, 0x1300);
>> +
>> + /* green 111 */
>> + KUNIT_EXPECT_GT(test, out.g, 0x6E00);
>> + KUNIT_EXPECT_LT(test, out.g, 0x7000);
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x0100);
>> +
>> + /* == red 140 - bt709 enc == */
>> + out.a = 0x0;
>> + out.r = 0x8c8c;
>> + out.g = 0x0;
>> + out.b = 0x0;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 30 */
>> + KUNIT_EXPECT_GT(test, out.r, 0x1D00);
>> + KUNIT_EXPECT_LT(test, out.r, 0x1F00);
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x100);
>> +
>> + /* blue 87 */
>> + KUNIT_EXPECT_GT(test, out.b, 0x5600);
>> + KUNIT_EXPECT_LT(test, out.b, 0x5800);
>> +
>> + /* == green 140 - bt709 enc == */
>> + out.a = 0x0;
>> + out.r = 0x0;
>> + out.g = 0x8c8c;
>> + out.b = 0x0;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 30 */
>> + KUNIT_EXPECT_GT(test, out.r, 0x6400);
>> + KUNIT_EXPECT_LT(test, out.r, 0x6600);
>> +
>> + /* green 0 */
>> + KUNIT_EXPECT_LT(test, out.g, 0x100);
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x100);
>> +
>> +
>> + /* == blue 140 - bt709 enc == */
>> + out.a = 0x0;
>> + out.r = 0x0;
>> + out.g = 0x0;
>> + out.b = 0x8c8c;
>> +
>> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
>> +
>> + /* red 30 */
>> + KUNIT_EXPECT_GT(test, out.r, 0x900);
>> + KUNIT_EXPECT_LT(test, out.r, 0xB00);
>> +
>> + /* green 61 */
>> + KUNIT_EXPECT_GT(test, out.g, 0x3C00);
>> + KUNIT_EXPECT_LT(test, out.g, 0x3E00);
>> +
>> + /* blue 0 */
>> + KUNIT_EXPECT_LT(test, out.b, 0x100);
>> +
>> +}
>> +
>> static struct kunit_case vkms_color_test_cases[] = {
>> KUNIT_CASE(vkms_color_test_get_lut_index),
>> KUNIT_CASE(vkms_color_test_lerp),
>> KUNIT_CASE(vkms_color_test_linear),
>> KUNIT_CASE(vkms_color_srgb_inv_srgb),
>> + KUNIT_CASE(vkms_color_ctm_3x4_50_desat),
>> + KUNIT_CASE(vkms_color_ctm_3x4_bt709),
>> {}
>> };
>>
>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
>> index c278fb223188..4ead5346007a 100644
>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
>> @@ -164,7 +164,7 @@ static void apply_lut(const struct vkms_crtc_state *crtc_state, struct line_buff
>> }
>> }
>>
>> -static void apply_3x4_matrix(struct pixel_argb_s32 *pixel, const struct drm_color_ctm_3x4 *matrix)
>> +void apply_3x4_matrix(struct pixel_argb_s32 *pixel, const struct drm_color_ctm_3x4 *matrix)
>> {
>> s64 rf, gf, bf;
>>
>
