| Message ID | 20230918192204.32263-9-jason-jh.lin@mediatek.com |
|---|---|
| State | New |
| Headers | show |
| Series | Add CMDQ secure driver for SVP | expand |
Hi, Jason: On Tue, 2023-09-19 at 03:21 +0800, Jason-JH.Lin wrote: > Add cmdq_pkt_finalize_loop to CMDQ driver. > > cmdq_pkt_finalize_loop appends end of command(EOC) instruction and > jump to start of command buffer instruction to make the command > buffer loopable. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++++++++++++++++++++++ > include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c > b/drivers/soc/mediatek/mtk-cmdq-helper.c > index 4be2a18a4a02..bbb127620bb3 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -475,6 +475,29 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > } > EXPORT_SYMBOL(cmdq_pkt_finalize); > > +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt) > +{ > + struct cmdq_instruction inst = { {0} }; > + int err; > + > + /* insert EOC and generate IRQ for each command iteration */ > + inst.op = CMDQ_CODE_EOC; > + inst.value = CMDQ_EOC_IRQ_EN; > + err = cmdq_pkt_append_command(pkt, inst); > + if (err < 0) > + return err; > + > + /* JUMP to start of pkt */ > + err = cmdq_pkt_jump(pkt, pkt->pa_base); > + if (err < 0) > + return err; Could you explain the case that a loop thread would trigger an interrupt? In DRM crc function, the loop thread need not to trigger interrupt, so I'm curious about this. Regards, CK > + > + pkt->loop = true; > + > + return err; > +} > +EXPORT_SYMBOL(cmdq_pkt_finalize_loop); > + > int cmdq_pkt_flush_async(struct cmdq_pkt *pkt) > { > int err; > diff --git a/include/linux/soc/mediatek/mtk-cmdq.h > b/include/linux/soc/mediatek/mtk-cmdq.h > index 837ad8656adc..38a8e47da338 100644 > --- a/include/linux/soc/mediatek/mtk-cmdq.h > +++ b/include/linux/soc/mediatek/mtk-cmdq.h > @@ -323,6 +323,14 @@ int cmdq_pkt_jump(struct cmdq_pkt *pkt, > dma_addr_t addr); > */ > int cmdq_pkt_finalize(struct cmdq_pkt *pkt); > > +/** > + * cmdq_pkt_finalize_loop() - Append EOC and jump to start command. > + * @pkt: the CMDQ packet > + * > + * Return: 0 for success; else the error code is returned > + */ > +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt); > + > /** > * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute > the CMDQ > * packet and call back at the end of done > packet
On 18/09/2023 21:21, Jason-JH.Lin wrote: > Add cmdq_pkt_finalize_loop to CMDQ driver. > > cmdq_pkt_finalize_loop appends end of command(EOC) instruction and > jump to start of command buffer instruction to make the command > buffer loopable. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++++++++++++++++++++++ > include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c > index 4be2a18a4a02..bbb127620bb3 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -475,6 +475,29 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > } > EXPORT_SYMBOL(cmdq_pkt_finalize); > > +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt) > +{ > + struct cmdq_instruction inst = { {0} }; > + int err; > + > + /* insert EOC and generate IRQ for each command iteration */ > + inst.op = CMDQ_CODE_EOC; > + inst.value = CMDQ_EOC_IRQ_EN; > + err = cmdq_pkt_append_command(pkt, inst); > + if (err < 0) > + return err; > + > + /* JUMP to start of pkt */ > + err = cmdq_pkt_jump(pkt, pkt->pa_base); > + if (err < 0) > + return err; > + > + pkt->loop = true; > + > + return err; > +} > +EXPORT_SYMBOL(cmdq_pkt_finalize_loop); 1. Missing GPL 2. Missing kerneldoc 3. Missing user 4. Are you going to split the patchset into one function per patch? No. Best regards, Krzysztof
On 18/09/2023 21:21, Jason-JH.Lin wrote: > Add cmdq_pkt_finalize_loop to CMDQ driver. > > cmdq_pkt_finalize_loop appends end of command(EOC) instruction and > jump to start of command buffer instruction to make the command > buffer loopable. > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> > --- > drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++++++++++++++++++++++ > include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++ > 2 files changed, 31 insertions(+) > > diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c > index 4be2a18a4a02..bbb127620bb3 100644 > --- a/drivers/soc/mediatek/mtk-cmdq-helper.c > +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c > @@ -475,6 +475,29 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) > } > EXPORT_SYMBOL(cmdq_pkt_finalize); > > +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt) > +{ > + struct cmdq_instruction inst = { {0} }; > + int err; > + > + /* insert EOC and generate IRQ for each command iteration */ > + inst.op = CMDQ_CODE_EOC; > + inst.value = CMDQ_EOC_IRQ_EN; > + err = cmdq_pkt_append_command(pkt, inst); > + if (err < 0) > + return err; > + > + /* JUMP to start of pkt */ > + err = cmdq_pkt_jump(pkt, pkt->pa_base); > + if (err < 0) > + return err; > + > + pkt->loop = true; > + > + return err; > +} > +EXPORT_SYMBOL(cmdq_pkt_finalize_loop); NAK. No users (and please carefully think before you answer that your other patch uses it). Best regards, Krzysztof
On 25/09/2023 08:04, Jason-JH Lin (林睿祥) wrote: > Hi Krzysztof, > > Thanks for the reviews. > > On Sat, 2023-09-23 at 20:08 +0200, Krzysztof Kozlowski wrote: >> >> External email : Please do not click links or open attachments until >> you have verified the sender or the content. >> On 18/09/2023 21:21, Jason-JH.Lin wrote: >>> Add cmdq_pkt_finalize_loop to CMDQ driver. >>> >>> cmdq_pkt_finalize_loop appends end of command(EOC) instruction and >>> jump to start of command buffer instruction to make the command >>> buffer loopable. >>> >>> Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> >>> --- >>> drivers/soc/mediatek/mtk-cmdq-helper.c | 23 >> +++++++++++++++++++++++ >>> include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++ >>> 2 files changed, 31 insertions(+) >>> >>> diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c >> b/drivers/soc/mediatek/mtk-cmdq-helper.c >>> index 4be2a18a4a02..bbb127620bb3 100644 >>> --- a/drivers/soc/mediatek/mtk-cmdq-helper.c >>> +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c >>> @@ -475,6 +475,29 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) >>> } >>> EXPORT_SYMBOL(cmdq_pkt_finalize); >>> >>> +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt) >>> +{ >>> +struct cmdq_instruction inst = { {0} }; >>> +int err; >>> + >>> +/* insert EOC and generate IRQ for each command iteration */ >>> +inst.op = CMDQ_CODE_EOC; >>> +inst.value = CMDQ_EOC_IRQ_EN; >>> +err = cmdq_pkt_append_command(pkt, inst); >>> +if (err < 0) >>> +return err; >>> + >>> +/* JUMP to start of pkt */ >>> +err = cmdq_pkt_jump(pkt, pkt->pa_base); >>> +if (err < 0) >>> +return err; >>> + >>> +pkt->loop = true; >>> + >>> +return err; >>> +} >>> +EXPORT_SYMBOL(cmdq_pkt_finalize_loop); >> >> NAK. No users (and please carefully think before you answer that your >> other patch uses it). >> > > As I know, the API with EXPORT_SYMBOL means it can be used by a dynamic > loaded module. > > Do you means that mtk-cmdq-sec-mailbox.c in [PATCH 10/15] is a built-in > module currently, so I should not add EXPORT_SYMBOL to this API? I mean there is no need for this. Please name the name of module where this function will be defined and name of module(s) using it. ".c" is not a module. ".ko" is. Best regards, Krzysztof
diff --git a/drivers/soc/mediatek/mtk-cmdq-helper.c b/drivers/soc/mediatek/mtk-cmdq-helper.c index 4be2a18a4a02..bbb127620bb3 100644 --- a/drivers/soc/mediatek/mtk-cmdq-helper.c +++ b/drivers/soc/mediatek/mtk-cmdq-helper.c @@ -475,6 +475,29 @@ int cmdq_pkt_finalize(struct cmdq_pkt *pkt) } EXPORT_SYMBOL(cmdq_pkt_finalize); +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt) +{ + struct cmdq_instruction inst = { {0} }; + int err; + + /* insert EOC and generate IRQ for each command iteration */ + inst.op = CMDQ_CODE_EOC; + inst.value = CMDQ_EOC_IRQ_EN; + err = cmdq_pkt_append_command(pkt, inst); + if (err < 0) + return err; + + /* JUMP to start of pkt */ + err = cmdq_pkt_jump(pkt, pkt->pa_base); + if (err < 0) + return err; + + pkt->loop = true; + + return err; +} +EXPORT_SYMBOL(cmdq_pkt_finalize_loop); + int cmdq_pkt_flush_async(struct cmdq_pkt *pkt) { int err; diff --git a/include/linux/soc/mediatek/mtk-cmdq.h b/include/linux/soc/mediatek/mtk-cmdq.h index 837ad8656adc..38a8e47da338 100644 --- a/include/linux/soc/mediatek/mtk-cmdq.h +++ b/include/linux/soc/mediatek/mtk-cmdq.h @@ -323,6 +323,14 @@ int cmdq_pkt_jump(struct cmdq_pkt *pkt, dma_addr_t addr); */ int cmdq_pkt_finalize(struct cmdq_pkt *pkt); +/** + * cmdq_pkt_finalize_loop() - Append EOC and jump to start command. + * @pkt: the CMDQ packet + * + * Return: 0 for success; else the error code is returned + */ +int cmdq_pkt_finalize_loop(struct cmdq_pkt *pkt); + /** * cmdq_pkt_flush_async() - trigger CMDQ to asynchronously execute the CMDQ * packet and call back at the end of done packet
Add cmdq_pkt_finalize_loop to CMDQ driver. cmdq_pkt_finalize_loop appends end of command(EOC) instruction and jump to start of command buffer instruction to make the command buffer loopable. Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com> --- drivers/soc/mediatek/mtk-cmdq-helper.c | 23 +++++++++++++++++++++++ include/linux/soc/mediatek/mtk-cmdq.h | 8 ++++++++ 2 files changed, 31 insertions(+)