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@xxxxxxxxxxxx>
> > ---
>
> [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
>
> > + 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);
> >
>
>
