Il 17/01/22 10:40, Yunfei Dong ha scritto:
Add support for VP8 decoding using the stateless API, as supported by MT8192.
Signed-off-by: Yunfei Dong yunfei.dong@mediatek.com
drivers/media/platform/mtk-vcodec/Makefile | 1 + .../mtk-vcodec/mtk_vcodec_dec_stateless.c | 24 +- .../platform/mtk-vcodec/mtk_vcodec_drv.h | 1 + .../mtk-vcodec/vdec/vdec_vp8_req_if.c | 440 ++++++++++++++++++ .../media/platform/mtk-vcodec/vdec_drv_if.c | 4 + .../media/platform/mtk-vcodec/vdec_drv_if.h | 1 + 6 files changed, 469 insertions(+), 2 deletions(-) create mode 100644 drivers/media/platform/mtk-vcodec/vdec/vdec_vp8_req_if.c
Hello Yunfei, I've found some issues in this patch, and there are also some other considerations to tidy it up.
(....snip....)
diff --git a/drivers/media/platform/mtk-vcodec/vdec/vdec_vp8_req_if.c b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp8_req_if.c new file mode 100644 index 000000000000..969568b98251 --- /dev/null +++ b/drivers/media/platform/mtk-vcodec/vdec/vdec_vp8_req_if.c @@ -0,0 +1,440 @@ +// SPDX-License-Identifier: GPL-2.0 +/*
- Copyright (c) 2021 MediaTek Inc.
- Author: Yunfei Dong yunfei.dong@mediatek.com
- */
+#include <linux/slab.h> +#include <media/v4l2-mem2mem.h> +#include <media/videobuf2-dma-contig.h> +#include <uapi/linux/v4l2-controls.h>
+#include "../mtk_vcodec_util.h" +#include "../mtk_vcodec_dec.h" +#include "../mtk_vcodec_intr.h" +#include "../vdec_drv_base.h" +#include "../vdec_drv_if.h" +#include "../vdec_vpu_if.h"
+/* Decoding picture buffer size (3 reference frames plus current frame) */ +#define VP8_DPB_SIZE 4
+/* HW working buffer size (bytes) */ +#define VP8_SEG_ID_SZ (256 * 1024)
This is SZ_256K
+#define VP8_PP_WRAPY_SZ (64 * 1024) +#define VP8_PP_WRAPC_SZ (64 * 1024) +#define VP8_VLD_PRED_SZ (64 * 1024)
And these are all SZ_64K.
+/**
- struct vdec_vp8_slice_info - decode misc information
- @vld_wrapper_dma : vld wrapper dma address
- @seg_id_buf_dma : seg id dma address
- @wrap_y_dma : wrap y dma address
- @wrap_c_dma : wrap y dma address
- @cur_y_fb_dma : current plane Y frame buffer dma address
- @cur_c_fb_dma : current plane C frame buffer dma address
- @bs_dma : bitstream dma address
- @bs_sz : bitstream size
- @resolution_changed: resolution change flag 1 - changed, 0 - not change
- @frame_header_type : current frame header type
- @wait_key_frame : wait key frame coming
- @crc : used to check whether hardware's status is right
- @timeout : decode timeout: 1 timeout, 0 no timeount
There's no `timeout` in this structure, did you forget to remove the documentation for this one?
- @reserved: : reserved, currently unused
- */
+struct vdec_vp8_slice_info {
- u64 vld_wrapper_dma;
- u64 seg_id_buf_dma;
- u64 wrap_y_dma;
- u64 wrap_c_dma;
- u64 cur_y_fb_dma;
- u64 cur_c_fb_dma;
- u64 bs_dma;
- u32 bs_sz;
- u32 resolution_changed;
- u32 frame_header_type;
- u32 crc[8];
- u32 reserved;
+};
+/**
- struct vdec_vp8_slice_dpb_info - vp8 reference information
- @y_dma_addr : Y bitstream physical address
- @c_dma_addr : CbCr bitstream physical address
- @reference_flag: reference picture flag
- @reserved : 64bit align
- */
+struct vdec_vp8_slice_dpb_info {
- dma_addr_t y_dma_addr;
- dma_addr_t c_dma_addr;
- int reference_flag;
- int reserved;
+};
+/**
- struct vdec_vp8_slice_vsi - VPU shared information
- @dec : decoding information
- @pic : picture information
- @vp8_dpb_info : reference buffer information
- */
+struct vdec_vp8_slice_vsi {
- struct vdec_vp8_slice_info dec;
- struct vdec_pic_info pic;
- struct vdec_vp8_slice_dpb_info vp8_dpb_info[3];
+};
+/**
- struct vdec_vp8_slice_inst - VP8 decoder instance
- @seg_id_buf : seg buffer
- @wrap_y_buf : wrapper y buffer
- @wrap_c_buf : wrapper c buffer
- @vld_wrapper_buf: vld wrapper buffer
- @ctx : V4L2 context
- @vpu : VPU instance for decoder
- @vsi : VPU share information
- */
+struct vdec_vp8_slice_inst {
- struct mtk_vcodec_mem seg_id_buf;
- struct mtk_vcodec_mem wrap_y_buf;
- struct mtk_vcodec_mem wrap_c_buf;
- struct mtk_vcodec_mem vld_wrapper_buf;
- struct mtk_vcodec_ctx *ctx;
- struct vdec_vpu_inst vpu;
- struct vdec_vp8_slice_vsi *vsi;
+};
+static void *vdec_vp8_slice_get_ctrl_ptr(struct mtk_vcodec_ctx *ctx, int id) +{
- struct v4l2_ctrl *ctrl = v4l2_ctrl_find(&ctx->ctrl_hdl, id);
You should check if ctrl is NULL here, and eventually return a ERR_PTR(-EINVAL) if that ever occurs... or you may get a NULL pointer kernel panic...
- return ctrl->p_cur.p;
+}
...snip...
+static void vdec_vp8_slice_get_decode_parameters(struct vdec_vp8_slice_inst *inst)
static int ..... ?
+{
- const struct v4l2_ctrl_vp8_frame *frame_header =
vdec_vp8_slice_get_ctrl_ptr(inst->ctx, V4L2_CTRL_TYPE_VP8_FRAME);
This is crashing the kernel, because it ends up being NULL... ... the solution to that is to look for V4L2_CID_STATELESS_VP8_FRAME instead of V4L2_CTRL_TYPE_VP8_FRAME, but you should really do an error check here and eventually bail out, in case anything goes horribly wrong...
- struct mtk_vcodec_ctx *ctx = inst->ctx;
- struct vb2_queue *vq;
- struct vb2_buffer *vb;
- u64 referenct_ts;
- int index, vb2_index;
...so you'd be checking for error pointer of frame_header here and returning.
- vq = v4l2_m2m_get_vq(ctx->m2m_ctx, V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE);
- for (index = 0; index < 3; index++) {
referenct_ts = vdec_vp8_slice_get_ref_by_ts(frame_header, index);vb2_index = vb2_find_timestamp(vq, referenct_ts, 0);if (vb2_index < 0) {if (!V4L2_VP8_FRAME_IS_KEY_FRAME(frame_header))mtk_vcodec_err(inst, "reference invalid: index(%d) ts(%lld)",index, referenct_ts);inst->vsi->vp8_dpb_info[index].reference_flag = 0;continue;}inst->vsi->vp8_dpb_info[index].reference_flag = 1;vb = vq->bufs[vb2_index];inst->vsi->vp8_dpb_info[index].y_dma_addr =vb2_dma_contig_plane_dma_addr(vb, 0);if (ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 2)inst->vsi->vp8_dpb_info[index].c_dma_addr =vb2_dma_contig_plane_dma_addr(vb, 1);elseinst->vsi->vp8_dpb_info[index].c_dma_addr =inst->vsi->vp8_dpb_info[index].y_dma_addr +ctx->picinfo.fb_sz[0];- }
- inst->vsi->dec.frame_header_type = frame_header->flags >> 1;
+}
+static int vdec_vp8_slice_init(struct mtk_vcodec_ctx *ctx) +{
- struct vdec_vp8_slice_inst *inst;
- int err;
- inst = kzalloc(sizeof(*inst), GFP_KERNEL);
- if (!inst)
return -ENOMEM;- inst->ctx = ctx;
- inst->vpu.id = SCP_IPI_VDEC_LAT;
- inst->vpu.core_id = SCP_IPI_VDEC_CORE;
- inst->vpu.ctx = ctx;
- inst->vpu.codec_type = ctx->current_codec;
- inst->vpu.capture_type = ctx->capture_fourcc;
- err = vpu_dec_init(&inst->vpu);
- if (err) {
mtk_vcodec_err(inst, "vdec_vp8 init err=%d", err);goto error_free_inst;- }
- inst->vsi = inst->vpu.vsi;
- err = vdec_vp8_slice_alloc_working_buf(inst);
- if (err)
goto error_deinit;- mtk_vcodec_debug(inst, "vp8 struct size = %d vsi: %d\n",
(int)sizeof(struct v4l2_ctrl_vp8_frame),(int)sizeof(struct vdec_vp8_slice_vsi));- mtk_vcodec_debug(inst, "vp8:%p, codec_type = 0x%x vsi: 0x%p",
inst, inst->vpu.codec_type, inst->vpu.vsi);- ctx->drv_handle = inst;
- return 0;
+error_deinit:
- vpu_dec_deinit(&inst->vpu);
+error_free_inst:
- kfree(inst);
- return err;
+}
+static int vdec_vp8_slice_decode(void *h_vdec, struct mtk_vcodec_mem *bs,
struct vdec_fb *fb, bool *res_chg)+{
- struct vdec_vp8_slice_inst *inst = h_vdec;
- struct vdec_vpu_inst *vpu = &inst->vpu;
- struct mtk_video_dec_buf *src_buf_info, *dst_buf_info;
- unsigned int data;
- u64 y_fb_dma, c_fb_dma;
- int err, timeout;
- /* Resolution changes are never initiated by us */
- *res_chg = false;
- /* bs NULL means flush decoder */
- if (!bs)
return vpu_dec_reset(vpu);- src_buf_info = container_of(bs, struct mtk_video_dec_buf, bs_buffer);
- fb = inst->ctx->dev->vdec_pdata->get_cap_buffer(inst->ctx);
- dst_buf_info = container_of(fb, struct mtk_video_dec_buf, frame_buffer);
- y_fb_dma = fb ? (u64)fb->base_y.dma_addr : 0;
- if (inst->ctx->q_data[MTK_Q_DATA_DST].fmt->num_planes == 1)
c_fb_dma = y_fb_dma +inst->ctx->picinfo.buf_w * inst->ctx->picinfo.buf_h;- else
c_fb_dma = fb ? (u64)fb->base_c.dma_addr : 0;- inst->vsi->dec.bs_dma = (unsigned long)bs->dma_addr;
inst->vsi->dec.bs_dma is u64... so please cast to u64.
- inst->vsi->dec.bs_sz = bs->size;
- inst->vsi->dec.cur_y_fb_dma = y_fb_dma;
- inst->vsi->dec.cur_c_fb_dma = c_fb_dma;
- mtk_vcodec_debug(inst, "frame[%d] bs(%zu 0x%lx) y/c(0x%llx 0x%llx)",
inst->ctx->decoded_frame_cnt,bs->size, (unsigned long)bs->dma_addr,
...and it would be useful if it was u64 here too, obviously.
y_fb_dma, c_fb_dma);- v4l2_m2m_buf_copy_metadata(&src_buf_info->m2m_buf.vb,
&dst_buf_info->m2m_buf.vb, true);- vdec_vp8_slice_get_decode_parameters(inst);
- err = vpu_dec_start(vpu, &data, 1);
- if (err) {
mtk_vcodec_debug(inst, "vp8 dec start err!");goto error;- }
Regards, Angelo