On Thu, 2024-02-15 at 11:29 +0100, AngeloGioacchino Del Regno wrote: > Il 15/02/24 01:49, Chun-Kuang Hu ha scritto: > > Because client driver has the information of struct cmdq_client, so > > it's not necessary to store struct cmdq_client in struct cmdq_pkt. > > cmdq_pkt_finalize() use struct cmdq_client in struct cmdq_pkt, so > > it's > > going to be abandoned. Therefore, use cmdq_pkt_eoc() and > > cmdq_pkt_nop() > > to replace cmdq_pkt_finalize(). > > No, it's not because cmdq_pkt_finalize() has cmdq_client, but because > we want > finer grain control over the CMDQ packets, as not all cases require > the NOP > packet to be appended after EOC. > > Besides, honestly I'm not even sure if the NOP is always required in > MDP3, so... > > ...Moudy, you know the MDP3 way better than anyone else - can you > please > check if NOP is actually needed here? > > Thanks! > Angelo > Hi Angelo, After confirming with the hardware designer and performing the video playback test, it was discovered that MDP3 is capable of excluding the NOP(jump) instruction. Sincerely, Moudy > > > > Signed-off-by: Chun-Kuang Hu <chunkuang.hu@xxxxxxxxxx> > > --- > > drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c | 3 ++- > > drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c | 2 ++ > > drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h | 1 + > > 3 files changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c > > index 6adac857a477..a420d492d879 100644 > > --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-cmdq.c > > @@ -471,7 +471,8 @@ int mdp_cmdq_send(struct mdp_dev *mdp, struct > > mdp_cmdq_param *param) > > dev_err(dev, "mdp_path_config error\n"); > > goto err_free_path; > > } > > - cmdq_pkt_finalize(&cmd->pkt); > > + cmdq_pkt_eoc(&cmd->pkt); > > + cmdq_pkt_nop(&cmd->pkt, mdp->cmdq_shift_pa); > > > > for (i = 0; i < num_comp; i++) > > memcpy(&comps[i], path->comps[i].comp, > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > index 94f4ed78523b..2214744c937c 100644 > > --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.c > > @@ -231,6 +231,8 @@ static int mdp_probe(struct platform_device > > *pdev) > > goto err_put_scp; > > } > > > > + mdp->cmdq_shift_pa = cmdq_get_shift_pa(mdp->cmdq_clt->chan); > > + > > init_waitqueue_head(&mdp->callback_wq); > > ida_init(&mdp->mdp_ida); > > platform_set_drvdata(pdev, mdp); > > diff --git a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > index 7e21d226ceb8..ed61e0bb69ee 100644 > > --- a/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > +++ b/drivers/media/platform/mediatek/mdp3/mtk-mdp3-core.h > > @@ -83,6 +83,7 @@ struct mdp_dev { > > u32 id_count; > > struct ida mdp_ida; > > struct cmdq_client *cmdq_clt; > > + u8 cmdq_shift_pa; > > wait_queue_head_t callback_wq; > > > > struct v4l2_device v4l2_dev; > >