[PATCH v2] media: imx-jpeg: Add support for encoder v1 descriptor configuration
Ming Qian(OSS)
ming.qian at oss.nxp.com
Sun Feb 1 21:59:28 PST 2026
Hi Frank,
On 1/30/2026 10:52 PM, Frank Li wrote:
> On Fri, Jan 30, 2026 at 02:22:33PM +0800, ming.qian at oss.nxp.com wrote:
>> From: Ming Qian <ming.qian at 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 at 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
>>
More information about the linux-arm-kernel
mailing list