Hi CK,
Sorry to reply your suggestion too late.
Thanks for your help to review this patch.
On Fri, 2024-05-17 at 09:53 +0000, CK Hu (胡俊光) wrote:
> Hi, Yunfei:
>
> On Thu, 2024-05-16 at 20:20 +0800, Yunfei Dong wrote:
> > Waiting interrupt in optee-os for svp mode, need to disable it in
> > kernel
> > in case of interrupt is cleaned.
> >
> > Signed-off-by: Yunfei Dong <yunfei.dong@xxxxxxxxxxxx>
> > ---
> > .../vcodec/decoder/mtk_vcodec_dec_hw.c | 34 +++++------
> > .../vcodec/decoder/mtk_vcodec_dec_pm.c | 6 +-
> > .../decoder/vdec/vdec_h264_req_multi_if.c | 57 +++++++++++--
> > ------
> > 3 files changed, 54 insertions(+), 43 deletions(-)
> >
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.
> > c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.
> > c
> > index 881d5de41e05..1982c088c6da 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.
> > c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_hw.
> > c
> > @@ -72,26 +72,28 @@ static irqreturn_t mtk_vdec_hw_irq_handler(int
> > irq, void *priv)
> >
> > ctx = mtk_vcodec_get_curr_ctx(dev->main_dev, dev->hw_idx);
> >
> > - /* check if HW active or not */
> > - cg_status = readl(dev->reg_base[VDEC_HW_SYS] +
> > VDEC_HW_ACTIVE_ADDR);
> > - if (cg_status & VDEC_HW_ACTIVE_MASK) {
> > - mtk_v4l2_vdec_err(ctx, "vdec active is not 0x0
> > (0x%08x)", cg_status);
> > - return IRQ_HANDLED;
> > - }
> > + if (!ctx->is_secure_playback) {
> > + /* check if HW active or not */
> > + cg_status = readl(dev->reg_base[VDEC_HW_SYS] +
> > VDEC_HW_ACTIVE_ADDR);
> > + if (cg_status & VDEC_HW_ACTIVE_MASK) {
> > + mtk_v4l2_vdec_err(ctx, "vdec active is not 0x0
> > (0x%08x)", cg_status);
> > + return IRQ_HANDLED;
> > + }
> >
> > - dec_done_status = readl(vdec_misc_addr);
> > - if ((dec_done_status & MTK_VDEC_IRQ_STATUS_DEC_SUCCESS) !=
> > - MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)
> > - return IRQ_HANDLED;
> > + dec_done_status = readl(vdec_misc_addr);
> > + if ((dec_done_status & MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)
> > !=
> > + MTK_VDEC_IRQ_STATUS_DEC_SUCCESS)
> > + return IRQ_HANDLED;
> >
> > - /* clear interrupt */
> > - writel(dec_done_status | VDEC_IRQ_CFG, vdec_misc_addr);
> > - writel(dec_done_status & ~VDEC_IRQ_CLR, vdec_misc_addr);
> > + /* clear interrupt */
> > + writel(dec_done_status | VDEC_IRQ_CFG, vdec_misc_addr);
> > + writel(dec_done_status & ~VDEC_IRQ_CLR,
> > vdec_misc_addr);
> >
> > - wake_up_dec_ctx(ctx, MTK_INST_IRQ_RECEIVED, dev->hw_idx);
> > + wake_up_dec_ctx(ctx, MTK_INST_IRQ_RECEIVED, dev-
> > >hw_idx);
> >
> > - mtk_v4l2_vdec_dbg(3, ctx, "wake up ctx %d, dec_done_status=%x",
> > - ctx->id, dec_done_status);
> > + mtk_v4l2_vdec_dbg(3, ctx, "wake up ctx %d,
> > dec_done_status=%x",
> > + ctx->id, dec_done_status);
> > + }
> >
> > return IRQ_HANDLED;
> > }
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.
> > c
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.
> > c
> > index aefd3e9e3061..a94eda936f16 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.
> > c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/mtk_vcodec_dec_pm.
> > c
> > @@ -238,7 +238,8 @@ void mtk_vcodec_dec_enable_hardware(struct
> > mtk_vcodec_dec_ctx *ctx, int hw_idx)
> > mtk_vcodec_dec_child_dev_on(ctx->dev, MTK_VDEC_LAT0);
> > mtk_vcodec_dec_child_dev_on(ctx->dev, hw_idx);
> >
> > - mtk_vcodec_dec_enable_irq(ctx->dev, hw_idx);
> > + if (!ctx->is_secure_playback)
> > + mtk_vcodec_dec_enable_irq(ctx->dev, hw_idx);
> >
> > if (IS_VDEC_INNER_RACING(ctx->dev->dec_capability))
> > mtk_vcodec_load_racing_info(ctx);
> > @@ -250,7 +251,8 @@ void mtk_vcodec_dec_disable_hardware(struct
> > mtk_vcodec_dec_ctx *ctx, int hw_idx)
> > if (IS_VDEC_INNER_RACING(ctx->dev->dec_capability))
> > mtk_vcodec_record_racing_info(ctx);
> >
> > - mtk_vcodec_dec_disable_irq(ctx->dev, hw_idx);
> > + if (!ctx->is_secure_playback)
> > + mtk_vcodec_dec_disable_irq(ctx->dev, hw_idx);
> >
> > mtk_vcodec_dec_child_dev_off(ctx->dev, hw_idx);
> > if (IS_VDEC_LAT_ARCH(ctx->dev->vdec_pdata->hw_arch) &&
> > diff --git
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
> > _multi_if.c
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
> > _multi_if.c
> > index f27d326f00ba..3186777dcc13 100644
> > ---
> > a/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
> > _multi_if.c
> > +++
> > b/drivers/media/platform/mediatek/vcodec/decoder/vdec/vdec_h264_req
> > _multi_if.c
> > @@ -593,14 +593,16 @@ static int vdec_h264_slice_core_decode(struct
> > vdec_lat_buf *lat_buf)
> > goto vdec_dec_end;
> > }
> >
> > - /* wait decoder done interrupt */
> > - timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx,
> > MTK_INST_IRQ_RECEIVED,
> > - WAIT_INTR_TIMEOUT_MS,
> > MTK_VDEC_CORE);
> > - if (timeout)
> > - mtk_vdec_err(ctx, "core decode timeout: pic_%d", ctx-
> > >decoded_frame_cnt);
> > - inst->vsi_core->dec.timeout = !!timeout;
> > -
> > - vpu_dec_core_end(vpu);
> > + if (!ctx->is_secure_playback) {
> > + /* wait decoder done interrupt */
> > + timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx,
> > MTK_INST_IRQ_RECEIVED,
> > + WAIT_INTR_TIMEOU
> > T_MS, MTK_VDEC_CORE);
> > + if (timeout)
> > + mtk_vdec_err(ctx, "core decode timeout:
> > pic_%d", ctx->decoded_frame_cnt);
> > + inst->vsi_core->dec.timeout = !!timeout;
> > +
> > + vpu_dec_core_end(vpu);
> > + }
>
> In svp mode, optee driver would wait the irq, but why optee driver
> does not notify normal world driver? The normal world driver here
> does not wait anything and keep going, so it would get the frame
> which has not been processed done and pass it to next module such as
> mdp? The normal world driver does not wait here, how does it make
> sure the frame is processed done?
>
Normal and secure world can receive the interrupt at the same time when
play secure video.
The driver process the interrupt and clean the interrupt in optee-os
when play secure video, not normal world. So the driver needn't to wait
interrupt and clean the interrupt in normal world
> Regards,
>
> CK
>
Best Regards,
Yunfei Dong
> > mtk_vdec_debug(ctx, "pic[%d] crc: 0x%x 0x%x 0x%x 0x%x 0x%x 0x%x
> > 0x%x 0x%x",
> > ctx->decoded_frame_cnt,
> > inst->vsi_core->dec.crc[0], inst->vsi_core-
> > >dec.crc[1],
> > @@ -724,14 +726,16 @@ static int vdec_h264_slice_lat_decode(void
> > *h_vdec, struct mtk_vcodec_mem *bs,
> > vdec_msg_queue_qbuf(&inst->ctx->msg_queue.core_ctx,
> > lat_buf);
> > }
> >
> > - /* wait decoder done interrupt */
> > - timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx,
> > MTK_INST_IRQ_RECEIVED,
> > - WAIT_INTR_TIMEOUT_MS,
> > MTK_VDEC_LAT0);
> > - if (timeout)
> > - mtk_vdec_err(inst->ctx, "lat decode timeout: pic_%d",
> > inst->slice_dec_num);
> > - inst->vsi->dec.timeout = !!timeout;
> > + if (!inst->ctx->is_secure_playback) {
> > + /* wait decoder done interrupt */
> > + timeout = mtk_vcodec_wait_for_done_ctx(inst->ctx,
> > MTK_INST_IRQ_RECEIVED,
> > + WAIT_INTR_TIMEOU
> > T_MS, MTK_VDEC_LAT0);
> > + if (timeout)
> > + mtk_vdec_err(inst->ctx, "lat decode timeout:
> > pic_%d", inst->slice_dec_num);
> > + inst->vsi->dec.timeout = !!timeout;
> >
> > - err = vpu_dec_end(vpu);
> > + err = vpu_dec_end(vpu);
> > + }
> > if (err == SLICE_HEADER_FULL || err == TRANS_BUFFER_FULL) {
> > if (!IS_VDEC_INNER_RACING(inst->ctx->dev-
> > >dec_capability))
> > vdec_msg_queue_qbuf(&inst->ctx-
> > >msg_queue.lat_ctx, lat_buf);
> > @@ -831,16 +835,19 @@ static int vdec_h264_slice_single_decode(void
> > *h_vdec, struct mtk_vcodec_mem *bs
> > if (err)
> > goto err_free_fb_out;
> >
> > - /* wait decoder done interrupt */
> > - err = mtk_vcodec_wait_for_done_ctx(inst->ctx,
> > MTK_INST_IRQ_RECEIVED,
> > - WAIT_INTR_TIMEOUT_MS,
> > MTK_VDEC_CORE);
> > - if (err)
> > - mtk_vdec_err(inst->ctx, "decode timeout: pic_%d", inst-
> > >ctx->decoded_frame_cnt);
> > -
> > - inst->vsi->dec.timeout = !!err;
> > - err = vpu_dec_end(vpu);
> > - if (err)
> > - goto err_free_fb_out;
> > + if (!inst->ctx->is_secure_playback) {
> > + /* wait decoder done interrupt */
> > + err = mtk_vcodec_wait_for_done_ctx(inst->ctx,
> > MTK_INST_IRQ_RECEIVED,
> > + WAIT_INTR_TIMEOUT_MS
> > , MTK_VDEC_CORE);
> > + if (err)
> > + mtk_vdec_err(inst->ctx, "decode timeout:
> > pic_%d",
> > + inst->ctx->decoded_frame_cnt);
> > +
> > + inst->vsi->dec.timeout = !!err;
> > + err = vpu_dec_end(vpu);
> > + if (err)
> > + goto err_free_fb_out;
> > + }
> >
> > memcpy(&inst->vsi_ctx, inst->vpu.vsi, sizeof(inst->vsi_ctx));
> > mtk_vdec_debug(inst->ctx, "pic[%d] crc: 0x%x 0x%x 0x%x 0x%x
> > 0x%x 0x%x 0x%x 0x%x",
