linux-kernel - Re: [PATCH 6/9] media: platform: mtk-mdp3: drop calling cmdq_pkt

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <af4bfdd52f0ca022730aedcb456bb1a0b105ae3e.camel@mediatek.com>
Date: Fri, 16 Feb 2024 06:20:54 +0000
From: Moudy Ho (何宗原) <Moudy.Ho@...iatek.com>
To: "linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
	"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
	"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "mchehab@...nel.org"
	<mchehab@...nel.org>, "linux-arm-kernel@...ts.infradead.org"
	<linux-arm-kernel@...ts.infradead.org>, "matthias.bgg@...il.com"
	<matthias.bgg@...il.com>, "angelogioacchino.delregno@...labora.com"
	<angelogioacchino.delregno@...labora.com>
Subject: Re: [PATCH 6/9] media: platform: mtk-mdp3: drop calling
 cmdq_pkt_finalize()

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@...nel.org>
> > ---
> >   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;
> 
>