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
Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.
Cc: Hans Verkuil <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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
Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.
Cc: Maxime Ripard <[email protected]>
Cc: Paul Kocialkowski <[email protected]>
Cc: Jernej Skrabec <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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
From: Ezequiel Garcia <[email protected]>
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 <[email protected]>
---
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
Use the newly introduced vb2_find_buffer API to get a vb2_buffer
given a buffer timestamp.
Cc: Dmitry Osipenko <[email protected]>
Signed-off-by: Ezequiel Garcia <[email protected]>
---
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
Hi Ezequiel,
On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
<[email protected]> 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
On Fri, Jul 8, 2022 at 1:47 PM Tomasz Figa <[email protected]> wrote:
>
> Hi Ezequiel,
>
> On Thu, Jul 7, 2022 at 3:27 AM Ezequiel Garcia
> <[email protected]> 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 <[email protected]>
to the entire series. The stylistic one can be ignored if there is no
other change needed.
Best regards,
Tomasz
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 <[email protected]>
> Cc: Paul Kocialkowski <[email protected]>
> Cc: Jernej Skrabec <[email protected]>
> Signed-off-by: Ezequiel Garcia <[email protected]>
> ---
> 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 |