From 0f090e2b2b94a70516a58bc9f87a8cef54ffd734 Mon Sep 17 00:00:00 2001 From: Tomasz Figa Date: Tue, 31 Mar 2015 22:40:03 +0900 Subject: [PATCH] CHROMIUM: [media] rk3288-vpu: Preserve picture positions in H264 DPB lists Even though H264 standard allows arbitrary order of DPB entries, the hardware requires picture with given POC to use the same DPB entry for its whole lifetime. This means that the driver needs to reorder DPB array to suit this requirement. This patch modifies the driver to reorder H264 DPB and should fix corruption issues when DPB array received from userspace does not meet hardware requirements. BUG=chrome-os-partner:38416 TEST=http://www.youtube.com/embed/YE7VzlLtp-4 Signed-off-by: Tomasz Figa Reviewed-on: https://chromium-review.googlesource.com/263370 Reviewed-by: Pawel Osciak Tested-by: Caesar Wang Change-Id: Ibbd0e2bc4e527aadd21ef08ec68866678bf8a659 Signed-off-by: Jeffy Chen Signed-off-by: Yakir Yang --- .../platform/rk3288-vpu/rk3288_vpu_common.h | 5 + .../platform/rk3288-vpu/rk3288_vpu_dec.c | 100 +++++++++++++++++- .../platform/rk3288-vpu/rk3288_vpu_hw_h264d.c | 41 +++---- 3 files changed, 125 insertions(+), 21 deletions(-) diff --git a/drivers/media/platform/rk3288-vpu/rk3288_vpu_common.h b/drivers/media/platform/rk3288-vpu/rk3288_vpu_common.h index 0ef96c3f186c..34828369875f 100644 --- a/drivers/media/platform/rk3288-vpu/rk3288_vpu_common.h +++ b/drivers/media/platform/rk3288-vpu/rk3288_vpu_common.h @@ -213,6 +213,9 @@ struct rk3288_vpu_vp8d_run { * @scaling_matrix: Pointer to a buffer containing scaling matrix. * @slice_param: Pointer to a buffer containing slice parameters array. * @decode_param: Pointer to a buffer containing decode parameters. + * @dpb: Array of DPB entries reordered to keep POC order. + * @dpb_map: Map of indices used in ref_pic_list_* into indices to + * reordered DPB array. */ struct rk3288_vpu_h264d_run { const struct v4l2_ctrl_h264_sps *sps; @@ -220,6 +223,8 @@ struct rk3288_vpu_h264d_run { const struct v4l2_ctrl_h264_scaling_matrix *scaling_matrix; const struct v4l2_ctrl_h264_slice_param *slice_param; const struct v4l2_ctrl_h264_decode_param *decode_param; + struct v4l2_h264_dpb_entry dpb[16]; + u8 dpb_map[16]; }; /** diff --git a/drivers/media/platform/rk3288-vpu/rk3288_vpu_dec.c b/drivers/media/platform/rk3288-vpu/rk3288_vpu_dec.c index 62fde2772353..e3c54146c53e 100644 --- a/drivers/media/platform/rk3288-vpu/rk3288_vpu_dec.c +++ b/drivers/media/platform/rk3288-vpu/rk3288_vpu_dec.c @@ -625,6 +625,99 @@ static int vidioc_streamoff(struct file *file, void *priv, return ret; } +static void rk3288_vpu_dec_set_dpb(struct rk3288_vpu_ctx *ctx, + struct v4l2_ctrl *ctrl) +{ + struct v4l2_ctrl_h264_decode_param *dec_param = ctrl->p_new.p; + const struct v4l2_h264_dpb_entry *new_dpb_entry; + u8 *dpb_map = ctx->run.h264d.dpb_map; + struct v4l2_h264_dpb_entry *cur_dpb_entry; + DECLARE_BITMAP(used, ARRAY_SIZE(ctx->run.h264d.dpb)) = { 0, }; + DECLARE_BITMAP(new, ARRAY_SIZE(dec_param->dpb)) = { 0, }; + int i, j; + + BUILD_BUG_ON(ARRAY_SIZE(ctx->run.h264d.dpb) != + ARRAY_SIZE(dec_param->dpb)); + + /* Disable all entries by default. */ + for (j = 0; j < ARRAY_SIZE(ctx->run.h264d.dpb); ++j) { + cur_dpb_entry = &ctx->run.h264d.dpb[j]; + + cur_dpb_entry->flags &= ~V4L2_H264_DPB_ENTRY_FLAG_ACTIVE; + } + + /* Try to match new DPB entries with existing ones by their POCs. */ + for (i = 0; i < ARRAY_SIZE(dec_param->dpb); ++i) { + new_dpb_entry = &dec_param->dpb[i]; + + if (!(new_dpb_entry->flags & V4L2_H264_DPB_ENTRY_FLAG_ACTIVE)) + continue; + + /* + * To cut off some comparisons, iterate only on target DPB + * entries which are not used yet. + */ + for_each_clear_bit(j, used, ARRAY_SIZE(ctx->run.h264d.dpb)) { + cur_dpb_entry = &ctx->run.h264d.dpb[j]; + + if (new_dpb_entry->top_field_order_cnt == + cur_dpb_entry->top_field_order_cnt && + new_dpb_entry->bottom_field_order_cnt == + cur_dpb_entry->bottom_field_order_cnt) { + memcpy(cur_dpb_entry, new_dpb_entry, + sizeof(*cur_dpb_entry)); + set_bit(j, used); + dpb_map[i] = j; + break; + } + } + + if (j == ARRAY_SIZE(ctx->run.h264d.dpb)) + set_bit(i, new); + } + + /* For entries that could not be matched, use remaining free slots. */ + for_each_set_bit(i, new, ARRAY_SIZE(dec_param->dpb)) { + new_dpb_entry = &dec_param->dpb[i]; + + j = find_first_zero_bit(used, ARRAY_SIZE(ctx->run.h264d.dpb)); + /* + * Both arrays are of the same sizes, so there is no way + * we can end up with no space in target array, unless + * something is buggy. + */ + if (WARN_ON(j >= ARRAY_SIZE(ctx->run.h264d.dpb))) + return; + + cur_dpb_entry = &ctx->run.h264d.dpb[j]; + memcpy(cur_dpb_entry, new_dpb_entry, sizeof(*cur_dpb_entry)); + set_bit(j, used); + dpb_map[i] = j; + } + + /* + * Verify that reference picture lists are in range, since they + * will be indexing dpb_map[] when programming the hardware. + * + * Fallback to 0 should be safe, as we will get at most corrupt + * decoding result, without any serious side effects. Moreover, + * even if entry 0 is unused, the hardware programming code will + * handle this properly. + */ + for (i = 0; i < ARRAY_SIZE(dec_param->ref_pic_list_b0); ++i) + if (dec_param->ref_pic_list_b0[i] + >= ARRAY_SIZE(ctx->run.h264d.dpb_map)) + dec_param->ref_pic_list_b0[i] = 0; + for (i = 0; i < ARRAY_SIZE(dec_param->ref_pic_list_b1); ++i) + if (dec_param->ref_pic_list_b1[i] + >= ARRAY_SIZE(ctx->run.h264d.dpb_map)) + dec_param->ref_pic_list_b1[i] = 0; + for (i = 0; i < ARRAY_SIZE(dec_param->ref_pic_list_p0); ++i) + if (dec_param->ref_pic_list_p0[i] + >= ARRAY_SIZE(ctx->run.h264d.dpb_map)) + dec_param->ref_pic_list_p0[i] = 0; +} + static int rk3288_vpu_dec_s_ctrl(struct v4l2_ctrl *ctrl) { struct rk3288_vpu_ctx *ctx = ctrl_to_ctx(ctrl); @@ -640,11 +733,16 @@ static int rk3288_vpu_dec_s_ctrl(struct v4l2_ctrl *ctrl) case V4L2_CID_MPEG_VIDEO_H264_PPS: case V4L2_CID_MPEG_VIDEO_H264_SCALING_MATRIX: case V4L2_CID_MPEG_VIDEO_H264_SLICE_PARAM: - case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: case V4L2_CID_MPEG_VIDEO_VP8_FRAME_HDR: /* These controls are used directly. */ break; + case V4L2_CID_MPEG_VIDEO_H264_DECODE_PARAM: + if (ctrl->store) + break; + rk3288_vpu_dec_set_dpb(ctx, ctrl); + break; + default: v4l2_err(&dev->v4l2_dev, "Invalid control, id=%d, val=%d\n", ctrl->id, ctrl->val); diff --git a/drivers/media/platform/rk3288-vpu/rk3288_vpu_hw_h264d.c b/drivers/media/platform/rk3288-vpu/rk3288_vpu_hw_h264d.c index 132a76013d32..b03491efbf32 100644 --- a/drivers/media/platform/rk3288-vpu/rk3288_vpu_hw_h264d.c +++ b/drivers/media/platform/rk3288-vpu/rk3288_vpu_hw_h264d.c @@ -226,7 +226,7 @@ static void rk3288_vpu_h264d_prepare_table(struct rk3288_vpu_ctx *ctx) ctx->run.h264d.scaling_matrix; const struct v4l2_ctrl_h264_decode_param *dec_param = ctx->run.h264d.decode_param; - const struct v4l2_h264_dpb_entry *dpb = dec_param->dpb; + const struct v4l2_h264_dpb_entry *dpb = ctx->run.h264d.dpb; int i; /* @@ -367,7 +367,8 @@ static void rk3288_vpu_h264d_set_ref(struct rk3288_vpu_ctx *ctx) { const struct v4l2_ctrl_h264_decode_param *dec_param = ctx->run.h264d.decode_param; - const struct v4l2_h264_dpb_entry *dpb = dec_param->dpb; + const struct v4l2_h264_dpb_entry *dpb = ctx->run.h264d.dpb; + const u8 *dpb_map = ctx->run.h264d.dpb_map; struct rk3288_vpu_dev *vpu = ctx->dev; u32 dpb_longterm = 0; u32 dpb_valid = 0; @@ -419,17 +420,17 @@ static void rk3288_vpu_h264d_set_ref(struct rk3288_vpu_ctx *ctx) reg_num = 0; for (i = 0; i < 15; i += 3) { reg = VDPU_REG_BD_REF_PIC_BINIT_RLIST_F0( - dec_param->ref_pic_list_b0[i + 0]) + dpb_map[dec_param->ref_pic_list_b0[i + 0]]) | VDPU_REG_BD_REF_PIC_BINIT_RLIST_F1( - dec_param->ref_pic_list_b0[i + 1]) + dpb_map[dec_param->ref_pic_list_b0[i + 1]]) | VDPU_REG_BD_REF_PIC_BINIT_RLIST_F2( - dec_param->ref_pic_list_b0[i + 2]) + dpb_map[dec_param->ref_pic_list_b0[i + 2]]) | VDPU_REG_BD_REF_PIC_BINIT_RLIST_B0( - dec_param->ref_pic_list_b1[i + 0]) + dpb_map[dec_param->ref_pic_list_b1[i + 0]]) | VDPU_REG_BD_REF_PIC_BINIT_RLIST_B1( - dec_param->ref_pic_list_b1[i + 1]) + dpb_map[dec_param->ref_pic_list_b1[i + 1]]) | VDPU_REG_BD_REF_PIC_BINIT_RLIST_B2( - dec_param->ref_pic_list_b1[i + 2]); + dpb_map[dec_param->ref_pic_list_b1[i + 2]]); vdpu_write_relaxed(vpu, reg, VDPU_REG_BD_REF_PIC(reg_num++)); } @@ -439,17 +440,17 @@ static void rk3288_vpu_h264d_set_ref(struct rk3288_vpu_ctx *ctx) * of P forward picture list. */ reg = VDPU_REG_BD_P_REF_PIC_BINIT_RLIST_F15( - dec_param->ref_pic_list_b0[15]) + dpb_map[dec_param->ref_pic_list_b0[15]]) | VDPU_REG_BD_P_REF_PIC_BINIT_RLIST_B15( - dec_param->ref_pic_list_b1[15]) + dpb_map[dec_param->ref_pic_list_b1[15]]) | VDPU_REG_BD_P_REF_PIC_PINIT_RLIST_F0( - dec_param->ref_pic_list_p0[0]) + dpb_map[dec_param->ref_pic_list_p0[0]]) | VDPU_REG_BD_P_REF_PIC_PINIT_RLIST_F1( - dec_param->ref_pic_list_p0[1]) + dpb_map[dec_param->ref_pic_list_p0[1]]) | VDPU_REG_BD_P_REF_PIC_PINIT_RLIST_F2( - dec_param->ref_pic_list_p0[2]) + dpb_map[dec_param->ref_pic_list_p0[2]]) | VDPU_REG_BD_P_REF_PIC_PINIT_RLIST_F3( - dec_param->ref_pic_list_p0[3]); + dpb_map[dec_param->ref_pic_list_p0[3]]); vdpu_write_relaxed(vpu, reg, VDPU_REG_BD_P_REF_PIC); /* @@ -459,17 +460,17 @@ static void rk3288_vpu_h264d_set_ref(struct rk3288_vpu_ctx *ctx) reg_num = 0; for (i = 4; i < RK3288_VPU_H264_NUM_DPB; i += 6) { reg = VDPU_REG_FWD_PIC_PINIT_RLIST_F0( - dec_param->ref_pic_list_p0[i + 0]) + dpb_map[dec_param->ref_pic_list_p0[i + 0]]) | VDPU_REG_FWD_PIC_PINIT_RLIST_F1( - dec_param->ref_pic_list_p0[i + 1]) + dpb_map[dec_param->ref_pic_list_p0[i + 1]]) | VDPU_REG_FWD_PIC_PINIT_RLIST_F2( - dec_param->ref_pic_list_p0[i + 2]) + dpb_map[dec_param->ref_pic_list_p0[i + 2]]) | VDPU_REG_FWD_PIC_PINIT_RLIST_F3( - dec_param->ref_pic_list_p0[i + 3]) + dpb_map[dec_param->ref_pic_list_p0[i + 3]]) | VDPU_REG_FWD_PIC_PINIT_RLIST_F4( - dec_param->ref_pic_list_p0[i + 4]) + dpb_map[dec_param->ref_pic_list_p0[i + 4]]) | VDPU_REG_FWD_PIC_PINIT_RLIST_F5( - dec_param->ref_pic_list_p0[i + 5]); + dpb_map[dec_param->ref_pic_list_p0[i + 5]]); vdpu_write_relaxed(vpu, reg, VDPU_REG_FWD_PIC(reg_num++)); } -- 2.34.1