On Tue, 10 Feb 2026 14:44:00 -0300 Ariel D'Alessandro <[email protected]> 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 <[email protected]> > --- > 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 cd85de4ffd03d..d53ea4189c97b 100644 > --- a/drivers/gpu/drm/vkms/vkms_composer.c > +++ b/drivers/gpu/drm/vkms/vkms_composer.c > @@ -17,6 +17,8 @@ > #include "vkms_composer.h" > #include "vkms_luts.h" > > +#define UNORM_16BIT_ONE (1ULL << 16) Hi, shouldn't this be 0xffff instead? > + > static u16 pre_mul_blend_channel(u16 src, u16 dst, u16 alpha) > { > u32 new_color; > @@ -139,20 +141,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); > Ok, so this is because r, g, b have the integer pixel value [0, 65535]. A casual reader would expect them to be normalized [0.0, 1.0] (as is possible without any loss of precision). But since they are not normalized, multiplication by normalized 1.0 must be carried out explicitly. If UNORM_16BIT_ONE was 0xffff, you would have my Reviewed-by: Pekka Paalanen <[email protected]> Thanks, pq
pgpn9EpSsCaNH.pgp
Description: OpenPGP digital signature
