[PATCH] media: hantro: Avoid global variable for jpeg quantization tables

* [PATCH] media: hantro: Avoid global variable for jpeg quantization tables
@ 2021-11-11 22:01 James Cowgill
  2021-11-17 10:58 ` Ezequiel Garcia
  0 siblings, 1 reply; 2+ messages in thread
From: James Cowgill @ 2021-11-11 22:01 UTC (permalink / raw)
  To: Ezequiel Garcia, Philipp Zabel, Mauro Carvalho Chehab,
	Greg Kroah-Hartman, Heiko Stuebner
  Cc: James Cowgill, linux-media, linux-rockchip, linux-staging,
	linux-kernel, linux-arm-kernel

On a system with multiple encoders present, it's possible for two
encoders to write to the global luma and chroma quantization tables at
the same time if they both submit a JPEG frame to be encoded. Avoid
this race by moving the tables into the `jpeg_ctx` structure which is
stored on the stack.

Signed-off-by: James Cowgill <james.cowgill@blaize.com>
---
 .../staging/media/hantro/hantro_h1_jpeg_enc.c |  5 ++-
 drivers/staging/media/hantro/hantro_jpeg.c    | 31 ++++++-------------
 drivers/staging/media/hantro/hantro_jpeg.h    |  4 ++-
 .../media/hantro/rockchip_vpu2_hw_jpeg_enc.c  |  5 ++-
 4 files changed, 16 insertions(+), 29 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
index 56cf261a8e95..20dafd6eb6b9 100644
--- a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
+++ b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c
@@ -113,9 +113,8 @@ int hantro_h1_jpeg_enc_run(struct hantro_ctx *ctx)
 
 	hantro_h1_set_src_img_ctrl(vpu, ctx);
 	hantro_h1_jpeg_enc_set_buffers(vpu, ctx, &src_buf->vb2_buf);
-	hantro_h1_jpeg_enc_set_qtable(vpu,
-				      hantro_jpeg_get_qtable(0),
-				      hantro_jpeg_get_qtable(1));
+	hantro_h1_jpeg_enc_set_qtable(vpu, jpeg_ctx.hw_luma_qtable,
+				      jpeg_ctx.hw_chroma_qtable);
 
 	reg = H1_REG_AXI_CTRL_OUTPUT_SWAP16
 		| H1_REG_AXI_CTRL_INPUT_SWAP16
diff --git a/drivers/staging/media/hantro/hantro_jpeg.c b/drivers/staging/media/hantro/hantro_jpeg.c
index 36c140fc6a36..df62fbdff7c9 100644
--- a/drivers/staging/media/hantro/hantro_jpeg.c
+++ b/drivers/staging/media/hantro/hantro_jpeg.c
@@ -36,8 +36,6 @@ static const unsigned char luma_q_table[] = {
 	0x48, 0x5c, 0x5f, 0x62, 0x70, 0x64, 0x67, 0x63
 };
 
-static unsigned char luma_q_table_reordered[ARRAY_SIZE(luma_q_table)];
-
 static const unsigned char chroma_q_table[] = {
 	0x11, 0x12, 0x18, 0x2f, 0x63, 0x63, 0x63, 0x63,
 	0x12, 0x15, 0x1a, 0x42, 0x63, 0x63, 0x63, 0x63,
@@ -49,8 +47,6 @@ static const unsigned char chroma_q_table[] = {
 	0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63, 0x63
 };
 
-static unsigned char chroma_q_table_reordered[ARRAY_SIZE(chroma_q_table)];
-
 static const unsigned char zigzag[64] = {
 	 0,  1,  8, 16,  9,  2,  3, 10,
 	17, 24, 32, 25, 18, 11,  4,  5,
@@ -277,7 +273,7 @@ jpeg_scale_quant_table(unsigned char *file_q_tab,
 	}
 }
 
-static void jpeg_set_quality(unsigned char *buffer, int quality)
+static void jpeg_set_quality(struct hantro_jpeg_ctx *ctx)
 {
 	int scale;
 
@@ -285,24 +281,15 @@ static void jpeg_set_quality(unsigned char *buffer, int quality)
 	 * Non-linear scaling factor:
 	 * [5,50] -> [1000..100], [51,100] -> [98..0]
 	 */
-	if (quality < 50)
-		scale = 5000 / quality;
+	if (ctx->quality < 50)
+		scale = 5000 / ctx->quality;
 	else
-		scale = 200 - 2 * quality;
-
-	jpeg_scale_quant_table(buffer + LUMA_QUANT_OFF,
-			       luma_q_table_reordered,
-			       luma_q_table, scale);
-	jpeg_scale_quant_table(buffer + CHROMA_QUANT_OFF,
-			       chroma_q_table_reordered,
-			       chroma_q_table, scale);
-}
+		scale = 200 - 2 * ctx->quality;
 
-unsigned char *hantro_jpeg_get_qtable(int index)
-{
-	if (index == 0)
-		return luma_q_table_reordered;
-	return chroma_q_table_reordered;
+	jpeg_scale_quant_table(ctx->buffer + LUMA_QUANT_OFF,
+			       ctx->hw_luma_qtable, luma_q_table, scale);
+	jpeg_scale_quant_table(ctx->buffer + CHROMA_QUANT_OFF,
+			       ctx->hw_chroma_qtable, chroma_q_table, scale);
 }
 
 void hantro_jpeg_header_assemble(struct hantro_jpeg_ctx *ctx)
@@ -324,7 +311,7 @@ void hantro_jpeg_header_assemble(struct hantro_jpeg_ctx *ctx)
 	memcpy(buf + HUFF_CHROMA_AC_OFF, chroma_ac_table,
 	       sizeof(chroma_ac_table));
 
-	jpeg_set_quality(buf, ctx->quality);
+	jpeg_set_quality(ctx);
 }
 
 int hantro_jpeg_enc_init(struct hantro_ctx *ctx)
diff --git a/drivers/staging/media/hantro/hantro_jpeg.h b/drivers/staging/media/hantro/hantro_jpeg.h
index 9474a00277f8..035ab25b803f 100644
--- a/drivers/staging/media/hantro/hantro_jpeg.h
+++ b/drivers/staging/media/hantro/hantro_jpeg.h
@@ -1,13 +1,15 @@
 /* SPDX-License-Identifier: GPL-2.0+ */
 
 #define JPEG_HEADER_SIZE	601
+#define JPEG_QUANT_SIZE		64
 
 struct hantro_jpeg_ctx {
 	int width;
 	int height;
 	int quality;
 	unsigned char *buffer;
+	unsigned char hw_luma_qtable[JPEG_QUANT_SIZE];
+	unsigned char hw_chroma_qtable[JPEG_QUANT_SIZE];
 };
 
-unsigned char *hantro_jpeg_get_qtable(int index);
 void hantro_jpeg_header_assemble(struct hantro_jpeg_ctx *ctx);
diff --git a/drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c b/drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c
index 991213ce1610..37f9707c3691 100644
--- a/drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c
+++ b/drivers/staging/media/hantro/rockchip_vpu2_hw_jpeg_enc.c
@@ -143,9 +143,8 @@ int rockchip_vpu2_jpeg_enc_run(struct hantro_ctx *ctx)
 
 	rockchip_vpu2_set_src_img_ctrl(vpu, ctx);
 	rockchip_vpu2_jpeg_enc_set_buffers(vpu, ctx, &src_buf->vb2_buf);
-	rockchip_vpu2_jpeg_enc_set_qtable(vpu,
-					  hantro_jpeg_get_qtable(0),
-					  hantro_jpeg_get_qtable(1));
+	rockchip_vpu2_jpeg_enc_set_qtable(vpu, jpeg_ctx.hw_luma_qtable,
+					  jpeg_ctx.hw_chroma_qtable);
 
 	reg = VEPU_REG_OUTPUT_SWAP32
 		| VEPU_REG_OUTPUT_SWAP16
-- 
2.33.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

^ permalink raw reply related	[flat|nested] 2+ messages in thread