From bff4d3cd3c9fcdd75a3d7fa0907de05526f74cf6 Mon Sep 17 00:00:00 2001 From: Harry Wentland Date: Fri, 14 Nov 2025 17:01:44 -0700 Subject: [PATCH] drm/vkms: Use s32 for internal color pipeline precision Certain operations require us to preserve values below 0.0 and above 1.0 (0x0 and 0xffff 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. Reviewed-by: Louis Chauvet Signed-off-by: Alex Hung Signed-off-by: Harry Wentland Reviewed-by: Daniel Stone Signed-off-by: Simon Ser Link: https://patch.msgid.link/20251115000237.3561250-20-alex.hung@amd.com --- 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 82116e283e6a..3c1ec430f509 100644 --- a/drivers/gpu/drm/vkms/vkms_composer.c +++ b/drivers/gpu/drm/vkms/vkms_composer.c @@ -128,7 +128,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) { struct drm_colorop_state *colorop_state = colorop->state; struct drm_device *dev = colorop->dev; @@ -157,9 +157,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; @@ -169,10 +186,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, 0xffff); + output_buffer->pixels[x].r = clamp_val(pixel.r, 0, 0xffff); + output_buffer->pixels[x].g = clamp_val(pixel.g, 0, 0xffff); + output_buffer->pixels[x].b = clamp_val(pixel.b, 0, 0xffff); } } diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h index b4e5e4b9958f..0933e4ce0ff0 100644 --- a/drivers/gpu/drm/vkms/vkms_drv.h +++ b/drivers/gpu/drm/vkms/vkms_drv.h @@ -45,6 +45,10 @@ struct vkms_frame_info { unsigned int rotation; }; +struct pixel_argb_s32 { + s32 a, r, g, b; +}; + /** * struct pixel_argb_u16 - Internal representation of a pixel color. * @a: Alpha component value, stored in 16 bits, without padding, using -- 2.47.3