From 1b3bb574569e5fe5aeb0a9c73848430b7e271c20 Mon Sep 17 00:00:00 2001 From: Andrzej Pietrasiewicz Date: Mon, 27 Jan 2020 15:30:09 +0100 Subject: [PATCH] media: hantro: Write quantization table registers in increasing addresses order Luma and chroma qtables need to be written into two 16-register blocks, each table consisting of 64 bytes total. The blocks are contiguous and start at offset 0 for luma and at offset 0x40 for chroma. The seemingly innocent optimization of writing the two blocks using one loop causes side effects which result in improper values of quantization tables being used by the hardware during encoding. Visually this results in macroblocking artifacts around contrasting edges in encoded images. The artifacts look like horizontally flipped shadows of the said edges. Changing the write operations to non-relaxed variant doesn't help. This patch removes this premature optimization and after this change the macroblocking artifacts around contrasting edges are gone. Signed-off-by: Andrzej Pietrasiewicz Tested-by: Ezequiel Garcia Signed-off-by: Hans Verkuil Signed-off-by: Mauro Carvalho Chehab --- drivers/staging/media/hantro/hantro_h1_jpeg_enc.c | 6 ++++++ drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c index f62ab96078c6..b22418436823 100644 --- a/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c +++ b/drivers/staging/media/hantro/hantro_h1_jpeg_enc.c @@ -73,10 +73,16 @@ hantro_h1_jpeg_enc_set_qtable(struct hantro_dev *vpu, luma_qtable_p = (__be32 *)luma_qtable; chroma_qtable_p = (__be32 *)chroma_qtable; + /* + * Quantization table registers must be written in contiguous blocks. + * DO NOT collapse the below two "for" loops into one. + */ for (i = 0; i < H1_JPEG_QUANT_TABLE_COUNT; i++) { reg = get_unaligned_be32(&luma_qtable_p[i]); vepu_write_relaxed(vpu, reg, H1_REG_JPEG_LUMA_QUAT(i)); + } + for (i = 0; i < H1_JPEG_QUANT_TABLE_COUNT; i++) { reg = get_unaligned_be32(&chroma_qtable_p[i]); vepu_write_relaxed(vpu, reg, H1_REG_JPEG_CHROMA_QUAT(i)); } diff --git a/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c b/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c index d248979908c3..3498e6124acd 100644 --- a/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c +++ b/drivers/staging/media/hantro/rk3399_vpu_hw_jpeg_enc.c @@ -103,10 +103,16 @@ rk3399_vpu_jpeg_enc_set_qtable(struct hantro_dev *vpu, luma_qtable_p = (__be32 *)luma_qtable; chroma_qtable_p = (__be32 *)chroma_qtable; + /* + * Quantization table registers must be written in contiguous blocks. + * DO NOT collapse the below two "for" loops into one. + */ for (i = 0; i < VEPU_JPEG_QUANT_TABLE_COUNT; i++) { reg = get_unaligned_be32(&luma_qtable_p[i]); vepu_write_relaxed(vpu, reg, VEPU_REG_JPEG_LUMA_QUAT(i)); + } + for (i = 0; i < VEPU_JPEG_QUANT_TABLE_COUNT; i++) { reg = get_unaligned_be32(&chroma_qtable_p[i]); vepu_write_relaxed(vpu, reg, VEPU_REG_JPEG_CHROMA_QUAT(i)); } -- 2.30.2