[PATCH 0/8] videobuf2: Replace vb2_find_timestamp() with vb2_find_buffer()

[PATCH 0/8] videobuf2: Replace vb2_find_timestamp() with vb2_find_buffer()

[Ezequiel Garcia]

All users of vb2_find_timestamp() combine it with vb2_get_buffer()
to retrieve a videobuf2 buffer, given a u64 timestamp.

Therefore, this series removes vb2_find_timestamp() and instead
introduces a vb2_find_buffer, which is more suitable, making
videobuf2 API slightly cleaner.

Ezequiel Garcia (8):
  videobuf2: Introduce vb2_find_buffer()
  mediatek: vcodec: Use vb2_find_buffer
  tegra-vde: Use vb2_find_buffer
  vicodec: Use vb2_find_buffer
  hantro: Use vb2_find_buffer
  rkvdec: Use vb2_find_buffer
  cedrus: Use vb2_find_buffer
  videobuf2: Remove vb2_find_timestamp()

 .../media/common/videobuf2/videobuf2-v4l2.c   | 12 ++---
 .../vcodec/vdec/vdec_h264_req_common.c        |  7 ++-
 .../mediatek/vcodec/vdec/vdec_vp8_req_if.c    |  7 ++-
 .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  8 +--
 .../media/platform/nvidia/tegra-vde/h264.c    |  9 ++--
 .../media/test-drivers/vicodec/vicodec-core.c |  8 +--
 drivers/staging/media/hantro/hantro_drv.c     |  6 +--
 .../staging/media/hantro/hantro_g2_vp9_dec.c  | 10 ++--
 drivers/staging/media/rkvdec/rkvdec-h264.c    | 41 ++++++---------
 drivers/staging/media/rkvdec/rkvdec-vp9.c     | 10 ++--
 drivers/staging/media/sunxi/cedrus/cedrus.h   | 13 +----
 .../staging/media/sunxi/cedrus/cedrus_h264.c  | 16 +++---
 .../staging/media/sunxi/cedrus/cedrus_h265.c  | 16 +++---
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 36 ++++++-------
 .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 50 ++++++-------------
 include/media/videobuf2-v4l2.h                | 12 ++---
 16 files changed, 100 insertions(+), 161 deletions(-)

-- 
2.34.3

[PATCH 1/8] videobuf2: Introduce vb2_find_buffer()

[Ezequiel Garcia]

From: Ezequiel Garcia <ezequiel@collabora.com>

All users of vb2_find_timestamp() combine it with vb2_get_buffer()
to retrieve a videobuf2 buffer, given a u64 timestamp.

Introduce an API for this use-case. Users will be converted to the new
API as follow-up commits.

Signed-off-by: Ezequiel Garcia <ezequiel@collabora.com>
---
 include/media/videobuf2-v4l2.h | 18 ++++++++++++++++++
 1 file changed, 18 insertions(+)

diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index d818d9707695..7f9ae5b39b78 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -78,6 +78,24 @@ struct vb2_v4l2_buffer {
 int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
 		       unsigned int start_idx);
 
+/**
+ * vb2_find_buffer() - Find a buffer with given timestamp
+ *
+ * @q:		pointer to &struct vb2_queue with videobuf2 queue.
+ * @timestamp:	the timestamp to find.
+ *
+ * Returns the buffer with the given @timestamp, or NULL if not found.
+ */
+static inline struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q,
+						 u64 timestamp)
+{
+	int index = vb2_find_timestamp(q, timestamp, 0);
+
+	if (index < 0)
+		return NULL;
+	return vb2_get_buffer(q, index);
+}
+
 int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
 
 /**
-- 
2.34.3

[PATCH 2/8] mediatek: vcodec: Use vb2_find_buffer

[Ezequiel Garcia]

Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.

Cc: Tiffany Lin <tiffany.lin@mediatek.com>
Cc: Andrew-CT Chen <andrew-ct.chen@mediatek.com>
Cc: Yunfei Dong <yunfei.dong@mediatek.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 .../platform/mediatek/vcodec/vdec/vdec_h264_req_common.c  | 7 +++----
 .../media/platform/mediatek/vcodec/vdec/vdec_vp8_req_if.c | 7 +++----
 .../platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c   | 8 ++++----
 3 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
index ca628321d272..580ce979e2a3 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
+++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_h264_req_common.c
@@ -51,7 +51,7 @@ void mtk_vdec_h264_fill_dpb_info(struct mtk_vcodec_ctx *ctx,
 	struct vb2_queue *vq;
 	struct vb2_buffer *vb;
 	struct vb2_v4l2_buffer *vb2_v4l2;
-	int index, vb2_index;
+	int index;
 
 	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
 
@@ -62,8 +62,8 @@ void mtk_vdec_h264_fill_dpb_info(struct mtk_vcodec_ctx *ctx,
 			continue;
 		}
 
-		vb2_index = vb2_find_timestamp(vq, dpb->reference_ts, 0);
-		if (vb2_index < 0) {
+		vb = vb2_find_buffer(vq, dpb->reference_ts);
+		if (!vb) {
 			dev_err(&ctx->dev->plat_dev->dev,
 				"Reference invalid: dpb_index(%d) reference_ts(%lld)",
 				index, dpb->reference_ts);
@@ -76,7 +76,6 @@ void mtk_vdec_h264_fill_dpb_info(struct mtk_vcodec_ctx *ctx,
 		else
 			h264_dpb_info[index].reference_flag = 2;
 
-		vb = vq->bufs[vb2_index];
 		vb2_v4l2 = container_of(vb, struct vb2_v4l2_buffer, vb2_buf);
 		h264_dpb_info[index].field = vb2_v4l2->field;
 
diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_req_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_req_if.c
index eef102f3f4f3..e1fe2603e92e 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_req_if.c
+++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp8_req_if.c
@@ -237,7 +237,7 @@ static int vdec_vp8_slice_get_decode_parameters(struct vdec_vp8_slice_inst *inst
 	struct vb2_queue *vq;
 	struct vb2_buffer *vb;
 	u64 referenct_ts;
-	int index, vb2_index;
+	int index;
 
 	frame_header = vdec_vp8_slice_get_ctrl_ptr(inst->ctx, V4L2_CID_STATELESS_VP8_FRAME);
 	if (IS_ERR(frame_header))
@@ -246,8 +246,8 @@ static int vdec_vp8_slice_get_decode_parameters(struct vdec_vp8_slice_inst *inst
 	vq = v4l2_m2m_get_vq(ctx->m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
 	for (index = 0; index < 3; index++) {
 		referenct_ts = vdec_vp8_slice_get_ref_by_ts(frame_header, index);
-		vb2_index = vb2_find_timestamp(vq, referenct_ts, 0);
-		if (vb2_index < 0) {
+		vb = vb2_find_buffer(vq, referenct_ts);
+		if (!vb) {
 			if (!V4L2_VP8_FRAME_IS_KEY_FRAME(frame_header))
 				mtk_vcodec_err(inst, "reference invalid: index(%d) ts(%lld)",
 					       index, referenct_ts);
@@ -256,7 +256,6 @@ static int vdec_vp8_slice_get_decode_parameters(struct vdec_vp8_slice_inst *inst
 		}
 		inst->vsi->vp8_dpb_info[index].reference_flag = 1;
 
-		vb = vq->bufs[vb2_index];
 		inst->vsi->vp8_dpb_info[index].y_dma_addr =
 			vb2_dma_contig_plane_dma_addr(vb, 0);
 		if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2)
diff --git a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
index 81de876d5126..fb1c36a3592d 100644
--- a/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
+++ b/drivers/media/platform/mediatek/vcodec/vdec/vdec_vp9_req_lat_if.c
@@ -1672,7 +1672,6 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst
 	struct vdec_vp9_slice_reference *ref;
 	int plane;
 	int size;
-	int idx;
 	int w;
 	int h;
 	int i;
@@ -1715,15 +1714,16 @@ static int vdec_vp9_slice_setup_core_buffer(struct vdec_vp9_slice_instance *inst
 	 */
 	for (i = 0; i < 3; i++) {
 		ref = &vsi->frame.ref[i];
-		idx = vb2_find_timestamp(vq, pfc->ref_idx[i], 0);
-		if (idx < 0) {
+		vb = vb2_find_buffer(vq, pfc->ref_idx[i]);
+		if (!vb) {
 			ref->frame_width = w;
 			ref->frame_height = h;
 			memset(&vsi->ref[i], 0, sizeof(vsi->ref[i]));
 		} else {
+			int idx = vb->index;
+
 			ref->frame_width = instance->dpb[idx].width;
 			ref->frame_height = instance->dpb[idx].height;
-			vb = vq->bufs[idx];
 			vsi->ref[i].y.dma_addr =
 				vb2_dma_contig_plane_dma_addr(vb, 0);
 			if (plane == 1)
-- 
2.34.3

[PATCH 3/8] tegra-vde: Use vb2_find_buffer

[Ezequiel Garcia]

Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.

Cc: Dmitry Osipenko <digetx@gmail.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/platform/nvidia/tegra-vde/h264.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/drivers/media/platform/nvidia/tegra-vde/h264.c b/drivers/media/platform/nvidia/tegra-vde/h264.c
index 88f81a134ba0..204e474d57f7 100644
--- a/drivers/media/platform/nvidia/tegra-vde/h264.c
+++ b/drivers/media/platform/nvidia/tegra-vde/h264.c
@@ -659,20 +659,19 @@ static struct vb2_buffer *get_ref_buf(struct tegra_ctx *ctx,
 {
 	const struct v4l2_h264_dpb_entry *dpb = ctx->h264.decode_params->dpb;
 	struct vb2_queue *cap_q = &ctx->fh.m2m_ctx->cap_q_ctx.q;
-	int buf_idx = -1;
+	struct vb2_buffer *vb = NULL;
 
 	if (dpb[dpb_idx].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
-		buf_idx = vb2_find_timestamp(cap_q,
-					     dpb[dpb_idx].reference_ts, 0);
+		vb = vb2_find_buffer(cap_q, dpb[dpb_idx].reference_ts);
 
 	/*
 	 * If a DPB entry is unused or invalid, address of current destination
 	 * buffer is returned.
 	 */
-	if (buf_idx < 0)
+	if (!vb)
 		return &dst->vb2_buf;
 
-	return vb2_get_buffer(cap_q, buf_idx);
+	return vb;
 }
 
 static int tegra_vde_validate_vb_size(struct tegra_ctx *ctx,
-- 
2.34.3

[PATCH 4/8] vicodec: Use vb2_find_buffer

[Ezequiel Garcia]

Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.

Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/media/test-drivers/vicodec/vicodec-core.c | 8 ++------
 1 file changed, 2 insertions(+), 6 deletions(-)

diff --git a/drivers/media/test-drivers/vicodec/vicodec-core.c b/drivers/media/test-drivers/vicodec/vicodec-core.c
index be43f7d32df9..1d1bee111732 100644
--- a/drivers/media/test-drivers/vicodec/vicodec-core.c
+++ b/drivers/media/test-drivers/vicodec/vicodec-core.c
@@ -280,17 +280,13 @@ static int device_process(struct vicodec_ctx *ctx,
 		 */
 		if (!(ntohl(ctx->state.header.flags) & V4L2_FWHT_FL_I_FRAME)) {
 			struct vb2_buffer *ref_vb2_buf;
-			int ref_buf_idx;
 			struct vb2_queue *vq_cap =
 				v4l2_m2m_get_vq(ctx->fh.m2m_ctx,
 						V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
-			ref_buf_idx = vb2_find_timestamp(vq_cap,
-							 ctx->state.ref_frame_ts, 0);
-			if (ref_buf_idx < 0)
+			ref_vb2_buf = vb2_find_buffer(vq_cap, ctx->state.ref_frame_ts);
+			if (!ref_vb2_buf)
 				return -EINVAL;
-
-			ref_vb2_buf = vq_cap->bufs[ref_buf_idx];
 			if (ref_vb2_buf->state == VB2_BUF_STATE_ERROR)
 				ret = -EINVAL;
 			ctx->state.ref_frame.buf =
-- 
2.34.3

[PATCH 5/8] hantro: Use vb2_find_buffer

[Ezequiel Garcia]

Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.

Cc: Philipp Zabel <p.zabel@pengutronix.de>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/staging/media/hantro/hantro_drv.c        |  6 ++----
 drivers/staging/media/hantro/hantro_g2_vp9_dec.c | 10 +++++-----
 2 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/drivers/staging/media/hantro/hantro_drv.c b/drivers/staging/media/hantro/hantro_drv.c
index 01d33dcb0467..8cb5cf53e5e7 100644
--- a/drivers/staging/media/hantro/hantro_drv.c
+++ b/drivers/staging/media/hantro/hantro_drv.c
@@ -47,12 +47,10 @@ dma_addr_t hantro_get_ref(struct hantro_ctx *ctx, u64 ts)
 {
 	struct vb2_queue *q = v4l2_m2m_get_dst_vq(ctx->fh.m2m_ctx);
 	struct vb2_buffer *buf;
-	int index;
 
-	index = vb2_find_timestamp(q, ts, 0);
-	if (index < 0)
+	buf = vb2_find_buffer(q, ts);
+	if (!buf)
 		return 0;
-	buf = vb2_get_buffer(q, index);
 	return hantro_get_dec_buf_addr(ctx, buf);
 }
 
diff --git a/drivers/staging/media/hantro/hantro_g2_vp9_dec.c b/drivers/staging/media/hantro/hantro_g2_vp9_dec.c
index 91c21b634fab..6d452c779633 100644
--- a/drivers/staging/media/hantro/hantro_g2_vp9_dec.c
+++ b/drivers/staging/media/hantro/hantro_g2_vp9_dec.c
@@ -111,17 +111,17 @@ get_ref_buf(struct hantro_ctx *ctx, struct vb2_v4l2_buffer *dst, u64 timestamp)
 {
 	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
 	struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
-	int buf_idx;
+	struct vb2_buffer *buf;
 
 	/*
 	 * If a ref is unused or invalid, address of current destination
 	 * buffer is returned.
 	 */
-	buf_idx = vb2_find_timestamp(cap_q, timestamp, 0);
-	if (buf_idx < 0)
-		return vb2_to_hantro_decoded_buf(&dst->vb2_buf);
+	buf = vb2_find_buffer(cap_q, timestamp);
+	if (!buf)
+		buf = &dst->vb2_buf;
 
-	return vb2_to_hantro_decoded_buf(vb2_get_buffer(cap_q, buf_idx));
+	return vb2_to_hantro_decoded_buf(buf);
 }
 
 static void update_dec_buf_info(struct hantro_decoded_buffer *buf,
-- 
2.34.3

[PATCH 6/8] rkvdec: Use vb2_find_buffer

[Ezequiel Garcia]

Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/staging/media/rkvdec/rkvdec-h264.c | 41 ++++++++--------------
 drivers/staging/media/rkvdec/rkvdec-vp9.c  | 10 +++---
 2 files changed, 19 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
index 2992fb87cf72..4af5a831bde0 100644
--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
@@ -109,7 +109,7 @@ struct rkvdec_h264_run {
 	const struct v4l2_ctrl_h264_sps *sps;
 	const struct v4l2_ctrl_h264_pps *pps;
 	const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
-	int ref_buf_idx[V4L2_H264_NUM_DPB_ENTRIES];
+	struct vb2_buffer *ref_buf[V4L2_H264_NUM_DPB_ENTRIES];
 };
 
 struct rkvdec_h264_ctx {
@@ -742,17 +742,16 @@ static void lookup_ref_buf_idx(struct rkvdec_ctx *ctx,
 		struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
 		const struct v4l2_h264_dpb_entry *dpb = run->decode_params->dpb;
 		struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
-		int buf_idx = -1;
+		struct vb2_buffer *buf = NULL;
 
 		if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE) {
-			buf_idx = vb2_find_timestamp(cap_q,
-						     dpb[i].reference_ts, 0);
-			if (buf_idx < 0)
+			buf = vb2_find_buffer(cap_q, dpb[i].reference_ts);
+			if (!buf)
 				pr_debug("No buffer for reference_ts %llu",
 					 dpb[i].reference_ts);
 		}
 
-		run->ref_buf_idx[i] = buf_idx;
+		run->ref_buf[i] = buf;
 	}
 }
 
@@ -805,7 +804,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
 			if (WARN_ON(ref->index >= ARRAY_SIZE(dec_params->dpb)))
 				continue;
 
-			dpb_valid = run->ref_buf_idx[ref->index] >= 0;
+			dpb_valid = run->ref_buf[ref->index] != NULL;
 			bottom = ref->fields == V4L2_H264_BOTTOM_FIELD_REF;
 
 			set_ps_field(hw_rps, DPB_INFO(i, j),
@@ -881,24 +880,6 @@ static const u32 poc_reg_tbl_bottom_field[16] = {
 	RKVDEC_REG_H264_POC_REFER2(1)
 };
 
-static struct vb2_buffer *
-get_ref_buf(struct rkvdec_ctx *ctx, struct rkvdec_h264_run *run,
-	    unsigned int dpb_idx)
-{
-	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
-	struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
-	int buf_idx = run->ref_buf_idx[dpb_idx];
-
-	/*
-	 * If a DPB entry is unused or invalid, address of current destination
-	 * buffer is returned.
-	 */
-	if (buf_idx < 0)
-		return &run->base.bufs.dst->vb2_buf;
-
-	return vb2_get_buffer(cap_q, buf_idx);
-}
-
 static void config_registers(struct rkvdec_ctx *ctx,
 			     struct rkvdec_h264_run *run)
 {
@@ -971,8 +952,14 @@ static void config_registers(struct rkvdec_ctx *ctx,
 
 	/* config ref pic address & poc */
 	for (i = 0; i < ARRAY_SIZE(dec_params->dpb); i++) {
-		struct vb2_buffer *vb_buf = get_ref_buf(ctx, run, i);
-
+		struct vb2_buffer *vb_buf = run->ref_buf[i];
+
+		/*
+		 * If a DPB entry is unused or invalid, address of current destination
+		 * buffer is returned.
+		 */
+		if (!vb_buf)
+			vb_buf = &dst_buf->vb2_buf;
 		refer_addr = vb2_dma_contig_plane_dma_addr(vb_buf, 0);
 
 		if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
diff --git a/drivers/staging/media/rkvdec/rkvdec-vp9.c b/drivers/staging/media/rkvdec/rkvdec-vp9.c
index c2f42e76be10..d8c1c0db15c7 100644
--- a/drivers/staging/media/rkvdec/rkvdec-vp9.c
+++ b/drivers/staging/media/rkvdec/rkvdec-vp9.c
@@ -383,17 +383,17 @@ get_ref_buf(struct rkvdec_ctx *ctx, struct vb2_v4l2_buffer *dst, u64 timestamp)
 {
 	struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
 	struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
-	int buf_idx;
+	struct vb2_buffer *buf;
 
 	/*
 	 * If a ref is unused or invalid, address of current destination
 	 * buffer is returned.
 	 */
-	buf_idx = vb2_find_timestamp(cap_q, timestamp, 0);
-	if (buf_idx < 0)
-		return vb2_to_rkvdec_decoded_buf(&dst->vb2_buf);
+	buf = vb2_find_buffer(cap_q, timestamp);
+	if (!buf)
+		buf = &dst->vb2_buf;
 
-	return vb2_to_rkvdec_decoded_buf(vb2_get_buffer(cap_q, buf_idx));
+	return vb2_to_rkvdec_decoded_buf(buf);
 }
 
 static dma_addr_t get_mv_base_addr(struct rkvdec_decoded_buffer *buf)
-- 
2.34.3

Re: [PATCH 6/8] rkvdec: Use vb2_find_buffer

[Tomasz Figa]

Hi Ezequiel,

On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Use the newly introduced vb2_find_buffer API to get a vb2_buffer
> given a buffer timestamp.
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/staging/media/rkvdec/rkvdec-h264.c | 41 ++++++++--------------
>  drivers/staging/media/rkvdec/rkvdec-vp9.c  | 10 +++---
>  2 files changed, 19 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> index 2992fb87cf72..4af5a831bde0 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> @@ -109,7 +109,7 @@ struct rkvdec_h264_run {
>         const struct v4l2_ctrl_h264_sps *sps;
>         const struct v4l2_ctrl_h264_pps *pps;
>         const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
> -       int ref_buf_idx[V4L2_H264_NUM_DPB_ENTRIES];
> +       struct vb2_buffer *ref_buf[V4L2_H264_NUM_DPB_ENTRIES];

How do we guarantee that those pointers remain valid through the
lifetime of this structure?

Best regards,
Tomasz

>  };
>
>  struct rkvdec_h264_ctx {
> @@ -742,17 +742,16 @@ static void lookup_ref_buf_idx(struct rkvdec_ctx *ctx,
>                 struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
>                 const struct v4l2_h264_dpb_entry *dpb = run->decode_params->dpb;
>                 struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> -               int buf_idx = -1;
> +               struct vb2_buffer *buf = NULL;
>
>                 if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE) {
> -                       buf_idx = vb2_find_timestamp(cap_q,
> -                                                    dpb[i].reference_ts, 0);
> -                       if (buf_idx < 0)
> +                       buf = vb2_find_buffer(cap_q, dpb[i].reference_ts);
> +                       if (!buf)
>                                 pr_debug("No buffer for reference_ts %llu",
>                                          dpb[i].reference_ts);
>                 }
>
> -               run->ref_buf_idx[i] = buf_idx;
> +               run->ref_buf[i] = buf;
>         }
>  }
>
> @@ -805,7 +804,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
>                         if (WARN_ON(ref->index >= ARRAY_SIZE(dec_params->dpb)))
>                                 continue;
>
> -                       dpb_valid = run->ref_buf_idx[ref->index] >= 0;
> +                       dpb_valid = run->ref_buf[ref->index] != NULL;
>                         bottom = ref->fields == V4L2_H264_BOTTOM_FIELD_REF;
>
>                         set_ps_field(hw_rps, DPB_INFO(i, j),
> @@ -881,24 +880,6 @@ static const u32 poc_reg_tbl_bottom_field[16] = {
>         RKVDEC_REG_H264_POC_REFER2(1)
>  };
>
> -static struct vb2_buffer *
> -get_ref_buf(struct rkvdec_ctx *ctx, struct rkvdec_h264_run *run,
> -           unsigned int dpb_idx)
> -{
> -       struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> -       struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> -       int buf_idx = run->ref_buf_idx[dpb_idx];
> -
> -       /*
> -        * If a DPB entry is unused or invalid, address of current destination
> -        * buffer is returned.
> -        */
> -       if (buf_idx < 0)
> -               return &run->base.bufs.dst->vb2_buf;
> -
> -       return vb2_get_buffer(cap_q, buf_idx);
> -}
> -
>  static void config_registers(struct rkvdec_ctx *ctx,
>                              struct rkvdec_h264_run *run)
>  {
> @@ -971,8 +952,14 @@ static void config_registers(struct rkvdec_ctx *ctx,
>
>         /* config ref pic address & poc */
>         for (i = 0; i < ARRAY_SIZE(dec_params->dpb); i++) {
> -               struct vb2_buffer *vb_buf = get_ref_buf(ctx, run, i);
> -
> +               struct vb2_buffer *vb_buf = run->ref_buf[i];
> +
> +               /*
> +                * If a DPB entry is unused or invalid, address of current destination
> +                * buffer is returned.
> +                */
> +               if (!vb_buf)
> +                       vb_buf = &dst_buf->vb2_buf;
>                 refer_addr = vb2_dma_contig_plane_dma_addr(vb_buf, 0);
>
>                 if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
> diff --git a/drivers/staging/media/rkvdec/rkvdec-vp9.c b/drivers/staging/media/rkvdec/rkvdec-vp9.c
> index c2f42e76be10..d8c1c0db15c7 100644
> --- a/drivers/staging/media/rkvdec/rkvdec-vp9.c
> +++ b/drivers/staging/media/rkvdec/rkvdec-vp9.c
> @@ -383,17 +383,17 @@ get_ref_buf(struct rkvdec_ctx *ctx, struct vb2_v4l2_buffer *dst, u64 timestamp)
>  {
>         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
>         struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> -       int buf_idx;
> +       struct vb2_buffer *buf;
>
>         /*
>          * If a ref is unused or invalid, address of current destination
>          * buffer is returned.
>          */
> -       buf_idx = vb2_find_timestamp(cap_q, timestamp, 0);
> -       if (buf_idx < 0)
> -               return vb2_to_rkvdec_decoded_buf(&dst->vb2_buf);
> +       buf = vb2_find_buffer(cap_q, timestamp);
> +       if (!buf)
> +               buf = &dst->vb2_buf;
>
> -       return vb2_to_rkvdec_decoded_buf(vb2_get_buffer(cap_q, buf_idx));
> +       return vb2_to_rkvdec_decoded_buf(buf);
>  }
>
>  static dma_addr_t get_mv_base_addr(struct rkvdec_decoded_buffer *buf)
> --
> 2.34.3
>

Re: [PATCH 6/8] rkvdec: Use vb2_find_buffer

[Ezequiel Garcia]

Hi Tomasz,

On Fri, Jul 08, 2022 at 01:40:53PM +0900, Tomasz Figa wrote:
> Hi Ezequiel,
> 
> On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > Use the newly introduced vb2_find_buffer API to get a vb2_buffer
> > given a buffer timestamp.
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > ---
> >  drivers/staging/media/rkvdec/rkvdec-h264.c | 41 ++++++++--------------
> >  drivers/staging/media/rkvdec/rkvdec-vp9.c  | 10 +++---
> >  2 files changed, 19 insertions(+), 32 deletions(-)
> >
> > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > index 2992fb87cf72..4af5a831bde0 100644
> > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > @@ -109,7 +109,7 @@ struct rkvdec_h264_run {
> >         const struct v4l2_ctrl_h264_sps *sps;
> >         const struct v4l2_ctrl_h264_pps *pps;
> >         const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
> > -       int ref_buf_idx[V4L2_H264_NUM_DPB_ENTRIES];
> > +       struct vb2_buffer *ref_buf[V4L2_H264_NUM_DPB_ENTRIES];
> 
> How do we guarantee that those pointers remain valid through the
> lifetime of this structure?
> 

The rkvdec_h264_run is populated in .device_run, and used to program
the hardware for each decode job.

So these videobuf2 buffer won't outlive a given decode job. The vb2
queue can't be released (so buffers can't be released) while
a job is runnning (i.e. the driver is in a "streaming" state).

We should be good, right?

Thanks for the review,
Ezequiel

> Best regards,
> Tomasz
> 
> >  };
> >
> >  struct rkvdec_h264_ctx {
> > @@ -742,17 +742,16 @@ static void lookup_ref_buf_idx(struct rkvdec_ctx *ctx,
> >                 struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> >                 const struct v4l2_h264_dpb_entry *dpb = run->decode_params->dpb;
> >                 struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> > -               int buf_idx = -1;
> > +               struct vb2_buffer *buf = NULL;
> >
> >                 if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE) {
> > -                       buf_idx = vb2_find_timestamp(cap_q,
> > -                                                    dpb[i].reference_ts, 0);
> > -                       if (buf_idx < 0)
> > +                       buf = vb2_find_buffer(cap_q, dpb[i].reference_ts);
> > +                       if (!buf)
> >                                 pr_debug("No buffer for reference_ts %llu",
> >                                          dpb[i].reference_ts);
> >                 }
> >
> > -               run->ref_buf_idx[i] = buf_idx;
> > +               run->ref_buf[i] = buf;
> >         }
> >  }
> >
> > @@ -805,7 +804,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> >                         if (WARN_ON(ref->index >= ARRAY_SIZE(dec_params->dpb)))
> >                                 continue;
> >
> > -                       dpb_valid = run->ref_buf_idx[ref->index] >= 0;
> > +                       dpb_valid = run->ref_buf[ref->index] != NULL;
> >                         bottom = ref->fields == V4L2_H264_BOTTOM_FIELD_REF;
> >
> >                         set_ps_field(hw_rps, DPB_INFO(i, j),
> > @@ -881,24 +880,6 @@ static const u32 poc_reg_tbl_bottom_field[16] = {
> >         RKVDEC_REG_H264_POC_REFER2(1)
> >  };
> >
> > -static struct vb2_buffer *
> > -get_ref_buf(struct rkvdec_ctx *ctx, struct rkvdec_h264_run *run,
> > -           unsigned int dpb_idx)
> > -{
> > -       struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > -       struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> > -       int buf_idx = run->ref_buf_idx[dpb_idx];
> > -
> > -       /*
> > -        * If a DPB entry is unused or invalid, address of current destination
> > -        * buffer is returned.
> > -        */
> > -       if (buf_idx < 0)
> > -               return &run->base.bufs.dst->vb2_buf;
> > -
> > -       return vb2_get_buffer(cap_q, buf_idx);
> > -}
> > -
> >  static void config_registers(struct rkvdec_ctx *ctx,
> >                              struct rkvdec_h264_run *run)
> >  {
> > @@ -971,8 +952,14 @@ static void config_registers(struct rkvdec_ctx *ctx,
> >
> >         /* config ref pic address & poc */
> >         for (i = 0; i < ARRAY_SIZE(dec_params->dpb); i++) {
> > -               struct vb2_buffer *vb_buf = get_ref_buf(ctx, run, i);
> > -
> > +               struct vb2_buffer *vb_buf = run->ref_buf[i];
> > +
> > +               /*
> > +                * If a DPB entry is unused or invalid, address of current destination
> > +                * buffer is returned.
> > +                */
> > +               if (!vb_buf)
> > +                       vb_buf = &dst_buf->vb2_buf;
> >                 refer_addr = vb2_dma_contig_plane_dma_addr(vb_buf, 0);
> >
> >                 if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
> > diff --git a/drivers/staging/media/rkvdec/rkvdec-vp9.c b/drivers/staging/media/rkvdec/rkvdec-vp9.c
> > index c2f42e76be10..d8c1c0db15c7 100644
> > --- a/drivers/staging/media/rkvdec/rkvdec-vp9.c
> > +++ b/drivers/staging/media/rkvdec/rkvdec-vp9.c
> > @@ -383,17 +383,17 @@ get_ref_buf(struct rkvdec_ctx *ctx, struct vb2_v4l2_buffer *dst, u64 timestamp)
> >  {
> >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> >         struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> > -       int buf_idx;
> > +       struct vb2_buffer *buf;
> >
> >         /*
> >          * If a ref is unused or invalid, address of current destination
> >          * buffer is returned.
> >          */
> > -       buf_idx = vb2_find_timestamp(cap_q, timestamp, 0);
> > -       if (buf_idx < 0)
> > -               return vb2_to_rkvdec_decoded_buf(&dst->vb2_buf);
> > +       buf = vb2_find_buffer(cap_q, timestamp);
> > +       if (!buf)
> > +               buf = &dst->vb2_buf;
> >
> > -       return vb2_to_rkvdec_decoded_buf(vb2_get_buffer(cap_q, buf_idx));
> > +       return vb2_to_rkvdec_decoded_buf(buf);
> >  }
> >
> >  static dma_addr_t get_mv_base_addr(struct rkvdec_decoded_buffer *buf)
> > --
> > 2.34.3
> >

Re: [PATCH 6/8] rkvdec: Use vb2_find_buffer

[Tomasz Figa]

On Fri, Jul 8, 2022 at 8:21 PM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Hi Tomasz,
>
> On Fri, Jul 08, 2022 at 01:40:53PM +0900, Tomasz Figa wrote:
> > Hi Ezequiel,
> >
> > On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
> > <ezequiel@vanguardiasur.com.ar> wrote:
> > >
> > > Use the newly introduced vb2_find_buffer API to get a vb2_buffer
> > > given a buffer timestamp.
> > >
> > > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > > ---
> > >  drivers/staging/media/rkvdec/rkvdec-h264.c | 41 ++++++++--------------
> > >  drivers/staging/media/rkvdec/rkvdec-vp9.c  | 10 +++---
> > >  2 files changed, 19 insertions(+), 32 deletions(-)
> > >
> > > diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > > index 2992fb87cf72..4af5a831bde0 100644
> > > --- a/drivers/staging/media/rkvdec/rkvdec-h264.c
> > > +++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
> > > @@ -109,7 +109,7 @@ struct rkvdec_h264_run {
> > >         const struct v4l2_ctrl_h264_sps *sps;
> > >         const struct v4l2_ctrl_h264_pps *pps;
> > >         const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix;
> > > -       int ref_buf_idx[V4L2_H264_NUM_DPB_ENTRIES];
> > > +       struct vb2_buffer *ref_buf[V4L2_H264_NUM_DPB_ENTRIES];
> >
> > How do we guarantee that those pointers remain valid through the
> > lifetime of this structure?
> >
>
> The rkvdec_h264_run is populated in .device_run, and used to program
> the hardware for each decode job.
>
> So these videobuf2 buffer won't outlive a given decode job. The vb2
> queue can't be released (so buffers can't be released) while
> a job is runnning (i.e. the driver is in a "streaming" state).
>
> We should be good, right?

Makes sense, thanks!

I think the only other comment was about the new helper being
initially inline and then turned back into a regular function, so if
we don't get any other comments in a few more days, feel free to
ignore that one.

Best regards,
Tomasz

>
> Thanks for the review,
> Ezequiel
>
> > Best regards,
> > Tomasz
> >
> > >  };
> > >
> > >  struct rkvdec_h264_ctx {
> > > @@ -742,17 +742,16 @@ static void lookup_ref_buf_idx(struct rkvdec_ctx *ctx,
> > >                 struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > >                 const struct v4l2_h264_dpb_entry *dpb = run->decode_params->dpb;
> > >                 struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> > > -               int buf_idx = -1;
> > > +               struct vb2_buffer *buf = NULL;
> > >
> > >                 if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE) {
> > > -                       buf_idx = vb2_find_timestamp(cap_q,
> > > -                                                    dpb[i].reference_ts, 0);
> > > -                       if (buf_idx < 0)
> > > +                       buf = vb2_find_buffer(cap_q, dpb[i].reference_ts);
> > > +                       if (!buf)
> > >                                 pr_debug("No buffer for reference_ts %llu",
> > >                                          dpb[i].reference_ts);
> > >                 }
> > >
> > > -               run->ref_buf_idx[i] = buf_idx;
> > > +               run->ref_buf[i] = buf;
> > >         }
> > >  }
> > >
> > > @@ -805,7 +804,7 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> > >                         if (WARN_ON(ref->index >= ARRAY_SIZE(dec_params->dpb)))
> > >                                 continue;
> > >
> > > -                       dpb_valid = run->ref_buf_idx[ref->index] >= 0;
> > > +                       dpb_valid = run->ref_buf[ref->index] != NULL;
> > >                         bottom = ref->fields == V4L2_H264_BOTTOM_FIELD_REF;
> > >
> > >                         set_ps_field(hw_rps, DPB_INFO(i, j),
> > > @@ -881,24 +880,6 @@ static const u32 poc_reg_tbl_bottom_field[16] = {
> > >         RKVDEC_REG_H264_POC_REFER2(1)
> > >  };
> > >
> > > -static struct vb2_buffer *
> > > -get_ref_buf(struct rkvdec_ctx *ctx, struct rkvdec_h264_run *run,
> > > -           unsigned int dpb_idx)
> > > -{
> > > -       struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > > -       struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> > > -       int buf_idx = run->ref_buf_idx[dpb_idx];
> > > -
> > > -       /*
> > > -        * If a DPB entry is unused or invalid, address of current destination
> > > -        * buffer is returned.
> > > -        */
> > > -       if (buf_idx < 0)
> > > -               return &run->base.bufs.dst->vb2_buf;
> > > -
> > > -       return vb2_get_buffer(cap_q, buf_idx);
> > > -}
> > > -
> > >  static void config_registers(struct rkvdec_ctx *ctx,
> > >                              struct rkvdec_h264_run *run)
> > >  {
> > > @@ -971,8 +952,14 @@ static void config_registers(struct rkvdec_ctx *ctx,
> > >
> > >         /* config ref pic address & poc */
> > >         for (i = 0; i < ARRAY_SIZE(dec_params->dpb); i++) {
> > > -               struct vb2_buffer *vb_buf = get_ref_buf(ctx, run, i);
> > > -
> > > +               struct vb2_buffer *vb_buf = run->ref_buf[i];
> > > +
> > > +               /*
> > > +                * If a DPB entry is unused or invalid, address of current destination
> > > +                * buffer is returned.
> > > +                */
> > > +               if (!vb_buf)
> > > +                       vb_buf = &dst_buf->vb2_buf;
> > >                 refer_addr = vb2_dma_contig_plane_dma_addr(vb_buf, 0);
> > >
> > >                 if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
> > > diff --git a/drivers/staging/media/rkvdec/rkvdec-vp9.c b/drivers/staging/media/rkvdec/rkvdec-vp9.c
> > > index c2f42e76be10..d8c1c0db15c7 100644
> > > --- a/drivers/staging/media/rkvdec/rkvdec-vp9.c
> > > +++ b/drivers/staging/media/rkvdec/rkvdec-vp9.c
> > > @@ -383,17 +383,17 @@ get_ref_buf(struct rkvdec_ctx *ctx, struct vb2_v4l2_buffer *dst, u64 timestamp)
> > >  {
> > >         struct v4l2_m2m_ctx *m2m_ctx = ctx->fh.m2m_ctx;
> > >         struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> > > -       int buf_idx;
> > > +       struct vb2_buffer *buf;
> > >
> > >         /*
> > >          * If a ref is unused or invalid, address of current destination
> > >          * buffer is returned.
> > >          */
> > > -       buf_idx = vb2_find_timestamp(cap_q, timestamp, 0);
> > > -       if (buf_idx < 0)
> > > -               return vb2_to_rkvdec_decoded_buf(&dst->vb2_buf);
> > > +       buf = vb2_find_buffer(cap_q, timestamp);
> > > +       if (!buf)
> > > +               buf = &dst->vb2_buf;
> > >
> > > -       return vb2_to_rkvdec_decoded_buf(vb2_get_buffer(cap_q, buf_idx));
> > > +       return vb2_to_rkvdec_decoded_buf(buf);
> > >  }
> > >
> > >  static dma_addr_t get_mv_base_addr(struct rkvdec_decoded_buffer *buf)
> > > --
> > > 2.34.3
> > >

[PATCH 7/8] cedrus: Use vb2_find_buffer

[Ezequiel Garcia]

Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.

Cc: Maxime Ripard <mripard@kernel.org>
Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 drivers/staging/media/sunxi/cedrus/cedrus.h   | 13 +----
 .../staging/media/sunxi/cedrus/cedrus_h264.c  | 16 +++---
 .../staging/media/sunxi/cedrus/cedrus_h265.c  | 16 +++---
 .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 36 ++++++-------
 .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 50 ++++++-------------
 5 files changed, 49 insertions(+), 82 deletions(-)

diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h b/drivers/staging/media/sunxi/cedrus/cedrus.h
index 3bc094eb497f..a9908cc5c848 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus.h
+++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
@@ -233,18 +233,9 @@ static inline dma_addr_t cedrus_buf_addr(struct vb2_buffer *buf,
 }
 
 static inline dma_addr_t cedrus_dst_buf_addr(struct cedrus_ctx *ctx,
-					     int index, unsigned int plane)
+					     struct vb2_buffer *buf,
+					     unsigned int plane)
 {
-	struct vb2_buffer *buf = NULL;
-	struct vb2_queue *vq;
-
-	if (index < 0)
-		return 0;
-
-	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
-	if (vq)
-		buf = vb2_get_buffer(vq, index);
-
 	return buf ? cedrus_buf_addr(buf, &ctx->dst_fmt, plane) : 0;
 }
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
index d8fb93035470..0559efeac125 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
@@ -111,16 +111,16 @@ static void cedrus_write_frame_list(struct cedrus_ctx *ctx,
 	for (i = 0; i < ARRAY_SIZE(decode->dpb); i++) {
 		const struct v4l2_h264_dpb_entry *dpb = &decode->dpb[i];
 		struct cedrus_buffer *cedrus_buf;
-		int buf_idx;
+		struct vb2_buffer *buf;
 
 		if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_VALID))
 			continue;
 
-		buf_idx = vb2_find_timestamp(cap_q, dpb->reference_ts, 0);
-		if (buf_idx < 0)
+		buf = vb2_find_buffer(cap_q, dpb->reference_ts);
+		if (!buf)
 			continue;
 
-		cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
+		cedrus_buf = vb2_to_cedrus_buffer(buf);
 		position = cedrus_buf->codec.h264.position;
 		used_dpbs |= BIT(position);
 
@@ -186,7 +186,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
 		const struct v4l2_h264_dpb_entry *dpb;
 		const struct cedrus_buffer *cedrus_buf;
 		unsigned int position;
-		int buf_idx;
+		struct vb2_buffer *buf;
 		u8 dpb_idx;
 
 		dpb_idx = ref_list[i].index;
@@ -195,11 +195,11 @@ static void _cedrus_write_ref_list(struct cedrus_ctx *ctx,
 		if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
 			continue;
 
-		buf_idx = vb2_find_timestamp(cap_q, dpb->reference_ts, 0);
-		if (buf_idx < 0)
+		buf = vb2_find_buffer(cap_q, dpb->reference_ts);
+		if (!buf)
 			continue;
 
-		cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
+		cedrus_buf = vb2_to_cedrus_buffer(buf);
 		position = cedrus_buf->codec.h264.position;
 
 		sram_array[i] |= position << 1;
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
index 44f385be9f6c..60cc13e4d0a9 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
@@ -102,14 +102,14 @@ static void cedrus_h265_frame_info_write_single(struct cedrus_ctx *ctx,
 						unsigned int index,
 						bool field_pic,
 						u32 pic_order_cnt[],
-						int buffer_index)
+						struct vb2_buffer *buf)
 {
 	struct cedrus_dev *dev = ctx->dev;
-	dma_addr_t dst_luma_addr = cedrus_dst_buf_addr(ctx, buffer_index, 0);
-	dma_addr_t dst_chroma_addr = cedrus_dst_buf_addr(ctx, buffer_index, 1);
+	dma_addr_t dst_luma_addr = cedrus_dst_buf_addr(ctx, buf, 0);
+	dma_addr_t dst_chroma_addr = cedrus_dst_buf_addr(ctx, buf, 1);
 	dma_addr_t mv_col_buf_addr[2] = {
-		cedrus_h265_frame_info_mv_col_buf_addr(ctx, buffer_index, 0),
-		cedrus_h265_frame_info_mv_col_buf_addr(ctx, buffer_index,
+		cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf->index, 0),
+		cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf->index,
 						       field_pic ? 1 : 0)
 	};
 	u32 offset = VE_DEC_H265_SRAM_OFFSET_FRAME_INFO +
@@ -141,7 +141,7 @@ static void cedrus_h265_frame_info_write_dpb(struct cedrus_ctx *ctx,
 	unsigned int i;
 
 	for (i = 0; i < num_active_dpb_entries; i++) {
-		int buffer_index = vb2_find_timestamp(vq, dpb[i].timestamp, 0);
+		struct vb2_buffer *buf = vb2_find_buffer(vq, dpb[i].timestamp);
 		u32 pic_order_cnt[2] = {
 			dpb[i].pic_order_cnt[0],
 			dpb[i].pic_order_cnt[1]
@@ -149,7 +149,7 @@ static void cedrus_h265_frame_info_write_dpb(struct cedrus_ctx *ctx,
 
 		cedrus_h265_frame_info_write_single(ctx, i, dpb[i].field_pic,
 						    pic_order_cnt,
-						    buffer_index);
+						    buf);
 	}
 }
 
@@ -616,7 +616,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
 	cedrus_h265_frame_info_write_single(ctx, output_pic_list_index,
 					    slice_params->pic_struct != 0,
 					    pic_order_cnt,
-					    run->dst->vb2_buf.index);
+					    &run->dst->vb2_buf);
 
 	cedrus_write(dev, VE_DEC_H265_OUTPUT_FRAME_IDX, output_pic_list_index);
 
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
index 5dad2f296c6d..ab9cfa001a49 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
@@ -13,6 +13,16 @@
 #include "cedrus_hw.h"
 #include "cedrus_regs.h"
 
+static void write_ref_buf_addr(struct cedrus_ctx *ctx, struct vb2_queue *q,
+			       u64 timestamp, u32 luma_reg, u32 chroma_reg)
+{
+	struct cedrus_dev *dev = ctx->dev;
+	struct vb2_buffer *buf = vb2_find_buffer(q, timestamp);
+
+	cedrus_write(dev, luma_reg, cedrus_dst_buf_addr(ctx, buf, 0));
+	cedrus_write(dev, chroma_reg, cedrus_dst_buf_addr(ctx, buf, 1));
+}
+
 static enum cedrus_irq_status cedrus_mpeg2_irq_status(struct cedrus_ctx *ctx)
 {
 	struct cedrus_dev *dev = ctx->dev;
@@ -54,13 +64,9 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	const struct v4l2_ctrl_mpeg2_picture *pic;
 	const struct v4l2_ctrl_mpeg2_quantisation *quantisation;
 	dma_addr_t src_buf_addr, dst_luma_addr, dst_chroma_addr;
-	dma_addr_t fwd_luma_addr, fwd_chroma_addr;
-	dma_addr_t bwd_luma_addr, bwd_chroma_addr;
 	struct cedrus_dev *dev = ctx->dev;
 	struct vb2_queue *vq;
 	const u8 *matrix;
-	int forward_idx;
-	int backward_idx;
 	unsigned int i;
 	u32 reg;
 
@@ -123,27 +129,17 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx, struct cedrus_run *run)
 	cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
 
 	/* Forward and backward prediction reference buffers. */
-
 	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE);
 
-	forward_idx = vb2_find_timestamp(vq, pic->forward_ref_ts, 0);
-	fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
-	fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
-
-	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
-	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, fwd_chroma_addr);
-
-	backward_idx = vb2_find_timestamp(vq, pic->backward_ref_ts, 0);
-	bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0);
-	bwd_chroma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 1);
-
-	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR, bwd_luma_addr);
-	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR, bwd_chroma_addr);
+	write_ref_buf_addr(ctx, vq, pic->forward_ref_ts,
+			   VE_DEC_MPEG_FWD_REF_LUMA_ADDR, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR);
+	write_ref_buf_addr(ctx, vq, pic->backward_ref_ts,
+			   VE_DEC_MPEG_BWD_REF_LUMA_ADDR, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR);
 
 	/* Destination luma and chroma buffers. */
 
-	dst_luma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 0);
-	dst_chroma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 1);
+	dst_luma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 0);
+	dst_chroma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 1);
 
 	cedrus_write(dev, VE_DEC_MPEG_REC_LUMA, dst_luma_addr);
 	cedrus_write(dev, VE_DEC_MPEG_REC_CHROMA, dst_chroma_addr);
diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
index f4016684b32d..a253f6b92135 100644
--- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
+++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
@@ -516,6 +516,16 @@ static void process_ref_frame_info(struct cedrus_dev *dev)
 	read_bits(dev, 1, VP8_PROB_HALF);
 }
 
+static void write_ref_buf_addr(struct cedrus_ctx *ctx, struct vb2_queue *q,
+			       u64 timestamp, u32 luma_reg, u32 chroma_reg)
+{
+	struct cedrus_dev *dev = ctx->dev;
+	struct vb2_buffer *buf = vb2_find_buffer(q, timestamp);
+
+	cedrus_write(dev, luma_reg, cedrus_dst_buf_addr(ctx, buf, 0));
+	cedrus_write(dev, chroma_reg, cedrus_dst_buf_addr(ctx, buf, 1));
+}
+
 static void cedrus_irq_clear(struct cedrus_dev *dev)
 {
 	cedrus_write(dev, VE_H264_STATUS,
@@ -661,7 +671,6 @@ static void cedrus_vp8_setup(struct cedrus_ctx *ctx,
 	dma_addr_t luma_addr, chroma_addr;
 	dma_addr_t src_buf_addr;
 	int header_size;
-	int qindex;
 	u32 reg;
 
 	cedrus_engine_enable(ctx, CEDRUS_CODEC_VP8);
@@ -805,43 +814,14 @@ static void cedrus_vp8_setup(struct cedrus_ctx *ctx,
 	reg |= VE_VP8_LF_DELTA0(slice->lf.mb_mode_delta[0]);
 	cedrus_write(dev, VE_VP8_MODE_LF_DELTA, reg);
 
-	luma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 0);
-	chroma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 1);
+	luma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 0);
+	chroma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 1);
 	cedrus_write(dev, VE_VP8_REC_LUMA, luma_addr);
 	cedrus_write(dev, VE_VP8_REC_CHROMA, chroma_addr);
 
-	qindex = vb2_find_timestamp(cap_q, slice->last_frame_ts, 0);
-	if (qindex >= 0) {
-		luma_addr = cedrus_dst_buf_addr(ctx, qindex, 0);
-		chroma_addr = cedrus_dst_buf_addr(ctx, qindex, 1);
-		cedrus_write(dev, VE_VP8_FWD_LUMA, luma_addr);
-		cedrus_write(dev, VE_VP8_FWD_CHROMA, chroma_addr);
-	} else {
-		cedrus_write(dev, VE_VP8_FWD_LUMA, 0);
-		cedrus_write(dev, VE_VP8_FWD_CHROMA, 0);
-	}
-
-	qindex = vb2_find_timestamp(cap_q, slice->golden_frame_ts, 0);
-	if (qindex >= 0) {
-		luma_addr = cedrus_dst_buf_addr(ctx, qindex, 0);
-		chroma_addr = cedrus_dst_buf_addr(ctx, qindex, 1);
-		cedrus_write(dev, VE_VP8_BWD_LUMA, luma_addr);
-		cedrus_write(dev, VE_VP8_BWD_CHROMA, chroma_addr);
-	} else {
-		cedrus_write(dev, VE_VP8_BWD_LUMA, 0);
-		cedrus_write(dev, VE_VP8_BWD_CHROMA, 0);
-	}
-
-	qindex = vb2_find_timestamp(cap_q, slice->alt_frame_ts, 0);
-	if (qindex >= 0) {
-		luma_addr = cedrus_dst_buf_addr(ctx, qindex, 0);
-		chroma_addr = cedrus_dst_buf_addr(ctx, qindex, 1);
-		cedrus_write(dev, VE_VP8_ALT_LUMA, luma_addr);
-		cedrus_write(dev, VE_VP8_ALT_CHROMA, chroma_addr);
-	} else {
-		cedrus_write(dev, VE_VP8_ALT_LUMA, 0);
-		cedrus_write(dev, VE_VP8_ALT_CHROMA, 0);
-	}
+	write_ref_buf_addr(ctx, cap_q, slice->last_frame_ts, VE_VP8_FWD_LUMA, VE_VP8_FWD_CHROMA);
+	write_ref_buf_addr(ctx, cap_q, slice->golden_frame_ts, VE_VP8_BWD_LUMA, VE_VP8_BWD_CHROMA);
+	write_ref_buf_addr(ctx, cap_q, slice->alt_frame_ts, VE_VP8_ALT_LUMA, VE_VP8_ALT_CHROMA);
 
 	cedrus_write(dev, VE_H264_CTRL, VE_H264_CTRL_VP8 |
 		     VE_H264_CTRL_DECODE_ERR_INT |
-- 
2.34.3

Re: [PATCH 7/8] cedrus: Use vb2_find_buffer

[Jernej Škrabec]

Hi Ezequiel,

Dne sreda, 06. julij 2022 ob 20:26:56 CEST je Ezequiel Garcia napisal(a):
> Use the newly introduced vb2_find_buffer API to get a vb2_buffer
> given a buffer timestamp.
> 
> Cc: Maxime Ripard <mripard@kernel.org>
> Cc: Paul Kocialkowski <paul.kocialkowski@bootlin.com>
> Cc: Jernej Skrabec <jernej.skrabec@gmail.com>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  drivers/staging/media/sunxi/cedrus/cedrus.h   | 13 +----
>  .../staging/media/sunxi/cedrus/cedrus_h264.c  | 16 +++---
>  .../staging/media/sunxi/cedrus/cedrus_h265.c  | 16 +++---
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 36 ++++++-------
>  .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 50 ++++++-------------
>  5 files changed, 49 insertions(+), 82 deletions(-)
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus.h
> b/drivers/staging/media/sunxi/cedrus/cedrus.h index
> 3bc094eb497f..a9908cc5c848 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus.h
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus.h
> @@ -233,18 +233,9 @@ static inline dma_addr_t cedrus_buf_addr(struct
> vb2_buffer *buf, }
> 
>  static inline dma_addr_t cedrus_dst_buf_addr(struct cedrus_ctx *ctx,
> -					     int index, 
unsigned int plane)
> +					     struct vb2_buffer 
*buf,
> +					     unsigned int 
plane)
>  {
> -	struct vb2_buffer *buf = NULL;
> -	struct vb2_queue *vq;
> -
> -	if (index < 0)
> -		return 0;
> -
> -	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, 
V4L2_BUF_TYPE_VIDEO_CAPTURE);
> -	if (vq)
> -		buf = vb2_get_buffer(vq, index);
> -
>  	return buf ? cedrus_buf_addr(buf, &ctx->dst_fmt, plane) : 0;
>  }
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c index
> d8fb93035470..0559efeac125 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h264.c
> @@ -111,16 +111,16 @@ static void cedrus_write_frame_list(struct cedrus_ctx
> *ctx, for (i = 0; i < ARRAY_SIZE(decode->dpb); i++) {
>  		const struct v4l2_h264_dpb_entry *dpb = &decode-
>dpb[i];
>  		struct cedrus_buffer *cedrus_buf;
> -		int buf_idx;
> +		struct vb2_buffer *buf;
> 
>  		if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_VALID))
>  			continue;
> 
> -		buf_idx = vb2_find_timestamp(cap_q, dpb->reference_ts, 
0);
> -		if (buf_idx < 0)
> +		buf = vb2_find_buffer(cap_q, dpb->reference_ts);
> +		if (!buf)
>  			continue;
> 
> -		cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
> +		cedrus_buf = vb2_to_cedrus_buffer(buf);
>  		position = cedrus_buf->codec.h264.position;
>  		used_dpbs |= BIT(position);
> 
> @@ -186,7 +186,7 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> *ctx, const struct v4l2_h264_dpb_entry *dpb;
>  		const struct cedrus_buffer *cedrus_buf;
>  		unsigned int position;
> -		int buf_idx;
> +		struct vb2_buffer *buf;
>  		u8 dpb_idx;
> 
>  		dpb_idx = ref_list[i].index;
> @@ -195,11 +195,11 @@ static void _cedrus_write_ref_list(struct cedrus_ctx
> *ctx, if (!(dpb->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
>  			continue;
> 
> -		buf_idx = vb2_find_timestamp(cap_q, dpb->reference_ts, 
0);
> -		if (buf_idx < 0)
> +		buf = vb2_find_buffer(cap_q, dpb->reference_ts);
> +		if (!buf)
>  			continue;
> 
> -		cedrus_buf = vb2_to_cedrus_buffer(cap_q->bufs[buf_idx]);
> +		cedrus_buf = vb2_to_cedrus_buffer(buf);
>  		position = cedrus_buf->codec.h264.position;
> 
>  		sram_array[i] |= position << 1;
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c index
> 44f385be9f6c..60cc13e4d0a9 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_h265.c
> @@ -102,14 +102,14 @@ static void cedrus_h265_frame_info_write_single(struct
> cedrus_ctx *ctx, unsigned int index,
>  						bool 
field_pic,
>  						u32 
pic_order_cnt[],
> -						int 
buffer_index)
> +						struct 
vb2_buffer *buf)
>  {
>  	struct cedrus_dev *dev = ctx->dev;
> -	dma_addr_t dst_luma_addr = cedrus_dst_buf_addr(ctx, buffer_index, 
0);
> -	dma_addr_t dst_chroma_addr = cedrus_dst_buf_addr(ctx, buffer_index, 
1);
> +	dma_addr_t dst_luma_addr = cedrus_dst_buf_addr(ctx, buf, 0);
> +	dma_addr_t dst_chroma_addr = cedrus_dst_buf_addr(ctx, buf, 1);
>  	dma_addr_t mv_col_buf_addr[2] = {
> -		cedrus_h265_frame_info_mv_col_buf_addr(ctx, 
buffer_index, 0),
> -		cedrus_h265_frame_info_mv_col_buf_addr(ctx, 
buffer_index,
> +		cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf->index, 
0),
> +		cedrus_h265_frame_info_mv_col_buf_addr(ctx, buf->index,
>  						       
field_pic ? 1 : 0)
>  	};
>  	u32 offset = VE_DEC_H265_SRAM_OFFSET_FRAME_INFO +
> @@ -141,7 +141,7 @@ static void cedrus_h265_frame_info_write_dpb(struct
> cedrus_ctx *ctx, unsigned int i;
> 
>  	for (i = 0; i < num_active_dpb_entries; i++) {
> -		int buffer_index = vb2_find_timestamp(vq, 
dpb[i].timestamp, 0);
> +		struct vb2_buffer *buf = vb2_find_buffer(vq, 
dpb[i].timestamp);
>  		u32 pic_order_cnt[2] = {
>  			dpb[i].pic_order_cnt[0],
>  			dpb[i].pic_order_cnt[1]
> @@ -149,7 +149,7 @@ static void cedrus_h265_frame_info_write_dpb(struct
> cedrus_ctx *ctx,
> 
>  		cedrus_h265_frame_info_write_single(ctx, i, 
dpb[i].field_pic,
>  						    
pic_order_cnt,
> -						    
buffer_index);
> +						    buf);
>  	}
>  }
> 
> @@ -616,7 +616,7 @@ static void cedrus_h265_setup(struct cedrus_ctx *ctx,
>  	cedrus_h265_frame_info_write_single(ctx, output_pic_list_index,
>  					    slice_params-
>pic_struct != 0,
>  					    pic_order_cnt,
> -					    run->dst-
>vb2_buf.index);
> +					    &run->dst-
>vb2_buf);
> 
>  	cedrus_write(dev, VE_DEC_H265_OUTPUT_FRAME_IDX, 
output_pic_list_index);
> 
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c index
> 5dad2f296c6d..ab9cfa001a49 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_mpeg2.c
> @@ -13,6 +13,16 @@
>  #include "cedrus_hw.h"
>  #include "cedrus_regs.h"
> 
> +static void write_ref_buf_addr(struct cedrus_ctx *ctx, struct vb2_queue *q,
> +			       u64 timestamp, u32 luma_reg, u32 
chroma_reg)
> +{
> +	struct cedrus_dev *dev = ctx->dev;
> +	struct vb2_buffer *buf = vb2_find_buffer(q, timestamp);
> +
> +	cedrus_write(dev, luma_reg, cedrus_dst_buf_addr(ctx, buf, 0));
> +	cedrus_write(dev, chroma_reg, cedrus_dst_buf_addr(ctx, buf, 1));
> +}

This function name doesn't conform to style used throughout whole driver. It 
should be prefixed by cedrus_mpeg2_. However, since same function is introduced 
in VP8 code, it should be prefixed with cedrus_ and moved to cedrus.h, so it 
can be used with both drivers.

Other than that, changes look correct.

Best regards,
Jernej

> +
>  static enum cedrus_irq_status cedrus_mpeg2_irq_status(struct cedrus_ctx
> *ctx) {
>  	struct cedrus_dev *dev = ctx->dev;
> @@ -54,13 +64,9 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx,
> struct cedrus_run *run) const struct v4l2_ctrl_mpeg2_picture *pic;
>  	const struct v4l2_ctrl_mpeg2_quantisation *quantisation;
>  	dma_addr_t src_buf_addr, dst_luma_addr, dst_chroma_addr;
> -	dma_addr_t fwd_luma_addr, fwd_chroma_addr;
> -	dma_addr_t bwd_luma_addr, bwd_chroma_addr;
>  	struct cedrus_dev *dev = ctx->dev;
>  	struct vb2_queue *vq;
>  	const u8 *matrix;
> -	int forward_idx;
> -	int backward_idx;
>  	unsigned int i;
>  	u32 reg;
> 
> @@ -123,27 +129,17 @@ static void cedrus_mpeg2_setup(struct cedrus_ctx *ctx,
> struct cedrus_run *run) cedrus_write(dev, VE_DEC_MPEG_PICBOUNDSIZE, reg);
> 
>  	/* Forward and backward prediction reference buffers. */
> -
>  	vq = v4l2_m2m_get_vq(ctx->fh.m2m_ctx, 
V4L2_BUF_TYPE_VIDEO_CAPTURE);
> 
> -	forward_idx = vb2_find_timestamp(vq, pic->forward_ref_ts, 0);
> -	fwd_luma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 0);
> -	fwd_chroma_addr = cedrus_dst_buf_addr(ctx, forward_idx, 1);
> -
> -	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_LUMA_ADDR, fwd_luma_addr);
> -	cedrus_write(dev, VE_DEC_MPEG_FWD_REF_CHROMA_ADDR, 
fwd_chroma_addr);
> -
> -	backward_idx = vb2_find_timestamp(vq, pic->backward_ref_ts, 0);
> -	bwd_luma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 0);
> -	bwd_chroma_addr = cedrus_dst_buf_addr(ctx, backward_idx, 1);
> -
> -	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_LUMA_ADDR, bwd_luma_addr);
> -	cedrus_write(dev, VE_DEC_MPEG_BWD_REF_CHROMA_ADDR, 
bwd_chroma_addr);
> +	write_ref_buf_addr(ctx, vq, pic->forward_ref_ts,
> +			   VE_DEC_MPEG_FWD_REF_LUMA_ADDR, 
VE_DEC_MPEG_FWD_REF_CHROMA_ADDR);
> +	write_ref_buf_addr(ctx, vq, pic->backward_ref_ts,
> +			   VE_DEC_MPEG_BWD_REF_LUMA_ADDR, 
VE_DEC_MPEG_BWD_REF_CHROMA_ADDR);
> 
>  	/* Destination luma and chroma buffers. */
> 
> -	dst_luma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 
0);
> -	dst_chroma_addr = cedrus_dst_buf_addr(ctx, run->dst-
>vb2_buf.index, 1);
> +	dst_luma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 0);
> +	dst_chroma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 1);
> 
>  	cedrus_write(dev, VE_DEC_MPEG_REC_LUMA, dst_luma_addr);
>  	cedrus_write(dev, VE_DEC_MPEG_REC_CHROMA, dst_chroma_addr);
> diff --git a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c index
> f4016684b32d..a253f6b92135 100644
> --- a/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> +++ b/drivers/staging/media/sunxi/cedrus/cedrus_vp8.c
> @@ -516,6 +516,16 @@ static void process_ref_frame_info(struct cedrus_dev
> *dev) read_bits(dev, 1, VP8_PROB_HALF);
>  }
> 
> +static void write_ref_buf_addr(struct cedrus_ctx *ctx, struct vb2_queue *q,
> +			       u64 timestamp, u32 luma_reg, u32 
chroma_reg)
> +{
> +	struct cedrus_dev *dev = ctx->dev;
> +	struct vb2_buffer *buf = vb2_find_buffer(q, timestamp);
> +
> +	cedrus_write(dev, luma_reg, cedrus_dst_buf_addr(ctx, buf, 0));
> +	cedrus_write(dev, chroma_reg, cedrus_dst_buf_addr(ctx, buf, 1));
> +}
> +
>  static void cedrus_irq_clear(struct cedrus_dev *dev)
>  {
>  	cedrus_write(dev, VE_H264_STATUS,
> @@ -661,7 +671,6 @@ static void cedrus_vp8_setup(struct cedrus_ctx *ctx,
>  	dma_addr_t luma_addr, chroma_addr;
>  	dma_addr_t src_buf_addr;
>  	int header_size;
> -	int qindex;
>  	u32 reg;
> 
>  	cedrus_engine_enable(ctx, CEDRUS_CODEC_VP8);
> @@ -805,43 +814,14 @@ static void cedrus_vp8_setup(struct cedrus_ctx *ctx,
>  	reg |= VE_VP8_LF_DELTA0(slice->lf.mb_mode_delta[0]);
>  	cedrus_write(dev, VE_VP8_MODE_LF_DELTA, reg);
> 
> -	luma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 0);
> -	chroma_addr = cedrus_dst_buf_addr(ctx, run->dst->vb2_buf.index, 
1);
> +	luma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 0);
> +	chroma_addr = cedrus_dst_buf_addr(ctx, &run->dst->vb2_buf, 1);
>  	cedrus_write(dev, VE_VP8_REC_LUMA, luma_addr);
>  	cedrus_write(dev, VE_VP8_REC_CHROMA, chroma_addr);
> 
> -	qindex = vb2_find_timestamp(cap_q, slice->last_frame_ts, 0);
> -	if (qindex >= 0) {
> -		luma_addr = cedrus_dst_buf_addr(ctx, qindex, 0);
> -		chroma_addr = cedrus_dst_buf_addr(ctx, qindex, 1);
> -		cedrus_write(dev, VE_VP8_FWD_LUMA, luma_addr);
> -		cedrus_write(dev, VE_VP8_FWD_CHROMA, chroma_addr);
> -	} else {
> -		cedrus_write(dev, VE_VP8_FWD_LUMA, 0);
> -		cedrus_write(dev, VE_VP8_FWD_CHROMA, 0);
> -	}
> -
> -	qindex = vb2_find_timestamp(cap_q, slice->golden_frame_ts, 0);
> -	if (qindex >= 0) {
> -		luma_addr = cedrus_dst_buf_addr(ctx, qindex, 0);
> -		chroma_addr = cedrus_dst_buf_addr(ctx, qindex, 1);
> -		cedrus_write(dev, VE_VP8_BWD_LUMA, luma_addr);
> -		cedrus_write(dev, VE_VP8_BWD_CHROMA, chroma_addr);
> -	} else {
> -		cedrus_write(dev, VE_VP8_BWD_LUMA, 0);
> -		cedrus_write(dev, VE_VP8_BWD_CHROMA, 0);
> -	}
> -
> -	qindex = vb2_find_timestamp(cap_q, slice->alt_frame_ts, 0);
> -	if (qindex >= 0) {
> -		luma_addr = cedrus_dst_buf_addr(ctx, qindex, 0);
> -		chroma_addr = cedrus_dst_buf_addr(ctx, qindex, 1);
> -		cedrus_write(dev, VE_VP8_ALT_LUMA, luma_addr);
> -		cedrus_write(dev, VE_VP8_ALT_CHROMA, chroma_addr);
> -	} else {
> -		cedrus_write(dev, VE_VP8_ALT_LUMA, 0);
> -		cedrus_write(dev, VE_VP8_ALT_CHROMA, 0);
> -	}
> +	write_ref_buf_addr(ctx, cap_q, slice->last_frame_ts, 
VE_VP8_FWD_LUMA,
> VE_VP8_FWD_CHROMA); +	write_ref_buf_addr(ctx, cap_q,
> slice->golden_frame_ts, VE_VP8_BWD_LUMA, VE_VP8_BWD_CHROMA);
> +	write_ref_buf_addr(ctx, cap_q, slice->alt_frame_ts, 
VE_VP8_ALT_LUMA,
> VE_VP8_ALT_CHROMA);
> 
>  	cedrus_write(dev, VE_H264_CTRL, VE_H264_CTRL_VP8 |
>  		     VE_H264_CTRL_DECODE_ERR_INT |

[PATCH 8/8] videobuf2: Remove vb2_find_timestamp()

[Ezequiel Garcia]

Now that we've transitioned all users to vb2_find_buffer API,
remove the unused vb2_find_timestamp().

Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
---
 .../media/common/videobuf2/videobuf2-v4l2.c   | 12 ++++-----
 include/media/videobuf2-v4l2.h                | 26 +------------------
 2 files changed, 7 insertions(+), 31 deletions(-)

diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
index 075d24ebf44c..a9696442dfba 100644
--- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
+++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
@@ -625,18 +625,18 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
 	.copy_timestamp		= __copy_timestamp,
 };
 
-int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
-		       unsigned int start_idx)
+struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp)
 {
 	unsigned int i;
 
-	for (i = start_idx; i < q->num_buffers; i++)
+	for (i = 0; i < q->num_buffers; i++)
 		if (q->bufs[i]->copied_timestamp &&
 		    q->bufs[i]->timestamp == timestamp)
-			return i;
-	return -1;
+			return vb2_get_buffer(q, i);
+
+	return NULL;
 }
-EXPORT_SYMBOL_GPL(vb2_find_timestamp);
+EXPORT_SYMBOL_GPL(vb2_find_buffer);
 
 /*
  * vb2_querybuf() - query video buffer information
diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
index 7f9ae5b39b78..5a845887850b 100644
--- a/include/media/videobuf2-v4l2.h
+++ b/include/media/videobuf2-v4l2.h
@@ -62,22 +62,6 @@ struct vb2_v4l2_buffer {
 #define to_vb2_v4l2_buffer(vb) \
 	container_of(vb, struct vb2_v4l2_buffer, vb2_buf)
 
-/**
- * vb2_find_timestamp() - Find buffer with given timestamp in the queue
- *
- * @q:		pointer to &struct vb2_queue with videobuf2 queue.
- * @timestamp:	the timestamp to find.
- * @start_idx:	the start index (usually 0) in the buffer array to start
- *		searching from. Note that there may be multiple buffers
- *		with the same timestamp value, so you can restart the search
- *		by setting @start_idx to the previously found index + 1.
- *
- * Returns the buffer index of the buffer with the given @timestamp, or
- * -1 if no buffer with @timestamp was found.
- */
-int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
-		       unsigned int start_idx);
-
 /**
  * vb2_find_buffer() - Find a buffer with given timestamp
  *
@@ -86,15 +70,7 @@ int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
  *
  * Returns the buffer with the given @timestamp, or NULL if not found.
  */
-static inline struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q,
-						 u64 timestamp)
-{
-	int index = vb2_find_timestamp(q, timestamp, 0);
-
-	if (index < 0)
-		return NULL;
-	return vb2_get_buffer(q, index);
-}
+struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp);
 
 int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
 
-- 
2.34.3

Re: [PATCH 8/8] videobuf2: Remove vb2_find_timestamp()

[Tomasz Figa]

Hi Ezequiel,

On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> Now that we've transitioned all users to vb2_find_buffer API,
> remove the unused vb2_find_timestamp().
>
> Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> ---
>  .../media/common/videobuf2/videobuf2-v4l2.c   | 12 ++++-----
>  include/media/videobuf2-v4l2.h                | 26 +------------------
>  2 files changed, 7 insertions(+), 31 deletions(-)
>
> diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> index 075d24ebf44c..a9696442dfba 100644
> --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> @@ -625,18 +625,18 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
>         .copy_timestamp         = __copy_timestamp,
>  };
>
> -int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> -                      unsigned int start_idx)
> +struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp)
>  {
>         unsigned int i;
>
> -       for (i = start_idx; i < q->num_buffers; i++)
> +       for (i = 0; i < q->num_buffers; i++)
>                 if (q->bufs[i]->copied_timestamp &&
>                     q->bufs[i]->timestamp == timestamp)
> -                       return i;
> -       return -1;
> +                       return vb2_get_buffer(q, i);
> +
> +       return NULL;
>  }
> -EXPORT_SYMBOL_GPL(vb2_find_timestamp);
> +EXPORT_SYMBOL_GPL(vb2_find_buffer);
>
>  /*
>   * vb2_querybuf() - query video buffer information
> diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> index 7f9ae5b39b78..5a845887850b 100644
> --- a/include/media/videobuf2-v4l2.h
> +++ b/include/media/videobuf2-v4l2.h
> @@ -62,22 +62,6 @@ struct vb2_v4l2_buffer {
>  #define to_vb2_v4l2_buffer(vb) \
>         container_of(vb, struct vb2_v4l2_buffer, vb2_buf)
>
> -/**
> - * vb2_find_timestamp() - Find buffer with given timestamp in the queue
> - *
> - * @q:         pointer to &struct vb2_queue with videobuf2 queue.
> - * @timestamp: the timestamp to find.
> - * @start_idx: the start index (usually 0) in the buffer array to start
> - *             searching from. Note that there may be multiple buffers
> - *             with the same timestamp value, so you can restart the search
> - *             by setting @start_idx to the previously found index + 1.
> - *
> - * Returns the buffer index of the buffer with the given @timestamp, or
> - * -1 if no buffer with @timestamp was found.
> - */
> -int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> -                      unsigned int start_idx);
> -
>  /**
>   * vb2_find_buffer() - Find a buffer with given timestamp
>   *
> @@ -86,15 +70,7 @@ int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
>   *
>   * Returns the buffer with the given @timestamp, or NULL if not found.
>   */
> -static inline struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q,
> -                                                u64 timestamp)
> -{
> -       int index = vb2_find_timestamp(q, timestamp, 0);
> -
> -       if (index < 0)
> -               return NULL;
> -       return vb2_get_buffer(q, index);
> -}
> +struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp);

Was there any specific reason to add it as an inline initially rather
than just having it close to the final shape from the very beginning?
Sorry for being picky, but I find it more difficult to review this
way.

Best regards,
Tomasz

>
>  int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
>
> --
> 2.34.3
>

Re: [PATCH 8/8] videobuf2: Remove vb2_find_timestamp()

[Ezequiel Garcia]

Hi Tomasz,

On Fri, Jul 08, 2022 at 01:45:35PM +0900, Tomasz Figa wrote:
> Hi Ezequiel,
> 
> On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > Now that we've transitioned all users to vb2_find_buffer API,
> > remove the unused vb2_find_timestamp().
> >
> > Signed-off-by: Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
> > ---
> >  .../media/common/videobuf2/videobuf2-v4l2.c   | 12 ++++-----
> >  include/media/videobuf2-v4l2.h                | 26 +------------------
> >  2 files changed, 7 insertions(+), 31 deletions(-)
> >
> > diff --git a/drivers/media/common/videobuf2/videobuf2-v4l2.c b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > index 075d24ebf44c..a9696442dfba 100644
> > --- a/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > +++ b/drivers/media/common/videobuf2/videobuf2-v4l2.c
> > @@ -625,18 +625,18 @@ static const struct vb2_buf_ops v4l2_buf_ops = {
> >         .copy_timestamp         = __copy_timestamp,
> >  };
> >
> > -int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> > -                      unsigned int start_idx)
> > +struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp)
> >  {
> >         unsigned int i;
> >
> > -       for (i = start_idx; i < q->num_buffers; i++)
> > +       for (i = 0; i < q->num_buffers; i++)
> >                 if (q->bufs[i]->copied_timestamp &&
> >                     q->bufs[i]->timestamp == timestamp)
> > -                       return i;
> > -       return -1;
> > +                       return vb2_get_buffer(q, i);
> > +
> > +       return NULL;
> >  }
> > -EXPORT_SYMBOL_GPL(vb2_find_timestamp);
> > +EXPORT_SYMBOL_GPL(vb2_find_buffer);
> >
> >  /*
> >   * vb2_querybuf() - query video buffer information
> > diff --git a/include/media/videobuf2-v4l2.h b/include/media/videobuf2-v4l2.h
> > index 7f9ae5b39b78..5a845887850b 100644
> > --- a/include/media/videobuf2-v4l2.h
> > +++ b/include/media/videobuf2-v4l2.h
> > @@ -62,22 +62,6 @@ struct vb2_v4l2_buffer {
> >  #define to_vb2_v4l2_buffer(vb) \
> >         container_of(vb, struct vb2_v4l2_buffer, vb2_buf)
> >
> > -/**
> > - * vb2_find_timestamp() - Find buffer with given timestamp in the queue
> > - *
> > - * @q:         pointer to &struct vb2_queue with videobuf2 queue.
> > - * @timestamp: the timestamp to find.
> > - * @start_idx: the start index (usually 0) in the buffer array to start
> > - *             searching from. Note that there may be multiple buffers
> > - *             with the same timestamp value, so you can restart the search
> > - *             by setting @start_idx to the previously found index + 1.
> > - *
> > - * Returns the buffer index of the buffer with the given @timestamp, or
> > - * -1 if no buffer with @timestamp was found.
> > - */
> > -int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> > -                      unsigned int start_idx);
> > -
> >  /**
> >   * vb2_find_buffer() - Find a buffer with given timestamp
> >   *
> > @@ -86,15 +70,7 @@ int vb2_find_timestamp(const struct vb2_queue *q, u64 timestamp,
> >   *
> >   * Returns the buffer with the given @timestamp, or NULL if not found.
> >   */
> > -static inline struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q,
> > -                                                u64 timestamp)
> > -{
> > -       int index = vb2_find_timestamp(q, timestamp, 0);
> > -
> > -       if (index < 0)
> > -               return NULL;
> > -       return vb2_get_buffer(q, index);
> > -}
> > +struct vb2_buffer *vb2_find_buffer(struct vb2_queue *q, u64 timestamp);
> 
> Was there any specific reason to add it as an inline initially rather
> than just having it close to the final shape from the very beginning?
> Sorry for being picky, but I find it more difficult to review this
> way.
> 

Yeah, that makes sense, I'll re-work it for the next iteration.

Thanks,
Ezequiel

> Best regards,
> Tomasz
> 
> >
> >  int vb2_querybuf(struct vb2_queue *q, struct v4l2_buffer *b);
> >
> > --
> > 2.34.3
> >

Re: [PATCH 0/8] videobuf2: Replace vb2_find_timestamp() with vb2_find_buffer()

[Tomasz Figa]

Hi Ezequiel,

On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
<ezequiel@vanguardiasur.com.ar> wrote:
>
> All users of vb2_find_timestamp() combine it with vb2_get_buffer()
> to retrieve a videobuf2 buffer, given a u64 timestamp.
>
> Therefore, this series removes vb2_find_timestamp() and instead
> introduces a vb2_find_buffer, which is more suitable, making
> videobuf2 API slightly cleaner.
>
> Ezequiel Garcia (8):
>   videobuf2: Introduce vb2_find_buffer()
>   mediatek: vcodec: Use vb2_find_buffer
>   tegra-vde: Use vb2_find_buffer
>   vicodec: Use vb2_find_buffer
>   hantro: Use vb2_find_buffer
>   rkvdec: Use vb2_find_buffer
>   cedrus: Use vb2_find_buffer
>   videobuf2: Remove vb2_find_timestamp()
>
>  .../media/common/videobuf2/videobuf2-v4l2.c   | 12 ++---
>  .../vcodec/vdec/vdec_h264_req_common.c        |  7 ++-
>  .../mediatek/vcodec/vdec/vdec_vp8_req_if.c    |  7 ++-
>  .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  8 +--
>  .../media/platform/nvidia/tegra-vde/h264.c    |  9 ++--
>  .../media/test-drivers/vicodec/vicodec-core.c |  8 +--
>  drivers/staging/media/hantro/hantro_drv.c     |  6 +--
>  .../staging/media/hantro/hantro_g2_vp9_dec.c  | 10 ++--
>  drivers/staging/media/rkvdec/rkvdec-h264.c    | 41 ++++++---------
>  drivers/staging/media/rkvdec/rkvdec-vp9.c     | 10 ++--
>  drivers/staging/media/sunxi/cedrus/cedrus.h   | 13 +----
>  .../staging/media/sunxi/cedrus/cedrus_h264.c  | 16 +++---
>  .../staging/media/sunxi/cedrus/cedrus_h265.c  | 16 +++---
>  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 36 ++++++-------
>  .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 50 ++++++-------------
>  include/media/videobuf2-v4l2.h                | 12 ++---
>  16 files changed, 100 insertions(+), 161 deletions(-)
>
> --
> 2.34.3
>

Thanks for the series! I think it's a nice cleanup indeed, but please
see a few comments in my replies to individual patches.

Best regards,
Tomasz

Re: [PATCH 0/8] videobuf2: Replace vb2_find_timestamp() with vb2_find_buffer()

[Tomasz Figa]

On Fri, Jul 8, 2022 at 1:47 PM Tomasz Figa <tfiga@chromium.org> wrote:
>
> Hi Ezequiel,
>
> On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
> <ezequiel@vanguardiasur.com.ar> wrote:
> >
> > All users of vb2_find_timestamp() combine it with vb2_get_buffer()
> > to retrieve a videobuf2 buffer, given a u64 timestamp.
> >
> > Therefore, this series removes vb2_find_timestamp() and instead
> > introduces a vb2_find_buffer, which is more suitable, making
> > videobuf2 API slightly cleaner.
> >
> > Ezequiel Garcia (8):
> >   videobuf2: Introduce vb2_find_buffer()
> >   mediatek: vcodec: Use vb2_find_buffer
> >   tegra-vde: Use vb2_find_buffer
> >   vicodec: Use vb2_find_buffer
> >   hantro: Use vb2_find_buffer
> >   rkvdec: Use vb2_find_buffer
> >   cedrus: Use vb2_find_buffer
> >   videobuf2: Remove vb2_find_timestamp()
> >
> >  .../media/common/videobuf2/videobuf2-v4l2.c   | 12 ++---
> >  .../vcodec/vdec/vdec_h264_req_common.c        |  7 ++-
> >  .../mediatek/vcodec/vdec/vdec_vp8_req_if.c    |  7 ++-
> >  .../vcodec/vdec/vdec_vp9_req_lat_if.c         |  8 +--
> >  .../media/platform/nvidia/tegra-vde/h264.c    |  9 ++--
> >  .../media/test-drivers/vicodec/vicodec-core.c |  8 +--
> >  drivers/staging/media/hantro/hantro_drv.c     |  6 +--
> >  .../staging/media/hantro/hantro_g2_vp9_dec.c  | 10 ++--
> >  drivers/staging/media/rkvdec/rkvdec-h264.c    | 41 ++++++---------
> >  drivers/staging/media/rkvdec/rkvdec-vp9.c     | 10 ++--
> >  drivers/staging/media/sunxi/cedrus/cedrus.h   | 13 +----
> >  .../staging/media/sunxi/cedrus/cedrus_h264.c  | 16 +++---
> >  .../staging/media/sunxi/cedrus/cedrus_h265.c  | 16 +++---
> >  .../staging/media/sunxi/cedrus/cedrus_mpeg2.c | 36 ++++++-------
> >  .../staging/media/sunxi/cedrus/cedrus_vp8.c   | 50 ++++++-------------
> >  include/media/videobuf2-v4l2.h                | 12 ++---
> >  16 files changed, 100 insertions(+), 161 deletions(-)
> >
> > --
> > 2.34.3
> >
>
> Thanks for the series! I think it's a nice cleanup indeed, but please
> see a few comments in my replies to individual patches.

As we clarified my concern in one of the patches and the other one was
purely stylistic, feel free to just add my

Acked-by: Tomasz Figa <tfiga@chromium.org>

to the entire series. The stylistic one can be ignored if there is no
other change needed.

Best regards,
Tomasz