On Mon, Mar 28, 2022 at 03:59:31PM -0400, Nicolas Dufresne wrote:
> @@ -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);
pr_debug() is too quiet. Make it pr_err(). Set buf_idx to zero instead
leaving it as an error code.
> + }
>
> 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;
These would be better as type bool.
regards,
dan carpenter
>
> 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);
> }
> }
> }
