Re: [PATCH RFT 5/7] media: hantro: jpeg: Add COM segment to JPEG header to align image scan
From: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Date: 2021-12-26 16:34:00
Also in:
linux-media, linux-rockchip, lkml
Hi, On Fri, Dec 24, 2021 at 04:42:46PM +0800, Chen-Yu Tsai wrote:
quoted hunk ↗ jump to hunk
The JPEG header size is not 64-bit aligned. This makes the driver require a bounce buffer for the encoded JPEG image scan output. Add a COM (comment) segment to the JPEG header so that the header size is a multiple of 64 bits. This will then allow dropping the use of the bounce buffer, and instead have the hardware write out to the capture buffer directly. Signed-off-by: Chen-Yu Tsai <wenst@chromium.org> --- drivers/staging/media/hantro/hantro_jpeg.c | 3 +++ drivers/staging/media/hantro/hantro_jpeg.h | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-)diff --git a/drivers/staging/media/hantro/hantro_jpeg.c b/drivers/staging/media/hantro/hantro_jpeg.c index 7d4018bd6876..51e67e5cf86f 100644 --- a/drivers/staging/media/hantro/hantro_jpeg.c +++ b/drivers/staging/media/hantro/hantro_jpeg.c@@ -247,6 +247,9 @@ static const unsigned char hantro_jpeg_header[JPEG_HEADER_SIZE] = { 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + /* COM */ + 0xff, 0xfe, 0x00, 0x03, 0x00, + /* SOS */ 0xff, 0xda, 0x00, 0x0c, 0x03, 0x01, 0x00, 0x02, 0x11, 0x03, 0x11, 0x00, 0x3f, 0x00,diff --git a/drivers/staging/media/hantro/hantro_jpeg.h b/drivers/staging/media/hantro/hantro_jpeg.h index f33c492134e4..0b49d0b82caa 100644 --- a/drivers/staging/media/hantro/hantro_jpeg.h +++ b/drivers/staging/media/hantro/hantro_jpeg.h@@ -1,6 +1,6 @@ /* SPDX-License-Identifier: GPL-2.0+ */ -#define JPEG_HEADER_SIZE 619 +#define JPEG_HEADER_SIZE 624
Can we add some compile-time check for the 8-byte alignment, so this is always enforced? Perhaps getting rid of the JPEG_HEADER_SIZE macro, something like this....
@@ -140,7 +140,7 @@ static const unsigned char chroma_ac_table[] = { * and we'll use fixed offsets to change the width, height * quantization tables, etc. */ -static const unsigned char hantro_jpeg_header[JPEG_HEADER_SIZE] = { +static const unsigned char hantro_jpeg_header[] = { /* SOI */ 0xff, 0xd8,
@@ -304,8 +304,13 @@ void hantro_jpeg_header_assemble(struct hantro_jpeg_ctx *ctx) { char *buf = ctx->buffer; - memcpy(buf, hantro_jpeg_header, - sizeof(hantro_jpeg_header)); + /* + * THE JPEG buffer is prepended with the JPEG header, + * so 64-bit alignment is needed for DMA. + */ + BUILD_BUG_ON(!IS_ALIGNED(sizeof(hantro_jpeg_header), 8)); + + memcpy(buf, hantro_jpeg_header, sizeof(hantro_jpeg_header));
Thanks, Ezequiel