[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <b3c384b6-7c8f-4428-a62e-1036b78217cf@collabora.com>
Date: Fri, 16 Feb 2024 08:56:08 +0100
From: AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
To: Moudy Ho (何宗原) <Moudy.Ho@...iatek.com>,
"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>
Subject: Re: [PATCH 6/9] media: platform: mtk-mdp3: drop calling
cmdq_pkt_finalize()
Il 16/02/24 07:20, Moudy Ho (何宗原) ha scritto:
> 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.
>
Thank you for this extremely fast clarification.
Cheers,
Angelo
> 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;
>>
>>
Powered by blists - more mailing lists