| Message ID | 20250207-topic-sm8650-pmu-ppi-partition-v1-2-dd3ba17b3eea@linaro.org (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | arm64: dts: qcom: sm8650: switch to 4 interrupt cells to add PPI partitions for PMUs | expand |
On 7.02.2025 11:31 AM, Neil Armstrong wrote: > The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper > interrupt partition maps and use the 4th interrupt cell to pass the > partition phandle for each ARM PMU node. > > Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> > --- > @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { > #size-cells = <2>; > ranges; > > + ppi-partitions { > + ppi_cluster0: interrupt-partition-0 { > + affinity = <&cpu0 &cpu1>; > + }; > + > + ppi_cluster1: interrupt-partition-1 { > + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; > + }; > + > + ppi_cluster2: interrupt-partition-2 { > + affinity = <&cpu7>; > + }; I'm not sure this is accurate. I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer Konrad
On 07/02/2025 21:30, Konrad Dybcio wrote: > On 7.02.2025 11:31 AM, Neil Armstrong wrote: >> The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper >> interrupt partition maps and use the 4th interrupt cell to pass the >> partition phandle for each ARM PMU node. >> >> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >> --- > >> @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { >> #size-cells = <2>; >> ranges; >> >> + ppi-partitions { >> + ppi_cluster0: interrupt-partition-0 { >> + affinity = <&cpu0 &cpu1>; >> + }; >> + >> + ppi_cluster1: interrupt-partition-1 { >> + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; >> + }; >> + >> + ppi_cluster2: interrupt-partition-2 { >> + affinity = <&cpu7>; >> + }; > > I'm not sure this is accurate. > > I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer Core 7 is a Cortex-X4, and has a dedicated PMU node, look at the cpu compatibles. Neil > > Konrad
On 9.02.2025 3:44 PM, Neil Armstrong wrote: > On 07/02/2025 21:30, Konrad Dybcio wrote: >> On 7.02.2025 11:31 AM, Neil Armstrong wrote: >>> The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper >>> interrupt partition maps and use the 4th interrupt cell to pass the >>> partition phandle for each ARM PMU node. >>> >>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>> --- >> >>> @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { >>> #size-cells = <2>; >>> ranges; >>> + ppi-partitions { >>> + ppi_cluster0: interrupt-partition-0 { >>> + affinity = <&cpu0 &cpu1>; >>> + }; >>> + >>> + ppi_cluster1: interrupt-partition-1 { >>> + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; >>> + }; >>> + >>> + ppi_cluster2: interrupt-partition-2 { >>> + affinity = <&cpu7>; >>> + }; >> >> I'm not sure this is accurate. >> >> I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer > > Core 7 is a Cortex-X4, and has a dedicated PMU node, look at the cpu compatibles. Look at what these compatibles do in code. Nothing special for the X. Konrad
On 10/02/2025 16:23, Konrad Dybcio wrote: > On 9.02.2025 3:44 PM, Neil Armstrong wrote: >> On 07/02/2025 21:30, Konrad Dybcio wrote: >>> On 7.02.2025 11:31 AM, Neil Armstrong wrote: >>>> The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper >>>> interrupt partition maps and use the 4th interrupt cell to pass the >>>> partition phandle for each ARM PMU node. >>>> >>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>> --- >>> >>>> @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { >>>> #size-cells = <2>; >>>> ranges; >>>> + ppi-partitions { >>>> + ppi_cluster0: interrupt-partition-0 { >>>> + affinity = <&cpu0 &cpu1>; >>>> + }; >>>> + >>>> + ppi_cluster1: interrupt-partition-1 { >>>> + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; >>>> + }; >>>> + >>>> + ppi_cluster2: interrupt-partition-2 { >>>> + affinity = <&cpu7>; >>>> + }; >>> >>> I'm not sure this is accurate. >>> >>> I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer >> >> Core 7 is a Cortex-X4, and has a dedicated PMU node, look at the cpu compatibles. > > Look at what these compatibles do in code. Nothing special for the X. So you suggest to revert Rob's change ? https://lore.kernel.org/all/20240417204247.3216703-1-robh@kernel.org/ So what I understood is that yes the code is the same, but the perf attributes are not necessarily the same between heterogeneous cores, so each instance here would load the attributes for each core type correctly instead of only using the ones from the first core Again, other SoCs uses this same scheme so I wonder what's the issue here ? Neil > > Konrad
On 10.02.2025 4:30 PM, neil.armstrong@linaro.org wrote: > On 10/02/2025 16:23, Konrad Dybcio wrote: >> On 9.02.2025 3:44 PM, Neil Armstrong wrote: >>> On 07/02/2025 21:30, Konrad Dybcio wrote: >>>> On 7.02.2025 11:31 AM, Neil Armstrong wrote: >>>>> The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper >>>>> interrupt partition maps and use the 4th interrupt cell to pass the >>>>> partition phandle for each ARM PMU node. >>>>> >>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>>> --- >>>> >>>>> @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { >>>>> #size-cells = <2>; >>>>> ranges; >>>>> + ppi-partitions { >>>>> + ppi_cluster0: interrupt-partition-0 { >>>>> + affinity = <&cpu0 &cpu1>; >>>>> + }; >>>>> + >>>>> + ppi_cluster1: interrupt-partition-1 { >>>>> + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; >>>>> + }; >>>>> + >>>>> + ppi_cluster2: interrupt-partition-2 { >>>>> + affinity = <&cpu7>; >>>>> + }; >>>> >>>> I'm not sure this is accurate. >>>> >>>> I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer >>> >>> Core 7 is a Cortex-X4, and has a dedicated PMU node, look at the cpu compatibles. >> >> Look at what these compatibles do in code. Nothing special for the X. > > So you suggest to revert Rob's change ? > > https://lore.kernel.org/all/20240417204247.3216703-1-robh@kernel.org/ > > So what I understood is that yes the code is the same, but the perf > attributes are not necessarily the same between heterogeneous cores, > so each instance here would load the attributes for each core type > correctly instead of only using the ones from the first core > > Again, other SoCs uses this same scheme so I wonder what's the issue here ? So I'm a little confused here. Is this partitioning scheme only describing a set of same-kind cores to Linux so that the PMU interrupts only arrive at one PMU device? Or does it reflect some actual physical topology of clusters and how they're connected to the GIC? If the former, I have no issues with this version of the patch. Konrad
On 10/02/2025 19:29, Konrad Dybcio wrote: > On 10.02.2025 4:30 PM, neil.armstrong@linaro.org wrote: >> On 10/02/2025 16:23, Konrad Dybcio wrote: >>> On 9.02.2025 3:44 PM, Neil Armstrong wrote: >>>> On 07/02/2025 21:30, Konrad Dybcio wrote: >>>>> On 7.02.2025 11:31 AM, Neil Armstrong wrote: >>>>>> The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper >>>>>> interrupt partition maps and use the 4th interrupt cell to pass the >>>>>> partition phandle for each ARM PMU node. >>>>>> >>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>>>> --- >>>>> >>>>>> @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { >>>>>> #size-cells = <2>; >>>>>> ranges; >>>>>> + ppi-partitions { >>>>>> + ppi_cluster0: interrupt-partition-0 { >>>>>> + affinity = <&cpu0 &cpu1>; >>>>>> + }; >>>>>> + >>>>>> + ppi_cluster1: interrupt-partition-1 { >>>>>> + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; >>>>>> + }; >>>>>> + >>>>>> + ppi_cluster2: interrupt-partition-2 { >>>>>> + affinity = <&cpu7>; >>>>>> + }; >>>>> >>>>> I'm not sure this is accurate. >>>>> >>>>> I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer >>>> >>>> Core 7 is a Cortex-X4, and has a dedicated PMU node, look at the cpu compatibles. >>> >>> Look at what these compatibles do in code. Nothing special for the X. >> >> So you suggest to revert Rob's change ? >> >> https://lore.kernel.org/all/20240417204247.3216703-1-robh@kernel.org/ >> >> So what I understood is that yes the code is the same, but the perf >> attributes are not necessarily the same between heterogeneous cores, >> so each instance here would load the attributes for each core type >> correctly instead of only using the ones from the first core >> >> Again, other SoCs uses this same scheme so I wonder what's the issue here ? > > So I'm a little confused here. Is this partitioning scheme only describing > a set of same-kind cores to Linux so that the PMU interrupts only arrive at > one PMU device? Or does it reflect some actual physical topology of clusters > and how they're connected to the GIC? > > If the former, I have no issues with this version of the patch. The former, we simply partition the PPI interrupt for each device node, it has obviously something to do with the topology, but the goal is not to describe the topology. Neil > > Konrad
On 11.02.2025 9:02 AM, neil.armstrong@linaro.org wrote: > On 10/02/2025 19:29, Konrad Dybcio wrote: >> On 10.02.2025 4:30 PM, neil.armstrong@linaro.org wrote: >>> On 10/02/2025 16:23, Konrad Dybcio wrote: >>>> On 9.02.2025 3:44 PM, Neil Armstrong wrote: >>>>> On 07/02/2025 21:30, Konrad Dybcio wrote: >>>>>> On 7.02.2025 11:31 AM, Neil Armstrong wrote: >>>>>>> The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper >>>>>>> interrupt partition maps and use the 4th interrupt cell to pass the >>>>>>> partition phandle for each ARM PMU node. >>>>>>> >>>>>>> Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> >>>>>>> --- >>>>>> >>>>>>> @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { >>>>>>> #size-cells = <2>; >>>>>>> ranges; >>>>>>> + ppi-partitions { >>>>>>> + ppi_cluster0: interrupt-partition-0 { >>>>>>> + affinity = <&cpu0 &cpu1>; >>>>>>> + }; >>>>>>> + >>>>>>> + ppi_cluster1: interrupt-partition-1 { >>>>>>> + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; >>>>>>> + }; >>>>>>> + >>>>>>> + ppi_cluster2: interrupt-partition-2 { >>>>>>> + affinity = <&cpu7>; >>>>>>> + }; >>>>>> >>>>>> I'm not sure this is accurate. >>>>>> >>>>>> I *think* it's cores 0-1 and 2-7, but I can't find a concrete answer >>>>> >>>>> Core 7 is a Cortex-X4, and has a dedicated PMU node, look at the cpu compatibles. >>>> >>>> Look at what these compatibles do in code. Nothing special for the X. >>> >>> So you suggest to revert Rob's change ? >>> >>> https://lore.kernel.org/all/20240417204247.3216703-1-robh@kernel.org/ >>> >>> So what I understood is that yes the code is the same, but the perf >>> attributes are not necessarily the same between heterogeneous cores, >>> so each instance here would load the attributes for each core type >>> correctly instead of only using the ones from the first core >>> >>> Again, other SoCs uses this same scheme so I wonder what's the issue here ? >> >> So I'm a little confused here. Is this partitioning scheme only describing >> a set of same-kind cores to Linux so that the PMU interrupts only arrive at >> one PMU device? Or does it reflect some actual physical topology of clusters >> and how they're connected to the GIC? >> >> If the former, I have no issues with this version of the patch. > > The former, we simply partition the PPI interrupt for each device node, it > has obviously something to do with the topology, but the goal is not to describe > the topology. Ok, good Reviewed-by: Konrad Dybcio <konrad.dybcio@oss.qualcomm.com> Konrad
diff --git a/arch/arm64/boot/dts/qcom/sm8650.dtsi b/arch/arm64/boot/dts/qcom/sm8650.dtsi index eea73474bc857260fce26ca417d286a737ac8ddb..47df1ca020331421a14fca3fc0002a46f2083291 100644 --- a/arch/arm64/boot/dts/qcom/sm8650.dtsi +++ b/arch/arm64/boot/dts/qcom/sm8650.dtsi @@ -428,17 +428,17 @@ memory@a0000000 { pmu-a520 { compatible = "arm,cortex-a520-pmu"; - interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH 0>; + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH &ppi_cluster0>; }; pmu-a720 { compatible = "arm,cortex-a720-pmu"; - interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH 0>; + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH &ppi_cluster1>; }; pmu-x4 { compatible = "arm,cortex-x4-pmu"; - interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH 0>; + interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH &ppi_cluster2>; }; psci { @@ -5309,6 +5309,20 @@ intc: interrupt-controller@17100000 { #size-cells = <2>; ranges; + ppi-partitions { + ppi_cluster0: interrupt-partition-0 { + affinity = <&cpu0 &cpu1>; + }; + + ppi_cluster1: interrupt-partition-1 { + affinity = <&cpu2 &cpu3 &cpu4 &cpu5 &cpu6>; + }; + + ppi_cluster2: interrupt-partition-2 { + affinity = <&cpu7>; + }; + }; + gic_its: msi-controller@17140000 { compatible = "arm,gic-v3-its"; reg = <0 0x17140000 0 0x20000>;
The PMUs shares the same per-cpu (PPI) interrupt, so declare the proper interrupt partition maps and use the 4th interrupt cell to pass the partition phandle for each ARM PMU node. Signed-off-by: Neil Armstrong <neil.armstrong@linaro.org> --- arch/arm64/boot/dts/qcom/sm8650.dtsi | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-)