.../media/platform/nxp/imx-jpeg/mxc-jpeg.c | 47 ++++++++++++++----- .../media/platform/nxp/imx-jpeg/mxc-jpeg.h | 1 + 2 files changed, 37 insertions(+), 11 deletions(-)
From: Ming Qian <ming.qian@oss.nxp.com>
Applications may set data_offset when it refers to an output queue. So
driver need to account for it when getting the start address of input
image in the plane.
Meanwhile the mxc-jpeg codec requires the address (plane address +
data_offset) to be 16-aligned.
Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
---
v2
- Verify the address alignment in buf_prepare()
---
.../media/platform/nxp/imx-jpeg/mxc-jpeg.c | 47 ++++++++++++++-----
.../media/platform/nxp/imx-jpeg/mxc-jpeg.h | 1 +
2 files changed, 37 insertions(+), 11 deletions(-)
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
index 5c17bc58181e..8681dd193033 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
@@ -598,6 +598,27 @@ static void _bswap16(u16 *a)
*a = ((*a & 0x00FF) << 8) | ((*a & 0xFF00) >> 8);
}
+static dma_addr_t mxc_jpeg_get_plane_dma_addr(struct vb2_buffer *buf, unsigned int plane_no)
+{
+ if (plane_no >= buf->num_planes)
+ return 0;
+ return vb2_dma_contig_plane_dma_addr(buf, plane_no) + buf->planes[plane_no].data_offset;
+}
+
+static void *mxc_jpeg_get_plane_vaddr(struct vb2_buffer *buf, unsigned int plane_no)
+{
+ if (plane_no >= buf->num_planes)
+ return NULL;
+ return vb2_plane_vaddr(buf, plane_no) + buf->planes[plane_no].data_offset;
+}
+
+static unsigned long mxc_jpeg_get_plane_payload(struct vb2_buffer *buf, unsigned int plane_no)
+{
+ if (plane_no >= buf->num_planes)
+ return 0;
+ return vb2_get_plane_payload(buf, plane_no) - buf->planes[plane_no].data_offset;
+}
+
static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
unsigned long len)
{
@@ -610,11 +631,11 @@ static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
return;
for (plane_no = 0; plane_no < buf->num_planes; plane_no++) {
- payload = vb2_get_plane_payload(buf, plane_no);
+ payload = mxc_jpeg_get_plane_payload(buf, plane_no);
if (len == 0)
len = payload;
- dma_addr = vb2_dma_contig_plane_dma_addr(buf, plane_no);
- vaddr = vb2_plane_vaddr(buf, plane_no);
+ dma_addr = mxc_jpeg_get_plane_dma_addr(buf, plane_no);
+ vaddr = mxc_jpeg_get_plane_vaddr(buf, plane_no);
v4l2_dbg(3, debug, &jpeg->v4l2_dev,
"plane %d (vaddr=%p dma_addr=%x payload=%ld):",
plane_no, vaddr, dma_addr, payload);
@@ -712,16 +733,15 @@ static void mxc_jpeg_addrs(struct mxc_jpeg_desc *desc,
struct mxc_jpeg_q_data *q_data;
q_data = mxc_jpeg_get_q_data(ctx, raw_buf->type);
- desc->buf_base0 = vb2_dma_contig_plane_dma_addr(raw_buf, 0);
+ desc->buf_base0 = mxc_jpeg_get_plane_dma_addr(raw_buf, 0);
desc->buf_base1 = 0;
if (img_fmt == STM_CTRL_IMAGE_FORMAT(MXC_JPEG_YUV420)) {
if (raw_buf->num_planes == 2)
- desc->buf_base1 = vb2_dma_contig_plane_dma_addr(raw_buf, 1);
+ desc->buf_base1 = mxc_jpeg_get_plane_dma_addr(raw_buf, 1);
else
desc->buf_base1 = desc->buf_base0 + q_data->sizeimage[0];
}
- desc->stm_bufbase = vb2_dma_contig_plane_dma_addr(jpeg_buf, 0) +
- offset;
+ desc->stm_bufbase = mxc_jpeg_get_plane_dma_addr(jpeg_buf, 0) + offset;
}
static bool mxc_jpeg_is_extended_sequential(const struct mxc_jpeg_fmt *fmt)
@@ -1029,8 +1049,8 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
vb2_set_plane_payload(&dst_buf->vb2_buf, 1, payload);
}
dev_dbg(dev, "Decoding finished, payload size: %ld + %ld\n",
- vb2_get_plane_payload(&dst_buf->vb2_buf, 0),
- vb2_get_plane_payload(&dst_buf->vb2_buf, 1));
+ mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 0),
+ mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 1));
}
/* short preview of the results */
@@ -1889,8 +1909,8 @@ static int mxc_jpeg_parse(struct mxc_jpeg_ctx *ctx, struct vb2_buffer *vb)
struct mxc_jpeg_sof *psof = NULL;
struct mxc_jpeg_sos *psos = NULL;
struct mxc_jpeg_src_buf *jpeg_src_buf = vb2_to_mxc_buf(vb);
- u8 *src_addr = (u8 *)vb2_plane_vaddr(vb, 0);
- u32 size = vb2_get_plane_payload(vb, 0);
+ u8 *src_addr = (u8 *)mxc_jpeg_get_plane_vaddr(vb, 0);
+ u32 size = mxc_jpeg_get_plane_payload(vb, 0);
int ret;
memset(&header, 0, sizeof(header));
@@ -2027,6 +2047,11 @@ static int mxc_jpeg_buf_prepare(struct vb2_buffer *vb)
i, vb2_plane_size(vb, i), sizeimage);
return -EINVAL;
}
+ if (!IS_ALIGNED(mxc_jpeg_get_plane_dma_addr(vb, i), MXC_JPEG_ADDR_ALIGNMENT)) {
+ dev_err(dev, "planes[%d] address is not %d aligned\n",
+ i, MXC_JPEG_ADDR_ALIGNMENT);
+ return -EINVAL;
+ }
}
if (V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type)) {
vb2_set_plane_payload(vb, 0, 0);
diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
index fdde45f7e163..44e46face6d1 100644
--- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
+++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
@@ -30,6 +30,7 @@
#define MXC_JPEG_MAX_PLANES 2
#define MXC_JPEG_PATTERN_WIDTH 128
#define MXC_JPEG_PATTERN_HEIGHT 64
+#define MXC_JPEG_ADDR_ALIGNMENT 16
enum mxc_jpeg_enc_state {
MXC_JPEG_ENCODING = 0, /* jpeg encode phase */
base-commit: 2e79181dfc85e1347a8655ea8d8a314158155c52
prerequisite-patch-id: 0000000000000000000000000000000000000000
--
2.43.0-rc1On Tue, May 06, 2025 at 04:08:15PM +0800, ming.qian@oss.nxp.com wrote:
> From: Ming Qian <ming.qian@oss.nxp.com>
>
> Applications may set data_offset when it refers to an output queue. So
> driver need to account for it when getting the start address of input
> image in the plane.
>
> Meanwhile the mxc-jpeg codec requires the address (plane address +
> data_offset) to be 16-aligned.
look like it is bug fix, missed consider data_offset.
So need fix tag.
Frank
>
> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
> ---
> v2
> - Verify the address alignment in buf_prepare()
>
> ---
> .../media/platform/nxp/imx-jpeg/mxc-jpeg.c | 47 ++++++++++++++-----
> .../media/platform/nxp/imx-jpeg/mxc-jpeg.h | 1 +
> 2 files changed, 37 insertions(+), 11 deletions(-)
>
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> index 5c17bc58181e..8681dd193033 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
> @@ -598,6 +598,27 @@ static void _bswap16(u16 *a)
> *a = ((*a & 0x00FF) << 8) | ((*a & 0xFF00) >> 8);
> }
>
> +static dma_addr_t mxc_jpeg_get_plane_dma_addr(struct vb2_buffer *buf, unsigned int plane_no)
> +{
> + if (plane_no >= buf->num_planes)
> + return 0;
> + return vb2_dma_contig_plane_dma_addr(buf, plane_no) + buf->planes[plane_no].data_offset;
> +}
> +
> +static void *mxc_jpeg_get_plane_vaddr(struct vb2_buffer *buf, unsigned int plane_no)
> +{
> + if (plane_no >= buf->num_planes)
> + return NULL;
> + return vb2_plane_vaddr(buf, plane_no) + buf->planes[plane_no].data_offset;
> +}
> +
> +static unsigned long mxc_jpeg_get_plane_payload(struct vb2_buffer *buf, unsigned int plane_no)
> +{
> + if (plane_no >= buf->num_planes)
> + return 0;
> + return vb2_get_plane_payload(buf, plane_no) - buf->planes[plane_no].data_offset;
> +}
> +
> static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
> unsigned long len)
> {
> @@ -610,11 +631,11 @@ static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
> return;
>
> for (plane_no = 0; plane_no < buf->num_planes; plane_no++) {
> - payload = vb2_get_plane_payload(buf, plane_no);
> + payload = mxc_jpeg_get_plane_payload(buf, plane_no);
> if (len == 0)
> len = payload;
> - dma_addr = vb2_dma_contig_plane_dma_addr(buf, plane_no);
> - vaddr = vb2_plane_vaddr(buf, plane_no);
> + dma_addr = mxc_jpeg_get_plane_dma_addr(buf, plane_no);
> + vaddr = mxc_jpeg_get_plane_vaddr(buf, plane_no);
> v4l2_dbg(3, debug, &jpeg->v4l2_dev,
> "plane %d (vaddr=%p dma_addr=%x payload=%ld):",
> plane_no, vaddr, dma_addr, payload);
> @@ -712,16 +733,15 @@ static void mxc_jpeg_addrs(struct mxc_jpeg_desc *desc,
> struct mxc_jpeg_q_data *q_data;
>
> q_data = mxc_jpeg_get_q_data(ctx, raw_buf->type);
> - desc->buf_base0 = vb2_dma_contig_plane_dma_addr(raw_buf, 0);
> + desc->buf_base0 = mxc_jpeg_get_plane_dma_addr(raw_buf, 0);
> desc->buf_base1 = 0;
> if (img_fmt == STM_CTRL_IMAGE_FORMAT(MXC_JPEG_YUV420)) {
> if (raw_buf->num_planes == 2)
> - desc->buf_base1 = vb2_dma_contig_plane_dma_addr(raw_buf, 1);
> + desc->buf_base1 = mxc_jpeg_get_plane_dma_addr(raw_buf, 1);
> else
> desc->buf_base1 = desc->buf_base0 + q_data->sizeimage[0];
> }
> - desc->stm_bufbase = vb2_dma_contig_plane_dma_addr(jpeg_buf, 0) +
> - offset;
> + desc->stm_bufbase = mxc_jpeg_get_plane_dma_addr(jpeg_buf, 0) + offset;
> }
>
> static bool mxc_jpeg_is_extended_sequential(const struct mxc_jpeg_fmt *fmt)
> @@ -1029,8 +1049,8 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
> vb2_set_plane_payload(&dst_buf->vb2_buf, 1, payload);
> }
> dev_dbg(dev, "Decoding finished, payload size: %ld + %ld\n",
> - vb2_get_plane_payload(&dst_buf->vb2_buf, 0),
> - vb2_get_plane_payload(&dst_buf->vb2_buf, 1));
> + mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 0),
> + mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 1));
> }
>
> /* short preview of the results */
> @@ -1889,8 +1909,8 @@ static int mxc_jpeg_parse(struct mxc_jpeg_ctx *ctx, struct vb2_buffer *vb)
> struct mxc_jpeg_sof *psof = NULL;
> struct mxc_jpeg_sos *psos = NULL;
> struct mxc_jpeg_src_buf *jpeg_src_buf = vb2_to_mxc_buf(vb);
> - u8 *src_addr = (u8 *)vb2_plane_vaddr(vb, 0);
> - u32 size = vb2_get_plane_payload(vb, 0);
> + u8 *src_addr = (u8 *)mxc_jpeg_get_plane_vaddr(vb, 0);
> + u32 size = mxc_jpeg_get_plane_payload(vb, 0);
> int ret;
>
> memset(&header, 0, sizeof(header));
> @@ -2027,6 +2047,11 @@ static int mxc_jpeg_buf_prepare(struct vb2_buffer *vb)
> i, vb2_plane_size(vb, i), sizeimage);
> return -EINVAL;
> }
> + if (!IS_ALIGNED(mxc_jpeg_get_plane_dma_addr(vb, i), MXC_JPEG_ADDR_ALIGNMENT)) {
> + dev_err(dev, "planes[%d] address is not %d aligned\n",
> + i, MXC_JPEG_ADDR_ALIGNMENT);
> + return -EINVAL;
> + }
> }
> if (V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type)) {
> vb2_set_plane_payload(vb, 0, 0);
> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> index fdde45f7e163..44e46face6d1 100644
> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
> @@ -30,6 +30,7 @@
> #define MXC_JPEG_MAX_PLANES 2
> #define MXC_JPEG_PATTERN_WIDTH 128
> #define MXC_JPEG_PATTERN_HEIGHT 64
> +#define MXC_JPEG_ADDR_ALIGNMENT 16
>
> enum mxc_jpeg_enc_state {
> MXC_JPEG_ENCODING = 0, /* jpeg encode phase */
>
> base-commit: 2e79181dfc85e1347a8655ea8d8a314158155c52
> prerequisite-patch-id: 0000000000000000000000000000000000000000
> --
> 2.43.0-rc1
>
Hi Frank,
On 2025/5/20 4:57, Frank Li wrote:
> On Tue, May 06, 2025 at 04:08:15PM +0800, ming.qian@oss.nxp.com wrote:
>> From: Ming Qian <ming.qian@oss.nxp.com>
>>
>> Applications may set data_offset when it refers to an output queue. So
>> driver need to account for it when getting the start address of input
>> image in the plane.
>>
>> Meanwhile the mxc-jpeg codec requires the address (plane address +
>> data_offset) to be 16-aligned.
>
> look like it is bug fix, missed consider data_offset.
>
> So need fix tag.
>
> Frank
>
Thanks for the reminder, will add it in v3.
regards,
Ming
>>
>> Signed-off-by: Ming Qian <ming.qian@oss.nxp.com>
>> ---
>> v2
>> - Verify the address alignment in buf_prepare()
>>
>> ---
>> .../media/platform/nxp/imx-jpeg/mxc-jpeg.c | 47 ++++++++++++++-----
>> .../media/platform/nxp/imx-jpeg/mxc-jpeg.h | 1 +
>> 2 files changed, 37 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> index 5c17bc58181e..8681dd193033 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.c
>> @@ -598,6 +598,27 @@ static void _bswap16(u16 *a)
>> *a = ((*a & 0x00FF) << 8) | ((*a & 0xFF00) >> 8);
>> }
>>
>> +static dma_addr_t mxc_jpeg_get_plane_dma_addr(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> + if (plane_no >= buf->num_planes)
>> + return 0;
>> + return vb2_dma_contig_plane_dma_addr(buf, plane_no) + buf->planes[plane_no].data_offset;
>> +}
>> +
>> +static void *mxc_jpeg_get_plane_vaddr(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> + if (plane_no >= buf->num_planes)
>> + return NULL;
>> + return vb2_plane_vaddr(buf, plane_no) + buf->planes[plane_no].data_offset;
>> +}
>> +
>> +static unsigned long mxc_jpeg_get_plane_payload(struct vb2_buffer *buf, unsigned int plane_no)
>> +{
>> + if (plane_no >= buf->num_planes)
>> + return 0;
>> + return vb2_get_plane_payload(buf, plane_no) - buf->planes[plane_no].data_offset;
>> +}
>> +
>> static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
>> unsigned long len)
>> {
>> @@ -610,11 +631,11 @@ static void print_mxc_buf(struct mxc_jpeg_dev *jpeg, struct vb2_buffer *buf,
>> return;
>>
>> for (plane_no = 0; plane_no < buf->num_planes; plane_no++) {
>> - payload = vb2_get_plane_payload(buf, plane_no);
>> + payload = mxc_jpeg_get_plane_payload(buf, plane_no);
>> if (len == 0)
>> len = payload;
>> - dma_addr = vb2_dma_contig_plane_dma_addr(buf, plane_no);
>> - vaddr = vb2_plane_vaddr(buf, plane_no);
>> + dma_addr = mxc_jpeg_get_plane_dma_addr(buf, plane_no);
>> + vaddr = mxc_jpeg_get_plane_vaddr(buf, plane_no);
>> v4l2_dbg(3, debug, &jpeg->v4l2_dev,
>> "plane %d (vaddr=%p dma_addr=%x payload=%ld):",
>> plane_no, vaddr, dma_addr, payload);
>> @@ -712,16 +733,15 @@ static void mxc_jpeg_addrs(struct mxc_jpeg_desc *desc,
>> struct mxc_jpeg_q_data *q_data;
>>
>> q_data = mxc_jpeg_get_q_data(ctx, raw_buf->type);
>> - desc->buf_base0 = vb2_dma_contig_plane_dma_addr(raw_buf, 0);
>> + desc->buf_base0 = mxc_jpeg_get_plane_dma_addr(raw_buf, 0);
>> desc->buf_base1 = 0;
>> if (img_fmt == STM_CTRL_IMAGE_FORMAT(MXC_JPEG_YUV420)) {
>> if (raw_buf->num_planes == 2)
>> - desc->buf_base1 = vb2_dma_contig_plane_dma_addr(raw_buf, 1);
>> + desc->buf_base1 = mxc_jpeg_get_plane_dma_addr(raw_buf, 1);
>> else
>> desc->buf_base1 = desc->buf_base0 + q_data->sizeimage[0];
>> }
>> - desc->stm_bufbase = vb2_dma_contig_plane_dma_addr(jpeg_buf, 0) +
>> - offset;
>> + desc->stm_bufbase = mxc_jpeg_get_plane_dma_addr(jpeg_buf, 0) + offset;
>> }
>>
>> static bool mxc_jpeg_is_extended_sequential(const struct mxc_jpeg_fmt *fmt)
>> @@ -1029,8 +1049,8 @@ static irqreturn_t mxc_jpeg_dec_irq(int irq, void *priv)
>> vb2_set_plane_payload(&dst_buf->vb2_buf, 1, payload);
>> }
>> dev_dbg(dev, "Decoding finished, payload size: %ld + %ld\n",
>> - vb2_get_plane_payload(&dst_buf->vb2_buf, 0),
>> - vb2_get_plane_payload(&dst_buf->vb2_buf, 1));
>> + mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 0),
>> + mxc_jpeg_get_plane_payload(&dst_buf->vb2_buf, 1));
>> }
>>
>> /* short preview of the results */
>> @@ -1889,8 +1909,8 @@ static int mxc_jpeg_parse(struct mxc_jpeg_ctx *ctx, struct vb2_buffer *vb)
>> struct mxc_jpeg_sof *psof = NULL;
>> struct mxc_jpeg_sos *psos = NULL;
>> struct mxc_jpeg_src_buf *jpeg_src_buf = vb2_to_mxc_buf(vb);
>> - u8 *src_addr = (u8 *)vb2_plane_vaddr(vb, 0);
>> - u32 size = vb2_get_plane_payload(vb, 0);
>> + u8 *src_addr = (u8 *)mxc_jpeg_get_plane_vaddr(vb, 0);
>> + u32 size = mxc_jpeg_get_plane_payload(vb, 0);
>> int ret;
>>
>> memset(&header, 0, sizeof(header));
>> @@ -2027,6 +2047,11 @@ static int mxc_jpeg_buf_prepare(struct vb2_buffer *vb)
>> i, vb2_plane_size(vb, i), sizeimage);
>> return -EINVAL;
>> }
>> + if (!IS_ALIGNED(mxc_jpeg_get_plane_dma_addr(vb, i), MXC_JPEG_ADDR_ALIGNMENT)) {
>> + dev_err(dev, "planes[%d] address is not %d aligned\n",
>> + i, MXC_JPEG_ADDR_ALIGNMENT);
>> + return -EINVAL;
>> + }
>> }
>> if (V4L2_TYPE_IS_CAPTURE(vb->vb2_queue->type)) {
>> vb2_set_plane_payload(vb, 0, 0);
>> diff --git a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> index fdde45f7e163..44e46face6d1 100644
>> --- a/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> +++ b/drivers/media/platform/nxp/imx-jpeg/mxc-jpeg.h
>> @@ -30,6 +30,7 @@
>> #define MXC_JPEG_MAX_PLANES 2
>> #define MXC_JPEG_PATTERN_WIDTH 128
>> #define MXC_JPEG_PATTERN_HEIGHT 64
>> +#define MXC_JPEG_ADDR_ALIGNMENT 16
>>
>> enum mxc_jpeg_enc_state {
>> MXC_JPEG_ENCODING = 0, /* jpeg encode phase */
>>
>> base-commit: 2e79181dfc85e1347a8655ea8d8a314158155c52
>> prerequisite-patch-id: 0000000000000000000000000000000000000000
>> --
>> 2.43.0-rc1
>>
© 2016 - 2025 Red Hat, Inc.