Re: [PATCH v1 19/24] media: rkvdec-h264: Add field decoding support

Sebastian Fricke
2022-03-30T07:10:39Z

Hey Nicolas,

On 28.03.2022 15:59, Nicolas Dufresne wrote:
>This makes use of the new feature in the reference builder to program
>up to 32 references when doing field decoding. It also signals the
>parity (top of bottom) of the field to the hardware.

s/top of bottom/top or bottom/

>
>Signed-off-by: Nicolas Dufresne <nicolas.dufresne@collabora.com>
Reviewed-by: Sebastian Fricke <sebastian.fricke@collabora.com>

Greetings,
Sebastian

>---
> drivers/staging/media/rkvdec/rkvdec-h264.c | 48 ++++++++++------------
> 1 file changed, 21 insertions(+), 27 deletions(-)
>
>diff --git a/drivers/staging/media/rkvdec/rkvdec-h264.c b/drivers/staging/media/rkvdec/rkvdec-h264.c
>index ec52b195bbd7..891c48bf6a51 100644
>--- a/drivers/staging/media/rkvdec/rkvdec-h264.c
>+++ b/drivers/staging/media/rkvdec/rkvdec-h264.c
>@@ -97,13 +97,10 @@ struct rkvdec_h264_priv_tbl {
> u8 err_info[RKV_ERROR_INFO_SIZE];
> };
>
>-#define RKVDEC_H264_DPB_SIZE 16
>-
> struct rkvdec_h264_reflists {
> struct v4l2_h264_reference p[V4L2_H264_REF_LIST_LEN];
> struct v4l2_h264_reference b0[V4L2_H264_REF_LIST_LEN];
> struct v4l2_h264_reference b1[V4L2_H264_REF_LIST_LEN];
>- u8 num_valid;
> };
>
> struct rkvdec_h264_run {
>@@ -738,23 +735,26 @@ static void lookup_ref_buf_idx(struct rkvdec_ctx *ctx,
> struct vb2_queue *cap_q = &m2m_ctx->cap_q_ctx.q;
> int buf_idx = -1;
>
>- if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)
>+ 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)
>+ pr_debug("No buffer for reference_ts %llu",
>+ dpb[i].reference_ts);
>+ }
>
> run->ref_buf_idx[i] = buf_idx;
> }
> }
>
> static void assemble_hw_rps(struct rkvdec_ctx *ctx,
>+ struct v4l2_h264_reflist_builder *builder,
> struct rkvdec_h264_run *run)
> {
> const struct v4l2_ctrl_h264_decode_params *dec_params = run->decode_params;
> const struct v4l2_h264_dpb_entry *dpb = dec_params->dpb;
> struct rkvdec_h264_ctx *h264_ctx = ctx->priv;
>- const struct v4l2_ctrl_h264_sps *sps = run->sps;
> struct rkvdec_h264_priv_tbl *priv_tbl = h264_ctx->priv_tbl.cpu;
>- u32 max_frame_num = 1 << (sps->log2_max_frame_num_minus4 + 4);
>
> u32 *hw_rps = priv_tbl->rps;
> u32 i, j;
>@@ -772,37 +772,36 @@ static void assemble_hw_rps(struct rkvdec_ctx *ctx,
> if (!(dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE))
> continue;
>
>- if (dpb[i].flags & V4L2_H264_DPB_ENTRY_FLAG_LONG_TERM ||
>- dpb[i].frame_num <= dec_params->frame_num) {
>- p[i] = dpb[i].frame_num;
>- continue;
>- }
>-
>- p[i] = dpb[i].frame_num - max_frame_num;
>+ p[i] = builder->refs[i].frame_num;
> }
>
> for (j = 0; j < RKVDEC_NUM_REFLIST; j++) {
>- for (i = 0; i < h264_ctx->reflists.num_valid; i++) {
>- u8 dpb_valid = run->ref_buf_idx[i] >= 0;
>- u8 idx = 0;
>+ for (i = 0; i < builder->num_valid; i++) {
>+ struct v4l2_h264_reference *ref;
>+ u8 dpb_valid;
>+ u8 bottom;
>
> switch (j) {
> case 0:
>- idx = h264_ctx->reflists.p[i].index;
>+ ref = &h264_ctx->reflists.p[i];
> break;
> case 1:
>- idx = h264_ctx->reflists.b0[i].index;
>+ ref = &h264_ctx->reflists.b0[i];
> break;
> case 2:
>- idx = h264_ctx->reflists.b1[i].index;
>+ ref = &h264_ctx->reflists.b1[i];
> break;
> }
>
>- if (idx >= ARRAY_SIZE(dec_params->dpb))
>+ if (WARN_ON(ref->index >= ARRAY_SIZE(dec_params->dpb)))
> continue;
>
>+ dpb_valid = run->ref_buf_idx[ref->index] >= 0;
>+ bottom = ref->fields == V4L2_H264_BOTTOM_FIELD_REF;
>+
> set_ps_field(hw_rps, DPB_INFO(i, j),
>- idx | dpb_valid << 4);
>+ ref->index | dpb_valid << 4);
>+ set_ps_field(hw_rps, BOTTOM_FLAG(i, j), bottom);
> }
> }
> }
>@@ -990,10 +989,6 @@ static void config_registers(struct rkvdec_ctx *ctx,
> rkvdec->regs + RKVDEC_REG_H264_BASE_REFER15);
> }
>
>- /*
>- * Since support frame mode only
>- * top_field_order_cnt is the same as bottom_field_order_cnt
>- */
> reg = RKVDEC_CUR_POC(dec_params->top_field_order_cnt);
> writel_relaxed(reg, rkvdec->regs + RKVDEC_REG_CUR_POC0);
>
>@@ -1109,7 +1104,6 @@ static int rkvdec_h264_run(struct rkvdec_ctx *ctx)
> /* Build the P/B{0,1} ref lists. */
> v4l2_h264_init_reflist_builder(&reflist_builder, run.decode_params,
> run.sps, run.decode_params->dpb);
>- h264_ctx->reflists.num_valid = reflist_builder.num_valid;
> v4l2_h264_build_p_ref_list(&reflist_builder, h264_ctx->reflists.p);
> v4l2_h264_build_b_ref_lists(&reflist_builder, h264_ctx->reflists.b0,
> h264_ctx->reflists.b1);
>@@ -1117,7 +1111,7 @@ static int rkvdec_h264_run(struct rkvdec_ctx *ctx)
> assemble_hw_scaling_list(ctx, &run);
> assemble_hw_pps(ctx, &run);
> lookup_ref_buf_idx(ctx, &run);
>- assemble_hw_rps(ctx, &run);
>+ assemble_hw_rps(ctx, &reflist_builder, &run);
> config_registers(ctx, &run);
>
> rkvdec_run_postamble(ctx, &run.base);
>--
>2.34.1
>