[PATCH v2] media: imx-jpeg: Add support for encoder v1 descriptor configuration

ming.qian@oss.nxp.com posted 1 patch 1 week, 1 day ago
.../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |   1 +
.../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 104 +++++++++++++++---
.../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  22 ++++
3 files changed, 113 insertions(+), 14 deletions(-)
[PATCH v2] media: imx-jpeg: Add support for encoder v1 descriptor configuration
Posted by ming.qian@oss.nxp.com 1 week, 1 day ago
From: Ming Qian <ming.qian@oss.nxp.com>

Support the upgraded JPEG encoder v1 found on i.MX952 SoC.

Detect the encoder hardware version via the version register.

The v1 encoder uses an expanded descriptor format that allows all
encoding parameters, including JPEG quality, to be configured directly
in the descriptor.

This removes the manual register-based configuration step required by v0
and reduces the interrupt count from two to one per frame.

V0 encoding flow:
  1. Write quality to registers -> trigger config interrupt
  2. Start encoding -> trigger completion interrupt

V1 encoding flow:
  1. Configure descriptor with all parameters including quality
  2. Start encoding -> trigger completion interrupt

Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>

---
v2
- Improve commit message
- Use GENMASK_U32
- make mxc_jpeg_get_version() static
- Check version in probe()
- Remove noise that update copyright years
---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |   1 +
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 104 +++++++++++++++---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  22 ++++
 3 files changed, 113 insertions(+), 14 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
index adb93e977be9..0d78443cb270 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
@@ -73,6 +73,7 @@
 #define GLB_CTRL_DEC_GO					(0x1 << 2)
 #define GLB_CTRL_L_ENDIAN(le)				((le) << 3)
 #define GLB_CTRL_SLOT_EN(slot)				(0x1 << ((slot) + 4))
+#define GLB_CTRL_CUR_VERSION(r)				FIELD_GET(GENMASK_U32(19, 16), r)
 
 /* COM_STAUS fields */
 #define COM_STATUS_DEC_ONGOING(r)		(((r) & (1 << 31)) >> 31)
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index b558700d1d96..71f4a1d292ac 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -64,6 +64,12 @@
 #include "mxc-jpeg-hw.h"
 #include "mxc-jpeg.h"
 
+#define call_void_jpeg_enc_ops(jpeg, op, args...)			\
+	do {								\
+		if ((jpeg)->enc_cfg_ops && (jpeg)->enc_cfg_ops->op)	\
+			(jpeg)->enc_cfg_ops->op(args);			\
+	} while (0)
+
 static const struct mxc_jpeg_fmt mxc_formats[] = {
 	{
 		.name		= "JPEG",
@@ -1030,11 +1036,7 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
 
 	if (jpeg->mode == MXC_JPEG_ENCODE &&
 	    ctx->enc_state == MXC_JPEG_ENC_CONF) {
-		q_data = mxc_jpeg_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
-		ctx->enc_state = MXC_JPEG_ENCODING;
-		dev_dbg(dev, "Encoder config finished. Start encoding...\n");
-		mxc_jpeg_enc_set_quality(dev, reg, ctx->jpeg_quality);
-		mxc_jpeg_enc_mode_go(dev, reg, mxc_jpeg_is_extended_sequential(q_data->fmt));
+		call_void_jpeg_enc_ops(jpeg, exit_config_mode, ctx);
 		goto job_unlock;
 	}
 	if (jpeg->mode == MXC_JPEG_DECODE && jpeg_src_buf->dht_needed &&
@@ -1272,6 +1274,7 @@ static void mxc_jpeg_config_dec_desc(struct vb2_buffer *out_buf,
 
 	jpeg_src_buf = vb2_to_mxc_buf(src_buf);
 
+	ctx->extseq = mxc_jpeg_is_extended_sequential(jpeg_src_buf->fmt);
 	/* setup the decoding descriptor */
 	desc->next_descpt_ptr = 0; /* end of chain */
 	q_data_cap = mxc_jpeg_get_q_data(ctx, cap_type);
@@ -1335,9 +1338,15 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
 	struct mxc_jpeg_q_data *q_data;
 	enum mxc_jpeg_image_format img_fmt;
 	int w, h;
+	bool extseq;
 
 	q_data = mxc_jpeg_get_q_data(ctx, src_buf->vb2_queue->type);
+	extseq = mxc_jpeg_is_extended_sequential(q_data->fmt);
+
+	ctx->extseq = extseq;
 
+	memset(desc, 0, sizeof(struct mxc_jpeg_desc));
+	memset(cfg_desc, 0, sizeof(struct mxc_jpeg_desc));
 	jpeg->slot_data.cfg_stream_size =
 			mxc_jpeg_setup_cfg_stream(cfg_stream_vaddr,
 						  q_data->fmt->fourcc,
@@ -1348,11 +1357,6 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
 	cfg_desc->next_descpt_ptr = desc_handle | MXC_NXT_DESCPT_EN;
 
 	cfg_desc->buf_base0 = jpeg->slot_data.cfg_stream_handle;
-	cfg_desc->buf_base1 = 0;
-	cfg_desc->line_pitch = 0;
-	cfg_desc->stm_bufbase = 0; /* no output expected */
-	cfg_desc->stm_bufsize = 0x0;
-	cfg_desc->imgsize = 0;
 	cfg_desc->stm_ctrl = STM_CTRL_CONFIG_MOD(1);
 	cfg_desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
 
@@ -1372,11 +1376,14 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
 	desc->stm_ctrl = STM_CTRL_CONFIG_MOD(0) |
 			 STM_CTRL_IMAGE_FORMAT(img_fmt);
 	desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
-	if (mxc_jpeg_is_extended_sequential(q_data->fmt))
+	if (extseq)
 		desc->stm_ctrl |= STM_CTRL_PIXEL_PRECISION;
 	else
 		desc->stm_ctrl &= ~STM_CTRL_PIXEL_PRECISION;
 	mxc_jpeg_addrs(desc, src_buf, dst_buf, 0);
+
+	call_void_jpeg_enc_ops(jpeg, setup_desc, ctx);
+
 	dev_dbg(jpeg->dev, "cfg_desc:\n");
 	print_descriptor_info(jpeg->dev, cfg_desc);
 	dev_dbg(jpeg->dev, "enc desc:\n");
@@ -1388,6 +1395,54 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
 	mxc_jpeg_set_desc(cfg_desc_handle, reg, slot);
 }
 
+static void mxc_jpeg_enc_start_config_manually(struct mxc_jpeg_ctx *ctx)
+{
+	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
+	void __iomem *reg = jpeg->base_reg;
+	struct device *dev = jpeg->dev;
+
+	ctx->enc_state = MXC_JPEG_ENC_CONF;
+	mxc_jpeg_enc_mode_conf(dev, reg, ctx->extseq);
+}
+
+static void mxc_jpeg_enc_finish_config_manually(struct mxc_jpeg_ctx *ctx)
+{
+	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
+	void __iomem *reg = jpeg->base_reg;
+	struct device *dev = jpeg->dev;
+
+	ctx->enc_state = MXC_JPEG_ENCODING;
+	dev_dbg(dev, "Encoder config finished. Start encoding...\n");
+	mxc_jpeg_enc_set_quality(dev, reg, ctx->jpeg_quality);
+	mxc_jpeg_enc_mode_go(dev, reg, ctx->extseq);
+}
+
+static void mxc_jpeg_enc_configure_desc(struct mxc_jpeg_ctx *ctx)
+{
+	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
+	struct mxc_jpeg_desc *desc = jpeg->slot_data.desc;
+	struct mxc_jpeg_desc *cfg_desc = jpeg->slot_data.cfg_desc;
+
+	ctx->enc_state = MXC_JPEG_ENCODING;
+	cfg_desc->mode = (ctx->extseq) ? 0xb0 : 0xa0;
+	cfg_desc->cfg_mode = 0x3ff;
+
+	desc->mode = (ctx->extseq) ? 0x150 : 0x140;
+	desc->cfg_mode = 0x3ff;
+	desc->quality = ctx->jpeg_quality;
+	desc->lumth = 0xffff;
+	desc->chrth = 0xffff;
+}
+
+static const struct mxc_jpeg_enc_ops mxc_jpeg_enc_cfg_ops_v0 = {
+	.enter_config_mode = mxc_jpeg_enc_start_config_manually,
+	.exit_config_mode = mxc_jpeg_enc_finish_config_manually
+};
+
+static const struct mxc_jpeg_enc_ops mxc_jpeg_enc_cfg_ops_v1 = {
+	.setup_desc = mxc_jpeg_enc_configure_desc
+};
+
 static const struct mxc_jpeg_fmt *mxc_jpeg_get_sibling_format(const struct mxc_jpeg_fmt *fmt)
 {
 	int i;
@@ -1593,12 +1648,10 @@ static void mxc_jpeg_device_run(void *priv)
 
 	if (jpeg->mode == MXC_JPEG_ENCODE) {
 		dev_dbg(dev, "Encoding on slot %d\n", ctx->slot);
-		ctx->enc_state = MXC_JPEG_ENC_CONF;
 		mxc_jpeg_config_enc_desc(&dst_buf->vb2_buf, ctx,
 					 &src_buf->vb2_buf, &dst_buf->vb2_buf);
 		/* start config phase */
-		mxc_jpeg_enc_mode_conf(dev, reg,
-				       mxc_jpeg_is_extended_sequential(q_data_out->fmt));
+		call_void_jpeg_enc_ops(jpeg, enter_config_mode, ctx);
 	} else {
 		dev_dbg(dev, "Decoding on slot %d\n", ctx->slot);
 		print_mxc_buf(jpeg, &src_buf->vb2_buf, 0);
@@ -2842,6 +2895,14 @@ static int mxc_jpeg_attach_pm_domains(struct mxc_jpeg_dev *jpeg)
 	return ret;
 }
 
+static int mxc_jpeg_get_version(void __iomem *reg)
+{
+	u32 regval;
+
+	regval = readl(reg + GLB_CTRL);
+	return GLB_CTRL_CUR_VERSION(regval);
+}
+
 static int mxc_jpeg_probe(struct platform_device *pdev)
 {
 	struct mxc_jpeg_dev *jpeg;
@@ -2976,8 +3037,23 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 	platform_set_drvdata(pdev, jpeg);
 	pm_runtime_enable(dev);
 
+	if (mode == MXC_JPEG_ENCODE) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0)
+			goto err_check_version;
+
+		if (mxc_jpeg_get_version(jpeg->base_reg) == 0)
+			jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v0;
+		else
+			jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v1;
+
+		pm_runtime_put_sync(dev);
+	}
+
 	return 0;
 
+err_check_version:
+	pm_runtime_disable(&pdev->dev);
 err_vdev_register:
 	video_device_release(jpeg->dec_vdev);
 
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
index 9c5b4f053ded..c00c13549746 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
@@ -81,6 +81,17 @@ struct mxc_jpeg_desc {
 	u32 stm_bufsize;
 	u32 imgsize;
 	u32 stm_ctrl;
+	/* below parameters are valid for v1 */
+	u32 mode;
+	u32 cfg_mode;
+	u32 quality;
+	u32 rc_regs_sel;
+	u32 lumth;
+	u32 chrth;
+	u32 nomfrsize_lo;
+	u32 nomfrsize_hi;
+	u32 ofbsize_lo;
+	u32 ofbsize_hi;
 } __packed;
 
 struct mxc_jpeg_q_data {
@@ -105,6 +116,7 @@ struct mxc_jpeg_ctx {
 	unsigned int			source_change;
 	bool				need_initial_source_change_evt;
 	bool				header_parsed;
+	bool				extseq;
 	struct v4l2_ctrl_handler	ctrl_handler;
 	u8				jpeg_quality;
 	struct delayed_work		task_timer;
@@ -125,6 +137,15 @@ struct mxc_jpeg_slot_data {
 	dma_addr_t cfg_dec_daddr;
 };
 
+struct mxc_jpeg_enc_ops {
+	/* Manual configuration (v0 hardware) - two-phase process */
+	void (*enter_config_mode)(struct mxc_jpeg_ctx *ctx);
+	void (*exit_config_mode)(struct mxc_jpeg_ctx *ctx);
+
+	/* Descriptor-based configuration (v1 hardware) - single-phase */
+	void (*setup_desc)(struct mxc_jpeg_ctx *ctx);
+};
+
 struct mxc_jpeg_dev {
 	spinlock_t			hw_lock; /* hardware access lock */
 	unsigned int			mode;
@@ -142,6 +163,7 @@ struct mxc_jpeg_dev {
 	struct device			**pd_dev;
 	struct device_link		**pd_link;
 	struct gen_pool			*sram_pool;
+	const struct mxc_jpeg_enc_ops	*enc_cfg_ops;
 };
 
 /**

base-commit: c824345288d11e269ce41b36c105715bc2286050
prerequisite-patch-id: 0000000000000000000000000000000000000000
-- 
2.52.0
Re: [PATCH v2] media: imx-jpeg: Add support for encoder v1 descriptor configuration
Posted by Frank Li 1 week ago
On Fri, Jan 30, 2026 at 02:22:33PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> Support the upgraded JPEG encoder v1 found on i.MX952 SoC.
>
> Detect the encoder hardware version via the version register.
>
> The v1 encoder uses an expanded descriptor format that allows all
> encoding parameters, including JPEG quality, to be configured directly
> in the descriptor.
>
> This removes the manual register-based configuration step required by v0
> and reduces the interrupt count from two to one per frame.
>
> V0 encoding flow:
>   1. Write quality to registers -> trigger config interrupt
>   2. Start encoding -> trigger completion interrupt
>
> V1 encoding flow:
>   1. Configure descriptor with all parameters including quality
>   2. Start encoding -> trigger completion interrupt
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>
> ---
> v2
> - Improve commit message
> - Use GENMASK_U32
> - make mxc_jpeg_get_version() static
> - Check version in probe()
> - Remove noise that update copyright years
> ---
>  .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |   1 +
>  .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 104 +++++++++++++++---
>  .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  22 ++++
>  3 files changed, 113 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
> index adb93e977be9..0d78443cb270 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
> @@ -73,6 +73,7 @@
>  #define GLB_CTRL_DEC_GO					(0x1 << 2)
>  #define GLB_CTRL_L_ENDIAN(le)				((le) << 3)
>  #define GLB_CTRL_SLOT_EN(slot)				(0x1 << ((slot) + 4))
> +#define GLB_CTRL_CUR_VERSION(r)				FIELD_GET(GENMASK_U32(19, 16), r)
>
>  /* COM_STAUS fields */
>  #define COM_STATUS_DEC_ONGOING(r)		(((r) & (1 << 31)) >> 31)
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index b558700d1d96..71f4a1d292ac 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -64,6 +64,12 @@
>  #include "mxc-jpeg-hw.h"
>  #include "mxc-jpeg.h"
>
> +#define call_void_jpeg_enc_ops(jpeg, op, args...)			\
> +	do {								\
> +		if ((jpeg)->enc_cfg_ops && (jpeg)->enc_cfg_ops->op)	\
> +			(jpeg)->enc_cfg_ops->op(args);			\
> +	} while (0)
> +
>  static const struct mxc_jpeg_fmt mxc_formats[] = {
>  	{
>  		.name		= "JPEG",
> @@ -1030,11 +1036,7 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>
>  	if (jpeg->mode == MXC_JPEG_ENCODE &&
>  	    ctx->enc_state == MXC_JPEG_ENC_CONF) {
> -		q_data = mxc_jpeg_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
> -		ctx->enc_state = MXC_JPEG_ENCODING;
> -		dev_dbg(dev, "Encoder config finished. Start encoding...\n");
> -		mxc_jpeg_enc_set_quality(dev, reg, ctx->jpeg_quality);
> -		mxc_jpeg_enc_mode_go(dev, reg, mxc_jpeg_is_extended_sequential(q_data->fmt));
> +		call_void_jpeg_enc_ops(jpeg, exit_config_mode, ctx);
>  		goto job_unlock;
>  	}
>  	if (jpeg->mode == MXC_JPEG_DECODE && jpeg_src_buf->dht_needed &&
> @@ -1272,6 +1274,7 @@ static void mxc_jpeg_config_dec_desc(struct vb2_buffer *out_buf,
>
>  	jpeg_src_buf = vb2_to_mxc_buf(src_buf);
>
> +	ctx->extseq = mxc_jpeg_is_extended_sequential(jpeg_src_buf->fmt);
>  	/* setup the decoding descriptor */
>  	desc->next_descpt_ptr = 0; /* end of chain */
>  	q_data_cap = mxc_jpeg_get_q_data(ctx, cap_type);
> @@ -1335,9 +1338,15 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>  	struct mxc_jpeg_q_data *q_data;
>  	enum mxc_jpeg_image_format img_fmt;
>  	int w, h;
> +	bool extseq;
>
>  	q_data = mxc_jpeg_get_q_data(ctx, src_buf->vb2_queue->type);
> +	extseq = mxc_jpeg_is_extended_sequential(q_data->fmt);
> +
> +	ctx->extseq = extseq;
>
> +	memset(desc, 0, sizeof(struct mxc_jpeg_desc));
> +	memset(cfg_desc, 0, sizeof(struct mxc_jpeg_desc));
>  	jpeg->slot_data.cfg_stream_size =
>  			mxc_jpeg_setup_cfg_stream(cfg_stream_vaddr,
>  						  q_data->fmt->fourcc,
> @@ -1348,11 +1357,6 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>  	cfg_desc->next_descpt_ptr = desc_handle | MXC_NXT_DESCPT_EN;
>
>  	cfg_desc->buf_base0 = jpeg->slot_data.cfg_stream_handle;
> -	cfg_desc->buf_base1 = 0;
> -	cfg_desc->line_pitch = 0;
> -	cfg_desc->stm_bufbase = 0; /* no output expected */
> -	cfg_desc->stm_bufsize = 0x0;
> -	cfg_desc->imgsize = 0;

this change and memset belong code cleanup, it'd better use seperate patch.

>  	cfg_desc->stm_ctrl = STM_CTRL_CONFIG_MOD(1);
>  	cfg_desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
>
> @@ -1372,11 +1376,14 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>  	desc->stm_ctrl = STM_CTRL_CONFIG_MOD(0) |
>  			 STM_CTRL_IMAGE_FORMAT(img_fmt);
>  	desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
> -	if (mxc_jpeg_is_extended_sequential(q_data->fmt))
> +	if (extseq)
>  		desc->stm_ctrl |= STM_CTRL_PIXEL_PRECISION;
>  	else
>  		desc->stm_ctrl &= ~STM_CTRL_PIXEL_PRECISION;
>  	mxc_jpeg_addrs(desc, src_buf, dst_buf, 0);
> +
> +	call_void_jpeg_enc_ops(jpeg, setup_desc, ctx);
> +
>  	dev_dbg(jpeg->dev, "cfg_desc:\n");
>  	print_descriptor_info(jpeg->dev, cfg_desc);
>  	dev_dbg(jpeg->dev, "enc desc:\n");
> @@ -1388,6 +1395,54 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>  	mxc_jpeg_set_desc(cfg_desc_handle, reg, slot);
>  }
>
> +static void mxc_jpeg_enc_start_config_manually(struct mxc_jpeg_ctx *ctx)
> +{
> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
> +	void __iomem *reg = jpeg->base_reg;
> +	struct device *dev = jpeg->dev;
> +
> +	ctx->enc_state = MXC_JPEG_ENC_CONF;
> +	mxc_jpeg_enc_mode_conf(dev, reg, ctx->extseq);
> +}
> +
> +static void mxc_jpeg_enc_finish_config_manually(struct mxc_jpeg_ctx *ctx)
> +{
> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
> +	void __iomem *reg = jpeg->base_reg;
> +	struct device *dev = jpeg->dev;
> +
> +	ctx->enc_state = MXC_JPEG_ENCODING;
> +	dev_dbg(dev, "Encoder config finished. Start encoding...\n");
> +	mxc_jpeg_enc_set_quality(dev, reg, ctx->jpeg_quality);
> +	mxc_jpeg_enc_mode_go(dev, reg, ctx->extseq);
> +}

If I do this, I prefer mxc_jpeg_enc_start_config_manually() and
mxc_jpeg_enc_finish_config_manually() as patch, just do code re-org.

Then base that, add mxc_jpeg_enc_configure_desc() will straight forward.

Some maintainer accept this if change is not bigger.  The squash patches
is trivials by maintainers.

Frank
> +
> +static void mxc_jpeg_enc_configure_desc(struct mxc_jpeg_ctx *ctx)
> +{
> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
> +	struct mxc_jpeg_desc *desc = jpeg->slot_data.desc;
> +	struct mxc_jpeg_desc *cfg_desc = jpeg->slot_data.cfg_desc;
> +
> +	ctx->enc_state = MXC_JPEG_ENCODING;
> +	cfg_desc->mode = (ctx->extseq) ? 0xb0 : 0xa0;
> +	cfg_desc->cfg_mode = 0x3ff;
> +
> +	desc->mode = (ctx->extseq) ? 0x150 : 0x140;
> +	desc->cfg_mode = 0x3ff;
> +	desc->quality = ctx->jpeg_quality;
> +	desc->lumth = 0xffff;
> +	desc->chrth = 0xffff;
> +}
> +
> +static const struct mxc_jpeg_enc_ops mxc_jpeg_enc_cfg_ops_v0 = {
> +	.enter_config_mode = mxc_jpeg_enc_start_config_manually,
> +	.exit_config_mode = mxc_jpeg_enc_finish_config_manually
> +};
> +
> +static const struct mxc_jpeg_enc_ops mxc_jpeg_enc_cfg_ops_v1 = {
> +	.setup_desc = mxc_jpeg_enc_configure_desc
> +};
> +
>  static const struct mxc_jpeg_fmt *mxc_jpeg_get_sibling_format(const struct mxc_jpeg_fmt *fmt)
>  {
>  	int i;
> @@ -1593,12 +1648,10 @@ static void mxc_jpeg_device_run(void *priv)
>
>  	if (jpeg->mode == MXC_JPEG_ENCODE) {
>  		dev_dbg(dev, "Encoding on slot %d\n", ctx->slot);
> -		ctx->enc_state = MXC_JPEG_ENC_CONF;
>  		mxc_jpeg_config_enc_desc(&dst_buf->vb2_buf, ctx,
>  					 &src_buf->vb2_buf, &dst_buf->vb2_buf);
>  		/* start config phase */
> -		mxc_jpeg_enc_mode_conf(dev, reg,
> -				       mxc_jpeg_is_extended_sequential(q_data_out->fmt));
> +		call_void_jpeg_enc_ops(jpeg, enter_config_mode, ctx);
>  	} else {
>  		dev_dbg(dev, "Decoding on slot %d\n", ctx->slot);
>  		print_mxc_buf(jpeg, &src_buf->vb2_buf, 0);
> @@ -2842,6 +2895,14 @@ static int mxc_jpeg_attach_pm_domains(struct mxc_jpeg_dev *jpeg)
>  	return ret;
>  }
>
> +static int mxc_jpeg_get_version(void __iomem *reg)
> +{
> +	u32 regval;
> +
> +	regval = readl(reg + GLB_CTRL);
> +	return GLB_CTRL_CUR_VERSION(regval);
> +}
> +
>  static int mxc_jpeg_probe(struct platform_device *pdev)
>  {
>  	struct mxc_jpeg_dev *jpeg;
> @@ -2976,8 +3037,23 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>  	platform_set_drvdata(pdev, jpeg);
>  	pm_runtime_enable(dev);
>
> +	if (mode == MXC_JPEG_ENCODE) {
> +		ret = pm_runtime_resume_and_get(dev);
> +		if (ret < 0)
> +			goto err_check_version;
> +
> +		if (mxc_jpeg_get_version(jpeg->base_reg) == 0)
> +			jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v0;
> +		else
> +			jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v1;
> +
> +		pm_runtime_put_sync(dev);
> +	}
> +
>  	return 0;
>
> +err_check_version:
> +	pm_runtime_disable(&pdev->dev);
>  err_vdev_register:
>  	video_device_release(jpeg->dec_vdev);
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> index 9c5b4f053ded..c00c13549746 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> @@ -81,6 +81,17 @@ struct mxc_jpeg_desc {
>  	u32 stm_bufsize;
>  	u32 imgsize;
>  	u32 stm_ctrl;
> +	/* below parameters are valid for v1 */
> +	u32 mode;
> +	u32 cfg_mode;
> +	u32 quality;
> +	u32 rc_regs_sel;
> +	u32 lumth;
> +	u32 chrth;
> +	u32 nomfrsize_lo;
> +	u32 nomfrsize_hi;
> +	u32 ofbsize_lo;
> +	u32 ofbsize_hi;
>  } __packed;
>
>  struct mxc_jpeg_q_data {
> @@ -105,6 +116,7 @@ struct mxc_jpeg_ctx {
>  	unsigned int			source_change;
>  	bool				need_initial_source_change_evt;
>  	bool				header_parsed;
> +	bool				extseq;
>  	struct v4l2_ctrl_handler	ctrl_handler;
>  	u8				jpeg_quality;
>  	struct delayed_work		task_timer;
> @@ -125,6 +137,15 @@ struct mxc_jpeg_slot_data {
>  	dma_addr_t cfg_dec_daddr;
>  };
>
> +struct mxc_jpeg_enc_ops {
> +	/* Manual configuration (v0 hardware) - two-phase process */
> +	void (*enter_config_mode)(struct mxc_jpeg_ctx *ctx);
> +	void (*exit_config_mode)(struct mxc_jpeg_ctx *ctx);
> +
> +	/* Descriptor-based configuration (v1 hardware) - single-phase */
> +	void (*setup_desc)(struct mxc_jpeg_ctx *ctx);
> +};
> +
>  struct mxc_jpeg_dev {
>  	spinlock_t			hw_lock; /* hardware access lock */
>  	unsigned int			mode;
> @@ -142,6 +163,7 @@ struct mxc_jpeg_dev {
>  	struct device			**pd_dev;
>  	struct device_link		**pd_link;
>  	struct gen_pool			*sram_pool;
> +	const struct mxc_jpeg_enc_ops	*enc_cfg_ops;
>  };
>
>  /**
>
> base-commit: c824345288d11e269ce41b36c105715bc2286050
> prerequisite-patch-id: 0000000000000000000000000000000000000000
> --
> 2.52.0
>
Re: [PATCH v2] media: imx-jpeg: Add support for encoder v1 descriptor configuration
Posted by Ming Qian(OSS) 5 days, 6 hours ago
Hi Frank,

On 1/30/2026 10:52 PM, Frank Li wrote:
> On Fri, Jan 30, 2026 at 02:22:33PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> Support the upgraded JPEG encoder v1 found on i.MX952 SoC.
>>
>> Detect the encoder hardware version via the version register.
>>
>> The v1 encoder uses an expanded descriptor format that allows all
>> encoding parameters, including JPEG quality, to be configured directly
>> in the descriptor.
>>
>> This removes the manual register-based configuration step required by v0
>> and reduces the interrupt count from two to one per frame.
>>
>> V0 encoding flow:
>>    1. Write quality to registers -> trigger config interrupt
>>    2. Start encoding -> trigger completion interrupt
>>
>> V1 encoding flow:
>>    1. Configure descriptor with all parameters including quality
>>    2. Start encoding -> trigger completion interrupt
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>>
>> ---
>> v2
>> - Improve commit message
>> - Use GENMASK_U32
>> - make mxc_jpeg_get_version() static
>> - Check version in probe()
>> - Remove noise that update copyright years
>> ---
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |   1 +
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 104 +++++++++++++++---
>>   .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  22 ++++
>>   3 files changed, 113 insertions(+), 14 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
>> index adb93e977be9..0d78443cb270 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h
>> @@ -73,6 +73,7 @@
>>   #define GLB_CTRL_DEC_GO					(0x1 << 2)
>>   #define GLB_CTRL_L_ENDIAN(le)				((le) << 3)
>>   #define GLB_CTRL_SLOT_EN(slot)				(0x1 << ((slot) + 4))
>> +#define GLB_CTRL_CUR_VERSION(r)				FIELD_GET(GENMASK_U32(19, 16), r)
>>
>>   /* COM_STAUS fields */
>>   #define COM_STATUS_DEC_ONGOING(r)		(((r) & (1 << 31)) >> 31)
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index b558700d1d96..71f4a1d292ac 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -64,6 +64,12 @@
>>   #include "mxc-jpeg-hw.h"
>>   #include "mxc-jpeg.h"
>>
>> +#define call_void_jpeg_enc_ops(jpeg, op, args...)			\
>> +	do {								\
>> +		if ((jpeg)->enc_cfg_ops && (jpeg)->enc_cfg_ops->op)	\
>> +			(jpeg)->enc_cfg_ops->op(args);			\
>> +	} while (0)
>> +
>>   static const struct mxc_jpeg_fmt mxc_formats[] = {
>>   	{
>>   		.name		= "JPEG",
>> @@ -1030,11 +1036,7 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>>
>>   	if (jpeg->mode == MXC_JPEG_ENCODE &&
>>   	    ctx->enc_state == MXC_JPEG_ENC_CONF) {
>> -		q_data = mxc_jpeg_get_q_data(ctx, V4L2_BUF_TYPE_VIDEO_OUTPUT_MPLANE);
>> -		ctx->enc_state = MXC_JPEG_ENCODING;
>> -		dev_dbg(dev, "Encoder config finished. Start encoding...\n");
>> -		mxc_jpeg_enc_set_quality(dev, reg, ctx->jpeg_quality);
>> -		mxc_jpeg_enc_mode_go(dev, reg, mxc_jpeg_is_extended_sequential(q_data->fmt));
>> +		call_void_jpeg_enc_ops(jpeg, exit_config_mode, ctx);
>>   		goto job_unlock;
>>   	}
>>   	if (jpeg->mode == MXC_JPEG_DECODE && jpeg_src_buf->dht_needed &&
>> @@ -1272,6 +1274,7 @@ static void mxc_jpeg_config_dec_desc(struct vb2_buffer *out_buf,
>>
>>   	jpeg_src_buf = vb2_to_mxc_buf(src_buf);
>>
>> +	ctx->extseq = mxc_jpeg_is_extended_sequential(jpeg_src_buf->fmt);
>>   	/* setup the decoding descriptor */
>>   	desc->next_descpt_ptr = 0; /* end of chain */
>>   	q_data_cap = mxc_jpeg_get_q_data(ctx, cap_type);
>> @@ -1335,9 +1338,15 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>>   	struct mxc_jpeg_q_data *q_data;
>>   	enum mxc_jpeg_image_format img_fmt;
>>   	int w, h;
>> +	bool extseq;
>>
>>   	q_data = mxc_jpeg_get_q_data(ctx, src_buf->vb2_queue->type);
>> +	extseq = mxc_jpeg_is_extended_sequential(q_data->fmt);
>> +
>> +	ctx->extseq = extseq;
>>
>> +	memset(desc, 0, sizeof(struct mxc_jpeg_desc));
>> +	memset(cfg_desc, 0, sizeof(struct mxc_jpeg_desc));
>>   	jpeg->slot_data.cfg_stream_size =
>>   			mxc_jpeg_setup_cfg_stream(cfg_stream_vaddr,
>>   						  q_data->fmt->fourcc,
>> @@ -1348,11 +1357,6 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>>   	cfg_desc->next_descpt_ptr = desc_handle | MXC_NXT_DESCPT_EN;
>>
>>   	cfg_desc->buf_base0 = jpeg->slot_data.cfg_stream_handle;
>> -	cfg_desc->buf_base1 = 0;
>> -	cfg_desc->line_pitch = 0;
>> -	cfg_desc->stm_bufbase = 0; /* no output expected */
>> -	cfg_desc->stm_bufsize = 0x0;
>> -	cfg_desc->imgsize = 0;
> 
> this change and memset belong code cleanup, it'd better use seperate patch.
> 

Sure, I'll make it a separate patch in v3

>>   	cfg_desc->stm_ctrl = STM_CTRL_CONFIG_MOD(1);
>>   	cfg_desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
>>
>> @@ -1372,11 +1376,14 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>>   	desc->stm_ctrl = STM_CTRL_CONFIG_MOD(0) |
>>   			 STM_CTRL_IMAGE_FORMAT(img_fmt);
>>   	desc->stm_ctrl |= STM_CTRL_BITBUF_PTR_CLR(1);
>> -	if (mxc_jpeg_is_extended_sequential(q_data->fmt))
>> +	if (extseq)
>>   		desc->stm_ctrl |= STM_CTRL_PIXEL_PRECISION;
>>   	else
>>   		desc->stm_ctrl &= ~STM_CTRL_PIXEL_PRECISION;
>>   	mxc_jpeg_addrs(desc, src_buf, dst_buf, 0);
>> +
>> +	call_void_jpeg_enc_ops(jpeg, setup_desc, ctx);
>> +
>>   	dev_dbg(jpeg->dev, "cfg_desc:\n");
>>   	print_descriptor_info(jpeg->dev, cfg_desc);
>>   	dev_dbg(jpeg->dev, "enc desc:\n");
>> @@ -1388,6 +1395,54 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
>>   	mxc_jpeg_set_desc(cfg_desc_handle, reg, slot);
>>   }
>>
>> +static void mxc_jpeg_enc_start_config_manually(struct mxc_jpeg_ctx *ctx)
>> +{
>> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
>> +	void __iomem *reg = jpeg->base_reg;
>> +	struct device *dev = jpeg->dev;
>> +
>> +	ctx->enc_state = MXC_JPEG_ENC_CONF;
>> +	mxc_jpeg_enc_mode_conf(dev, reg, ctx->extseq);
>> +}
>> +
>> +static void mxc_jpeg_enc_finish_config_manually(struct mxc_jpeg_ctx *ctx)
>> +{
>> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
>> +	void __iomem *reg = jpeg->base_reg;
>> +	struct device *dev = jpeg->dev;
>> +
>> +	ctx->enc_state = MXC_JPEG_ENCODING;
>> +	dev_dbg(dev, "Encoder config finished. Start encoding...\n");
>> +	mxc_jpeg_enc_set_quality(dev, reg, ctx->jpeg_quality);
>> +	mxc_jpeg_enc_mode_go(dev, reg, ctx->extseq);
>> +}
> 
> If I do this, I prefer mxc_jpeg_enc_start_config_manually() and
> mxc_jpeg_enc_finish_config_manually() as patch, just do code re-org.
> 
> Then base that, add mxc_jpeg_enc_configure_desc() will straight forward.
> 
> Some maintainer accept this if change is not bigger.  The squash patches
> is trivials by maintainers.
> 
> Frank

OK, I'll split them into separate patches in v3

Regards,
Ming

>> +
>> +static void mxc_jpeg_enc_configure_desc(struct mxc_jpeg_ctx *ctx)
>> +{
>> +	struct mxc_jpeg_dev *jpeg = ctx->mxc_jpeg;
>> +	struct mxc_jpeg_desc *desc = jpeg->slot_data.desc;
>> +	struct mxc_jpeg_desc *cfg_desc = jpeg->slot_data.cfg_desc;
>> +
>> +	ctx->enc_state = MXC_JPEG_ENCODING;
>> +	cfg_desc->mode = (ctx->extseq) ? 0xb0 : 0xa0;
>> +	cfg_desc->cfg_mode = 0x3ff;
>> +
>> +	desc->mode = (ctx->extseq) ? 0x150 : 0x140;
>> +	desc->cfg_mode = 0x3ff;
>> +	desc->quality = ctx->jpeg_quality;
>> +	desc->lumth = 0xffff;
>> +	desc->chrth = 0xffff;
>> +}
>> +
>> +static const struct mxc_jpeg_enc_ops mxc_jpeg_enc_cfg_ops_v0 = {
>> +	.enter_config_mode = mxc_jpeg_enc_start_config_manually,
>> +	.exit_config_mode = mxc_jpeg_enc_finish_config_manually
>> +};
>> +
>> +static const struct mxc_jpeg_enc_ops mxc_jpeg_enc_cfg_ops_v1 = {
>> +	.setup_desc = mxc_jpeg_enc_configure_desc
>> +};
>> +
>>   static const struct mxc_jpeg_fmt *mxc_jpeg_get_sibling_format(const struct mxc_jpeg_fmt *fmt)
>>   {
>>   	int i;
>> @@ -1593,12 +1648,10 @@ static void mxc_jpeg_device_run(void *priv)
>>
>>   	if (jpeg->mode == MXC_JPEG_ENCODE) {
>>   		dev_dbg(dev, "Encoding on slot %d\n", ctx->slot);
>> -		ctx->enc_state = MXC_JPEG_ENC_CONF;
>>   		mxc_jpeg_config_enc_desc(&dst_buf->vb2_buf, ctx,
>>   					 &src_buf->vb2_buf, &dst_buf->vb2_buf);
>>   		/* start config phase */
>> -		mxc_jpeg_enc_mode_conf(dev, reg,
>> -				       mxc_jpeg_is_extended_sequential(q_data_out->fmt));
>> +		call_void_jpeg_enc_ops(jpeg, enter_config_mode, ctx);
>>   	} else {
>>   		dev_dbg(dev, "Decoding on slot %d\n", ctx->slot);
>>   		print_mxc_buf(jpeg, &src_buf->vb2_buf, 0);
>> @@ -2842,6 +2895,14 @@ static int mxc_jpeg_attach_pm_domains(struct mxc_jpeg_dev *jpeg)
>>   	return ret;
>>   }
>>
>> +static int mxc_jpeg_get_version(void __iomem *reg)
>> +{
>> +	u32 regval;
>> +
>> +	regval = readl(reg + GLB_CTRL);
>> +	return GLB_CTRL_CUR_VERSION(regval);
>> +}
>> +
>>   static int mxc_jpeg_probe(struct platform_device *pdev)
>>   {
>>   	struct mxc_jpeg_dev *jpeg;
>> @@ -2976,8 +3037,23 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>   	platform_set_drvdata(pdev, jpeg);
>>   	pm_runtime_enable(dev);
>>
>> +	if (mode == MXC_JPEG_ENCODE) {
>> +		ret = pm_runtime_resume_and_get(dev);
>> +		if (ret < 0)
>> +			goto err_check_version;
>> +
>> +		if (mxc_jpeg_get_version(jpeg->base_reg) == 0)
>> +			jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v0;
>> +		else
>> +			jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v1;
>> +
>> +		pm_runtime_put_sync(dev);
>> +	}
>> +
>>   	return 0;
>>
>> +err_check_version:
>> +	pm_runtime_disable(&pdev->dev);
>>   err_vdev_register:
>>   	video_device_release(jpeg->dec_vdev);
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> index 9c5b4f053ded..c00c13549746 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> @@ -81,6 +81,17 @@ struct mxc_jpeg_desc {
>>   	u32 stm_bufsize;
>>   	u32 imgsize;
>>   	u32 stm_ctrl;
>> +	/* below parameters are valid for v1 */
>> +	u32 mode;
>> +	u32 cfg_mode;
>> +	u32 quality;
>> +	u32 rc_regs_sel;
>> +	u32 lumth;
>> +	u32 chrth;
>> +	u32 nomfrsize_lo;
>> +	u32 nomfrsize_hi;
>> +	u32 ofbsize_lo;
>> +	u32 ofbsize_hi;
>>   } __packed;
>>
>>   struct mxc_jpeg_q_data {
>> @@ -105,6 +116,7 @@ struct mxc_jpeg_ctx {
>>   	unsigned int			source_change;
>>   	bool				need_initial_source_change_evt;
>>   	bool				header_parsed;
>> +	bool				extseq;
>>   	struct v4l2_ctrl_handler	ctrl_handler;
>>   	u8				jpeg_quality;
>>   	struct delayed_work		task_timer;
>> @@ -125,6 +137,15 @@ struct mxc_jpeg_slot_data {
>>   	dma_addr_t cfg_dec_daddr;
>>   };
>>
>> +struct mxc_jpeg_enc_ops {
>> +	/* Manual configuration (v0 hardware) - two-phase process */
>> +	void (*enter_config_mode)(struct mxc_jpeg_ctx *ctx);
>> +	void (*exit_config_mode)(struct mxc_jpeg_ctx *ctx);
>> +
>> +	/* Descriptor-based configuration (v1 hardware) - single-phase */
>> +	void (*setup_desc)(struct mxc_jpeg_ctx *ctx);
>> +};
>> +
>>   struct mxc_jpeg_dev {
>>   	spinlock_t			hw_lock; /* hardware access lock */
>>   	unsigned int			mode;
>> @@ -142,6 +163,7 @@ struct mxc_jpeg_dev {
>>   	struct device			**pd_dev;
>>   	struct device_link		**pd_link;
>>   	struct gen_pool			*sram_pool;
>> +	const struct mxc_jpeg_enc_ops	*enc_cfg_ops;
>>   };
>>
>>   /**
>>
>> base-commit: c824345288d11e269ce41b36c105715bc2286050
>> prerequisite-patch-id: 0000000000000000000000000000000000000000
>> --
>> 2.52.0
>>