Re: [PATCH v3 3/4] media: imx-jpeg: Change the pattern size to 128x64

[Frank Li <Frank.li@nxp.com>]

On Tue, Apr 08, 2025 at 10:57:19AM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> In order to decode a motion-jpeg bitstream, which doesn't provide a DHT,
> the driver will first decode a pattern jpeg and use the DHT found in the
> pattern to decode the first actual frame. This mode is called
> "repeat-mode" and it utilizes linked descriptors.

here new empty line for new paragraph

> The smallest supported resolution of 64x64 was used for that pattern to
> not cause unneeded performance delay. This choice, however, can cause a
> corrupted decoded picture of the first frame after the pattern, when the
> resolution of that frame is larger than the pattern and is not aligned
> to 64.

Here need empty line.

> By altering the pattern size to 128x64, this corruption can be avoided.
> That size has been confirmed to be safe by the hardware designers.
> Additionally, a DMA buffer needs to be allocated to store the decoded
> picture of the pattern image.
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> Reviewed-by: Frank Li <Frank.Li@nxp.com>
> ---
> v3
> - Improve commit message
>
>  .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 42 +++++++++++++++----
>  .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  5 +++
>  2 files changed, 39 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 12661c177f5a..45705c606769 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -535,7 +535,18 @@ static const unsigned char jpeg_sos_maximal[] = {
>  };
>
>  static const unsigned char jpeg_image_red[] = {
> -	0xFC, 0x5F, 0xA2, 0xBF, 0xCA, 0x73, 0xFE, 0xFE,
> +	0xF9, 0xFE, 0x8A, 0xFC, 0x34, 0xFD, 0xC4, 0x28,
> +	0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A,
> +	0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0,
> +	0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00,
> +	0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02,
> +	0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28,
> +	0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A,
> +	0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0,
> +	0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00,
> +	0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02,
> +	0x8A, 0x00, 0x28, 0xA0, 0x0F, 0xFF, 0xD0, 0xF9,
> +	0xFE, 0x8A, 0xFC, 0x34, 0xFD, 0xC4, 0x28, 0xA0,
>  	0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00,
>  	0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02,
>  	0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28,
> @@ -545,7 +556,7 @@ static const unsigned char jpeg_image_red[] = {
>  	0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02,
>  	0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00, 0x28,
>  	0xA0, 0x02, 0x8A, 0x00, 0x28, 0xA0, 0x02, 0x8A,
> -	0x00, 0x28, 0xA0, 0x02, 0x8A, 0x00
> +	0x00, 0x28, 0xA0, 0x0F
>  };
>
>  static const unsigned char jpeg_eoi[] = {
> @@ -775,6 +786,13 @@ static void mxc_jpeg_free_slot_data(struct mxc_jpeg_dev *jpeg)
>  	jpeg->slot_data.cfg_stream_vaddr = NULL;
>  	jpeg->slot_data.cfg_stream_handle = 0;
>
> +	dma_free_coherent(jpeg->dev, jpeg->slot_data.cfg_dec_size,
> +			  jpeg->slot_data.cfg_dec_vaddr,
> +			  jpeg->slot_data.cfg_dec_daddr);
> +	jpeg->slot_data.cfg_dec_size = 0;
> +	jpeg->slot_data.cfg_dec_vaddr = NULL;
> +	jpeg->slot_data.cfg_dec_daddr = 0;
> +
>  	jpeg->slot_data.used = false;
>  }
>
> @@ -814,6 +832,14 @@ static bool mxc_jpeg_alloc_slot_data(struct mxc_jpeg_dev *jpeg)
>  		goto err;
>  	jpeg->slot_data.cfg_stream_vaddr = cfg_stm;
>
> +	jpeg->slot_data.cfg_dec_size = MXC_JPEG_PATTERN_WIDTH * MXC_JPEG_PATTERN_HEIGHT * 2;
> +	jpeg->slot_data.cfg_dec_vaddr = dma_alloc_coherent(jpeg->dev,
> +							   jpeg->slot_data.cfg_dec_size,
> +							   &jpeg->slot_data.cfg_dec_daddr,
> +							   GFP_ATOMIC);
> +	if (!jpeg->slot_data.cfg_dec_vaddr)
> +		goto err;
> +
>  skip_alloc:
>  	jpeg->slot_data.used = true;
>
> @@ -1216,14 +1242,14 @@ static void mxc_jpeg_config_dec_desc(struct vb2_buffer *out_buf,
>  	 */
>  	*cfg_size = mxc_jpeg_setup_cfg_stream(cfg_stream_vaddr,
>  					      V4L2_PIX_FMT_YUYV,
> -					      MXC_JPEG_MIN_WIDTH,
> -					      MXC_JPEG_MIN_HEIGHT);
> +					      MXC_JPEG_PATTERN_WIDTH,
> +					      MXC_JPEG_PATTERN_HEIGHT);
>  	cfg_desc->next_descpt_ptr = desc_handle | MXC_NXT_DESCPT_EN;
> -	cfg_desc->buf_base0 = vb2_dma_contig_plane_dma_addr(dst_buf, 0);
> +	cfg_desc->buf_base0 = jpeg->slot_data.cfg_dec_daddr;
>  	cfg_desc->buf_base1 = 0;
> -	cfg_desc->imgsize = MXC_JPEG_MIN_WIDTH << 16;
> -	cfg_desc->imgsize |= MXC_JPEG_MIN_HEIGHT;
> -	cfg_desc->line_pitch = MXC_JPEG_MIN_WIDTH * 2;
> +	cfg_desc->imgsize = MXC_JPEG_PATTERN_WIDTH << 16;
> +	cfg_desc->imgsize |= MXC_JPEG_PATTERN_HEIGHT;
> +	cfg_desc->line_pitch = MXC_JPEG_PATTERN_WIDTH * 2;
>  	cfg_desc->stm_ctrl = STM_CTRL_IMAGE_FORMAT(MXC_JPEG_YUV422);
>  	cfg_desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
>  	cfg_desc->stm_bufbase = cfg_stream_handle;
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> index 86e324b21aed..fdde45f7e163 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> @@ -28,6 +28,8 @@
>  #define MXC_JPEG_W_ALIGN		3
>  #define MXC_JPEG_MAX_SIZEIMAGE		0xFFFFFC00
>  #define MXC_JPEG_MAX_PLANES		2
> +#define MXC_JPEG_PATTERN_WIDTH		128
> +#define MXC_JPEG_PATTERN_HEIGHT		64
>
>  enum mxc_jpeg_enc_state {
>  	MXC_JPEG_ENCODING	= 0, /* jpeg encode phase */
> @@ -117,6 +119,9 @@ struct mxc_jpeg_slot_data {
>  	dma_addr_t desc_handle;
>  	dma_addr_t cfg_desc_handle; // configuration descriptor dma address
>  	dma_addr_t cfg_stream_handle; // configuration bitstream dma address
> +	dma_addr_t cfg_dec_size;
> +	void *cfg_dec_vaddr;
> +	dma_addr_t cfg_dec_daddr;
>  };
>
>  struct mxc_jpeg_dev {
> --
> 2.43.0-rc1
>