<html><body><p>
<pre>
Hi CK,

Thanks for the reviews.

On Wed, 2024-12-11 at 04:04 +0000, CK Hu (胡俊光) wrote:
> On Wed, 2024-12-11 at 11:46 +0800, CK Hu wrote:
> > Hi, Jason:
> >
> > On Wed, 2024-12-11 at 11:22 +0800, Jason-JH.Lin wrote:
> > > To support hardware without subsys IDs on new SoCs, add a
> > > programming
> > > flow that checks whether the subsys ID is valid. If the subsys ID
> > > is
> > > invalid, the flow will call 2 alternative CMDQ APIs:
> > > cmdq_pkt_assign() and cmdq_pkt_write_s_value() to achieve the
> > > same
> > > functionality.
> > >
> > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@mediatek.com>
> > > ---
> > > drivers/gpu/drm/mediatek/mtk_ddp_comp.c | 34
> > > ++++++++++++++++++++-----
> > > 1 file changed, 28 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > > b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > > index edc6417639e6..0792c895526f 100644
> > > --- a/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > > +++ b/drivers/gpu/drm/mediatek/mtk_ddp_comp.c
> > > @@ -66,14 +66,38 @@ struct mtk_ddp_comp_dev {
> > > struct cmdq_client_reg cmdq_reg;
> > > };
> > >
> > > +#if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > > +static void mtk_ddp_write_cmdq_pkt(struct cmdq_pkt *cmdq_pkt,
> > > struct cmdq_client_reg *cmdq_reg,
> > > + unsigned int offset, unsigned int
> > > value, unsigned int mask)
> >
> > Drop this function.
>
> Sorry, it seems cmdq_subsys_is_valid() is used to check the SoC
> support new API or not.
> But I would try to find out a way not to always check using new API
> or not.
>

OK, I can help you test it, if you have any idea for this.

I'll use `cl->chan, cmdq_reg->subsys == INVALID_SUBSYS` instead of
calling `cmdq_subsys_is_valid()` to avoid function calls.

> Regards,
> CK
>
> >
> > > +{
> > > +struct cmdq_client *cl = (struct cmdq_client *)cmdq_pkt->cl;
> > > +
> > > +offset += cmdq_reg->offset;
> > > +
> > > +if (cmdq_subsys_is_valid(cl->chan, cmdq_reg->subsys)) {
> > > +if (mask == GENMASK(31, 0))
> > > +cmdq_pkt_write(cmdq_pkt, cmdq_reg->subsys,
> > > offset, value);
> > > +else
> > > +cmdq_pkt_write_mask(cmdq_pkt, cmdq_reg->subsys,
> > > offset, value, mask);
> > > +} else {
> > > +/* only MMIO access, no need to check mminfro_offset */
> > > +cmdq_pkt_assign(cmdq_pkt, 0, CMDQ_ADDR_HIGH(cmdq_reg-
> > > >pa_base));
> > > +if (mask == GENMASK(31, 0))
> > > +cmdq_pkt_write_s_value(cmdq_pkt, 0,
> > > CMDQ_ADDR_LOW(offset), value);
> > > +else
> > > +cmdq_pkt_write_s_mask_value(cmdq_pkt, 0,
> > > CMDQ_ADDR_LOW(offset),
> > > + value, mask);
> > > +}
> > > +}
> > > +#endif

[snip]

> > > else
> > > #endif
> > > writel_relaxed(value, regs + offset);
> > > @@ -98,8 +121,7 @@ void mtk_ddp_write_mask(struct cmdq_pkt
> > > *cmdq_pkt, unsigned int value,
> > > {
> > > #if IS_REACHABLE(CONFIG_MTK_CMDQ)
> > > if (cmdq_pkt) {
> > > -cmdq_pkt_write_mask(cmdq_pkt, cmdq_reg->subsys,
> > > - cmdq_reg->offset + offset, value,
> > > mask);
> > > +mtk_ddp_write_cmdq_pkt(cmdq_pkt, cmdq_reg, offset,
> > > value, mask);
> >
> > /* only MMIO access, no need to check mminfro_offset */
> > cmdq_pkt_assign(cmdq_pkt, CMDQ_XXXREG_0, CMDQ_ADDR_HIGH(cmdq_reg-
> > >pa_base));
> > cmdq_pkt_write_s_mask_value(cmdq_pkt, CMDQ_XXXREG_0,
> > CMDQ_ADDR_LOW(offset),
> > value, mask);
> >
> > CMDQ_XXXREG_0 is defined in cmdq header file.
> >

OK, I'll use CMDQ_THR_SPR_IDX0 instead.

Regards,
Jason-JH.Lin

> > Regards,
> > CK

</pre>
</p></body></html><!--type:text--><!--{--><pre>************* MEDIATEK Confidentiality Notice ********************
The information contained in this e-mail message (including any 
attachments) may be confidential, proprietary, privileged, or otherwise
exempt from disclosure under applicable laws. It is intended to be 
conveyed only to the designated recipient(s). Any use, dissemination, 
distribution, printing, retaining or copying of this e-mail (including its 
attachments) by unintended recipient(s) is strictly prohibited and may 
be unlawful. If you are not an intended recipient of this e-mail, or believe 
that you have received this e-mail in error, please notify the sender 
immediately (by replying to this e-mail), delete any and all copies of 
this e-mail (including any attachments) from your system, and do not
disclose the content of this e-mail to any other person. Thank you!
</pre><!--}-->