From: "Jernej Škrabec" <jernej.skrabec@gmail.com>
To: linux-media@vger.kernel.org, linux-kernel@vger.kernel.org,
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>
Cc: Hans Verkuil <hverkuil-cisco@xs4all.nl>,
Tomasz Figa <tfiga@chromium.org>,
Marek Szyprowski <m.szyprowski@samsung.com>,
Ezequiel Garcia <ezequiel@vanguardiasur.com.ar>,
Maxime Ripard <mripard@kernel.org>,
Paul Kocialkowski <paul.kocialkowski@bootlin.com>
Subject: Re: [PATCH 7/8] cedrus: Use vb2_find_buffer
Date: Mon, 11 Jul 2022 20:33:02 +0200 [thread overview]
Message-ID: <8960558.CDJkKcVGEf@jernej-laptop> (raw)
In-Reply-To: <20220706182657.210650-8-ezequiel@vanguardiasur.com.ar>
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 |
next prev parent reply other threads:[~2022-07-11 18:33 UTC|newest]
Thread overview: 17+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-06 18:26 [PATCH 0/8] videobuf2: Replace vb2_find_timestamp() with vb2_find_buffer() Ezequiel Garcia
2022-07-06 18:26 ` [PATCH 1/8] videobuf2: Introduce vb2_find_buffer() Ezequiel Garcia
2022-07-06 18:26 ` [PATCH 2/8] mediatek: vcodec: Use vb2_find_buffer Ezequiel Garcia
2022-07-06 18:26 ` [PATCH 3/8] tegra-vde: " Ezequiel Garcia
2022-07-06 18:26 ` [PATCH 4/8] vicodec: " Ezequiel Garcia
2022-07-06 18:26 ` [PATCH 5/8] hantro: " Ezequiel Garcia
2022-07-06 18:26 ` [PATCH 6/8] rkvdec: " Ezequiel Garcia
2022-07-08 4:40 ` Tomasz Figa
2022-07-08 11:21 ` Ezequiel Garcia
2022-07-08 11:45 ` Tomasz Figa
2022-07-06 18:26 ` [PATCH 7/8] cedrus: " Ezequiel Garcia
2022-07-11 18:33 ` Jernej Škrabec [this message]
2022-07-06 18:26 ` [PATCH 8/8] videobuf2: Remove vb2_find_timestamp() Ezequiel Garcia
2022-07-08 4:45 ` Tomasz Figa
2022-07-08 11:15 ` Ezequiel Garcia
2022-07-08 4:47 ` [PATCH 0/8] videobuf2: Replace vb2_find_timestamp() with vb2_find_buffer() Tomasz Figa
2022-07-08 11:47 ` Tomasz Figa
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=8960558.CDJkKcVGEf@jernej-laptop \
--to=jernej.skrabec@gmail.com \
--cc=ezequiel@vanguardiasur.com.ar \
--cc=hverkuil-cisco@xs4all.nl \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-media@vger.kernel.org \
--cc=m.szyprowski@samsung.com \
--cc=mripard@kernel.org \
--cc=paul.kocialkowski@bootlin.com \
--cc=tfiga@chromium.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.