On Fri, 2025-05-30 at 13:38 -0400, Nicolas Dufresne wrote:
> External email : Please do not click links or open attachments until
> you have verified the sender or the content.
>
>
> Hi,
>
> Le vendredi 30 mai 2025 à 15:45 +0800, Kyrie Wu a écrit :
> > 1. change dst buffer size to same as struct mtk_jpeg_src_buf
> > to make sure all params of mtk_jpeg_src_buf could get a memory.
> > 2. For memory alloc operation:
> > the v4l2 framework malloc a memory, the base addr is vb2_buffer and
> > the size is sizeof(struct mtk_jpeg_src_buf), mtk_jpeg_src_buf could
> > get
> > itself addr by container_of like that:
> > vb2_buffer -> vb2_v4l2_buffer -> mtk_jpeg_src_buf.
> > vb2_v4l2_buffer must keep on the top of mtk_jpeg_src_buf.
>
> The subject imply a refactoring, but the most important change in
> your
> patch is to fix the wrong buf strut size. Can you rework the subject
> and message to state what you are fixing please.
>
> Add a Fixes tag, and moved it at the start of the series to show this
> isn't a problem you have introduced in previous patch.
Dear Nicolas,
Thanks for your suggestion and I will change the tag from "refactoring"
to "fixes" and add the tag.
Thanks.
>
> >
> > Signed-off-by: Kyrie Wu <kyrie.wu@mediatek.com>
> > ---
> > 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 52d59bb5c9ad..7e3509be6f69 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.c
> > @@ -1103,7 +1103,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 655dc9c3280c..186cd1862028 100644
> > --- a/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > +++ b/drivers/media/platform/mediatek/jpeg/mtk_jpeg_core.h
> > @@ -96,10 +96,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;
>
> This "refactoring" should be split, it does not fix anything.
Thanks. I will change it in the coming version 6.
>
> Nicolas
Regards,
Kyrie.
>
> > struct mtk_jpeg_dec_param dec_param;
> >
> > struct mtk_jpeg_ctx *curr_ctx;