[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <f4e6e86e0c929d45ca1af123fd7f4bb98072addb.camel@mediatek.com>
Date: Thu, 21 Nov 2024 10:41:55 +0000
From: Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>
To: "chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "mchehab@...nel.org"
<mchehab@...nel.org>, CK Hu (胡俊光)
<ck.hu@...iatek.com>, "conor+dt@...nel.org" <conor+dt@...nel.org>,
"robh@...nel.org" <robh@...nel.org>, "matthias.bgg@...il.com"
<matthias.bgg@...il.com>, "jassisinghbrar@...il.com"
<jassisinghbrar@...il.com>, "krzk+dt@...nel.org" <krzk+dt@...nel.org>,
AngeloGioacchino Del Regno <angelogioacchino.delregno@...labora.com>
CC: "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
"linux-mediatek@...ts.infradead.org" <linux-mediatek@...ts.infradead.org>,
Singo Chang (張興國) <Singo.Chang@...iatek.com>,
"linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
"devicetree@...r.kernel.org" <devicetree@...r.kernel.org>, "simona@...ll.ch"
<simona@...ll.ch>, Nancy Lin (林欣螢)
<Nancy.Lin@...iatek.com>, Moudy Ho (何宗原)
<Moudy.Ho@...iatek.com>, "dri-devel@...ts.freedesktop.org"
<dri-devel@...ts.freedesktop.org>, Project_Global_Chrome_Upstream_Group
<Project_Global_Chrome_Upstream_Group@...iatek.com>, "airlied@...il.com"
<airlied@...il.com>, "linux-arm-kernel@...ts.infradead.org"
<linux-arm-kernel@...ts.infradead.org>
Subject: Re: [PATCH 4/8] soc: mediatek: mtk-cmdq: Add unsupported subsys ID
programing flow
Hi CK,
Thanks for the review.
On Thu, 2024-11-21 at 07:10 +0000, CK Hu (胡俊光) wrote:
> Hi, Jason:
>
> On Thu, 2024-11-21 at 14:38 +0800, CK Hu wrote:
> > Hi, Jason:
> >
> > On Thu, 2024-11-21 at 12:25 +0800, Jason-JH.Lin wrote:
> > > When GCE executes instructions, the corresponding hardware
> > > register
> > > can be found through the subsys ID.
> > > For unsupported subsys ID hardware, the physical address need to
> > > be used
> > > to generate GCE instructions.
> > >
> > > Add the pa_base interface to the instruction programming flow for
> > > these
> > > unsupported subsys ID hardware.
> > >
> > > Signed-off-by: Jason-JH.Lin <jason-jh.lin@...iatek.com>
> > > ---
> >
> > [snip]
> >
> > > -int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u16 offset,
> > > u32 value)
> > > +int cmdq_pkt_write(struct cmdq_pkt *pkt, u8 subsys, u32 pa_base,
> > > u16 offset, u32 value)
> > > {
> > > + struct cmdq_client *cl = (struct cmdq_client *)pkt->cl;
> > > struct cmdq_instruction inst = {
> > > .op = CMDQ_CODE_WRITE,
> > > .value = value,
> > > .offset = offset,
> > > .subsys = subsys
> > > };
> > > - return cmdq_pkt_append_command(pkt, inst);
> > > + int err;
> > > +
> > > + if (!cl) {
> > > + pr_err("%s %d: pkt->cl is NULL!\n", __func__,
> > > __LINE__);
> > > + return -EINVAL;
> > > + }
> > > +
> > > + if (cmdq_subsys_is_valid(cl->chan, subsys)) {
> >
> > I would like to have a new API for no subsys. Maybe
> > cmdq_pkt_write_pa().
> > If some client driver always have subsys, it could use
> > cmdq_pkt_write().
> > If some client driver have no subsys, it could use
> > cmdq_pkt_write_pa().
> > This would prevent frequently conditional jump in this function.
> > If some client driver have subsys in some SoC and have no subsys in
> > other SoC,
> > let the conditional jump happen in that client driver.
> > (The client driver could use 'likely' or 'unlikely' to uptimize)
> > In the view point to let client driver have fine-grained control,
> > maybe client could use cmdq_pkt_assign() and
> > cmdq_pkt_write_s_value() to achieve this so it's not necessary to
> > invent new API.
>
> For a client driver, the high address is usually a constant value.
> So the client could have command like:
>
> cmdq_pkt_assign(pkt, 0, CMDQ_ADDR_HIGH(pa_base));
>
> cmdq_pkt_write_s_value(pkt, 0, CMDQ_ADDR_LOW(offset1), value1);
> cmdq_pkt_write_s_value(pkt, 0, CMDQ_ADDR_LOW(offset2), value2);
> cmdq_pkt_write_s_value(pkt, 0, CMDQ_ADDR_LOW(offset3), value3);
> cmdq_pkt_write_s_value(pkt, 0, CMDQ_ADDR_LOW(offset4), value4);
> cmdq_pkt_write_s_value(pkt, 0, CMDQ_ADDR_LOW(offset5), value5);
> cmdq_pkt_write_s_value(pkt, 0, CMDQ_ADDR_LOW(offset6), value6);
>
> This would reduce the command size.
> GCE would execute more quickly.
>
> Regards,
> CK
>
> >
> > Regards,
> > CK
I think that might be a wide rage for changing all the client's code.
But I'll try to implement that in client drivers and see if there is
any problem and feedback to you.
Regards,
Jason-JH.Lin
> >
> > > + err = cmdq_pkt_append_command(pkt, inst);
> > > + } else {
> > > + err = cmdq_pkt_assign(pkt, 0, CMDQ_ADDR_HIGH(pa_base));
> > > + if (err < 0)
> > > + return err;
> > > +
> > > + err = cmdq_pkt_write_s_value(pkt, 0,
> > > CMDQ_ADDR_LOW(offset), value);
> > > + }
> > > +
> > > + return err;
> > > }
> > > EXPORT_SYMBOL(cmdq_pkt_write);
> > >
> >
> >
Powered by blists - more mailing lists