[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <28d9f246-8952-4f03-94bc-60eadca78c2f@bootlin.com>
Date: Mon, 29 Dec 2025 10:33:53 +0100
From: Louis Chauvet <louis.chauvet@...tlin.com>
To: Ariel D'Alessandro <ariel.dalessandro@...labora.com>,
Haneen Mohammed <hamohammed.sa@...il.com>, Simona Vetter <simona@...ll.ch>,
Melissa Wen <melissa.srw@...il.com>,
Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>,
Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann <tzimmermann@...e.de>,
David Airlie <airlied@...il.com>
Cc: dri-devel@...ts.freedesktop.org, linux-kernel@...r.kernel.org,
nfraprado@...labora.com, pekka.paalanen@...labora.com,
daniels@...labora.com, kernel@...labora.com
Subject: Re: [PATCH] drm/vkms: Fix bad matrix offset component multiplication
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.dalessandro@...labora.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,
Powered by blists - more mailing lists