Le jeudi 25 décembre 2025 à 06:05 +0000, Kyrie Wu (吴晗) a écrit :

On Tue, 2025-12-16 at 16:22 -0500, Nicolas Dufresne wrote:

quoted

Hi,

Le mardi 02 décembre 2025 à 17:47 +0800, Kyrie Wu a écrit :

quoted

For memory alloc operation of jpeg dst buffer: the mallocing
memory function interface use vb2_buffer as the base addr.
If structure mtk_jpeg_src_buf wants to be allocated to memory,
it needs to be placed vb2_v4l2_buffer at the starting position,
because structure vb2_buffer is at the starting position of
vb2_v4l2_buffer, and the allocated size is set to the size of
structure mtk_jpeg_src_buf, so as to ensure that structures
mtk_jpeg_src_buf, vb2_v4l2_buffer and vb2_buffer can all be
allocated memory.

Please correct the wording, "mallocing" is not a word, addr ->
address. I tend
to do the same, but refrain from giving the code a personality, the
vb2_buffer
have no will. This is overall complicated way to simply say:

	Fix the size of the allocated mtk_jpeg_src_buf structure.

quoted

Fixes: 5fb1c2361e56 ("mtk-jpegenc: add jpeg encode worker
interface")

Drop the blank line.

quoted

Signed-off-by: Kyrie Wu <redacted>
---
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c | 2 +-
 drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c

b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
index 0cf3dc5407e4..bd0afc93d491 100644

--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c

@@ -1099,7 +1099,7 @@ static int mtk_jpeg_queue_init(void *priv,

struct vb2_queue *src_vq,
 	dst_vq->type = V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE;
 	dst_vq->io_modes = VB2_DMABUF | VB2_MMAP;
 	dst_vq->drv_priv = ctx;
-	dst_vq->buf_struct_size = sizeof(struct v4l2_m2m_buffer);
+	dst_vq->buf_struct_size = sizeof(struct mtk_jpeg_src_buf);
 	dst_vq->ops = jpeg->variant->qops;
 	dst_vq->mem_ops = &vb2_dma_contig_memops;
 	dst_vq->timestamp_flags = V4L2_BUF_FLAG_TIMESTAMP_COPY;

diff --git a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h

b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
index 6be5cf30dea1..148fd41759b7 100644

--- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
+++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h

@@ -85,10 +85,10 @@ struct mtk_jpeg_variant {

 };
 
 struct mtk_jpeg_src_buf {
-	u32 frame_num;
 	struct vb2_v4l2_buffer b;
 	struct list_head list;
 	u32 bs_size;
+	u32 frame_num;

I like the change, but this shouldn't be an issue if you use
container_of, which
is safer then a plain cast. Please review the code and make sure to
use it. It
may be confusing to include a cosmetic change in a fixes.

Nicolas

Dear Nicolas,

The driver requests memory space for mtk_jpeg_src_buf in the
function of mtk_jpeg_queue_init,
and it used struct vb2_v4l2_buffer b as the starting base address,
the parameter,frame_num, will not get a memory if we keep the old
structure. And a unknown memory would be get if we used container_of
to get the starting address of mtk_jpeg_src_buf.

You are right, and the documentation says so:

 *              The first field of the
 *		driver-specific buffer structure must be the subsystem-specific
 *		struct (vb2_v4l2_buffer in the case of V4L2).

This is inconsistent with how other objects are overloaded, notably the request
object, which does not have this limitation. We should probably fix that
someday, meanwhile.

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

Thanks.

Regards,
Kyrie.

quoted

quoted

 	struct mtk_jpeg_dec_param dec_param;
 
 	struct mtk_jpeg_ctx *curr_ctx;