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 <[email protected]>
---
.../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
Hi James,
Thanks for the patch.
On Thu, Nov 11, 2021 at 10:01:08PM +0000, James Cowgill wrote:
> 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 <[email protected]>
Reviewed-by: Ezequiel Garcia <[email protected]>