| Message ID | 20190604222939.195471-1-swboyd@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | arm64: dts: sdm845: Add iommus property to qup1 | expand |
On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > The SMMU that sits in front of the QUP needs to be programmed properly > so that the i2c geni driver can allocate DMA descriptors. Failure to do > this leads to faults when using devices such as an i2c touchscreen where > the transaction is larger than 32 bytes and we use a DMA buffer. > I'm pretty sure I've run into this problem, but before we marked the smmu bypass_disable and as such didn't get the fault, thanks. > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > Add the right SID and mask so this works. > > Cc: Sibi Sankar <sibis@codeaurora.org> > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index fcb93300ca62..2e57e861e17c 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -900,6 +900,7 @@ > #address-cells = <2>; > #size-cells = <2>; > ranges; > + iommus = <&apps_smmu 0x6c0 0x3>; According to the docs this stream belongs to TZ, the HLOS stream should be 0x6c3. Regards, Bjorn > status = "disabled"; > > i2c8: i2c@a80000 { > -- > Sent by a computer through tubes >
Quoting Bjorn Andersson (2019-06-04 15:37:00) > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > The SMMU that sits in front of the QUP needs to be programmed properly > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > this leads to faults when using devices such as an i2c touchscreen where > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > I'm pretty sure I've run into this problem, but before we marked the > smmu bypass_disable and as such didn't get the fault, thanks. > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > Add the right SID and mask so this works. > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index fcb93300ca62..2e57e861e17c 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -900,6 +900,7 @@ > > #address-cells = <2>; > > #size-cells = <2>; > > ranges; > > + iommus = <&apps_smmu 0x6c0 0x3>; > > According to the docs this stream belongs to TZ, the HLOS stream should > be 0x6c3. Aye, I saw this line in the downstream kernel but it doesn't work for me. If I specify <&apps_smmu 0x6c3 0x0> it still blows up. I wonder if my firmware perhaps is missing some initialization here to make the QUP operate in HLOS mode? Otherwise, I thought that the 0x3 at the end was the mask and so it should be split off to the second cell in the DT specifier but that seemed a little weird.
Hi, On Tue, Jun 4, 2019 at 3:37 PM Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > The SMMU that sits in front of the QUP needs to be programmed properly > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > this leads to faults when using devices such as an i2c touchscreen where > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > I'm pretty sure I've run into this problem, but before we marked the > smmu bypass_disable and as such didn't get the fault, thanks. > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > Add the right SID and mask so this works. > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > --- > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > index fcb93300ca62..2e57e861e17c 100644 > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > @@ -900,6 +900,7 @@ > > #address-cells = <2>; > > #size-cells = <2>; > > ranges; > > + iommus = <&apps_smmu 0x6c0 0x3>; > > According to the docs this stream belongs to TZ, the HLOS stream should > be 0x6c3. Since you've already got the docs in front of you, how about looking up the value for qup0 so we can fix both of 'em? -Doug
On Tue 04 Jun 15:48 PDT 2019, Doug Anderson wrote: > Hi, > > On Tue, Jun 4, 2019 at 3:37 PM Bjorn Andersson > <bjorn.andersson@linaro.org> wrote: > > > > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > > > The SMMU that sits in front of the QUP needs to be programmed properly > > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > > this leads to faults when using devices such as an i2c touchscreen where > > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > > > > I'm pretty sure I've run into this problem, but before we marked the > > smmu bypass_disable and as such didn't get the fault, thanks. > > > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > > > Add the right SID and mask so this works. > > > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > --- > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > index fcb93300ca62..2e57e861e17c 100644 > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > @@ -900,6 +900,7 @@ > > > #address-cells = <2>; > > > #size-cells = <2>; > > > ranges; > > > + iommus = <&apps_smmu 0x6c0 0x3>; > > > > According to the docs this stream belongs to TZ, the HLOS stream should > > be 0x6c3. > > Since you've already got the docs in front of you, how about looking > up the value for qup0 so we can fix both of 'em? > Good thinking. QUP_1 is at stream 0x3. Regards, Bjorn
On Tue 04 Jun 15:46 PDT 2019, Stephen Boyd wrote: > Quoting Bjorn Andersson (2019-06-04 15:37:00) > > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > > > The SMMU that sits in front of the QUP needs to be programmed properly > > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > > this leads to faults when using devices such as an i2c touchscreen where > > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > > > > I'm pretty sure I've run into this problem, but before we marked the > > smmu bypass_disable and as such didn't get the fault, thanks. > > > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > > > Add the right SID and mask so this works. > > > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > --- > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > index fcb93300ca62..2e57e861e17c 100644 > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > @@ -900,6 +900,7 @@ > > > #address-cells = <2>; > > > #size-cells = <2>; > > > ranges; > > > + iommus = <&apps_smmu 0x6c0 0x3>; > > > > According to the docs this stream belongs to TZ, the HLOS stream should > > be 0x6c3. > > Aye, I saw this line in the downstream kernel but it doesn't work for > me. If I specify <&apps_smmu 0x6c3 0x0> it still blows up. I wonder if > my firmware perhaps is missing some initialization here to make the QUP > operate in HLOS mode? Otherwise, I thought that the 0x3 at the end was > the mask and so it should be split off to the second cell in the DT > specifier but that seemed a little weird. > Both 0x3 and 0x6c3 are documented as the actual HLOS stream id, with mask of 0x0. Looping in Vivek as well. Regards, Bjorn
On Wed, Jun 5, 2019 at 4:16 AM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Bjorn Andersson (2019-06-04 15:37:00) > > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > > > The SMMU that sits in front of the QUP needs to be programmed properly > > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > > this leads to faults when using devices such as an i2c touchscreen where > > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > > > > I'm pretty sure I've run into this problem, but before we marked the > > smmu bypass_disable and as such didn't get the fault, thanks. > > > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > > > Add the right SID and mask so this works. > > > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > --- > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > > 1 file changed, 1 insertion(+) > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > index fcb93300ca62..2e57e861e17c 100644 > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > @@ -900,6 +900,7 @@ > > > #address-cells = <2>; > > > #size-cells = <2>; > > > ranges; > > > + iommus = <&apps_smmu 0x6c0 0x3>; > > > > According to the docs this stream belongs to TZ, the HLOS stream should > > be 0x6c3. > > Aye, I saw this line in the downstream kernel but it doesn't work for > me. If I specify <&apps_smmu 0x6c3 0x0> it still blows up. I wonder if > my firmware perhaps is missing some initialization here to make the QUP > operate in HLOS mode? Otherwise, I thought that the 0x3 at the end was > the mask and so it should be split off to the second cell in the DT > specifier but that seemed a little weird. Two things here - 0x6c0 - TZ SID. Do you see above fault on MTP sdm845 devices? 0x6c3/0x6c6 - HLOS SIDs. 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. P.S. As you rightly said, the second cell in iommus property is the mask so that the iommu is able to reserve all that SIDs that are covered with the starting SID and the mask. Best regards Vivek
Quoting Vivek Gautam (2019-06-04 21:55:26) > On Wed, Jun 5, 2019 at 4:16 AM Stephen Boyd <swboyd@chromium.org> wrote: > > > > Quoting Bjorn Andersson (2019-06-04 15:37:00) > > > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > > > > > The SMMU that sits in front of the QUP needs to be programmed properly > > > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > > > this leads to faults when using devices such as an i2c touchscreen where > > > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > > > > > > > I'm pretty sure I've run into this problem, but before we marked the > > > smmu bypass_disable and as such didn't get the fault, thanks. > > > > > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > > > > > Add the right SID and mask so this works. > > > > > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > > --- > > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > > index fcb93300ca62..2e57e861e17c 100644 > > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > > @@ -900,6 +900,7 @@ > > > > #address-cells = <2>; > > > > #size-cells = <2>; > > > > ranges; > > > > + iommus = <&apps_smmu 0x6c0 0x3>; > > > > > > According to the docs this stream belongs to TZ, the HLOS stream should > > > be 0x6c3. > > > > Aye, I saw this line in the downstream kernel but it doesn't work for > > me. If I specify <&apps_smmu 0x6c3 0x0> it still blows up. I wonder if > > my firmware perhaps is missing some initialization here to make the QUP > > operate in HLOS mode? Otherwise, I thought that the 0x3 at the end was > > the mask and so it should be split off to the second cell in the DT > > specifier but that seemed a little weird. > > Two things here - > 0x6c0 - TZ SID. Do you see above fault on MTP sdm845 devices? No. I see the above fault on Cheza. > 0x6c3/0x6c6 - HLOS SIDs. Why are there two? I see some mentions of GSI mode near these SIDs so maybe GSI has to be used for DMA here to get the above two SIDs at the IOMMU? Otherwise if we do the non-GSI mode of DMA we're going to use the "TZ" SID? > > 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? > > P.S. > As you rightly said, the second cell in iommus property is the mask so that > the iommu is able to reserve all that SIDs that are covered with the > starting SID > and the mask. > Well if 0x6c6 is another possibility maybe it should be <&apps_smmu 0x6c0 0x7> to cover the 0x6c3 and 0x6c6 SIDs?
Hi Stephen, On Thu, Jun 6, 2019 at 2:27 AM Stephen Boyd <swboyd@chromium.org> wrote: > > Quoting Vivek Gautam (2019-06-04 21:55:26) > > On Wed, Jun 5, 2019 at 4:16 AM Stephen Boyd <swboyd@chromium.org> wrote: > > > > > > Quoting Bjorn Andersson (2019-06-04 15:37:00) > > > > On Tue 04 Jun 15:29 PDT 2019, Stephen Boyd wrote: > > > > > > > > > The SMMU that sits in front of the QUP needs to be programmed properly > > > > > so that the i2c geni driver can allocate DMA descriptors. Failure to do > > > > > this leads to faults when using devices such as an i2c touchscreen where > > > > > the transaction is larger than 32 bytes and we use a DMA buffer. > > > > > > > > > > > > > I'm pretty sure I've run into this problem, but before we marked the > > > > smmu bypass_disable and as such didn't get the fault, thanks. > > > > > > > > > arm-smmu 15000000.iommu: Unexpected global fault, this could be serious > > > > > arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 > > > > > > > > > > Add the right SID and mask so this works. > > > > > > > > > > Cc: Sibi Sankar <sibis@codeaurora.org> > > > > > Signed-off-by: Stephen Boyd <swboyd@chromium.org> > > > > > --- > > > > > arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + > > > > > 1 file changed, 1 insertion(+) > > > > > > > > > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > > > index fcb93300ca62..2e57e861e17c 100644 > > > > > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > > > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > > > > > @@ -900,6 +900,7 @@ > > > > > #address-cells = <2>; > > > > > #size-cells = <2>; > > > > > ranges; > > > > > + iommus = <&apps_smmu 0x6c0 0x3>; > > > > > > > > According to the docs this stream belongs to TZ, the HLOS stream should > > > > be 0x6c3. > > > > > > Aye, I saw this line in the downstream kernel but it doesn't work for > > > me. If I specify <&apps_smmu 0x6c3 0x0> it still blows up. I wonder if > > > my firmware perhaps is missing some initialization here to make the QUP > > > operate in HLOS mode? Otherwise, I thought that the 0x3 at the end was > > > the mask and so it should be split off to the second cell in the DT > > > specifier but that seemed a little weird. > > > > Two things here - > > 0x6c0 - TZ SID. Do you see above fault on MTP sdm845 devices? > > No. I see the above fault on Cheza. Right, expected. > > > 0x6c3/0x6c6 - HLOS SIDs. My bad, the other SID is 0x6D6. > > Why are there two? I see some mentions of GSI mode near these SIDs so > maybe GSI has to be used for DMA here to get the above two SIDs at the > IOMMU? Otherwise if we do the non-GSI mode of DMA we're going to use the > "TZ" SID? Yea, one for GSI, and the other one for non-GSI DMA. I am unsure at this point about the use of TZ SID, but i would assume this is the SID that's used by the qup firmware, and therefore on MTP TZ programs this SID. > > > > > 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? > > > > > P.S. > > As you rightly said, the second cell in iommus property is the mask so that > > the iommu is able to reserve all that SIDs that are covered with the > > starting SID > > and the mask. > > > > Well if 0x6c6 is another possibility maybe it should be <&apps_smmu > 0x6c0 0x7> to cover the 0x6c3 and 0x6c6 SIDs? The other SID was 0x6D6. Best regards Vivek
On 06/06/2019 13:17, Vivek Gautam wrote:
> <&apps_smmu 0x6c0 0x3> // for both 0x6c0 (TZ) and 0x6c3 (HLOS)
Another possibility is to list both:
<&apps_smmu 0x6c0 0x0>
<&apps_smmu 0x6c3 0x0>
which leaves 0x6c1 and 0x6c2 out of the picture, and makes 0x6c3
appear explicitly (for anyone checking the documentation).
Regards.
Quoting Vivek Gautam (2019-06-06 04:17:16) > Hi Stephen, > > On Thu, Jun 6, 2019 at 2:27 AM Stephen Boyd <swboyd@chromium.org> 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.
On 6/11/2019 4:51 AM, Stephen Boyd wrote: > Quoting Vivek Gautam (2019-06-06 04:17:16) >> Hi Stephen, >> >> On Thu, Jun 6, 2019 at 2:27 AM Stephen Boyd <swboyd@chromium.org> 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. I don't think SMMU limits any such programming of SIDs. It's a design decision to program few SIDs in TZ/Hyp and allocate the corresponding context banks and create respective mappings. You should be able to see these SMR configurations before kernel boots up. I use a simple T32 command - smmu.add "<name>" <smmu_type> <base_address> smmu.streammaptable <name> e.g. for sdm845 apps_smmu smmu.add "apps" MMU500 a:0x15000000 smmu.StreamMapTable apps This dumps all the information regarding the smmu. > > 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? I will get back with this information. BRs Vivek > > 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. >
Quoting Vivek Gautam (2019-06-12 02:26:20) > > > On 6/11/2019 4:51 AM, Stephen Boyd wrote: > > 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. > > I don't think SMMU limits any such programming of SIDs. It's a design > decision > to program few SIDs in TZ/Hyp and allocate the corresponding context banks > and create respective mappings. You should be able to see these SMR > configurations > before kernel boots up. I use a simple T32 command - > > smmu.add "<name>" <smmu_type> <base_address> > smmu.streammaptable <name> > > e.g. for sdm845 apps_smmu > > smmu.add "apps" MMU500 a:0x15000000 > smmu.StreamMapTable apps > > This dumps all the information regarding the smmu. Preferably I can see this by using devmem instead of jtag and T32. Do you know the address of the register? Otherwise I'll go dig into the documentation and try to figure it out. > > > > > 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? > > I will get back with this information. > Any update?
Quoting Stephen Boyd (2019-07-16 16:47:07) > Quoting Vivek Gautam (2019-06-12 02:26:20) > > > > > > On 6/11/2019 4:51 AM, Stephen Boyd wrote: > > > 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. > > > > I don't think SMMU limits any such programming of SIDs. It's a design > > decision > > to program few SIDs in TZ/Hyp and allocate the corresponding context banks > > and create respective mappings. You should be able to see these SMR > > configurations > > before kernel boots up. I use a simple T32 command - > > > > smmu.add "<name>" <smmu_type> <base_address> > > smmu.streammaptable <name> > > > > e.g. for sdm845 apps_smmu > > > > smmu.add "apps" MMU500 a:0x15000000 > > smmu.StreamMapTable apps > > > > This dumps all the information regarding the smmu. > > Preferably I can see this by using devmem instead of jtag and T32. Do > you know the address of the register? Otherwise I'll go dig into the > documentation and try to figure it out. And this won't really help me will it? I thought the stream ID was "fixed" by hardware, so it seems sort of weird that we're talking about limiting it in TZ/hyp. Here's the S2CR table from Cheza in case that is useful. localhost ~ # mem Welcome to mem interactive mode (featuring python!) Available functions: - r(addr) # Read a single 32-bit word (little endian). - rm(addr, nbytes) # Read memory at the given addr as a string. - w(addr, val) # Write a single 32-bit word (little endian). - wm(addr, val) # Write a string to memory at the given addr. >>> for x in range(64): ... r(0x15000c00 + (x << 2)) ... 0x00000000 0x00000000 0x00000001 0x00000002 0x00000003 0x00000004 0x00000005 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 0x00020000 > > > > > > > > > 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? > > > > I will get back with this information. > > > > Any update? Any news?
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index fcb93300ca62..2e57e861e17c 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -900,6 +900,7 @@ #address-cells = <2>; #size-cells = <2>; ranges; + iommus = <&apps_smmu 0x6c0 0x3>; status = "disabled"; i2c8: i2c@a80000 {
The SMMU that sits in front of the QUP needs to be programmed properly so that the i2c geni driver can allocate DMA descriptors. Failure to do this leads to faults when using devices such as an i2c touchscreen where the transaction is larger than 32 bytes and we use a DMA buffer. arm-smmu 15000000.iommu: Unexpected global fault, this could be serious arm-smmu 15000000.iommu: GFSR 0x00000002, GFSYNR0 0x00000002, GFSYNR1 0x000006c0, GFSYNR2 0x00000000 Add the right SID and mask so this works. Cc: Sibi Sankar <sibis@codeaurora.org> Signed-off-by: Stephen Boyd <swboyd@chromium.org> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 1 + 1 file changed, 1 insertion(+)