linux-kernel - Re: [PATCH 4/8] soc: mediatek: mtk-cmdq: Add unsupported subsys ID programing flow

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <89d7718e29dd7ec80a2b457bca9a6f9cae888bac.camel@mediatek.com>
Date: Thu, 21 Nov 2024 07:10:55 +0000
From: CK Hu (胡俊光) <ck.hu@...iatek.com>
To: Jason-JH Lin (林睿祥) <Jason-JH.Lin@...iatek.com>,
	"chunkuang.hu@...nel.org" <chunkuang.hu@...nel.org>, "mchehab@...nel.org"
	<mchehab@...nel.org>, "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>, Moudy Ho (何宗原)
	<Moudy.Ho@...iatek.com>, Nancy Lin (林欣螢)
	<Nancy.Lin@...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, 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
> 
> > +		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);
> >  
> 
>