Wayland - WebOS Graphics Training Request for LG Soft Bangalore

2024-10-04 Thread channaraj

Dear Devel,

Greetings! Hope this mail finds you well.

We need you Support in delivering this training to our client LG Soft 
India. Find the more details below.


We have a technical training requirement, please find the details listed 
below and let us know your availability for call and training, cost, 
course content, duration of the training (approx.)



Training Name

webOS graphics (trainer can suggest apt name basis the topics)

Objective

The objective of this training is to provide participants with a strong 
understanding of the Wayland protocol, develop skills in Wayland 
compositor creation, and enhance proficiency in graphics programming 
using OpenGL, Vulkan, and related technologies.


Detailed Content

Wayland Protocol:
- Overview of Wayland architecture and protocol.
- Understanding Wayland clients and servers.
- Key components like Weston (reference compositor)

Compositor Development:
- Basics of developing a Wayland compositor.
- Handling input/output devices.
- Rendering using OpenGL, Vulkan, or similar.

Graphics Programming:
- Rendering pipelines (OpenGL, Vulkan).
- EGL, GBM (Generic Buffer Management), and DRM (Direct Rendering 
Manager).


No. of Participants

30 to 35 (max)

Please help us with the following -

1. Updated Trainer Profile

2. Training Content Outline as per the above inputs

3. Training Proposal

4. Availbility of the trainer for 30 Minutes next week

Thanks & Regards,
Channa Raj. S
Founder - Corporate Trainer Services
M: +91 8884040488
LinkedIn Profile: https://www.linkedin.com/in/vlsraj11
Website: https://www.vebsters.com/

Re: [PATCH v6 18/44] drm/vkms: Use s32 for internal color pipeline precision

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:01, Harry Wentland wrote:
> Certain operations require us to preserve values below 0.0 and
> above 1.0 (0x0 and 0x respectively in 16 bpc unorm). One
> such operation is a BT709 encoding operation followed by its
> decoding operation, or the reverse.
> 
> We'll use s32 values as intermediate in and outputs of our
> color operations, for the operations where it matters.
> 
> For now this won't apply to LUT operations. We might want to
> update those to work on s32 as well, but it's unclear how
> that should work for unorm LUT definitions. We'll revisit
> that once we add LUT + CTM tests.
> 
> In order to allow for this we'll also invert the nesting of our
> colorop processing loops. We now use the pixel iteration loop
> on the outside and the colorop iteration on the inside.
> 
> Signed-off-by: Harry Wentland 
> ---
> v6:
>  - use clamp_val instead of manual clamping (Louis Chauvet)

Perfect!
 
> v4:
>  - Clarify that we're packing 16-bit UNORM into s32, not
>converting values to a different representation (Pekka)
> 
> v3:
>  - Use new colorop->next pointer
> 
>  drivers/gpu/drm/vkms/vkms_composer.c | 27 +--
>  drivers/gpu/drm/vkms/vkms_drv.h  |  4 
>  2 files changed, 29 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index b4aaad2bf45f..01fa81ebc93b 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -159,7 +159,7 @@ static void apply_lut(const struct vkms_crtc_state 
> *crtc_state, struct line_buff
>   }
>  }
> 
> -static void apply_colorop(struct pixel_argb_u16 *pixel, struct drm_colorop 
> *colorop)
> +static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop 
> *colorop)

I agree with this change, but I think we may face conversion issues 
in apply_lut_to_channel_value, as it takes u16 and returns u16, but with 
your change, you pass s32 and expect s32.

If it is not an issue: Reviewed-by: Louis Chauvet 

>  {
>   struct drm_colorop_state *colorop_state = colorop->state;
> 
> @@ -185,9 +185,26 @@ static void apply_colorop(struct pixel_argb_u16 *pixel, 
> struct drm_colorop *colo
> 
>  static void pre_blend_color_transform(const struct vkms_plane_state 
> *plane_state, struct line_buffer *output_buffer)
>  {
> + struct pixel_argb_s32 pixel;
> +
>   for (size_t x = 0; x < output_buffer->n_pixels; x++) {
>   struct drm_colorop *colorop = 
> plane_state->base.base.color_pipeline;
> 
> + /*
> +  * Some operations, such as applying a BT709 encoding matrix,
> +  * followed by a decoding matrix, require that we preserve
> +  * values above 1.0 and below 0.0 until the end of the pipeline.
> +  *
> +  * Pack the 16-bit UNORM values into s32 to give us head-room to
> +  * avoid clipping until we're at the end of the pipeline. Clip
> +  * intentionally at the end of the pipeline before packing
> +  * UNORM values back into u16.
> +  */
> + pixel.a = output_buffer->pixels[x].a;
> + pixel.r = output_buffer->pixels[x].r;
> + pixel.g = output_buffer->pixels[x].g;
> + pixel.b = output_buffer->pixels[x].b;
> +
>   while (colorop) {
>   struct drm_colorop_state *colorop_state;
> 
> @@ -197,10 +214,16 @@ static void pre_blend_color_transform(const struct 
> vkms_plane_state *plane_state
>   return;
> 
>   if (!colorop_state->bypass)
> - apply_colorop(&output_buffer->pixels[x], 
> colorop);
> + apply_colorop(&pixel, colorop);
> 
>   colorop = colorop->next;
>   }
> +
> + /* clamp values */
> + output_buffer->pixels[x].a = clamp_val(pixel.a, 0, 0x);
> + output_buffer->pixels[x].r = clamp_val(pixel.r, 0, 0x);
> + output_buffer->pixels[x].g = clamp_val(pixel.g, 0, 0x);
> + output_buffer->pixels[x].b = clamp_val(pixel.b, 0, 0x);
>   }
>  }
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
> index 278cf3533f58..b78bc2611746 100644
> --- a/drivers/gpu/drm/vkms/vkms_drv.h
> +++ b/drivers/gpu/drm/vkms/vkms_drv.h
> @@ -36,6 +36,10 @@ struct vkms_frame_info {
>   unsigned int cpp;
>  };
> 
> +struct pixel_argb_s32 {
> + s32 a, r, g, b;
> +};
> +
>  struct pixel_argb_u16 {
>   u16 a, r, g, b;
>  };
> --
> 2.46.2
> 


Re: [PATCH v6 21/44] drm/vkms: Add tests for CTM handling

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:01, 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.
> 
> Signed-off-by: Harry Wentland 

Reviewed-by: Louis Chauvet 

> ---
> 
> v6:
>  - update reference values since we're now rounding
> 
> v5:
>  - Make apply_3x4_matrix static
> 
> 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
> 
>  drivers/gpu/drm/vkms/tests/vkms_color_test.c | 250 +++
>  drivers/gpu/drm/vkms/vkms_composer.c |   3 +-
>  drivers/gpu/drm/vkms/vkms_composer.h |   1 +
>  3 files changed, 253 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_test.c 
> b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
> index c36e67c7909e..d5eb1e4e9b67 100644
> --- a/drivers/gpu/drm/vkms/tests/vkms_color_test.c
> +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
> @@ -187,11 +187,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 = 0x8000;
> + ref.g = 0x4000;
> + ref.b = 0x4000;
> +
> + 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

Re: [PATCH v6 41/44] drm/colorop: allow non-bypass colorops

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:01, Harry Wentland wrote:
> Not all HW will be able to do bypass on all color
> operations. Introduce an 'allow_bypass' boolean for
> all colorop init functions and only create the BYPASS
> property when it's true.
> 
> Signed-off-by: Harry Wentland 
> ---
>  .../amd/display/amdgpu_dm/amdgpu_dm_colorop.c | 22 +---
>  drivers/gpu/drm/drm_atomic.c  |  3 +-
>  drivers/gpu/drm/drm_colorop.c | 51 ---
>  drivers/gpu/drm/vkms/vkms_colorop.c   |  8 +--
>  include/drm/drm_colorop.h | 10 ++--
>  5 files changed, 60 insertions(+), 34 deletions(-)
> 

[...]

> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -31,7 +31,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane 
> *plane, struct drm_pr
>   goto cleanup;
>   }
>  
> - ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
> + ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs, 
> true);
>   if (ret)
>   goto cleanup;
>  
> @@ -48,7 +48,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane 
> *plane, struct drm_pr
>   goto cleanup;
>   }
>  
> - ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane);
> + ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane, true);
>   if (ret)
>   goto cleanup;
>  
> @@ -64,7 +64,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane 
> *plane, struct drm_pr
>   goto cleanup;
>   }
>  
> - ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane);
> + ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane, true);
>   if (ret)
>   goto cleanup;
>  
> @@ -80,7 +80,7 @@ static int vkms_initialize_color_pipeline(struct drm_plane 
> *plane, struct drm_pr
>   goto cleanup;
>   }
>  
> - ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
> + ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs, 
> true);
>   if (ret)
>   goto cleanup;

You allow the bypass here, but you forgot to add a check in apply_colorop 
to bypass the colorop when this is set. It seems to be the case in the AMD 
driver too. Or maybe you wanted to pass false in "allow_bypass"?

>  
> diff --git a/include/drm/drm_colorop.h b/include/drm/drm_colorop.h
> index d3a00296973d..b8c1c4da3444 100644
> --- a/include/drm/drm_colorop.h
> +++ b/include/drm/drm_colorop.h
> @@ -333,14 +333,16 @@ static inline struct drm_colorop 
> *drm_colorop_find(struct drm_device *dev,
>  }
>  
>  int drm_colorop_curve_1d_init(struct drm_device *dev, struct drm_colorop 
> *colorop,
> -   struct drm_plane *plane, u64 supported_tfs);
> +   struct drm_plane *plane, u64 supported_tfs,
> +   bool allow_bypass);
>  int drm_colorop_curve_1d_lut_init(struct drm_device *dev, struct drm_colorop 
> *colorop,
> struct drm_plane *plane, uint32_t lut_size,
> -   enum drm_colorop_lut1d_interpolation_type 
> lut1d_interpolation);
> +   enum drm_colorop_lut1d_interpolation_type 
> lut1d_interpolation,
> +   bool allow_bypass);
>  int drm_colorop_ctm_3x4_init(struct drm_device *dev, struct drm_colorop 
> *colorop,
> -  struct drm_plane *plane);
> +  struct drm_plane *plane, bool allow_bypass);
>  int drm_colorop_mult_init(struct drm_device *dev, struct drm_colorop 
> *colorop,
> -   struct drm_plane *plane);
> +   struct drm_plane *plane, bool allow_bypass);
>  
>  struct drm_colorop_state *
>  drm_atomic_helper_colorop_duplicate_state(struct drm_colorop *colorop);
> -- 
> 2.46.2
> 


Re: [PATCH v6 03/44] drm/vkms: Add kunit tests for VKMS LUT handling

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:00, Harry Wentland wrote:
> Debugging LUT math is much easier when we can unit test
> it. Add kunit functionality to VKMS and add tests for
>  - get_lut_index
>  - lerp_u16
> 
> Signed-off-by: Harry Wentland 
> Cc: Arthur Grillo 
> ---
> v6:
>  - Eliminate need to include test as .c file (Louis Chauvet)

Thanks for the modification!

With the checkpatch warning fixed:
Reviewed-by: Louis Chauvet 

> v5:
>  - Bring back static for lerp_u16 and get_lut_index (Arthur)
> 
> v4:
>  - Test the critical points of the lerp function (Pekka)
> 
> v3:
>  - Use include way of testing static functions (Arthur)
> 
>  drivers/gpu/drm/vkms/Kconfig |  15 ++
>  drivers/gpu/drm/vkms/Makefile|   1 +
>  drivers/gpu/drm/vkms/tests/.kunitconfig  |   4 +
>  drivers/gpu/drm/vkms/tests/Makefile  |   3 +
>  drivers/gpu/drm/vkms/tests/vkms_color_test.c | 168 +++
>  drivers/gpu/drm/vkms/vkms_composer.c |  10 +-
>  drivers/gpu/drm/vkms/vkms_composer.h |  13 ++
>  7 files changed, 211 insertions(+), 3 deletions(-)
>  create mode 100644 drivers/gpu/drm/vkms/tests/.kunitconfig
>  create mode 100644 drivers/gpu/drm/vkms/tests/Makefile
>  create mode 100644 drivers/gpu/drm/vkms/tests/vkms_color_test.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_composer.h
> 
> diff --git a/drivers/gpu/drm/vkms/Kconfig b/drivers/gpu/drm/vkms/Kconfig
> index b9ecdebecb0b..59c4a32adb9d 100644
> --- a/drivers/gpu/drm/vkms/Kconfig
> +++ b/drivers/gpu/drm/vkms/Kconfig
> @@ -13,3 +13,18 @@ config DRM_VKMS
> a VKMS.
> 
> If M is selected the module will be called vkms.
> +
> +config DRM_VKMS_KUNIT_TESTS
> + tristate "KUnit tests for VKMS." if !KUNIT_ALL_TESTS
> + depends on DRM_VKMS=y && KUNIT
> + default KUNIT_ALL_TESTS
> + help
> +   This builds unit tests for VKMS. This option is not useful for
> +   distributions or general kernels, but only for kernel
> +   developers working on VKMS.
> +
> +   For more information on KUnit and unit tests in general,
> +   please refer to the KUnit documentation in
> +   Documentation/dev-tools/kunit/.
> +
> +   If in doubt, say "N".
> diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> index 1b28a6a32948..8d3e46dde635 100644
> --- a/drivers/gpu/drm/vkms/Makefile
> +++ b/drivers/gpu/drm/vkms/Makefile
> @@ -9,3 +9,4 @@ vkms-y := \
>   vkms_writeback.o
> 
>  obj-$(CONFIG_DRM_VKMS) += vkms.o
> +obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += tests/
> diff --git a/drivers/gpu/drm/vkms/tests/.kunitconfig 
> b/drivers/gpu/drm/vkms/tests/.kunitconfig
> new file mode 100644
> index ..70e378228cbd
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/.kunitconfig
> @@ -0,0 +1,4 @@
> +CONFIG_KUNIT=y
> +CONFIG_DRM=y
> +CONFIG_DRM_VKMS=y
> +CONFIG_DRM_VKMS_KUNIT_TESTS=y
> diff --git a/drivers/gpu/drm/vkms/tests/Makefile 
> b/drivers/gpu/drm/vkms/tests/Makefile
> new file mode 100644
> index ..7876ca7a3c42
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0-only
> +
> +obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += vkms_color_test.o
> \ No newline at end of file
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_test.c 
> b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
> new file mode 100644
> index ..efe139978860
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
> @@ -0,0 +1,168 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */

checkpatch ask for a // comment here for the SPDX license (If you know, 
why checkpatch ask for a // in .c but /* */ in .h?)

> +
> +#include 
> +
> +#include 
> +#include 
> +#include "../vkms_drv.h"
> +#include "../vkms_composer.h"
> +
> +#define TEST_LUT_SIZE 16
> +
> +MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING);
> +
> +static struct drm_color_lut test_linear_array[TEST_LUT_SIZE] = {
> + { 0x0, 0x0, 0x0, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> + { 0x, 0x, 0x, 0 },
> +};
> +
> +const struct vkms_color_lut test_linear_lut = {
> + .base = test_linear_array,
> + .lut_length = TEST_LUT_SIZE,
> + .channel_value2index_ratio = 0xf000fll
> +};
> +
> +

checkpatch: Please don't use multiple blank lines

> +static void vkms_color_test_get_lut_index(struct kunit *test)
> +{
> + int i;
> +
> + KUNIT_EXPECT_EQ(test, drm_fixp2int(get_lut_index(&test_linear_lut, 
> test_linear_array[0].red)), 0);

checkpatch: 

Re: [PATCH v6 20/44] drm/tests: Add a few tests around drm_fixed.h

2024-10-04 Thread Louis Chauvet
Few checkpatch warnings:
- line length of 103 exceeds 100 columns
- Blank lines aren't necessary before a close brace '}'
- adding a line without newline at end of file

With those fixed:
Reviewd-by: Louis Chauvet 

On 03/10/24 - 16:01, Harry Wentland wrote:
> While working on the CTM implementation of VKMS I had to ascertain
> myself of a few assumptions. One of those is whether drm_fixed.h
> treats its numbers using signed-magnitude or twos-complement. It is
> twos-complement.
> 
> In order to make someone else's day easier I am adding the
> drm_test_int2fixp test that validates the above assumption.
> 
> I am also adding a test for the new sm2fixp function that converts
> from a signed-magnitude fixed point to the twos-complement fixed
> point.
> 
> Signed-off-by: Harry Wentland 
> ---
> 
> v6:
>  - add missing MODULE_DESCRIPTION (Jeff Johnson)
>  - fix buffer overflow
> 
>  drivers/gpu/drm/tests/Makefile|  3 +-
>  drivers/gpu/drm/tests/drm_fixp_test.c | 70 +++
>  2 files changed, 72 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/tests/drm_fixp_test.c
> 
> diff --git a/drivers/gpu/drm/tests/Makefile b/drivers/gpu/drm/tests/Makefile
> index 56dab563abd7..bd69df0eee33 100644
> --- a/drivers/gpu/drm/tests/Makefile
> +++ b/drivers/gpu/drm/tests/Makefile
> @@ -20,6 +20,7 @@ obj-$(CONFIG_DRM_KUNIT_TEST) += \
>   drm_modes_test.o \
>   drm_plane_helper_test.o \
>   drm_probe_helper_test.o \
> - drm_rect_test.o
> + drm_rect_test.o \
> + drm_fixp_test.o
> 
>  CFLAGS_drm_mm_test.o := $(DISABLE_STRUCTLEAK_PLUGIN)
> diff --git a/drivers/gpu/drm/tests/drm_fixp_test.c 
> b/drivers/gpu/drm/tests/drm_fixp_test.c
> new file mode 100644
> index ..24a801cf38be
> --- /dev/null
> +++ b/drivers/gpu/drm/tests/drm_fixp_test.c
> @@ -0,0 +1,70 @@
> +// SPDX-License-Identifier: MIT
> +/*
> + * Copyright 2022 Advanced Micro Devices, Inc.
> + */
> +
> +#include 
> +#include 
> +
> +static void drm_test_sm2fixp(struct kunit *test)
> +{
> + KUNIT_EXPECT_EQ(test, 0x7fffll, ((1ull << 63) - 1));
> +
> + /* 1 */
> + KUNIT_EXPECT_EQ(test, drm_int2fixp(1), drm_sm2fixp(1ull << 
> DRM_FIXED_POINT));
> +
> + /* -1 */
> + KUNIT_EXPECT_EQ(test, drm_int2fixp(-1), drm_sm2fixp((1ull << 63) | 
> (1ull << DRM_FIXED_POINT)));
> +
> + /* 0.5 */
> + KUNIT_EXPECT_EQ(test, drm_fixp_from_fraction(1, 2), drm_sm2fixp(1ull << 
> (DRM_FIXED_POINT - 1)));
> +
> + /* -0.5 */
> + KUNIT_EXPECT_EQ(test, drm_fixp_from_fraction(-1, 2), drm_sm2fixp((1ull 
> << 63) | (1ull << (DRM_FIXED_POINT - 1;
> +
> +}
> +
> +static void drm_test_int2fixp(struct kunit *test)
> +{
> + /* 1 */
> + KUNIT_EXPECT_EQ(test, 1ll << 32, drm_int2fixp(1));
> +
> + /* -1 */
> + KUNIT_EXPECT_EQ(test, -(1ll << 32), drm_int2fixp(-1));
> +
> + /* 1 + (-1) = 0 */
> + KUNIT_EXPECT_EQ(test, 0, drm_int2fixp(1) + drm_int2fixp(-1));
> +
> + /* 1 / 2 */
> + KUNIT_EXPECT_EQ(test, 1ll << 31, drm_fixp_from_fraction(1, 2));
> +
> + /* -0.5 */
> + KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(-1, 2));
> +
> + /* (1 / 2) + (-1) = 0.5 */
> + KUNIT_EXPECT_EQ(test, 1ll << 31, drm_fixp_from_fraction(-1, 2) + 
> drm_int2fixp(1));
> +
> + /* (1 / 2) - 1) = 0.5 */
> + KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(1, 2) + 
> drm_int2fixp(-1));
> +
> + /* (1 / 2) - 1) = 0.5 */
> + KUNIT_EXPECT_EQ(test, -(1ll << 31), drm_fixp_from_fraction(1, 2) - 
> drm_int2fixp(1));
> +
> +}
> +
> +static struct kunit_case drm_fixp_tests[] = {
> + KUNIT_CASE(drm_test_int2fixp),
> + KUNIT_CASE(drm_test_sm2fixp),
> + { }
> +};
> +
> +static struct kunit_suite drm_rect_test_suite = {
> + .name = "drm_fixp",
> + .test_cases = drm_fixp_tests,
> +};
> +
> +kunit_test_suite(drm_rect_test_suite);
> +
> +MODULE_AUTHOR("AMD");
> +MODULE_LICENSE("GPL and additional rights");
> +MODULE_DESCRIPTION("Unit tests for drm_fixed.h");
> \ No newline at end of file
> --
> 2.46.2
> 


Re: [PATCH v6 19/44] drm/vkms: add 3x4 matrix in color pipeline

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:01, Harry Wentland wrote:
> We add two 3x4 matrices into the VKMS color pipeline. The reason
> we're adding matrices is so that we can test that application
> of a matrix and its inverse yields an output equal to the input
> image.
> 
> One complication with the matrix implementation has to do with
> the fact that the matrix entries are in signed-magnitude fixed
> point, whereas the drm_fixed.h implementation uses 2s-complement.
> The latter one is the one that we want for easy addition and
> subtraction, so we convert all entries to 2s-complement.
> 
> Signed-off-by: Harry Wentland 
> ---
> 
> v6:
>  - pre-compute colors (Louis Chauvet)
>  - round matrix output (Louis Chauvet)
> 
>  drivers/gpu/drm/vkms/vkms_colorop.c  | 34 +++-
>  drivers/gpu/drm/vkms/vkms_composer.c | 32 ++
>  2 files changed, 65 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c 
> b/drivers/gpu/drm/vkms/vkms_colorop.c
> index aebd467c4e61..f0c264820a81 100644
> --- a/drivers/gpu/drm/vkms/vkms_colorop.c
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -12,7 +12,7 @@ static const u64 supported_tfs =
>   BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
>   BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF);
> 
> -#define MAX_COLOR_PIPELINE_OPS 2
> +#define MAX_COLOR_PIPELINE_OPS 4
> 
>  static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct 
> drm_prop_enum_list *list)
>  {
> @@ -48,6 +48,38 @@ static int vkms_initialize_color_pipeline(struct drm_plane 
> *plane, struct drm_pr
>   goto cleanup;
>   }
> 
> + ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane);
> + if (ret)
> + goto cleanup;
> +
> + drm_colorop_set_next_property(ops[i-1], ops[i]);

checkpatch: spaces preferred around that '-' (ctx:VxV)

> +
> + i++;
> +
> + /* 3rd op: 3x4 matrix */
> + ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);

checkpatch: Prefer kzalloc(sizeof(*ops[i])...) over kzalloc(sizeof(struct 
drm_colorop)...)

> + if (!ops[i]) {
> + DRM_ERROR("KMS: Failed to allocate colorop\n");
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> +
> + ret = drm_colorop_ctm_3x4_init(dev, ops[i], plane);
> + if (ret)
> + goto cleanup;
> +
> + drm_colorop_set_next_property(ops[i-1], ops[i]);

checkpatch: preferred around that '-' (ctx:VxV)

> +
> + i++;
> +
> + /* 4th op: 1d curve */
> + ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);

checkpatch: Prefer kzalloc(sizeof(*ops[i])...) over kzalloc(sizeof(struct 
drm_colorop)...)

> + if (!ops[i]) {
> + DRM_ERROR("KMS: Failed to allocate colorop\n");
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> +
>   ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
>   if (ret)
>   goto cleanup;
> diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
> b/drivers/gpu/drm/vkms/vkms_composer.c
> index 01fa81ebc93b..c8b9b9d7f78f 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -159,6 +159,35 @@ 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)
> +{
> + s64 rf, gf, bf;
> + s64 r, g, b;
> +
> + r = drm_int2fixp(pixel->r);
> + g = drm_int2fixp(pixel->g);
> + b = drm_int2fixp(pixel->b);
> +
> + rf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[0]), r) +
> +  drm_fixp_mul(drm_sm2fixp(matrix->matrix[1]), g) +
> +  drm_fixp_mul(drm_sm2fixp(matrix->matrix[2]), b) +
> +  drm_sm2fixp(matrix->matrix[3]);
> +
> + gf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[4]), r) +
> +  drm_fixp_mul(drm_sm2fixp(matrix->matrix[5]), g) +
> +  drm_fixp_mul(drm_sm2fixp(matrix->matrix[6]), b) +
> +  drm_sm2fixp(matrix->matrix[7]);
> +
> + bf = drm_fixp_mul(drm_sm2fixp(matrix->matrix[8]), r) +
> +  drm_fixp_mul(drm_sm2fixp(matrix->matrix[9]), g) +
> +  drm_fixp_mul(drm_sm2fixp(matrix->matrix[10]), b) +
> +  drm_sm2fixp(matrix->matrix[11]);
> +
> + pixel->r = drm_fixp2int_round(rf);
> + pixel->g = drm_fixp2int_round(gf);
> + pixel->b = drm_fixp2int_round(bf);
> +}
> +
>  static void apply_colorop(struct pixel_argb_s32 *pixel, struct drm_colorop 
> *colorop)
>  {
>   struct drm_colorop_state *colorop_state = colorop->state;
> @@ -179,6 +208,9 @@ static void apply_colorop(struct pixel_argb_s32 *pixel, 
> struct drm_colorop *colo
>   DRM_DEBUG_DRIVER("unkown colorop 1D curve type 
> %d\n", colorop_state->curve_1d_type);
>   break;
>   }
> + } else if (colorop->type == DRM_COLOROP_CTM_3X4) {
> + if (colorop_state->data)
> + apply_3x4_matrix(pixel,

Re: [PATCH v6 15/44] drm/vkms: Add kunit tests for linear and sRGB LUTs

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:00, Harry Wentland wrote:

checkpatch: commit description - Add an appropriate one

> Signed-off-by: Harry Wentland 
> ---
>  drivers/gpu/drm/vkms/tests/vkms_color_test.c | 38 +++-
>  drivers/gpu/drm/vkms/vkms_composer.c | 15 ++--
>  drivers/gpu/drm/vkms/vkms_composer.h | 13 +++
>  3 files changed, 53 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/gpu/drm/vkms/tests/vkms_color_test.c 
> b/drivers/gpu/drm/vkms/tests/vkms_color_test.c
> index efe139978860..c36e67c7909e 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 @@ const struct vkms_color_lut test_linear_lut = {
>   .channel_value2index_ratio = 0xf000fll
>  };
>  
> -
>  static void vkms_color_test_get_lut_index(struct kunit *test)
>  {
>   int i;
> @@ -45,6 +45,19 @@ static void vkms_color_test_get_lut_index(struct kunit 
> *test)
>  
>   for (i = 0; i < TEST_LUT_SIZE; i++)
>   KUNIT_EXPECT_EQ(test, 
> drm_fixp2int_ceil(get_lut_index(&test_linear_lut, test_linear_array[i].red)), 
> 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);
>  }
>  
>  static void vkms_color_test_lerp(struct kunit *test)
> @@ -153,9 +166,32 @@ 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);

checkpatch: Missing a blank line after declarations

> + 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);

checkpatch: spaces preferred around that '-/+' (ctx:VxV)

> + }
> +}
> +
>  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 a35466e68237..b4aaad2bf45f 100644
> --- a/drivers/gpu/drm/vkms/vkms_composer.c
> +++ b/drivers/gpu/drm/vkms/vkms_composer.c
> @@ -113,18 +113,7 @@ 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,
> +VISIBLE_IF_KUNIT u16 apply_lut_to_channel_value(const struct vkms_color_lut 
> *lut, u16 channel_value,
> enum lut_channel channel)

checkpatch: Alignment should match open parenthesis

>  {
>   s64 lut_index = get_lut_index(lut, channel_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)
> 

Re: [PATCH v6 14/44] drm/vkms: Add enumerated 1D curve colorop

2024-10-04 Thread Louis Chauvet
On 03/10/24 - 16:00, Harry Wentland wrote:
> This patch introduces a VKMS color pipeline that includes two
> drm_colorops for named transfer functions. For now the only ones
> supported are sRGB EOTF, sRGB Inverse EOTF, and a Linear TF.
> We will expand this in the future but I don't want to do so
> without accompanying IGT tests.
> 
> We introduce a new vkms_luts.c file that hard-codes sRGB EOTF,
> sRGB Inverse EOTF, and a linear EOTF LUT. These have been
> generated with 256 entries each as IGT is currently testing
> only 8 bpc surfaces. We will likely need higher precision
> but I'm reluctant to make that change without clear indication
> that we need it. We'll revisit and, if necessary, regenerate
> the LUTs when we have IGT tests for higher precision buffers.
> 
> Signed-off-by: Harry Wentland 
> Signed-off-by: Alex Hung 
> ---
> 
> v6:
>  - drop 'len' var (Louis Chauvet)
>  - cleanup if colorop alloc or init fails (Louis Chauvet)
>  - switch loop in pre_blend_transform (Louis Chauvet)
>  - drop extraneous if (colorop) inside while (colorop) (Louis Chauvet)

thanks!

> v5:
>  - Squash with "Pull apply_colorop out of pre_blend_color_transform"
>(Sebastian)
>  - Fix warnings
>  - Fix include
>  - Drop TODOs
> 
> v4:
>  - Drop _tf_ from color_pipeline init function
>  - Pass supported TFs into colorop init
>  - Create bypass pipeline in DRM helper (Pekka)
> 
> v2:
>  - Add commit description
>  - Fix sRGB EOTF LUT definition
>  - Add linear and sRGB inverse EOTF LUTs
> 
>  drivers/gpu/drm/vkms/Makefile|   4 +-
>  drivers/gpu/drm/vkms/vkms_colorop.c  |  81 +++
>  drivers/gpu/drm/vkms/vkms_composer.c |  49 ++
>  drivers/gpu/drm/vkms/vkms_drv.h  |   4 +
>  drivers/gpu/drm/vkms/vkms_luts.c | 802 +++
>  drivers/gpu/drm/vkms/vkms_luts.h |  12 +
>  drivers/gpu/drm/vkms/vkms_plane.c|   2 +
>  7 files changed, 953 insertions(+), 1 deletion(-)
>  create mode 100644 drivers/gpu/drm/vkms/vkms_colorop.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.c
>  create mode 100644 drivers/gpu/drm/vkms/vkms_luts.h
> 
> diff --git a/drivers/gpu/drm/vkms/Makefile b/drivers/gpu/drm/vkms/Makefile
> index 8d3e46dde635..0bf3c116f1ae 100644
> --- a/drivers/gpu/drm/vkms/Makefile
> +++ b/drivers/gpu/drm/vkms/Makefile
> @@ -6,7 +6,9 @@ vkms-y := \
>   vkms_formats.o \
>   vkms_crtc.o \
>   vkms_composer.o \
> - vkms_writeback.o
> + vkms_writeback.o \
> + vkms_colorop.o \
> + vkms_luts.o
> 
>  obj-$(CONFIG_DRM_VKMS) += vkms.o
>  obj-$(CONFIG_DRM_VKMS_KUNIT_TESTS) += tests/
> diff --git a/drivers/gpu/drm/vkms/vkms_colorop.c 
> b/drivers/gpu/drm/vkms/vkms_colorop.c
> new file mode 100644
> index ..aebd467c4e61
> --- /dev/null
> +++ b/drivers/gpu/drm/vkms/vkms_colorop.c
> @@ -0,0 +1,81 @@
> +/* SPDX-License-Identifier: GPL-2.0+ */

checkpatch: Improper SPDX comment style for 
'drivers/gpu/drm/vkms/vkms_colorop.c', please use '//' instead

> +
> +#include 
> +#include 
> +#include 
> +#include 
> +#include 
> +
> +#include "vkms_drv.h"
> +
> +static const u64 supported_tfs =
> + BIT(DRM_COLOROP_1D_CURVE_SRGB_EOTF) |
> + BIT(DRM_COLOROP_1D_CURVE_SRGB_INV_EOTF);
> +
> +#define MAX_COLOR_PIPELINE_OPS 2
> +
> +static int vkms_initialize_color_pipeline(struct drm_plane *plane, struct 
> drm_prop_enum_list *list)
> +{
> + struct drm_colorop *ops[MAX_COLOR_PIPELINE_OPS];
> + struct drm_device *dev = plane->dev;
> + int ret;
> + int i = 0;
> +
> + memset(ops, 0, sizeof(ops));
> +
> + /* 1st op: 1d curve */
> + ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);

checkpatch: Prefer kzalloc(sizeof(*ops[i])...) over kzalloc(sizeof(struct 
drm_colorop)...)

> + if (!ops[i]) {
> + DRM_ERROR("KMS: Failed to allocate colorop\n");
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> +
> + ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
> + if (ret)
> + goto cleanup;
> +
> + list->type = ops[i]->base.id;
> + list->name = kasprintf(GFP_KERNEL, "Color Pipeline %d", 
> ops[i]->base.id);
> +
> + i++;
> +
> + /* 2nd op: 1d curve */
> + ops[i] = kzalloc(sizeof(struct drm_colorop), GFP_KERNEL);

ditto

> + if (!ops[i]) {
> + DRM_ERROR("KMS: Failed to allocate colorop\n");
> + ret = -ENOMEM;
> + goto cleanup;
> + }
> +
> + ret = drm_colorop_curve_1d_init(dev, ops[i], plane, supported_tfs);
> + if (ret)
> + goto cleanup;
> +
> + drm_colorop_set_next_property(ops[i-1], ops[i]);

checkpatch: spaces preferred around that '-' (ctx:VxV)

> +
> + return 0;
> +
> +cleanup:
> + for (; i >= 0; i--)
> + if (ops[i])
> + kfree(ops[i]);

I think this is not sufficient, drm_colorop_curve_1d_init seems to 
call many drm_* helpers to create properties and store pointers to dev.

Issues I found:

drm_colorop.c:105
list_add_tail(&colorop-

Re: [PATCH v6 15/44] drm/vkms: Add kunit tests for linear and sRGB LUTs

2024-10-04 Thread kernel test robot
Hi Harry,

kernel test robot noticed the following build errors:

[auto build test ERROR on drm/drm-next]
[also build test ERROR on drm-exynos/exynos-drm-next drm-misc/drm-misc-next 
linus/master v6.12-rc1 next-20241004]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:
https://github.com/intel-lab-lkp/linux/commits/Harry-Wentland/drm-Add-helper-for-conversion-from-signed-magnitude/20241004-040629
base:   git://anongit.freedesktop.org/drm/drm drm-next
patch link:
https://lore.kernel.org/r/20241003200129.1732122-16-harry.wentland%40amd.com
patch subject: [PATCH v6 15/44] drm/vkms: Add kunit tests for linear and sRGB 
LUTs
config: x86_64-randconfig-101-20241005 
(https://download.01.org/0day-ci/archive/20241005/202410051303.vy1ejqpj-...@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 
3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): 
(https://download.01.org/0day-ci/archive/20241005/202410051303.vy1ejqpj-...@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot 
| Closes: 
https://lore.kernel.org/oe-kbuild-all/202410051303.vy1ejqpj-...@intel.com/

All errors (new ones prefixed by >>, old ones prefixed by <<):

WARNING: modpost: missing MODULE_DESCRIPTION() in 
drivers/gpu/drm/vkms/tests/vkms_color_test.o
>> ERROR: modpost: "srgb_eotf" [drivers/gpu/drm/vkms/tests/vkms_color_test.ko] 
>> undefined!
>> ERROR: modpost: "srgb_inv_eotf" 
>> [drivers/gpu/drm/vkms/tests/vkms_color_test.ko] undefined!
>> ERROR: modpost: "linear_eotf" 
>> [drivers/gpu/drm/vkms/tests/vkms_color_test.ko] undefined!

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki