On 2025-02-25 06:19, Louis Chauvet wrote:
>
>
> Le 20/12/2024 à 05:33, Alex Hung a écrit :
>> From: Harry Wentland
>>
>> Two tests are added to VKMS LUT handling:
>> - linear
>> - inv_srgb
>>
>> Reviewed-by: Louis Chauvet
>> Signed-off-by: Alex Hung
>> Signed-off-by: Harry Wentland
>> ---
>> v7:
>> - Fix checkpatch warnings (Louis Chauvet)
>> - Adde a commit messages
>> - Fix code styles by adding and removing spaces (new lines, tabs and so
>> on)
>>
>> drivers/gpu/drm/vkms/tests/vkms_color_test.c | 39 +++-
>> drivers/gpu/drm/vkms/vkms_composer.c | 17 ++---
>> drivers/gpu/drm/vkms/vkms_composer.h | 13 +++
>> 3 files changed, 55 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_test.c
>> b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
>> index b53beaac2703..d765c5eb5d88 100644
>> --- a/drivers/gpu/drm/vkms/tests/vkms_color_test.c
>> +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
>> @@ -6,6 +6,7 @@
>> #include
>> #include "../vkms_drv.h"
>> #include "../vkms_composer.h"
>> +#include "../vkms_luts.h"
>> #define TEST_LUT_SIZE 16
>> @@ -36,7 +37,6 @@ static const struct vkms_color_lut test_linear_lut = {
>> .channel_value2index_ratio = 0xf000fll
>> };
>> -
>> static void vkms_color_test_get_lut_index(struct kunit *test)
>> {
>> s64 lut_index;
>> @@ -49,6 +49,19 @@ static void vkms_color_test_get_lut_index(struct kunit
>> *test)
>> lut_index = get_lut_index(&test_linear_lut,
>> test_linear_array[i].red);
>> KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(lut_index), i);
>> }
>> +
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&srgb_eotf, 0x0)),
>> 0x0);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
>> 0x0)), 0x0);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
>> 0x101)), 0x1);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
>> 0x202)), 0x2);
>> +
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&srgb_inv_eotf, 0x0)),
>> 0x0);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_inv_eotf,
>> 0x0)), 0x0);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_inv_eotf,
>> 0x101)), 0x1);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_inv_eotf,
>> 0x202)), 0x2);
>> +
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
>> 0xfefe)), 0xfe);
>> + KUNIT_EXPECT_EQ(test, drm_fixp2int_ceil(get_lut_index(&srgb_eotf,
>> 0x)), 0xff);
>
> Did you see the kernel bot warning? I think you can simply add
> EXPORT_SYMBOL_IF_KUNIT(srgb_eotf) in vkms_lut.h.
>
I did not and don't see any warnings if I run this locally.
Adding EXPORT_SYMBOL_IF_KUNIT would require pulling in kunit
headers into vkms_luts.h. I would prefer not to do that if
it's not needed.
Harry
>> }
>> static void vkms_color_test_lerp(struct kunit *test)
>> @@ -155,9 +168,33 @@ static void vkms_color_test_lerp(struct kunit *test)
>> KUNIT_EXPECT_EQ(test, lerp_u16(0x0, 0x1, 0x8000), 0x1);
>> }
>> +static void vkms_color_test_linear(struct kunit *test)
>> +{
>> + for (int i = 0; i < LUT_SIZE; i++) {
>> + int linear = apply_lut_to_channel_value(&linear_eotf, i * 0x101,
>> LUT_RED);
>> +
>> + KUNIT_EXPECT_EQ(test, DIV_ROUND_CLOSEST(linear, 0x101), i);
>> + }
>> +}
>> +
>> +static void vkms_color_srgb_inv_srgb(struct kunit *test)
>> +{
>> + u16 srgb, final;
>> +
>> + for (int i = 0; i < LUT_SIZE; i++) {
>> + srgb = apply_lut_to_channel_value(&srgb_eotf, i * 0x101, LUT_RED);
>> + final = apply_lut_to_channel_value(&srgb_inv_eotf, srgb, LUT_RED);
>> +
>> + KUNIT_EXPECT_GE(test, final / 0x101, i - 1);
>> + KUNIT_EXPECT_LE(test, final / 0x101, i + 1);
>> + }
>> +}
>> +
>> 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),
>> {}
>> };
>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c
>> b/drivers/gpu/drm/vkms/vkms_composer.c
>> index 983654540ee5..ee3cfe153d8f 100644
>> --- a/drivers/gpu/drm/vkms/vkms_composer.c
>> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
>> @@ -113,19 +113,8 @@ VISIBLE_IF_KUNIT s64 get_lut_index(const struct
>> vkms_color_lut *lut, u16 channel
>> }
>> EXPORT_SYMBOL_IF_KUNIT(get_lut_index);
>> -/*
>> - * This enum is related to the positions of the variables inside
>> - * `struct drm_color_lut`, so the order of both needs to be the same.
>> - */
>> -enum lut_channel {
>> - LUT_RED = 0,
>> - LUT_GREEN,
>> - LUT_BLUE,
>> - LUT_RESERVED
>> -};
>> -
>> -static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16
>> channel_value,
>> - enum lut_channel channel)
>> +VISIBLE_IF_KUNIT u16 a
