Re: [PATCH v6, 8/8] media: mediatek: vcodec: Return encoding result in asynchronous mode

From: Allen-KH Cheng (程冠勳)
Date: Tue Oct 04 2022 - 08:09:00 EST

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH v3 00/23] Improvements to Intel perf metrics"
Previous message: Niklas Schnelle: "[PATCH v4 5/5] iommu/s390: Fix incorrect pgsize_bitmap"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

Hi Irui,

On Sat, 2022-10-01 at 11:17 +0800, Irui Wang wrote:
> when enable multi-core encoding, the wait IRQ done synchronous
> function
> should not be called, so the encoding result can't return to client
> in
> device_run. Move the buffer done function in IRQ handler.
>
> Signed-off-by: Irui Wang <irui.wang@xxxxxxxxxxxx>
> ---
> .../platform/mediatek/vcodec/mtk_vcodec_drv.h | 6 ++
> .../platform/mediatek/vcodec/mtk_vcodec_enc.c | 72
> +++++++++++++++++--
> .../platform/mediatek/vcodec/mtk_vcodec_enc.h | 7 +-
> .../mediatek/vcodec/mtk_vcodec_enc_drv.c | 28 +++++++-
> .../mediatek/vcodec/mtk_vcodec_enc_hw.c | 13 +++-
> .../mediatek/vcodec/mtk_vcodec_enc_pm.c | 1 +
> .../mediatek/vcodec/mtk_vcodec_util.h | 1 +
> .../mediatek/vcodec/venc/venc_h264_if.c | 20 ++++--
> .../platform/mediatek/vcodec/venc_drv_if.h | 2 +
> 9 files changed, 137 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> index a75ba675f72b..d9c27ebcf3c3 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_drv.h
> @@ -291,6 +291,9 @@ struct vdec_pic_info {
> * @msg_queue: msg queue used to store lat buffer information.
> * @q_mutex: vb2_queue mutex.
> * @encoded_frame_cnt: number of encoded frames
> + * @pfrm_buf: used to store current ctx's frame buffer
> + * @pbs_buf: used to store current ctx's bitstream buffer
> + * @hdr_size: used to store prepend header size
> */
> struct mtk_vcodec_ctx {
> enum mtk_instance_type type;
> @@ -340,6 +343,9 @@ struct mtk_vcodec_ctx {
>
> struct mutex q_mutex;
> int encoded_frame_cnt;
> + struct vb2_v4l2_buffer *pfrm_buf[MTK_VENC_HW_MAX];
> + struct vb2_v4l2_buffer *pbs_buf[MTK_VENC_HW_MAX];
> + unsigned int hdr_size;
> };
>
> /*
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> index d15e106fb246..3424da4aaa26 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.c
> @@ -958,6 +958,8 @@ static void vb2ops_venc_stop_streaming(struct
> vb2_queue *q)
>
> mtk_v4l2_debug(2, "[%d]-> type=%d", ctx->id, q->type);
>
> + mtk_venc_lock_all(ctx);
> +
> if (q->type == V4L2_BUF_TYPE_VIDEO_CAPTURE_MPLANE) {
> while ((dst_buf = v4l2_m2m_dst_buf_remove(ctx-
> >m2m_ctx))) {
> vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
> @@ -1193,6 +1195,7 @@ static void mtk_venc_worker(struct work_struct
> *work)
> * is dequeued.
> */
> if (src_buf == &ctx->empty_flush_buf.vb) {
> + mtk_venc_lock_all(ctx);
> vb2_set_plane_payload(&dst_buf->vb2_buf, 0, 0);
> dst_buf->flags |= V4L2_BUF_FLAG_LAST;
> v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
> @@ -1207,9 +1210,12 @@ static void mtk_venc_worker(struct work_struct
> *work)
> frm_buf.fb_addr[i].size =
> (size_t)src_buf-
> >vb2_buf.planes[i].length;
> }
> + frm_buf.frm_addr = src_buf;
> +
> bs_buf.va = vb2_plane_vaddr(&dst_buf->vb2_buf, 0);
> bs_buf.dma_addr = vb2_dma_contig_plane_dma_addr(&dst_buf-
> >vb2_buf, 0);
> bs_buf.size = (size_t)dst_buf->vb2_buf.planes[0].length;
> + bs_buf.buf = dst_buf;
>
> mtk_v4l2_debug(2,
> "Framebuf PA=%llx Size=0x%zx;PA=0x%llx
> Size=0x%zx;PA=0x%llx Size=%zu",
> @@ -1235,11 +1241,14 @@ static void mtk_venc_worker(struct
> work_struct *work)
> v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_ERROR);
> mtk_v4l2_err("venc_if_encode failed=%d", ret);
> } else {
> - v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
> - vb2_set_plane_payload(&dst_buf->vb2_buf, 0,
> enc_result.bs_size);
> - v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
> - mtk_v4l2_debug(2, "venc_if_encode bs size=%d",
> - enc_result.bs_size);
> + if (!IS_VENC_MULTICORE(ctx->dev->enc_capability)) {
> + v4l2_m2m_buf_done(src_buf, VB2_BUF_STATE_DONE);
> + vb2_set_plane_payload(&dst_buf->vb2_buf, 0,
> + enc_result.bs_size);
> + v4l2_m2m_buf_done(dst_buf, VB2_BUF_STATE_DONE);
> + mtk_v4l2_debug(2, "venc_if_encode bs size=%d",
> + enc_result.bs_size);
> + }
> }
>
> ctx->encoded_frame_cnt++;
> @@ -1453,6 +1462,34 @@ int mtk_vcodec_enc_queue_init(void *priv,
> struct vb2_queue *src_vq,
> return vb2_queue_init(dst_vq);
> }
>
> +void mtk_venc_buf_done(struct mtk_vcodec_ctx *ctx, int hw_id,
> + unsigned int bs_size, bool time_out, bool
> key_frame)
> +{
> + struct vb2_v4l2_buffer *src_vb2_v4l2 = NULL;
> + struct vb2_v4l2_buffer *dst_vb2_v4l2 = NULL;
> +
> + /*
> + * the frm_buf(src_buf) and bs_buf(dst_buf) can be obtained
> from ctx,
> + * then put them to done list, user can get them by dqbuf call
> + */
> + src_vb2_v4l2 = ctx->pfrm_buf[hw_id];
> + dst_vb2_v4l2 = ctx->pbs_buf[hw_id];
> +

Do those buffers require a Null check?

or we can just write
struct vb2_v4l2_buffer *src_vb2_v4l2 = tx->pfrm_buf[hw_id];
struct vb2_v4l2_buffer *dst_vb2_v4l2 = ctx->pbs_buf[hw_id];

BRs,
Allen

> + if (src_vb2_v4l2 && dst_vb2_v4l2) {
> + dst_vb2_v4l2->vb2_buf.timestamp =
> + src_vb2_v4l2->vb2_buf.timestamp;
> + dst_vb2_v4l2->timecode = src_vb2_v4l2->timecode;
> +
> + if (key_frame)
> + dst_vb2_v4l2->flags |= V4L2_BUF_FLAG_KEYFRAME;
> +
> + v4l2_m2m_buf_done(src_vb2_v4l2, VB2_BUF_STATE_DONE);
> + vb2_set_plane_payload(&dst_vb2_v4l2->vb2_buf, 0,
> bs_size);
> + v4l2_m2m_buf_done(dst_vb2_v4l2, VB2_BUF_STATE_DONE);
> + }
> +}
> +EXPORT_SYMBOL_GPL(mtk_venc_buf_done);
> +
> int mtk_venc_unlock(struct mtk_vcodec_ctx *ctx, int hw_id)
> {
> struct mtk_vcodec_dev *dev = ctx->dev;
> @@ -1460,6 +1497,7 @@ int mtk_venc_unlock(struct mtk_vcodec_ctx *ctx,
> int hw_id)
> mutex_unlock(&dev->enc_mutex[hw_id]);
> return 0;
> }
> +EXPORT_SYMBOL_GPL(mtk_venc_unlock);
>
> int mtk_venc_lock(struct mtk_vcodec_ctx *ctx, int hw_id)
> {
> @@ -1468,6 +1506,30 @@ int mtk_venc_lock(struct mtk_vcodec_ctx *ctx,
> int hw_id)
> mutex_lock(&dev->enc_mutex[hw_id]);
> return 0;
> }
> +EXPORT_SYMBOL_GPL(mtk_venc_lock);
> +
> +void mtk_venc_lock_all(struct mtk_vcodec_ctx *ctx)
> +{
> + unsigned int i;
> + struct mtk_vcodec_dev *dev = ctx->dev;
> +
> + /*
> + * For multi-core mode encoding, there are may be bufs being
> encoded
> + * when get the empty flush buffer or stop streaming, for
> example, the
> + * buffer with LAST flag will return to client before the
> encoding
> + * buffers, which will cause frame lost.
> + * The encoder device mutex will be locked during encoding
> process,
> + * when encode done, the mutex unlocked. So if all encoder
> device mutex
> + * can be locked, which means there are no bufs being encoded
> at this
> + * time, then the buffer with LAST flag can return to client
> properly.
> + */
> +
> + for (i = 0; i < MTK_VENC_HW_MAX; i++) {
> + mutex_lock(&dev->enc_mutex[i]);
> + mutex_unlock(&dev->enc_mutex[i]);
> + }
> +}
> +EXPORT_SYMBOL_GPL(mtk_venc_lock_all);
>
> void mtk_vcodec_enc_release(struct mtk_vcodec_ctx *ctx)
> {
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.h
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.h
> index 29f5c8d1b59f..5ab17381c7ba 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc.h
> @@ -20,6 +20,9 @@
>
> #define MTK_VENC_IRQ_STATUS_OFFSET 0x05C
> #define MTK_VENC_IRQ_ACK_OFFSET 0x060
> +#define VENC_PIC_BITSTREAM_BYTE_CNT 0x0098
> +#define VENC_PIC_FRM_TYPE 0x0010
> +#define VENC_PIC_KEY_FRM 0x2
>
> /**
> * struct mtk_video_enc_buf - Private data related to each VB2
> buffer.
> @@ -46,5 +49,7 @@ int mtk_vcodec_enc_queue_init(void *priv, struct
> vb2_queue *src_vq,
> void mtk_vcodec_enc_release(struct mtk_vcodec_ctx *ctx);
> int mtk_vcodec_enc_ctrls_setup(struct mtk_vcodec_ctx *ctx);
> void mtk_vcodec_enc_set_default_params(struct mtk_vcodec_ctx *ctx);
> -
> +void mtk_venc_buf_done(struct mtk_vcodec_ctx *ctx, int hw_id,
> + unsigned int bs_size, bool time_out, bool
> key_frame);
> +void mtk_venc_lock_all(struct mtk_vcodec_ctx *ctx);
> #endif /* _MTK_VCODEC_ENC_H_ */
> diff --git
> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
> index ba9c19a4d2cc..b34d7583fccc 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_drv.c
> @@ -89,6 +89,9 @@ static irqreturn_t mtk_vcodec_enc_irq_handler(int
> irq, void *priv)
> struct mtk_vcodec_ctx *ctx;
> unsigned long flags;
> void __iomem *addr;
> + unsigned int bs_size;
> + unsigned int frm_type;
> + bool is_key_frame = 0;
>
> spin_lock_irqsave(&dev->irqlock, flags);
> ctx = dev->curr_enc_ctx[MTK_VENC_CORE_0];
> @@ -101,8 +104,32 @@ static irqreturn_t
> mtk_vcodec_enc_irq_handler(int irq, void *priv)
> ctx->irq_status = readl(dev->reg_base[dev->venc_pdata->core_id]
> +
> (MTK_VENC_IRQ_STATUS_OFFSET));
>
> + bs_size = readl(dev->reg_base[dev->venc_pdata->core_id] +
> + (VENC_PIC_BITSTREAM_BYTE_CNT));
> + frm_type = readl(dev->reg_base[dev->venc_pdata->core_id] +
> + (VENC_PIC_FRM_TYPE));
> +
> clean_irq_status(ctx->irq_status, addr);
>
> + if (IS_VENC_MULTICORE(dev->enc_capability)) {
> + if (ctx->irq_status & MTK_VENC_IRQ_STATUS_FRM) {
> + if (ctx->hdr_size != 0) {
> + bs_size += ctx->hdr_size;
> + ctx->hdr_size = 0;
> + }
> +
> + if (frm_type & VENC_PIC_KEY_FRM)
> + is_key_frame = 1;
> +
> + mtk_venc_buf_done(ctx, 0, bs_size, 0,
> is_key_frame);
> + mtk_vcodec_enc_clock_off(dev, 0);
> + mtk_venc_unlock(ctx, 0);
> + } else {
> + wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED, 0);
> + }
> + return IRQ_HANDLED;
> + }
> +
> wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED, 0);
> return IRQ_HANDLED;
> }
> @@ -284,7 +311,6 @@ static int mtk_vcodec_probe(struct
> platform_device *pdev)
> goto err_res;
> }
>
> - irq_set_status_flags(dev->enc_irq, IRQ_NOAUTOEN);
> ret = devm_request_irq(&pdev->dev, dev->enc_irq,
> mtk_vcodec_enc_irq_handler,
> 0, pdev->name, dev);
> diff --git
> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_hw.c
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_hw.c
> index 6df5221b742f..0367e59b20a9 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_hw.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_hw.c
> @@ -50,6 +50,9 @@ static irqreturn_t mtk_enc_hw_irq_handler(int irq,
> void *priv)
> struct mtk_vcodec_ctx *ctx;
> unsigned long flags;
> void __iomem *addr;
> + unsigned int bs_size;
> + unsigned int frm_type;
> + bool is_key_frame = 0;
>
> spin_lock_irqsave(&main_dev->irqlock, flags);
> ctx = main_dev->curr_enc_ctx[dev->hw_id];
> @@ -61,9 +64,17 @@ static irqreturn_t mtk_enc_hw_irq_handler(int irq,
> void *priv)
>
> addr = dev->reg_base + MTK_VENC_IRQ_ACK_OFFSET;
> ctx->irq_status = readl(dev->reg_base +
> MTK_VENC_IRQ_STATUS_OFFSET);
> + bs_size = readl(dev->reg_base + VENC_PIC_BITSTREAM_BYTE_CNT);
> + frm_type = readl(dev->reg_base + VENC_PIC_FRM_TYPE);
> clean_hw_irq_status(ctx->irq_status, addr);
>
> - wake_up_ctx(ctx, MTK_INST_IRQ_RECEIVED, 0);
> + if (frm_type & VENC_PIC_KEY_FRM)
> + is_key_frame = 1;
> +
> + mtk_venc_buf_done(ctx, dev->hw_id, bs_size, 0, is_key_frame);
> + mtk_vcodec_enc_clock_off(main_dev, dev->hw_id);
> + mtk_venc_unlock(ctx, dev->hw_id);
> +
> return IRQ_HANDLED;
> }
>
> diff --git
> a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> index 2f83aade779a..af2968a8d2e5 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_enc_pm.c
> @@ -235,3 +235,4 @@ void mtk_vcodec_enc_clock_off(struct
> mtk_vcodec_dev *dev,
> for (i = enc_clk->clk_num - 1; i >= 0; i--)
> clk_disable(enc_clk->clk_info[i].vcodec_clk);
> }
> +EXPORT_SYMBOL_GPL(mtk_vcodec_enc_clock_off);
> diff --git a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> index 0033c53d5589..cecf78d6d4c6 100644
> --- a/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> +++ b/drivers/media/platform/mediatek/vcodec/mtk_vcodec_util.h
> @@ -15,6 +15,7 @@ struct mtk_vcodec_mem {
> size_t size;
> void *va;
> dma_addr_t dma_addr;
> + void *buf;
> };
>
> struct mtk_vcodec_fb {
> diff --git
> a/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> b/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> index 32497a35afb1..a6990221d845 100644
> --- a/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> +++ b/drivers/media/platform/mediatek/vcodec/venc/venc_h264_if.c
> @@ -22,7 +22,6 @@
> static const char h264_filler_marker[] = {0x0, 0x0, 0x0, 0x1, 0xc};
>
> #define H264_FILLER_MARKER_SIZE ARRAY_SIZE(h264_filler_marker)
> -#define VENC_PIC_BITSTREAM_BYTE_CNT 0x0098
>
> /*
> * enum venc_h264_frame_type - h264 encoder output bitstream frame
> type
> @@ -620,6 +619,11 @@ static int h264_encode_frame(struct
> venc_h264_inst *inst,
> return ret;
> }
>
> + if (IS_VENC_MULTICORE(ctx->dev->enc_capability)) {
> + ++inst->frm_cnt;
> + return ret;
> + }
> +
> irq_status = h264_enc_wait_venc_done(inst);
> if (irq_status != MTK_VENC_IRQ_STATUS_FRM) {
> mtk_vcodec_err(inst, "irq_status=%d failed",
> irq_status);
> @@ -705,8 +709,6 @@ static int h264_enc_encode(void *handle,
>
> mtk_vcodec_debug(inst, "opt %d ->", opt);
>
> - enable_irq(ctx->dev->enc_irq);
> -
> switch (opt) {
> case VENC_START_OPT_ENCODE_SEQUENCE_HEADER: {
> unsigned int bs_size_hdr;
> @@ -729,6 +731,13 @@ static int h264_enc_encode(void *handle,
> unsigned int bs_size_hdr;
> unsigned int bs_size_frm;
>
> + /*
> + * the frm_buf and bs_buf need to recorded into current
> ctx,
> + * when encoding done, the target buffers can be got
> from ctx.
> + */
> + ctx->pfrm_buf[ctx->hw_id] = frm_buf->frm_addr;
> + ctx->pbs_buf[ctx->hw_id] = bs_buf->buf;
> +
> if (!inst->prepend_hdr) {
> ret = h264_encode_frame(inst, frm_buf, bs_buf,
> &result->bs_size, ctx-
> >hw_id);
> @@ -763,7 +772,9 @@ static int h264_enc_encode(void *handle,
> if (ret)
> goto encode_err;
>
> - result->bs_size = hdr_sz + filler_sz + bs_size_frm;
> + ctx->hdr_size = hdr_sz + filler_sz;
> +
> + result->bs_size = ctx->hdr_size + bs_size_frm;
>
> mtk_vcodec_debug(inst, "hdr %d filler %d frame %d bs
> %d",
> hdr_sz, filler_sz, bs_size_frm,
> @@ -782,7 +793,6 @@ static int h264_enc_encode(void *handle,
>
> encode_err:
>
> - disable_irq(ctx->dev->enc_irq);
> mtk_vcodec_debug(inst, "opt %d <-", opt);
>
> return ret;
> diff --git a/drivers/media/platform/mediatek/vcodec/venc_drv_if.h
> b/drivers/media/platform/mediatek/vcodec/venc_drv_if.h
> index e676ccf6bd25..7e24b7f573d7 100644
> --- a/drivers/media/platform/mediatek/vcodec/venc_drv_if.h
> +++ b/drivers/media/platform/mediatek/vcodec/venc_drv_if.h
> @@ -108,9 +108,11 @@ struct venc_frame_info {
> /*
> * struct venc_frm_buf - frame buffer information used in
> venc_if_encode()
> * @fb_addr: plane frame buffer addresses
> + * @frm_addr: current v4l2 buffer address
> */
> struct venc_frm_buf {
> struct mtk_vcodec_fb fb_addr[MTK_VCODEC_MAX_PLANES];
> + void *frm_addr;
> };
>
> /*

Next message: Arnaldo Carvalho de Melo: "Re: [PATCH v3 00/23] Improvements to Intel perf metrics"
Previous message: Niklas Schnelle: "[PATCH v4 5/5] iommu/s390: Fix incorrect pgsize_bitmap"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]