diff mbox series

[v4,4/7] media: mediatek: vcodec: using input information to get vb2 buffer

Message ID 20240807082444.21280-5-yunfei.dong@mediatek.com (mailing list archive)
State New
Headers show
Series media: mediatek: vcodec: fix v4l2_ctrl_request_complete fail | expand

Commit Message

Yunfei Dong (董云飞) Aug. 7, 2024, 8:24 a.m. UTC 
vb2 buffer may be removed from ready list when lat try to get next
src buffer, leading to vb2 buffer not the current one. Need to get
vb2 buffer according to current input memory information.

Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
---
 .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c     | 13 +++++++------
 .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c     | 15 +++++++--------
 2 files changed, 14 insertions(+), 14 deletions(-)

Comments

sebastian.fricke@collabora.com Aug. 23, 2024, 12:40 p.m. UTC | #1 
Hey Yunfei,

I would rename the title to something like this:

media: mediatek: vcodec: Get SRC buffer from bitstream instead of M2M

On 07.08.2024 16:24, Yunfei Dong wrote:
>vb2 buffer may be removed from ready list when lat try to get next
>src buffer, leading to vb2 buffer not the current one. Need to get
>vb2 buffer according to current input memory information.

And I would rewrite the commit log like this:

Getting the SRC buffer from the M2M buffer-queue risks picking a
different SRC buffer than the one used for the current decode operation.
Get the SRC buffer therefore from the bitstream data, which was set up
earlier during the decode.

Did I get that right?

Also could you explain why this change is required in this series?

Regards,
Sebastian Fricke

>
>Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
>---
> .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c     | 13 +++++++------
> .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c     | 15 +++++++--------
> 2 files changed, 14 insertions(+), 14 deletions(-)
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
>index 90217cc8e242..a744740ba5f1 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
>@@ -1062,19 +1062,20 @@ static inline void vdec_av1_slice_vsi_to_remote(struct vdec_av1_slice_vsi *vsi,
>
> static int vdec_av1_slice_setup_lat_from_src_buf(struct vdec_av1_slice_instance *instance,
> 						 struct vdec_av1_slice_vsi *vsi,
>+						 struct mtk_vcodec_mem *bs,
> 						 struct vdec_lat_buf *lat_buf)
> {
>-	struct vb2_v4l2_buffer *src;
>+	struct mtk_video_dec_buf *src_buf_info;
> 	struct vb2_v4l2_buffer *dst;
>
>-	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
>-	if (!src)
>+	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
>+	if (!src_buf_info)
> 		return -EINVAL;
>
>-	lat_buf->vb2_v4l2_src = src;
>+	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
>
> 	dst = &lat_buf->ts_info;
>-	v4l2_m2m_buf_copy_metadata(src, dst, true);
>+	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, dst, true);
> 	vsi->frame.cur_ts = dst->vb2_buf.timestamp;
>
> 	return 0;
>@@ -1724,7 +1725,7 @@ static int vdec_av1_slice_setup_lat(struct vdec_av1_slice_instance *instance,
> 	struct vdec_av1_slice_vsi *vsi = &pfc->vsi;
> 	int ret;
>
>-	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, lat_buf);
>+	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, bs, lat_buf);
> 	if (ret)
> 		return ret;
>
>diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
>index 3dceb668ba1c..c50a454ab4f7 100644
>--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
>+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
>@@ -712,19 +712,18 @@ int vdec_vp9_slice_setup_single_from_src_to_dst(struct vdec_vp9_slice_instance *
> }
>
> static int vdec_vp9_slice_setup_lat_from_src_buf(struct vdec_vp9_slice_instance *instance,
>+						 struct mtk_vcodec_mem *bs,
> 						 struct vdec_lat_buf *lat_buf)
> {
>-	struct vb2_v4l2_buffer *src;
>-	struct vb2_v4l2_buffer *dst;
>+	struct mtk_video_dec_buf *src_buf_info;
>
>-	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
>-	if (!src)
>+	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
>+	if (!src_buf_info)
> 		return -EINVAL;
>
>-	lat_buf->vb2_v4l2_src = src;
>+	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
>
>-	dst = &lat_buf->ts_info;
>-	v4l2_m2m_buf_copy_metadata(src, dst, true);
>+	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, &lat_buf->ts_info, true);
> 	return 0;
> }
>
>@@ -1154,7 +1153,7 @@ static int vdec_vp9_slice_setup_lat(struct vdec_vp9_slice_instance *instance,
> 	struct vdec_vp9_slice_vsi *vsi = &pfc->vsi;
> 	int ret;
>
>-	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, lat_buf);
>+	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, bs, lat_buf);
> 	if (ret)
> 		goto err;
>
>-- 
>2.46.0
>
>
Yunfei Dong (董云飞) Aug. 27, 2024, 2:47 a.m. UTC | #2 
Hi Sebastian,

Thanks for your suggestion.
On Fri, 2024-08-23 at 14:40 +0200, Sebastian Fricke wrote:
> Hey Yunfei,
> 
> I would rename the title to something like this:
> 
> media: mediatek: vcodec: Get SRC buffer from bitstream instead of M2M
> 
> On 07.08.2024 16:24, Yunfei Dong wrote:
> > vb2 buffer may be removed from ready list when lat try to get next
> > src buffer, leading to vb2 buffer not the current one. Need to get
> > vb2 buffer according to current input memory information.
> 
> And I would rewrite the commit log like this:
> 
> Getting the SRC buffer from the M2M buffer-queue risks picking a
> different SRC buffer than the one used for the current decode
> operation.
> Get the SRC buffer therefore from the bitstream data, which was set
> up
> earlier during the decode.
> 
> Did I get that right?
> 
> Also could you explain why this change is required in this series?
> 
Must make sure the src buffer is removed from ready list, in case of
the buffer is removed when core work try to set the buffer done.

Best Regards,
Yunfei Dong
> Regards,
> Sebastian Fricke
> 
> > 
> > Signed-off-by: Yunfei Dong <yunfei.dong@mediatek.com>
> > ---
> > .../vcodec/decoder/vdec/vdec_av1_req_lat_if.c     | 13 +++++++-----
> > -
> > .../vcodec/decoder/vdec/vdec_vp9_req_lat_if.c     | 15 +++++++-----
> > ---
> > 2 files changed, 14 insertions(+), 14 deletions(-)
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_
> > lat_if.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_
> > lat_if.c
> > index 90217cc8e242..a744740ba5f1 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_
> > lat_if.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_
> > lat_if.c
> > @@ -1062,19 +1062,20 @@ static inline void
> > vdec_av1_slice_vsi_to_remote(struct vdec_av1_slice_vsi *vsi,
> > 
> > static int vdec_av1_slice_setup_lat_from_src_buf(struct
> > vdec_av1_slice_instance *instance,
> > 						 struct
> > vdec_av1_slice_vsi *vsi,
> > +						 struct mtk_vcodec_mem
> > *bs,
> > 						 struct vdec_lat_buf
> > *lat_buf)
> > {
> > -	struct vb2_v4l2_buffer *src;
> > +	struct mtk_video_dec_buf *src_buf_info;
> > 	struct vb2_v4l2_buffer *dst;
> > 
> > -	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
> > -	if (!src)
> > +	src_buf_info = container_of(bs, struct mtk_video_dec_buf,
> > bs_buffer);
> > +	if (!src_buf_info)
> > 		return -EINVAL;
> > 
> > -	lat_buf->vb2_v4l2_src = src;
> > +	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
> > 
> > 	dst = &lat_buf->ts_info;
> > -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> > +	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, dst, true);
> > 	vsi->frame.cur_ts = dst->vb2_buf.timestamp;
> > 
> > 	return 0;
> > @@ -1724,7 +1725,7 @@ static int vdec_av1_slice_setup_lat(struct
> > vdec_av1_slice_instance *instance,
> > 	struct vdec_av1_slice_vsi *vsi = &pfc->vsi;
> > 	int ret;
> > 
> > -	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi,
> > lat_buf);
> > +	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, bs,
> > lat_buf);
> > 	if (ret)
> > 		return ret;
> > 
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_
> > lat_if.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_
> > lat_if.c
> > index 3dceb668ba1c..c50a454ab4f7 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_
> > lat_if.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_
> > lat_if.c
> > @@ -712,19 +712,18 @@ int
> > vdec_vp9_slice_setup_single_from_src_to_dst(struct
> > vdec_vp9_slice_instance *
> > }
> > 
> > static int vdec_vp9_slice_setup_lat_from_src_buf(struct
> > vdec_vp9_slice_instance *instance,
> > +						 struct mtk_vcodec_mem
> > *bs,
> > 						 struct vdec_lat_buf
> > *lat_buf)
> > {
> > -	struct vb2_v4l2_buffer *src;
> > -	struct vb2_v4l2_buffer *dst;
> > +	struct mtk_video_dec_buf *src_buf_info;
> > 
> > -	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
> > -	if (!src)
> > +	src_buf_info = container_of(bs, struct mtk_video_dec_buf,
> > bs_buffer);
> > +	if (!src_buf_info)
> > 		return -EINVAL;
> > 
> > -	lat_buf->vb2_v4l2_src = src;
> > +	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
> > 
> > -	dst = &lat_buf->ts_info;
> > -	v4l2_m2m_buf_copy_metadata(src, dst, true);
> > +	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, &lat_buf-
> > >ts_info, true);
> > 	return 0;
> > }
> > 
> > @@ -1154,7 +1153,7 @@ static int vdec_vp9_slice_setup_lat(struct
> > vdec_vp9_slice_instance *instance,
> > 	struct vdec_vp9_slice_vsi *vsi = &pfc->vsi;
> > 	int ret;
> > 
> > -	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, lat_buf);
> > +	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, bs,
> > lat_buf);
> > 	if (ret)
> > 		goto err;
> > 
> > -- 
> > 2.46.0
> > 
> > 
> 
>
diff mbox series

Patch

diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
index 90217cc8e242..a744740ba5f1 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_av1_req_lat_if.c
@@ -1062,19 +1062,20 @@  static inline void vdec_av1_slice_vsi_to_remote(struct vdec_av1_slice_vsi *vsi,
 
 static int vdec_av1_slice_setup_lat_from_src_buf(struct vdec_av1_slice_instance *instance,
 						 struct vdec_av1_slice_vsi *vsi,
+						 struct mtk_vcodec_mem *bs,
 						 struct vdec_lat_buf *lat_buf)
 {
-	struct vb2_v4l2_buffer *src;
+	struct mtk_video_dec_buf *src_buf_info;
 	struct vb2_v4l2_buffer *dst;
 
-	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
-	if (!src)
+	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
+	if (!src_buf_info)
 		return -EINVAL;
 
-	lat_buf->vb2_v4l2_src = src;
+	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
 
 	dst = &lat_buf->ts_info;
-	v4l2_m2m_buf_copy_metadata(src, dst, true);
+	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, dst, true);
 	vsi->frame.cur_ts = dst->vb2_buf.timestamp;
 
 	return 0;
@@ -1724,7 +1725,7 @@  static int vdec_av1_slice_setup_lat(struct vdec_av1_slice_instance *instance,
 	struct vdec_av1_slice_vsi *vsi = &pfc->vsi;
 	int ret;
 
-	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, lat_buf);
+	ret = vdec_av1_slice_setup_lat_from_src_buf(instance, vsi, bs, lat_buf);
 	if (ret)
 		return ret;
 
diff --git a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
index 3dceb668ba1c..c50a454ab4f7 100644
--- a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_vp9_req_lat_if.c
@@ -712,19 +712,18 @@  int vdec_vp9_slice_setup_single_from_src_to_dst(struct vdec_vp9_slice_instance *
 }
 
 static int vdec_vp9_slice_setup_lat_from_src_buf(struct vdec_vp9_slice_instance *instance,
+						 struct mtk_vcodec_mem *bs,
 						 struct vdec_lat_buf *lat_buf)
 {
-	struct vb2_v4l2_buffer *src;
-	struct vb2_v4l2_buffer *dst;
+	struct mtk_video_dec_buf *src_buf_info;
 
-	src = v4l2_m2m_next_src_buf(instance->ctx->m2m_ctx);
-	if (!src)
+	src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
+	if (!src_buf_info)
 		return -EINVAL;
 
-	lat_buf->vb2_v4l2_src = src;
+	lat_buf->vb2_v4l2_src = &src_buf_info->m2m_buf.vb;
 
-	dst = &lat_buf->ts_info;
-	v4l2_m2m_buf_copy_metadata(src, dst, true);
+	v4l2_m2m_buf_copy_metadata(lat_buf->vb2_v4l2_src, &lat_buf->ts_info, true);
 	return 0;
 }
 
@@ -1154,7 +1153,7 @@  static int vdec_vp9_slice_setup_lat(struct vdec_vp9_slice_instance *instance,
 	struct vdec_vp9_slice_vsi *vsi = &pfc->vsi;
 	int ret;
 
-	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, lat_buf);
+	ret = vdec_vp9_slice_setup_lat_from_src_buf(instance, bs, lat_buf);
 	if (ret)
 		goto err;