Re: [RFC PATCH v4 25/42] drm/vkms: Add tests for CTM handling

2024-03-14 Thread Pekka Paalanen
On Mon, 26 Feb 2024 16:10:39 -0500
Harry Wentland  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.
> 
> v4:
>  - Comment on origin of bt709_enc matrix (Pekka)
>  - Use full opaque alpha (Pekka)
>  - Add additional check for Y < 0x (Pekka)
>  - Remove unused code (Pekka)
>  - Rename red, green, blue to Y, U, V where applicable
> 
> Signed-off-by: Harry Wentland 
> ---
>  drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 251 ++
>  drivers/gpu/drm/vkms/vkms_composer.c  |   2 +-
>  2 files changed, 252 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 e6ac01dee830..83d07f7bae37 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 
>  
>  #include 
> +#include 
>  
>  #define TEST_LUT_SIZE 16
>  
> @@ -181,11 +182,261 @@ 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

These are supposed to be sign-magnitude, not fixed-point, to my
understanding. It just happens that these specific values have the same
bit pattern in both representations.


> +} };
> +
> +static void vkms_color_ctm_3x4_50_desat(struct kunit *test)
> +{
> + struct pixel_argb_s32 ref, out;
> +
> + /* full white */
> + ref.a = 0x;
> + ref.r = 0x;
> + ref.g = 0x;
> + ref.b = 0x;
> +
> + 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 = 0x;
> + 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 = 0x;
> + 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 = 0x;
> + ref.r = 0x7fff;
> + ref.g = 0x3fff;
> + ref.b = 0x3fff;
> +
> + out.a = 0x;
> + out.r = 0x;
> + out.g = 0x0;
> + out.b = 0x0;
> +
> + apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
> +
> + KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
> +}
> +
> +/*
> + * BT.709 encoding matrix
> + *
> + * Values printed from within IGT when converting
> + * igt_matrix_3x4_bt709_enc to the fixed-point format expected
> + * by DRM/KMS.

Shouldn't that be sign-magnitude, not fixed point?

I was hoping to get a reference to a spec like BT.709 and a formula for
getting the blow out of it. IGT can change over time, and I don't know
where IGT for that matrix from.

But ok, this is a test with an arbitrary matrix, not necessarily the
BT.709 matrix specifically.

Btw. which BT.709 matrix is this? YUV->RGB, RGB->XYZ, or the inverse of
one of those? Limited or full quantization range?

Judging by the tests, I guess RGB->YUV full range.

> + */
> +const struct drm_color_ctm_3x4 test_matrix_3x4_bt709_enc = { {
> + 0x366cf400ull, 0xb7175900ull, 0x000127bb300ull, 0,
> + 0x80001993b3a0ull, 0x80005609fe80ull, 0x6f9db200ull, 0,
> + 0x9d70a400ull, 0x80008f011100ull, 0x8e6f9330ull, 0
> +} };
> +
> +static void vkms_color_ctm_3x4_bt709(struct kunit *test)
> +{
> + struct pixel_argb_s32 out;
> +
> + /* full white to bt709 */
> + out.a = 0x;
> + out.r = 0x;
> + out.g = 0x;
> + out.b = 0x;
> +
> + apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
> +
> + /* Y 255 */
> + KUNIT_EXPECT_GT(test, out.r, 0xfe00);
> + KUNIT_EXPECT_LT(test, out.r, 0x1);
> +
> + /* U 0 */
> + KUNIT_EXPECT_LT(test, out.g, 0x0100);

For all of these U/V too, you may want to enforce a range. The value
must be approximately 0x100. Something like 0x17 would very wrong.

Hmm, wait...

Neutral color should have U and V neutral, that is, zero, right? So
what is zero in this integer representation?

You don't seem to be testing negative U or V...


Thanks,
pq

> +
> + /* V 0 */
> + KUNIT_EXPECT_LT(test, out.b, 0x0100);
> +
> + /* full black to bt709 */
> + out.a = 0xf

[RFC PATCH v4 25/42] drm/vkms: Add tests for CTM handling

2024-02-26 Thread Harry Wentland
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.

v4:
 - Comment on origin of bt709_enc matrix (Pekka)
 - Use full opaque alpha (Pekka)
 - Add additional check for Y < 0x (Pekka)
 - Remove unused code (Pekka)
 - Rename red, green, blue to Y, U, V where applicable

Signed-off-by: Harry Wentland 
---
 drivers/gpu/drm/vkms/tests/vkms_color_tests.c | 251 ++
 drivers/gpu/drm/vkms/vkms_composer.c  |   2 +-
 2 files changed, 252 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 e6ac01dee830..83d07f7bae37 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 
 
 #include 
+#include 
 
 #define TEST_LUT_SIZE 16
 
@@ -181,11 +182,261 @@ 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 = 0x;
+   ref.r = 0x;
+   ref.g = 0x;
+   ref.b = 0x;
+
+   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 = 0x;
+   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 = 0x;
+   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 = 0x;
+   ref.r = 0x7fff;
+   ref.g = 0x3fff;
+   ref.b = 0x3fff;
+
+   out.a = 0x;
+   out.r = 0x;
+   out.g = 0x0;
+   out.b = 0x0;
+
+   apply_3x4_matrix(&out, &test_matrix_3x4_50_desat);
+
+   KUNIT_EXPECT_MEMEQ(test, &ref, &out, sizeof(out));
+}
+
+/*
+ * BT.709 encoding matrix
+ *
+ * Values printed from within IGT when converting
+ * igt_matrix_3x4_bt709_enc to the fixed-point format expected
+ * by DRM/KMS.
+ */
+const struct drm_color_ctm_3x4 test_matrix_3x4_bt709_enc = { {
+   0x366cf400ull, 0xb7175900ull, 0x000127bb300ull, 0,
+   0x80001993b3a0ull, 0x80005609fe80ull, 0x6f9db200ull, 0,
+   0x9d70a400ull, 0x80008f011100ull, 0x8e6f9330ull, 0
+} };
+
+static void vkms_color_ctm_3x4_bt709(struct kunit *test)
+{
+   struct pixel_argb_s32 out;
+
+   /* full white to bt709 */
+   out.a = 0x;
+   out.r = 0x;
+   out.g = 0x;
+   out.b = 0x;
+
+   apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+   /* Y 255 */
+   KUNIT_EXPECT_GT(test, out.r, 0xfe00);
+   KUNIT_EXPECT_LT(test, out.r, 0x1);
+
+   /* U 0 */
+   KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+   /* V 0 */
+   KUNIT_EXPECT_LT(test, out.b, 0x0100);
+
+   /* full black to bt709 */
+   out.a = 0x;
+   out.r = 0x0;
+   out.g = 0x0;
+   out.b = 0x0;
+
+   apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+   /* Y 0 */
+   KUNIT_EXPECT_LT(test, out.r, 0x100);
+
+   /* U 0 */
+   KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+   /* V 0 */
+   KUNIT_EXPECT_LT(test, out.b, 0x0100);
+
+   /* gray to bt709 */
+   out.a = 0x;
+   out.r = 0x7fff;
+   out.g = 0x7fff;
+   out.b = 0x7fff;
+
+   apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+   /* Y 127 */
+   KUNIT_EXPECT_GT(test, out.r, 0x7e00);
+   KUNIT_EXPECT_LT(test, out.r, 0x8000);
+
+   /* U 0 */
+   KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+   /* V 0 */
+   KUNIT_EXPECT_LT(test, out.b, 0x0100);
+
+   /* == red 255 - bt709 enc == */
+   out.a = 0x;
+   out.r = 0x;
+   out.g = 0x0;
+   out.b = 0x0;
+
+   apply_3x4_matrix(&out, &test_matrix_3x4_bt709_enc);
+
+   /* Y 54 */
+   KUNIT_EXPECT_GT(test, out.r, 0x3500);
+   KUNIT_EXPECT_LT(test, out.r, 0x3700);
+
+   /* U 0 */
+   KUNIT_EXPECT_LT(test, out.g, 0x0100);
+
+   /* V 157 */
+   KUNIT_EXPECT_GT(test, out.b, 0x9C00);
+   KUNIT_EX