[PATCH v4 0/4] media: imx-jpeg: Add v1 hardware encoder support for i.MX952

[PATCH v4 0/4] media: imx-jpeg: Add v1 hardware encoder support for i.MX952

[ming.qian]

From: Ming Qian <ming.qian@oss.nxp.com>

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

V1 hardware uses enhanced descriptors with mode, quality, and
threshold fields, configured in a single phase via setup_desc
callback.
V0 hardware continues using the existing two-phase manual configuration.

---
Change logs
v4
- Use devm_pm_runtime_enable() helper
- Correct the typo in the title

v3
- Split the changes into separate patches

v2
- Improve commit message
- Use GENMASK_U32
- make mxc_jpeg_get_version() static
- Check version in probe()
- Remove noise that update copyright years

Ming Qian (4):
  media: imx-jpeg: Simplify descriptor initialization with memset
  media: imx-jpeg: Use devm_pm_runtime_enable() helper
  media: imx-jpeg: Add encoder ops layer for hardware abstraction
  media: imx-jpeg: Add support for encoder v1 descriptor configuration

 .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |   1 +
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 117 +++++++++++++++---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    |  22 ++++
 3 files changed, 123 insertions(+), 17 deletions(-)


base-commit: c824345288d11e269ce41b36c105715bc2286050
prerequisite-patch-id: 0000000000000000000000000000000000000000
-- 
2.52.0

[PATCH v4 1/4] media: imx-jpeg: Simplify descriptor initialization with memset

[ming.qian]

From: Ming Qian <ming.qian@oss.nxp.com>

Use memset() to zero-initialize desc and cfg_desc structures instead of
assigning individual fields to zero. This is cleaner and ensures all
descriptor fields are properly initialized.

No functional change.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
 drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index b558700d1d96..39022c1bf36d 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -1338,6 +1338,8 @@ static void mxc_jpeg_config_enc_desc(struct vb2_buffer *out_buf,
 
 	q_data = mxc_jpeg_get_q_data(ctx, src_buf->vb2_queue->type);
 
+	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 +1350,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);
 
-- 
2.52.0

[PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

[ming.qian]

From: Ming Qian <ming.qian@oss.nxp.com>

Use devm_pm_runtime_enable() to simplify probe and exit paths.

No functional change.

Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
 drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
 1 file changed, 12 insertions(+), 3 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 39022c1bf36d..877e6c4f7406 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 			  jpeg->dec_vdev->minor);
 
 	platform_set_drvdata(pdev, jpeg);
-	pm_runtime_enable(dev);
+	ret = devm_pm_runtime_enable(dev);
+	if (ret) {
+		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
+		goto err_pm;
+	}
 
 	return 0;
 
+err_pm:
+	video_unregister_device(jpeg->dec_vdev);
+	/* set NULL to prevent double-free */
+	jpeg->dec_vdev = NULL;
 err_vdev_register:
-	video_device_release(jpeg->dec_vdev);
+	/* Only release if allocation succeeded but registration failed */
+	if (jpeg->dec_vdev)
+		video_device_release(jpeg->dec_vdev);
 
 err_vdev_alloc:
 	v4l2_m2m_release(jpeg->m2m_dev);
@@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
 
 	mxc_jpeg_free_slot_data(jpeg);
 
-	pm_runtime_disable(&pdev->dev);
 	video_unregister_device(jpeg->dec_vdev);
 	v4l2_m2m_release(jpeg->m2m_dev);
 	v4l2_device_unregister(&jpeg->v4l2_dev);
-- 
2.52.0

[PATCH v4 3/4] media: imx-jpeg: Add encoder ops layer for hardware abstraction

[ming.qian]

From: Ming Qian <ming.qian@oss.nxp.com>

Introduce mxc_jpeg_enc_ops function pointer structure to abstract
encoder configuration differences between hardware versions.

Extract the existing two-phase manual configuration into dedicated
functions (enter_config_mode/exit_config_mode) for v0 hardware.
Add setup_desc callback placeholder for future v1 hardware support
which will use descriptor-based configuration.

Store the extended sequential mode flag in the context to avoid
recalculating it during configuration phases.

No functional change.

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 56 ++++++++++++++++---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    | 11 ++++
 2 files changed, 58 insertions(+), 9 deletions(-)

diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 877e6c4f7406..c2e2c028b410 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,8 +1338,12 @@ 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));
@@ -1369,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");
@@ -1385,6 +1395,33 @@ 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 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_fmt *mxc_jpeg_get_sibling_format(const struct mxc_jpeg_fmt *fmt)
 {
 	int i;
@@ -1590,12 +1627,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);
@@ -2977,6 +3012,9 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 		goto err_pm;
 	}
 
+	if (mode == MXC_JPEG_ENCODE)
+		jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v0;
+
 	return 0;
 
 err_pm:
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
index 9c5b4f053ded..cdfefb68346a 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
@@ -105,6 +105,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 +126,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 +152,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;
 };
 
 /**
-- 
2.52.0

[PATCH v4 4/4] media: imx-jpeg: Add support for encoder v1 descriptor configuration

[ming.qian]

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

Reviewed-by: Frank Li <Frank.Li@nxp.com>
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
 .../media/platform/nxp/imx-jpeg/mxc-jpeg-hw.h |  1 +
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.c    | 43 ++++++++++++++++++-
 .../media/platform/nxp/imx-jpeg/mxc-jpeg.h    | 11 +++++
 3 files changed, 53 insertions(+), 2 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 c2e2c028b410..5723ae677153 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -1417,11 +1417,32 @@ static void mxc_jpeg_enc_finish_config_manually(struct mxc_jpeg_ctx *ctx)
 	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;
@@ -2874,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;
@@ -3012,8 +3041,18 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
 		goto err_pm;
 	}
 
-	if (mode == MXC_JPEG_ENCODE)
-		jpeg->enc_cfg_ops = &mxc_jpeg_enc_cfg_ops_v0;
+	if (mode == MXC_JPEG_ENCODE) {
+		ret = pm_runtime_resume_and_get(dev);
+		if (ret < 0)
+			goto err_pm;
+
+		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;
 
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
index cdfefb68346a..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 {
-- 
2.52.0

Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

[Frank Li]

On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> Use devm_pm_runtime_enable() to simplify probe and exit paths.
>
> No functional change.
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 39022c1bf36d..877e6c4f7406 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>  			  jpeg->dec_vdev->minor);
>
>  	platform_set_drvdata(pdev, jpeg);
> -	pm_runtime_enable(dev);
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
> +		goto err_pm;
> +	}
>
>  	return 0;
>
> +err_pm:
> +	video_unregister_device(jpeg->dec_vdev);
> +	/* set NULL to prevent double-free */
> +	jpeg->dec_vdev = NULL;

code have not simplied, you add addtional goto label. You can use
devm_add_action_or_reset().

Frank

>  err_vdev_register:
> -	video_device_release(jpeg->dec_vdev);
> +	/* Only release if allocation succeeded but registration failed */
> +	if (jpeg->dec_vdev)
> +		video_device_release(jpeg->dec_vdev);
>
>  err_vdev_alloc:
>  	v4l2_m2m_release(jpeg->m2m_dev);
> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>
>  	mxc_jpeg_free_slot_data(jpeg);
>
> -	pm_runtime_disable(&pdev->dev);
>  	video_unregister_device(jpeg->dec_vdev);
>  	v4l2_m2m_release(jpeg->m2m_dev);
>  	v4l2_device_unregister(&jpeg->v4l2_dev);
> --
> 2.52.0
>

Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

[Ming Qian(OSS)]

Hi Frank,

On 2/4/2026 12:56 AM, Frank Li wrote:
> On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> Use devm_pm_runtime_enable() to simplify probe and exit paths.
>>
>> No functional change.
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>>   drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>>   1 file changed, 12 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 39022c1bf36d..877e6c4f7406 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>   			  jpeg->dec_vdev->minor);
>>
>>   	platform_set_drvdata(pdev, jpeg);
>> -	pm_runtime_enable(dev);
>> +	ret = devm_pm_runtime_enable(dev);
>> +	if (ret) {
>> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
>> +		goto err_pm;
>> +	}
>>
>>   	return 0;
>>
>> +err_pm:
>> +	video_unregister_device(jpeg->dec_vdev);
>> +	/* set NULL to prevent double-free */
>> +	jpeg->dec_vdev = NULL;
> 
> code have not simplied, you add addtional goto label. You can use
> devm_add_action_or_reset().
> 
> Frank
> 

I don't quite understand the difference compared to directly using
devm_pm_runtime_enable.

int devm_pm_runtime_enable(struct device *dev)
{
	pm_runtime_enable(dev);

	return devm_add_action_or_reset(dev, pm_runtime_disable_action, dev);
}

We still need to check the return value of devm_add_action_or_reset and
perform the corresponding error handling.
We may still add the goto label.

Regards,
Ming

>>   err_vdev_register:
>> -	video_device_release(jpeg->dec_vdev);
>> +	/* Only release if allocation succeeded but registration failed */
>> +	if (jpeg->dec_vdev)
>> +		video_device_release(jpeg->dec_vdev);
>>
>>   err_vdev_alloc:
>>   	v4l2_m2m_release(jpeg->m2m_dev);
>> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>>
>>   	mxc_jpeg_free_slot_data(jpeg);
>>
>> -	pm_runtime_disable(&pdev->dev);
>>   	video_unregister_device(jpeg->dec_vdev);
>>   	v4l2_m2m_release(jpeg->m2m_dev);
>>   	v4l2_device_unregister(&jpeg->v4l2_dev);
>> --
>> 2.52.0
>>

Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

[Frank Li]

On Wed, Feb 04, 2026 at 09:36:06AM +0800, Ming Qian(OSS) wrote:
> Hi Frank,
>
> On 2/4/2026 12:56 AM, Frank Li wrote:
> > On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
> > > From: Ming Qian <ming.qian@oss.nxp.com>
> > >
> > > Use devm_pm_runtime_enable() to simplify probe and exit paths.
> > >
> > > No functional change.
> > >
> > > Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> > > ---
> > >   drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
> > >   1 file changed, 12 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > > index 39022c1bf36d..877e6c4f7406 100644
> > > --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > > +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> > > @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
> > >   			  jpeg->dec_vdev->minor);
> > >
> > >   	platform_set_drvdata(pdev, jpeg);
> > > -	pm_runtime_enable(dev);
> > > +	ret = devm_pm_runtime_enable(dev);
> > > +	if (ret) {
> > > +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
> > > +		goto err_pm;
> > > +	}
> > >
> > >   	return 0;
> > >
> > > +err_pm:
> > > +	video_unregister_device(jpeg->dec_vdev);
> > > +	/* set NULL to prevent double-free */
> > > +	jpeg->dec_vdev = NULL;
> >
> > code have not simplied, you add addtional goto label. You can use
> > devm_add_action_or_reset().
> >
> > Frank
> >
>
> I don't quite understand the difference compared to directly using
> devm_pm_runtime_enable.
>
> int devm_pm_runtime_enable(struct device *dev)
> {
> 	pm_runtime_enable(dev);
>
> 	return devm_add_action_or_reset(dev, pm_runtime_disable_action, dev);
> }
>
> We still need to check the return value of devm_add_action_or_reset and
> perform the corresponding error handling.
> We may still add the goto label.

Sorry, my means put video_device_release() to devm_add_action_or_reset() to
avoid goto.

If it is complex, you can keep original one.

Frank
>
> Regards,
> Ming
>
> > >   err_vdev_register:
> > > -	video_device_release(jpeg->dec_vdev);
> > > +	/* Only release if allocation succeeded but registration failed */
> > > +	if (jpeg->dec_vdev)
> > > +		video_device_release(jpeg->dec_vdev);
> > >
> > >   err_vdev_alloc:
> > >   	v4l2_m2m_release(jpeg->m2m_dev);
> > > @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
> > >
> > >   	mxc_jpeg_free_slot_data(jpeg);
> > >
> > > -	pm_runtime_disable(&pdev->dev);
> > >   	video_unregister_device(jpeg->dec_vdev);
> > >   	v4l2_m2m_release(jpeg->m2m_dev);
> > >   	v4l2_device_unregister(&jpeg->v4l2_dev);
> > > --
> > > 2.52.0
> > >

Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

[Ming Qian(OSS)]

Hi Frank,

On 2/4/2026 11:42 PM, Frank Li wrote:
> On Wed, Feb 04, 2026 at 09:36:06AM +0800, Ming Qian(OSS) wrote:
>> Hi Frank,
>>
>> On 2/4/2026 12:56 AM, Frank Li wrote:
>>> On Tue, Feb 03, 2026 at 04:23:39PM +0800, ming.qian@oss.nxp.com wrote:
>>>> From: Ming Qian <ming.qian@oss.nxp.com>
>>>>
>>>> Use devm_pm_runtime_enable() to simplify probe and exit paths.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>>>> ---
>>>>    drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>>>>    1 file changed, 12 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>>> index 39022c1bf36d..877e6c4f7406 100644
>>>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>>>> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>>>>    			  jpeg->dec_vdev->minor);
>>>>
>>>>    	platform_set_drvdata(pdev, jpeg);
>>>> -	pm_runtime_enable(dev);
>>>> +	ret = devm_pm_runtime_enable(dev);
>>>> +	if (ret) {
>>>> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
>>>> +		goto err_pm;
>>>> +	}
>>>>
>>>>    	return 0;
>>>>
>>>> +err_pm:
>>>> +	video_unregister_device(jpeg->dec_vdev);
>>>> +	/* set NULL to prevent double-free */
>>>> +	jpeg->dec_vdev = NULL;
>>>
>>> code have not simplied, you add addtional goto label. You can use
>>> devm_add_action_or_reset().
>>>
>>> Frank
>>>
>>
>> I don't quite understand the difference compared to directly using
>> devm_pm_runtime_enable.
>>
>> int devm_pm_runtime_enable(struct device *dev)
>> {
>> 	pm_runtime_enable(dev);
>>
>> 	return devm_add_action_or_reset(dev, pm_runtime_disable_action, dev);
>> }
>>
>> We still need to check the return value of devm_add_action_or_reset and
>> perform the corresponding error handling.
>> We may still add the goto label.
> 
> Sorry, my means put video_device_release() to devm_add_action_or_reset() to
> avoid goto.
> 
> If it is complex, you can keep original one.
> 
> Frank

OK, I get your point, and I prefer to keep it.

Regards,
Ming

>>
>> Regards,
>> Ming
>>
>>>>    err_vdev_register:
>>>> -	video_device_release(jpeg->dec_vdev);
>>>> +	/* Only release if allocation succeeded but registration failed */
>>>> +	if (jpeg->dec_vdev)
>>>> +		video_device_release(jpeg->dec_vdev);
>>>>
>>>>    err_vdev_alloc:
>>>>    	v4l2_m2m_release(jpeg->m2m_dev);
>>>> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>>>>
>>>>    	mxc_jpeg_free_slot_data(jpeg);
>>>>
>>>> -	pm_runtime_disable(&pdev->dev);
>>>>    	video_unregister_device(jpeg->dec_vdev);
>>>>    	v4l2_m2m_release(jpeg->m2m_dev);
>>>>    	v4l2_device_unregister(&jpeg->v4l2_dev);
>>>> --
>>>> 2.52.0
>>>>

Re: [PATCH v4 2/4] media: imx-jpeg: Use devm_pm_runtime_enable() helper

[Nicolas Dufresne]

[-- Attachment #1: Type: text/plain, Size: 1818 bytes --]

Le mardi 03 février 2026 à 16:23 +0800, ming.qian@oss.nxp.com a écrit :
> From: Ming Qian <ming.qian@oss.nxp.com>
> 
> Use devm_pm_runtime_enable() to simplify probe and exit paths.
> 
> No functional change.
> 
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>

Reviewed-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>

> ---
>  drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 39022c1bf36d..877e6c4f7406 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -2971,12 +2971,22 @@ static int mxc_jpeg_probe(struct platform_device *pdev)
>  			  jpeg->dec_vdev->minor);
>  
>  	platform_set_drvdata(pdev, jpeg);
> -	pm_runtime_enable(dev);
> +	ret = devm_pm_runtime_enable(dev);
> +	if (ret) {
> +		dev_err(dev, "Failed to enable runtime PM: %d\n", ret);
> +		goto err_pm;
> +	}
>  
>  	return 0;
>  
> +err_pm:
> +	video_unregister_device(jpeg->dec_vdev);
> +	/* set NULL to prevent double-free */
> +	jpeg->dec_vdev = NULL;
>  err_vdev_register:
> -	video_device_release(jpeg->dec_vdev);
> +	/* Only release if allocation succeeded but registration failed */
> +	if (jpeg->dec_vdev)
> +		video_device_release(jpeg->dec_vdev);
>  
>  err_vdev_alloc:
>  	v4l2_m2m_release(jpeg->m2m_dev);
> @@ -3047,7 +3057,6 @@ static void mxc_jpeg_remove(struct platform_device *pdev)
>  
>  	mxc_jpeg_free_slot_data(jpeg);
>  
> -	pm_runtime_disable(&pdev->dev);
>  	video_unregister_device(jpeg->dec_vdev);
>  	v4l2_m2m_release(jpeg->m2m_dev);
>  	v4l2_device_unregister(&jpeg->v4l2_dev);

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 228 bytes --]