Re: [PATCH] arm64: dts: sdm845: Add iommus property to qup1

[Stephen Boyd]

Quoting Vivek Gautam (2019-06-06 04:17:16)
> Hi Stephen,
> 
> On Thu, Jun 6, 2019 at 2:27 AM Stephen Boyd <swboyd@xxxxxxxxxxxx> wrote:
> >
> > Quoting Vivek Gautam (2019-06-04 21:55:26)
> >
> > >
> > > Cheza will throw faults for anything that is programmed with TZ on mtp
> > > as all of that should be handled in HLOS. The firmwares of all these
> > > peripherals assume that the SID reservation is done (whether in TZ or HLOS).
> > >
> > > I am inclined to moving the iommus property for all 'TZ' to board dts files.
> > > MTP wouldn't need those SIDs. So, the SOC level dtsi will have just the
> > > HLOS SIDs.
> >
> > So you're saying you'd like to have the SID be <&apps_smmu 0x6c3 0x0> in
> > the sdm845.dtsi file and then override this on Cheza because our SID is
> > different (possibly because we don't use GSI)? Why can't we program the
> > SID in Cheza firmware to match the "HLOS" SID of 0x6c3?
> 
> Sorry my bad, I missed the overriding part.
> May be we add the lists of SIDs in board dts only. So, cheza dts will
> have all these SIDs -
> <&apps_smmu 0x6c0 0x3>   // for both 0x6c0 (TZ) and 0x6c3 (HLOS)
> <&apps_smmu 0x6d6 0x0>   // if we want to use the GSI dma.
> and
> MTP will have
> <&apps_smmu 0x6c3 0x0>
> <&apps_smmu 0x6d6 0x0>
> WDUT?

I'd prefer to fix the firmware so that the HLOS SID is used even on this
board. Making Cheza use something different from MTP doesn't sound so
good. Do you know how that works? Is there some configuration register
or something that I should be looking for to see why the SID is not the
HLOS one? It's definitely generating SIDs for the TZ SID (0x6c0), but
I'd like to make sure that we can't change it because it's tied to some
hardware signal like the NS bit and/or the Execution Level. Hopefully
it's a config and then our difference from MTP can be minimized.

As far as I can tell, HLOS on SDM845 has always used GPI (yet another
DMA engine) to do the DMA transfers. And the GPI is the hardware block
that uses the SID of 0x6d6 above, so putting that into iommus for the
geniqup doesn't make any sense given that GPI is another node. Can you
confirm this is the case? Furthermore, the SID of 0x6c3 sounds untested?
Has it ever been generated on SDM845 MTP?

If we ever support GPI, I'd expect to see something like this in DT:

	gpi_dma: gpi@a00000 {
		reg = <0x00a00000 0x60000>;
		iommus = <&apps_smmu 0x6d6 0x0>;
		...
	};

	geniqup@ac0000 {
		reg = <0x00ac0000 0x6000>;
		iommus = <&apps_smmu 0x6c3 0x0>;

		i2c@....{

			dmas = <&gpi_dma ....>;
		};

But now I'm worried that the geniqup needs the proper geniqup wrapper
clks to talk to it. Most likely the GPI is embedded inside the geniqup
wrapper and sits right next to the bus to do bus DMA mastering. From the
DT side, it means we should either put it inside the geniqup node, or we
should add the wrapper clks to the GPI node and hope things work out
with regards to clks and shared resources being used at the right time.

If we're left with trying to figure out how to express the different
SIDs depending on the CPU execution state then it may be easier to push
for GPI upstreaming and use that dma engine to "fold" the SID
numberspace into one SID for the GPI. This would avoid having to deal
with the HLOS vs. TZ SID problem by adding a whole other driver. Or we
could just rip out the non-GPI DMA support in this driver because the
SID is all confused.