[RFC PATCH v3 20/23] drm/vkms: Use s32 for internal color pipeline precision

[Harry Wentland]

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.

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.

v3:
 - Use new colorop->next pointer

Signed-off-by: Harry Wentland <harry.wentland at amd.com>
---
 drivers/gpu/drm/vkms/vkms_composer.c | 55 +++++++++++++++++++++-------
 drivers/gpu/drm/vkms/vkms_drv.h      |  4 ++
 2 files changed, 46 insertions(+), 13 deletions(-)

diff --git a/drivers/gpu/drm/vkms/vkms_composer.c b/drivers/gpu/drm/vkms/vkms_composer.c
index 9010415e4bd6..d04a235b9fcd 100644
--- a/drivers/gpu/drm/vkms/vkms_composer.c
+++ b/drivers/gpu/drm/vkms/vkms_composer.c
@@ -164,7 +164,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)
 {
 	/* TODO is this right? */
 	struct drm_colorop_state *colorop_state = colorop->state;
@@ -191,25 +191,54 @@ 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 drm_colorop *colorop = plane_state->base.base.color_pipeline;
+	struct drm_colorop *colorop;
+	struct pixel_argb_s32 pixel;
 
-	while (colorop) {
-		struct drm_colorop_state *colorop_state;
+	for (size_t x = 0; x < output_buffer->n_pixels; x++) {
+
+		/*
+		 * 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.
+		 *
+		 * Convert values to s32 for our internal pipeline and go back
+		 * to u16 values at the end.
+		 */
+		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;
+
+		colorop = plane_state->base.base.color_pipeline;
+		while (colorop) {
+			struct drm_colorop_state *colorop_state;
 
-		if (!colorop)
-			return;
+			if (!colorop)
+				return;
 
-		/* TODO this is probably wrong */
-		colorop_state = colorop->state;
+			/* TODO this is probably wrong */
+			colorop_state = colorop->state;
 
-		if (!colorop_state)
-			return;
+			if (!colorop_state)
+				return;
 
-		for (size_t x = 0; x < output_buffer->n_pixels; x++)
 			if (!colorop_state->bypass)
-				apply_colorop(&output_buffer->pixels[x], colorop);
+				apply_colorop(&pixel, colorop);
+
+			colorop = colorop->next;
+		}
 
-		colorop = colorop->next;
+		/* clamp pixel */
+		pixel.a = max(min(pixel.a, 0xffff), 0x0);
+		pixel.r = max(min(pixel.r, 0xffff), 0x0);
+		pixel.g = max(min(pixel.g, 0xffff), 0x0);
+		pixel.b = max(min(pixel.b, 0xffff), 0x0);
+
+		/* put back to output_buffer */
+		output_buffer->pixels[x].a = pixel.a;
+		output_buffer->pixels[x].r = pixel.r;
+		output_buffer->pixels[x].g = pixel.g;
+		output_buffer->pixels[x].b = pixel.b;
 	}
 }
 
diff --git a/drivers/gpu/drm/vkms/vkms_drv.h b/drivers/gpu/drm/vkms/vkms_drv.h
index 2bcc24c196a2..fadb7685a360 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.42.1