| Message ID | 20240417133731.2055383-6-quic_c_gdjako@quicinc.com |
|---|---|
| State | Accepted |
| Commit | 7bb38c20f2b64a65423e64e6765bd70a5eadee81 |
| Headers | show |
| Series | Add support for Translation Buffer Units | expand |
On Wed, 17 Apr 2024 at 16:39, Georgi Djakov <quic_c_gdjako@quicinc.com> wrote: > > Add the device-tree nodes for the TBUs (translation buffer units) that > are present on the sdm845 platforms. The TBUs can be used debug the > kernel and provide additional information when a context faults occur. > > Describe the all registers, clocks, interconnects and power-domain > resources that are needed for each of the TBUs. > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> This patch now prevents interconnect drivers from hitting the sync state on SDM845. The TBU driver is enabled only when the ARM_SMMU_QCOM_DEBUG is enabled, which is not a typical case on a normal system: [ 26.209151] qnoc-sdm845 1500000.interconnect: sync_state() pending due to 150c5000.tbu [ 26.217228] qnoc-sdm845 1620000.interconnect: sync_state() pending due to 150c5000.tbu [ 26.229926] qnoc-sdm845 1500000.interconnect: sync_state() pending due to 150c9000.tbu [ 26.238008] qnoc-sdm845 1620000.interconnect: sync_state() pending due to 150c9000.tbu [ 26.249068] qnoc-sdm845 1740000.interconnect: sync_state() pending due to 150cd000.tbu [ 26.257127] qnoc-sdm845 1740000.interconnect: sync_state() pending due to 150d1000.tbu [ 26.265159] qnoc-sdm845 1740000.interconnect: sync_state() pending due to 150d5000.tbu [ 26.273189] qnoc-sdm845 1500000.interconnect: sync_state() pending due to 150d9000.tbu [ 26.281206] qnoc-sdm845 1620000.interconnect: sync_state() pending due to 150d9000.tbu [ 26.289203] qnoc-sdm845 1500000.interconnect: sync_state() pending due to 150dd000.tbu [ 26.297196] qnoc-sdm845 1620000.interconnect: sync_state() pending due to 150dd000.tbu [ 26.305201] qnoc-sdm845 1500000.interconnect: sync_state() pending due to 150e1000.tbu [ 26.313207] qnoc-sdm845 1620000.interconnect: sync_state() pending due to 150e1000.tbu > --- > arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 ++++++++++++++++++++++++++++ > 1 file changed, 73 insertions(+) > > diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi > index 2f20be99ee7e..fa9403aad96f 100644 > --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi > +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi > @@ -15,6 +15,7 @@ > #include <dt-bindings/dma/qcom-gpi.h> > #include <dt-bindings/firmware/qcom,scm.h> > #include <dt-bindings/gpio/gpio.h> > +#include <dt-bindings/interconnect/qcom,icc.h> > #include <dt-bindings/interconnect/qcom,osm-l3.h> > #include <dt-bindings/interconnect/qcom,sdm845.h> > #include <dt-bindings/interrupt-controller/arm-gic.h> > @@ -5085,6 +5086,78 @@ apps_smmu: iommu@15000000 { > <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; > }; > > + anoc_1_tbu: tbu@150c5000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150c5000 0x0 0x1000>; > + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_TBU1_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0x0 0x400>; > + }; > + > + anoc_2_tbu: tbu@150c9000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150c9000 0x0 0x1000>; > + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_TBU2_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0x400 0x400>; > + }; > + > + mnoc_hf_0_tbu: tbu@150cd000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150cd000 0x0 0x1000>; > + interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ACTIVE_ONLY > + &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0x800 0x400>; > + }; > + > + mnoc_hf_1_tbu: tbu@150d1000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150d1000 0x0 0x1000>; > + interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ACTIVE_ONLY > + &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0xc00 0x400>; > + }; > + > + mnoc_sf_0_tbu: tbu@150d5000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150d5000 0x0 0x1000>; > + interconnects = <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ACTIVE_ONLY > + &mmss_noc SLAVE_MNOC_SF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0x1000 0x400>; > + }; > + > + compute_dsp_tbu: tbu@150d9000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150d9000 0x0 0x1000>; > + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; > + qcom,stream-id-range = <&apps_smmu 0x1400 0x400>; > + }; > + > + adsp_tbu: tbu@150dd000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150dd000 0x0 0x1000>; > + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_AUDIO_TBU_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0x1800 0x400>; > + }; > + > + anoc_1_pcie_tbu: tbu@150e1000 { > + compatible = "qcom,sdm845-tbu"; > + reg = <0x0 0x150e1000 0x0 0x1000>; > + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; > + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY > + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; > + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; > + qcom,stream-id-range = <&apps_smmu 0x1c00 0x400>; > + }; > + > lpasscc: clock-controller@17014000 { > compatible = "qcom,sdm845-lpasscc"; > reg = <0 0x17014000 0 0x1f004>, <0 0x17300000 0 0x200>; >
On Fri, 14 Jun 2024 at 21:05, Dmitry Baryshkov <dmitry.baryshkov@linaro.org> wrote: > > On Wed, 17 Apr 2024 at 16:39, Georgi Djakov <quic_c_gdjako@quicinc.com> wrote: > > > > Add the device-tree nodes for the TBUs (translation buffer units) that > > are present on the sdm845 platforms. The TBUs can be used debug the > > kernel and provide additional information when a context faults occur. > > > > Describe the all registers, clocks, interconnects and power-domain > > resources that are needed for each of the TBUs. > > > > Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > > This patch now prevents interconnect drivers from hitting the sync > state on SDM845. > The TBU driver is enabled only when the ARM_SMMU_QCOM_DEBUG is > enabled, which is not a typical case on a normal system: Georgi, before I start acting like a bull in a china shop and sending reverts, any update from your side? > > [ 26.209151] qnoc-sdm845 1500000.interconnect: sync_state() pending > due to 150c5000.tbu > [ 26.217228] qnoc-sdm845 1620000.interconnect: sync_state() pending > due to 150c5000.tbu > [ 26.229926] qnoc-sdm845 1500000.interconnect: sync_state() pending > due to 150c9000.tbu > [ 26.238008] qnoc-sdm845 1620000.interconnect: sync_state() pending > due to 150c9000.tbu > [ 26.249068] qnoc-sdm845 1740000.interconnect: sync_state() pending > due to 150cd000.tbu > [ 26.257127] qnoc-sdm845 1740000.interconnect: sync_state() pending > due to 150d1000.tbu > [ 26.265159] qnoc-sdm845 1740000.interconnect: sync_state() pending > due to 150d5000.tbu > [ 26.273189] qnoc-sdm845 1500000.interconnect: sync_state() pending > due to 150d9000.tbu > [ 26.281206] qnoc-sdm845 1620000.interconnect: sync_state() pending > due to 150d9000.tbu > [ 26.289203] qnoc-sdm845 1500000.interconnect: sync_state() pending > due to 150dd000.tbu > [ 26.297196] qnoc-sdm845 1620000.interconnect: sync_state() pending > due to 150dd000.tbu > [ 26.305201] qnoc-sdm845 1500000.interconnect: sync_state() pending > due to 150e1000.tbu > [ 26.313207] qnoc-sdm845 1620000.interconnect: sync_state() pending > due to 150e1000.tbu
On 25.06.24 10:50, Dmitry Baryshkov wrote: > On Fri, 14 Jun 2024 at 21:05, Dmitry Baryshkov > <dmitry.baryshkov@linaro.org> wrote: >> >> On Wed, 17 Apr 2024 at 16:39, Georgi Djakov <quic_c_gdjako@quicinc.com> wrote: >>> >>> Add the device-tree nodes for the TBUs (translation buffer units) that >>> are present on the sdm845 platforms. The TBUs can be used debug the >>> kernel and provide additional information when a context faults occur. >>> >>> Describe the all registers, clocks, interconnects and power-domain >>> resources that are needed for each of the TBUs. >>> >>> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> >> >> This patch now prevents interconnect drivers from hitting the sync >> state on SDM845. >> The TBU driver is enabled only when the ARM_SMMU_QCOM_DEBUG is >> enabled, which is not a typical case on a normal system: > > Georgi, before I start acting like a bull in a china shop and sending > reverts, any update from your side? Hi Dmitry! Thanks for the report! We can easily add status = "disabled" to the DT nodes, but please give me some time to take a look what would be the best way to handle this, as i was out last week and now i am still catching up. BR, Georgi > >> >> [ 26.209151] qnoc-sdm845 1500000.interconnect: sync_state() pending >> due to 150c5000.tbu >> [ 26.217228] qnoc-sdm845 1620000.interconnect: sync_state() pending >> due to 150c5000.tbu >> [ 26.229926] qnoc-sdm845 1500000.interconnect: sync_state() pending >> due to 150c9000.tbu >> [ 26.238008] qnoc-sdm845 1620000.interconnect: sync_state() pending >> due to 150c9000.tbu >> [ 26.249068] qnoc-sdm845 1740000.interconnect: sync_state() pending >> due to 150cd000.tbu >> [ 26.257127] qnoc-sdm845 1740000.interconnect: sync_state() pending >> due to 150d1000.tbu >> [ 26.265159] qnoc-sdm845 1740000.interconnect: sync_state() pending >> due to 150d5000.tbu >> [ 26.273189] qnoc-sdm845 1500000.interconnect: sync_state() pending >> due to 150d9000.tbu >> [ 26.281206] qnoc-sdm845 1620000.interconnect: sync_state() pending >> due to 150d9000.tbu >> [ 26.289203] qnoc-sdm845 1500000.interconnect: sync_state() pending >> due to 150dd000.tbu >> [ 26.297196] qnoc-sdm845 1620000.interconnect: sync_state() pending >> due to 150dd000.tbu >> [ 26.305201] qnoc-sdm845 1500000.interconnect: sync_state() pending >> due to 150e1000.tbu >> [ 26.313207] qnoc-sdm845 1620000.interconnect: sync_state() pending >> due to 150e1000.tbu >
On Tue, Jun 25, 2024 at 03:59:27PM +0300, Dmitry Baryshkov wrote: > On Tue, 25 Jun 2024 at 15:57, Georgi Djakov <djakov@kernel.org> wrote: > > > > On 25.06.24 10:50, Dmitry Baryshkov wrote: > > > On Fri, 14 Jun 2024 at 21:05, Dmitry Baryshkov > > > <dmitry.baryshkov@linaro.org> wrote: > > >> > > >> On Wed, 17 Apr 2024 at 16:39, Georgi Djakov <quic_c_gdjako@quicinc.com> wrote: > > >>> > > >>> Add the device-tree nodes for the TBUs (translation buffer units) that > > >>> are present on the sdm845 platforms. The TBUs can be used debug the > > >>> kernel and provide additional information when a context faults occur. > > >>> > > >>> Describe the all registers, clocks, interconnects and power-domain > > >>> resources that are needed for each of the TBUs. > > >>> > > >>> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > > >> > > >> This patch now prevents interconnect drivers from hitting the sync > > >> state on SDM845. > > >> The TBU driver is enabled only when the ARM_SMMU_QCOM_DEBUG is > > >> enabled, which is not a typical case on a normal system: > > > > > > Georgi, before I start acting like a bull in a china shop and sending > > > reverts, any update from your side? > > > > Hi Dmitry! > > Thanks for the report! We can easily add status = "disabled" to the DT > > nodes, but please give me some time to take a look what would be the best > > way to handle this, as i was out last week and now i am still catching up. > > I think the simplest thing would be to move the TBU driver to the > arm-qcom-smmu.c instead of having it in the -debug.c The TBUs aren't used for anything other than debugging, so I'd really rather they live with the debug code. Will
On Tue, 2 Jul 2024 at 19:39, Will Deacon <will@kernel.org> wrote: > > On Tue, Jun 25, 2024 at 03:59:27PM +0300, Dmitry Baryshkov wrote: > > On Tue, 25 Jun 2024 at 15:57, Georgi Djakov <djakov@kernel.org> wrote: > > > > > > On 25.06.24 10:50, Dmitry Baryshkov wrote: > > > > On Fri, 14 Jun 2024 at 21:05, Dmitry Baryshkov > > > > <dmitry.baryshkov@linaro.org> wrote: > > > >> > > > >> On Wed, 17 Apr 2024 at 16:39, Georgi Djakov <quic_c_gdjako@quicinc.com> wrote: > > > >>> > > > >>> Add the device-tree nodes for the TBUs (translation buffer units) that > > > >>> are present on the sdm845 platforms. The TBUs can be used debug the > > > >>> kernel and provide additional information when a context faults occur. > > > >>> > > > >>> Describe the all registers, clocks, interconnects and power-domain > > > >>> resources that are needed for each of the TBUs. > > > >>> > > > >>> Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> > > > >> > > > >> This patch now prevents interconnect drivers from hitting the sync > > > >> state on SDM845. > > > >> The TBU driver is enabled only when the ARM_SMMU_QCOM_DEBUG is > > > >> enabled, which is not a typical case on a normal system: > > > > > > > > Georgi, before I start acting like a bull in a china shop and sending > > > > reverts, any update from your side? > > > > > > Hi Dmitry! > > > Thanks for the report! We can easily add status = "disabled" to the DT > > > nodes, but please give me some time to take a look what would be the best > > > way to handle this, as i was out last week and now i am still catching up. > > > > I think the simplest thing would be to move the TBU driver to the > > arm-qcom-smmu.c instead of having it in the -debug.c > > The TBUs aren't used for anything other than debugging, so I'd really > rather they live with the debug code. The problem is that not having any driver bound to the TBU devices prevents interconnect drivers from hitting the sync state (and thus lowering the bandwidth to the requested values). Being that late in the development cycle, I think we should fix the issue in the fastest way. So I'm close to sending a revert of the DT changes. Setting status = "disabled" doesn't qualify as a logical change as having TBU units enabled on a platform-to-platform basis doesn't make sense.
diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index 2f20be99ee7e..fa9403aad96f 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -15,6 +15,7 @@ #include <dt-bindings/dma/qcom-gpi.h> #include <dt-bindings/firmware/qcom,scm.h> #include <dt-bindings/gpio/gpio.h> +#include <dt-bindings/interconnect/qcom,icc.h> #include <dt-bindings/interconnect/qcom,osm-l3.h> #include <dt-bindings/interconnect/qcom,sdm845.h> #include <dt-bindings/interrupt-controller/arm-gic.h> @@ -5085,6 +5086,78 @@ apps_smmu: iommu@15000000 { <GIC_SPI 343 IRQ_TYPE_LEVEL_HIGH>; }; + anoc_1_tbu: tbu@150c5000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150c5000 0x0 0x1000>; + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_TBU1_GDSC>; + qcom,stream-id-range = <&apps_smmu 0x0 0x400>; + }; + + anoc_2_tbu: tbu@150c9000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150c9000 0x0 0x1000>; + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_TBU2_GDSC>; + qcom,stream-id-range = <&apps_smmu 0x400 0x400>; + }; + + mnoc_hf_0_tbu: tbu@150cd000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150cd000 0x0 0x1000>; + interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ACTIVE_ONLY + &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_MMNOC_MMU_TBU_HF0_GDSC>; + qcom,stream-id-range = <&apps_smmu 0x800 0x400>; + }; + + mnoc_hf_1_tbu: tbu@150d1000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150d1000 0x0 0x1000>; + interconnects = <&mmss_noc MASTER_MDP0 QCOM_ICC_TAG_ACTIVE_ONLY + &mmss_noc SLAVE_MNOC_HF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_MMNOC_MMU_TBU_HF1_GDSC>; + qcom,stream-id-range = <&apps_smmu 0xc00 0x400>; + }; + + mnoc_sf_0_tbu: tbu@150d5000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150d5000 0x0 0x1000>; + interconnects = <&mmss_noc MASTER_CAMNOC_SF QCOM_ICC_TAG_ACTIVE_ONLY + &mmss_noc SLAVE_MNOC_SF_MEM_NOC QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_MMNOC_MMU_TBU_SF_GDSC>; + qcom,stream-id-range = <&apps_smmu 0x1000 0x400>; + }; + + compute_dsp_tbu: tbu@150d9000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150d9000 0x0 0x1000>; + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; + qcom,stream-id-range = <&apps_smmu 0x1400 0x400>; + }; + + adsp_tbu: tbu@150dd000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150dd000 0x0 0x1000>; + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_AUDIO_TBU_GDSC>; + qcom,stream-id-range = <&apps_smmu 0x1800 0x400>; + }; + + anoc_1_pcie_tbu: tbu@150e1000 { + compatible = "qcom,sdm845-tbu"; + reg = <0x0 0x150e1000 0x0 0x1000>; + clocks = <&gcc GCC_AGGRE_NOC_PCIE_TBU_CLK>; + interconnects = <&system_noc MASTER_GNOC_SNOC QCOM_ICC_TAG_ACTIVE_ONLY + &config_noc SLAVE_IMEM_CFG QCOM_ICC_TAG_ACTIVE_ONLY>; + power-domains = <&gcc HLOS1_VOTE_AGGRE_NOC_MMU_PCIE_TBU_GDSC>; + qcom,stream-id-range = <&apps_smmu 0x1c00 0x400>; + }; + lpasscc: clock-controller@17014000 { compatible = "qcom,sdm845-lpasscc"; reg = <0 0x17014000 0 0x1f004>, <0 0x17300000 0 0x200>;
Add the device-tree nodes for the TBUs (translation buffer units) that are present on the sdm845 platforms. The TBUs can be used debug the kernel and provide additional information when a context faults occur. Describe the all registers, clocks, interconnects and power-domain resources that are needed for each of the TBUs. Signed-off-by: Georgi Djakov <quic_c_gdjako@quicinc.com> --- arch/arm64/boot/dts/qcom/sdm845.dtsi | 73 ++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+)