Re: [PATCH] drm/vkms: Fix bad matrix offset component multiplication

[Louis Chauvet]

On 12/23/25 18:26, Ariel D'Alessandro wrote:
Pixels values are packed as 16-bit UNORM values, so the matrix offset
components must be multiplied properly by the idempotent element -i.e.
number 1 encoded as 16-bit UNORM-.

Signed-off-by: Ariel D'Alessandro <ariel.dalessan...@collabora.com>

Hi,

Thanks for this patch!

How did you catch the issue? If you have an example of failing color 
conversion, can you add a test case in vkms_color_test?

Thanks,
Louis Chauvet

---
  drivers/gpu/drm/vkms/vkms_composer.c | 13 ++++++++++---
  1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c 
b/drivers/gpu/drm/vkms/vkms_composer.c
index 3cf3f26e0d8ea..efdaf665435d9 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -16,6 +16,8 @@
  #include "vkms_composer.h"
  #include "vkms_luts.h"
  
+#define UNORM_16BIT_ONE			(1ULL << 16)
+
  static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha)
  {
        u32 new_color;
@@ -138,20 +140,25 @@ VISIBLE_IF_KUNIT void apply_3x4_matrix(struct 
pixel_argb_s32 *pixel,
        g = drm_int2fixp(pixel->g);
        b = drm_int2fixp(pixel->b);
  
+	/*
+        * Pixels values are packed as 16-bit UNORM values, so the matrix offset
+        * components must be multiplied properly by the idempotent element 
-i.e.
+        * number 1 encoded as 16-bit UNORM-.
+        */
        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]);
+            drm_fixp_mul(drm_sm2fixp(matrix->matrix[3]), 
drm_int2fixp(UNORM_16BIT_ONE));
  
  	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]);
+            drm_fixp_mul(drm_sm2fixp(matrix->matrix[7]), 
drm_int2fixp(UNORM_16BIT_ONE));
  
  	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]);
+            drm_fixp_mul(drm_sm2fixp(matrix->matrix[11]), 
drm_int2fixp(UNORM_16BIT_ONE));
  
  	pixel->r = drm_fixp2int_round(rf);
        pixel->g = drm_fixp2int_round(gf);

---
base-commit: b96bcfba104c65db41378a04f5ccac186f79578f
change-id: 20251223-vkms-composer-fix-matrix-aa2c593f4515

Best regards,