On 2025-02-25 06:19, Louis Chauvet wrote:
>
>
> Le 20/12/2024 à 05:33, Alex Hung a écrit :
>> From: Harry Wentland <harry.wentland@xxxxxxx>
>>
>> Two tests are added to VKMS LUT handling:
>> - linear
>> - inv_srgb
>>
>> Reviewed-by: Louis Chauvet <louis.chauvet@xxxxxxxxxxx>
>> Signed-off-by: Alex Hung <alex.hung@xxxxxxx>
>> Signed-off-by: Harry Wentland <harry.wentland@xxxxxxx>
>> ---
>> 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 <drm/drm_mode.h>
>> #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, 0xffff)), 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, 0x80000000), 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 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 channel_value,
>> + enum lut_channel channel)
>> {
>> s64 lut_index = get_lut_index(lut, channel_value);
>> u16 *floor_lut_value, *ceil_lut_value;
>> @@ -150,6 +139,8 @@ static u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 chan
>> return lerp_u16(floor_channel_value, ceil_channel_value,
>> lut_index & DRM_FIXED_DECIMAL_MASK);
>> }
>> +EXPORT_SYMBOL_IF_KUNIT(apply_lut_to_channel_value);
>> +
>> static void apply_lut(const struct vkms_crtc_state *crtc_state, struct line_buffer *output_buffer)
>> {
>> diff --git a/drivers/gpu/drm/vkms/vkms_composer.h b/drivers/gpu/drm/vkms/vkms_composer.h
>> index 9316a053e7d7..67ae09913460 100644
>> --- a/drivers/gpu/drm/vkms/vkms_composer.h
>> +++ b/drivers/gpu/drm/vkms/vkms_composer.h
>> @@ -5,9 +5,22 @@
>> #include <kunit/visibility.h>
>> +/*
>> + * 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
>> +};
>> +
>> #if IS_ENABLED(CONFIG_KUNIT)
>> u16 lerp_u16(u16 a, u16 b, s64 t);
>> s64 get_lut_index(const struct vkms_color_lut *lut, u16 channel_value);
>> +u16 apply_lut_to_channel_value(const struct vkms_color_lut *lut, u16 channel_value,
>> + enum lut_channel channel);
>> #endif
>> #endif /* _VKMS_COMPOSER_H_ */
>
