2019-04-12 17:27:05

by Willy Wolff

[permalink] [raw]
Subject: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

Add device tree entries for PMU of ARM CCI-400.

$ sudo ./perf stat -a -C 0 -e CCI_400/config=0xff,name=cycles/ sleep 1

Performance counter stats for 'system wide':

420,303,619 cycles

1.019058775 seconds time elapsed

Tested on Odroid-xu3 and 4.

Signed-off-by: Willy Wolff <[email protected]>
---
arch/arm/boot/dts/exynos5420.dtsi | 12 +++++++++++-
1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
index aaff15880761..be58650aca35 100644
--- a/arch/arm/boot/dts/exynos5420.dtsi
+++ b/arch/arm/boot/dts/exynos5420.dtsi
@@ -158,7 +158,7 @@
#address-cells = <1>;
#size-cells = <1>;
reg = <0x10d20000 0x1000>;
- ranges = <0x0 0x10d20000 0x6000>;
+ ranges = <0x0 0x10d20000 0x10000>;

cci_control0: slave-if@4000 {
compatible = "arm,cci-400-ctrl-if";
@@ -170,6 +170,16 @@
interface-type = "ace";
reg = <0x5000 0x1000>;
};
+
+ pmu@9000 {
+ compatible = "arm,cci-400-pmu,r0";
+ reg = <0x9000 0x5000>;
+ interrupts = <0 105 4>,
+ <0 101 4>,
+ <0 102 4>,
+ <0 103 4>,
+ <0 104 4>;
+ };
};

clock: clock-controller@10010000 {
--
2.11.0


2019-04-15 12:25:42

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

Hi Willy,

On Fri, 12 Apr 2019 at 22:55, Willy Wolff <[email protected]> wrote:
>
> Add device tree entries for PMU of ARM CCI-400.
>
> $ sudo ./perf stat -a -C 0 -e CCI_400/config=0xff,name=cycles/ sleep 1
>
> Performance counter stats for 'system wide':
>
> 420,303,619 cycles
>
> 1.019058775 seconds time elapsed
>
> Tested on Odroid-xu3 and 4.
>
> Signed-off-by: Willy Wolff <[email protected]>
> ---
> arch/arm/boot/dts/exynos5420.dtsi | 12 +++++++++++-
> 1 file changed, 11 insertions(+), 1 deletion(-)
>
> diff --git a/arch/arm/boot/dts/exynos5420.dtsi b/arch/arm/boot/dts/exynos5420.dtsi
> index aaff15880761..be58650aca35 100644
> --- a/arch/arm/boot/dts/exynos5420.dtsi
> +++ b/arch/arm/boot/dts/exynos5420.dtsi
> @@ -158,7 +158,7 @@
> #address-cells = <1>;
> #size-cells = <1>;
> reg = <0x10d20000 0x1000>;
> - ranges = <0x0 0x10d20000 0x6000>;
> + ranges = <0x0 0x10d20000 0x10000>;
>
> cci_control0: slave-if@4000 {
> compatible = "arm,cci-400-ctrl-if";
> @@ -170,6 +170,16 @@
> interface-type = "ace";
> reg = <0x5000 0x1000>;
> };
> +
> + pmu@9000 {
> + compatible = "arm,cci-400-pmu,r0";
> + reg = <0x9000 0x5000>;

As per Exynos 5422 user manual below interrupts should be as follow.

+ interrupts = <GIC_SPI 176
IRQ_TYPE_LEVEL_HIGH>, /* CCI_N_EVENT_CNT0_OVF */
+ <GIC_SPI 177
IRQ_TYPE_LEVEL_HIGH>, /* CCI_N_EVENT_CNT1_OVF */
+ <GIC_SPI 178
IRQ_TYPE_LEVEL_HIGH>, /* CCI_N_EVENT_CNT2_OVF */
+ <GIC_SPI 179
IRQ_TYPE_LEVEL_HIGH>, /* CCI_N_EVENT_CNT3_OVF */
+ <GIC_SPI 180
IRQ_TYPE_LEVEL_HIGH>, /* CCI_N_EVENT_CNT4_OVF */
+ <GIC_SPI 181
IRQ_TYPE_LEVEL_HIGH>; /* CCI_NERR */

> + interrupts = <0 105 4>,
> + <0 101 4>,
> + <0 102 4>,
> + <0 103 4>,
> + <0 104 4>;
> + };
> };
>
> clock: clock-controller@10010000 {
> --
> 2.11.0
>

But I am observing follow kernel warning after I enable
CONFIG_ARM_CCI_PMU + exynos_defconfig.

[ 4.557701] mmcblk0: p1
[ 4.561036] BUG: sleeping function called from invalid context at
kernel/locking/mutex.c:908
[ 4.568075] in_atomic(): 1, irqs_disabled(): 0, pid: 1, name: swapper/0
[ 4.574656] 1 lock held by swapper/0/1:
[ 4.578397] #0: (ptrval) (&dev->mutex){....}, at:
device_driver_attach+0x18/0x60
[ 4.585900] Preemption disabled at:
[ 4.585909] [<c077eca8>] cci_pmu_probe+0x1cc/0x4a0
[ 4.594122] CPU: 4 PID: 1 Comm: swapper/0 Not tainted 5.1.0-rc5-dirty #11
[ 4.600853] Hardware name: SAMSUNG EXYNOS (Flattened Device Tree)
[ 4.606928] [<c011255c>] (unwind_backtrace) from [<c010de98>]
(show_stack+0x10/0x14)
[ 4.614642] [<c010de98>] (show_stack) from [<c0a6ee0c>]
(dump_stack+0x98/0xc4)
[ 4.621832] [<c0a6ee0c>] (dump_stack) from [<c0156e68>]
(___might_sleep+0x20c/0x2c0)
[ 4.629545] [<c0156e68>] (___might_sleep) from [<c0a8a470>]
(__mutex_lock+0x3c/0xa34)
[ 4.637343] [<c0a8a470>] (__mutex_lock) from [<c0a8ae84>]
(mutex_lock_nested+0x1c/0x24)
[ 4.645318] [<c0a8ae84>] (mutex_lock_nested) from [<c021b498>]
(perf_pmu_register+0x20/0x40c)
[ 4.653812] [<c021b498>] (perf_pmu_register) from [<c077edd0>]
(cci_pmu_probe+0x2f4/0x4a0)
[ 4.662044] [<c077edd0>] (cci_pmu_probe) from [<c0591880>]
(platform_drv_probe+0x48/0x98)
[ 4.670186] [<c0591880>] (platform_drv_probe) from [<c058f1b8>]
(really_probe+0x24c/0x410)
[ 4.678418] [<c058f1b8>] (really_probe) from [<c058f530>]
(driver_probe_device+0x78/0x1c0)
[ 4.686651] [<c058f530>] (driver_probe_device) from [<c058f8d4>]
(device_driver_attach+0x58/0x60)
[ 4.695492] [<c058f8d4>] (device_driver_attach) from [<c058f994>]
(__driver_attach+0xb8/0x158)
[ 4.704070] [<c058f994>] (__driver_attach) from [<c058d1f4>]
(bus_for_each_dev+0x74/0xb4)
[ 4.712213] [<c058d1f4>] (bus_for_each_dev) from [<c058e398>]
(bus_add_driver+0x1c0/0x200)
[ 4.720446] [<c058e398>] (bus_add_driver) from [<c0590850>]
(driver_register+0x74/0x108)
[ 4.728507] [<c0590850>] (driver_register) from [<c010315c>]
(do_one_initcall+0x90/0x434)
[ 4.736655] [<c010315c>] (do_one_initcall) from [<c0f0131c>]
(kernel_init_freeable+0x448/0x4ec)
[ 4.745319] [<c0f0131c>] (kernel_init_freeable) from [<c0a87c44>]
(kernel_init+0x8/0x110)
[ 4.753460] [<c0a87c44>] (kernel_init) from [<c01010b4>]
(ret_from_fork+0x14/0x20)
[ 4.760995] Exception stack(0xe88e1fb0 to 0xe88e1ff8)
[ 4.766011] 1fa0: 00000000
00000000 00000000 00000000
[ 4.774170] 1fc0: 00000000 00000000 00000000 00000000 00000000
00000000 00000000 00000000
[ 4.782315] 1fe0: 00000000 00000000 00000000 00000000 00000013 00000000
[ 4.789337] ARM CCI_400 PMU driver probed
[ 4.797261] NET: Registered protocol family 10

Hi Krzysztof,

Cache Coherent Interface (CCI) among Cortex-A15 and Cortex-A7, G2D, G3D and SSS

Level 0 > CPU blocks such as Cortex-A15 (CA15), Cortex-A7 (CA7) are
joined as the member of Level 0 CCI bus

Level 1 > Display engine block (DISP) and 2D graphic engines (G2D) are
directly connected to Level 1.
DISP, MDMA, SSS.

Level 2 > While all the other IP is connected to Level 1 bus via Level 2 bus
G3D, MSCL, MFC, ISP, JPEG/Rotator/DMA/PERI, NAND/SD/EMMC.

So my question is the mapped with the cci ip block correct.
Level 0 (cci_control0)
Level 1 (cci_control1)
Level 2 (cci_control1)

Best Regards

-Anand

2019-04-16 10:21:53

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

On Mon, 15 Apr 2019 at 14:24, Anand Moon <[email protected]> wrote:
> Cache Coherent Interface (CCI) among Cortex-A15 and Cortex-A7, G2D, G3D and SSS
>
> Level 0 > CPU blocks such as Cortex-A15 (CA15), Cortex-A7 (CA7) are
> joined as the member of Level 0 CCI bus
>
> Level 1 > Display engine block (DISP) and 2D graphic engines (G2D) are
> directly connected to Level 1.
> DISP, MDMA, SSS.
>
> Level 2 > While all the other IP is connected to Level 1 bus via Level 2 bus
> G3D, MSCL, MFC, ISP, JPEG/Rotator/DMA/PERI, NAND/SD/EMMC.
>
> So my question is the mapped with the cci ip block correct.
> Level 0 (cci_control0)
> Level 1 (cci_control1)
> Level 2 (cci_control1)

Hi Anand,

I do not understand the question - what is mapped with correctly or not?

Best regards,
Krzysztof

2019-04-17 04:27:54

by Anand Moon

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

Hi Krzysztof,

On Tue, 16 Apr 2019 at 15:49, Krzysztof Kozlowski <[email protected]> wrote:
>
> On Mon, 15 Apr 2019 at 14:24, Anand Moon <[email protected]> wrote:
> > Cache Coherent Interface (CCI) among Cortex-A15 and Cortex-A7, G2D, G3D and SSS
> >
> > Level 0 > CPU blocks such as Cortex-A15 (CA15), Cortex-A7 (CA7) are
> > joined as the member of Level 0 CCI bus
> >
> > Level 1 > Display engine block (DISP) and 2D graphic engines (G2D) are
> > directly connected to Level 1.
> > DISP, MDMA, SSS.
> >
> > Level 2 > While all the other IP is connected to Level 1 bus via Level 2 bus
> > G3D, MSCL, MFC, ISP, JPEG/Rotator/DMA/PERI, NAND/SD/EMMC.
> >
> > So my question is the mapped with the cci ip block correct.
> > Level 0 (cci_control0)
> > Level 1 (cci_control1)
> > Level 2 (cci_control1)
>
> Hi Anand,
>
> I do not understand the question - what is mapped with correctly or not?
>
> Best regards,
> Krzysztof

Following the https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/cci.txt
CCI node linked to CPU and DMA nodes for example.

On this line below diagram from Exynos 5422 UM show various IP block
linked to CCI level.
Below image just elaborate overall architecture of Exynos 5422 CCI.

[0] https://imgur.com/gallery/0xJSwGQ

So we should map the various IP block to corresponding CCI level.

Best Regards
-Anand

2019-04-17 07:47:31

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

On Wed, 17 Apr 2019 at 06:26, Anand Moon <[email protected]> wrote:
>
> Hi Krzysztof,
>
> On Tue, 16 Apr 2019 at 15:49, Krzysztof Kozlowski <[email protected]> wrote:
> >
> > On Mon, 15 Apr 2019 at 14:24, Anand Moon <[email protected]> wrote:
> > > Cache Coherent Interface (CCI) among Cortex-A15 and Cortex-A7, G2D, G3D and SSS
> > >
> > > Level 0 > CPU blocks such as Cortex-A15 (CA15), Cortex-A7 (CA7) are
> > > joined as the member of Level 0 CCI bus
> > >
> > > Level 1 > Display engine block (DISP) and 2D graphic engines (G2D) are
> > > directly connected to Level 1.
> > > DISP, MDMA, SSS.
> > >
> > > Level 2 > While all the other IP is connected to Level 1 bus via Level 2 bus
> > > G3D, MSCL, MFC, ISP, JPEG/Rotator/DMA/PERI, NAND/SD/EMMC.
> > >
> > > So my question is the mapped with the cci ip block correct.
> > > Level 0 (cci_control0)
> > > Level 1 (cci_control1)
> > > Level 2 (cci_control1)
> >
> > Hi Anand,
> >
> > I do not understand the question - what is mapped with correctly or not?
> >
> > Best regards,
> > Krzysztof
>
> Following the https://www.kernel.org/doc/Documentation/devicetree/bindings/arm/cci.txt
> CCI node linked to CPU and DMA nodes for example.
>
> On this line below diagram from Exynos 5422 UM show various IP block
> linked to CCI level.
> Below image just elaborate overall architecture of Exynos 5422 CCI.
>
> [0] https://imgur.com/gallery/0xJSwGQ
>
> So we should map the various IP block to corresponding CCI level.

Willy's patch did not touch cci_control{0,1} nor any other CCI levels
so I do not get what are you commenting. As for other CCI ports - we
do not define them and I do not see any users of device CCI API
(cci_enable_port_by_device() and cci_disable_port_by_device()). But
feel free to propose patches changing this. In general - it is easier
to discuss if you show the code/patch, not talk about some theoretical
change.

Best regards,
Krzysztof

2019-04-19 18:24:40

by Willy Wolff

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

Hi,

This patch can be dropped, as it needs more work.

In fact, the interrupts seems to be wrong. The interrupts suggested by
Anand Moon gave the same following results.

export CCI_DEV=CCI_400
export OMP_NUM_THREADS=2
sudo --preserve-env ./perf stat -a \
-e armv7_cortex_a7/config=0x11,name=a7_cycles/ \
-e armv7_cortex_a15/config=0x11,name=a15_cycles/ \
-e armv7_cortex_a7/config=0x19,name=a7_bus/ \
-e armv7_cortex_a15/config=0x19,name=a15_bus/ \
-e ${CCI_DEV}/config=0xff,name=cci400_cycles/ \
-e ${CCI_DEV}/config=0x0,name=cci400_si_rrq_hs_any/ \
-e ${CCI_DEV}/config=0xc,name=cci400_si_wrq_hs_any/ \
taskset -c 0,7 /home/user/cg.x.A 1

[..]

Performance counter stats for 'system wide':

9,362,850,550 a7_cycles
1,682,125,760 a15_cycles
68,920,347 a7_bus
61,484,352 a15_bus
3,789,936,935 cci400_cycles
0 cci400_si_rrq_hs_any
0 cci400_si_wrq_hs_any

9.541340558 seconds time elapsed

cg.x.A comes from NAS benchmark suite, compiled with fopenmp support, setup
to run 2 threads and taskmapped to ran on both a7 and a15 clusters.
a7_bus and a15_bus report main memory accesses.

Only cci400_cycles seems to be correct. However, all pmcs from the master
interface are reported as unsupported and all pmcs from the slave interface
return 0, which is probably not correct.
http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0470f/CJHICFBF.html

Would it be possible that someone from Samsung provide the right
interrupts values?
Many thanks.

Regards,
Willy

2019-04-19 21:20:41

by Robin Murphy

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

On 2019-04-19 6:53 pm, Willy Wolff wrote:
> Hi,
>
> This patch can be dropped, as it needs more work.
>
> In fact, the interrupts seems to be wrong. The interrupts suggested by
> Anand Moon gave the same following results.
>
> export CCI_DEV=CCI_400
> export OMP_NUM_THREADS=2
> sudo --preserve-env ./perf stat -a \
> -e armv7_cortex_a7/config=0x11,name=a7_cycles/ \
> -e armv7_cortex_a15/config=0x11,name=a15_cycles/ \
> -e armv7_cortex_a7/config=0x19,name=a7_bus/ \
> -e armv7_cortex_a15/config=0x19,name=a15_bus/ \
> -e ${CCI_DEV}/config=0xff,name=cci400_cycles/ \
> -e ${CCI_DEV}/config=0x0,name=cci400_si_rrq_hs_any/ \
> -e ${CCI_DEV}/config=0xc,name=cci400_si_wrq_hs_any/ \

From the look of those configs, you'll be counting events on slave
interface 0, which may not even have anything connected anyway. The CPU
clusters on a CCI-400 will be on slave interfaces 3 and 4, so try
something like '-e CCI_400/cci400_si_rrq_hs_any,source=4/'.

The interrupts only matter for counter overflow, so confirming those
could be done by picking a sufficiently frequent event, counting for
long enough to capture slightly more than 2^32 of those, then seeing
whether the overflow accumulates correctly or the count appears to go
backwards (and/or checking what fired in /proc/interrupts). I believe
the cycle counter is also 32-bit on CCI, so that should be relatively
easy; for the other counters beyond the first one it should be feasible
to schedule additional dummy events before the event of interest in
order to trick pmu_get_event_idx() into allocating the desired counter
for it.

Robin.

> taskset -c 0,7 /home/user/cg.x.A 1
>
> [..]
>
> Performance counter stats for 'system wide':
>
> 9,362,850,550 a7_cycles
> 1,682,125,760 a15_cycles
> 68,920,347 a7_bus
> 61,484,352 a15_bus
> 3,789,936,935 cci400_cycles
> 0 cci400_si_rrq_hs_any
> 0 cci400_si_wrq_hs_any
>
> 9.541340558 seconds time elapsed
>
> cg.x.A comes from NAS benchmark suite, compiled with fopenmp support, setup
> to run 2 threads and taskmapped to ran on both a7 and a15 clusters.
> a7_bus and a15_bus report main memory accesses.
>
> Only cci400_cycles seems to be correct. However, all pmcs from the master
> interface are reported as unsupported and all pmcs from the slave interface
> return 0, which is probably not correct.
> http://infocenter.arm.com/help/index.jsp?topic=/com.arm.doc.ddi0470f/CJHICFBF.html
>
> Would it be possible that someone from Samsung provide the right
> interrupts values?
> Many thanks.
>
> Regards,
> Willy
>
> _______________________________________________
> linux-arm-kernel mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

2019-04-20 06:51:59

by Willy Wolff

[permalink] [raw]
Subject: Re: [PATCH] ARM: dts: exynos: add CCI-400 PMU nodes support to Exynos542x SoCs

Indeed, many thanks Robin.

Using this, values sound better.
export OMP_NUM_THREADS=2
sudo --preserve-env ./perf stat -a \
-e armv7_cortex_a7/config=0x11,name=a7_cycles/ \
-e armv7_cortex_a15/config=0x11,name=a15_cycles/ \
-e armv7_cortex_a7/config=0x19,name=a7_bus/ \
-e armv7_cortex_a15/config=0x19,name=a15_bus/ \
-e CCI_400/config=0xff,name=cci400_cycles/ \
-e CCI_400/config=0x0,source=4,name=cci400_si_rrq_hs_any_s4/ \
-e CCI_400/config=0xc,source=4,name=cci400_si_wrq_hs_any_s4/ \
-e CCI_400/config=0x0,source=3,name=cci400_si_rrq_hs_any_s3/ \
-e CCI_400/config=0xc,source=3,name=cci400_si_wrq_hs_any_s3/ \
taskset -c 0,4 /home/user/EnergyManager/temperature_bench_install/bin/cg.x.A 1

[..]
Performance counter stats for 'system wide':

6,201,513,834 a7_cycles
2,781,009,706 a15_cycles
64,200,721 a7_bus
60,237,019 a15_bus
1,158,303,323 cci400_cycles
11,390,649 cci400_si_rrq_hs_any_s4
1,253,383 cci400_si_wrq_hs_any_s4
13,379,256 cci400_si_rrq_hs_any_s3
13,200,717 cci400_si_wrq_hs_any_s3

3.685087167 seconds time elapsed


Do you think that I should write a v2 with a better cover letter that shows
how to access this? By the way, I see that you contribute to that driver. I
haven't seen anything about this source=, do you think that it should be
documented somewhere?

Also, 0 and 4 on the interrupts represent GIC_SPI and IRQ_TYPE_LEVEL_HIGH,
I will do a v2 for this. Don't you think that the doc at
Documentation/devicetree/bindings/arm/cci.txt should use this too?

Willy

On Fri, Apr 19, 2019 at 10:18:02PM +0100, Robin Murphy wrote:
> On 2019-04-19 6:53 pm, Willy Wolff wrote:
> > Hi,
> >
> > This patch can be dropped, as it needs more work.
> >
> > In fact, the interrupts seems to be wrong. The interrupts suggested by
> > Anand Moon gave the same following results.
> >
> > export CCI_DEV=CCI_400
> > export OMP_NUM_THREADS=2
> > sudo --preserve-env ./perf stat -a \
> > -e armv7_cortex_a7/config=0x11,name=a7_cycles/ \
> > -e armv7_cortex_a15/config=0x11,name=a15_cycles/ \
> > -e armv7_cortex_a7/config=0x19,name=a7_bus/ \
> > -e armv7_cortex_a15/config=0x19,name=a15_bus/ \
> > -e ${CCI_DEV}/config=0xff,name=cci400_cycles/ \
> > -e ${CCI_DEV}/config=0x0,name=cci400_si_rrq_hs_any/ \
> > -e ${CCI_DEV}/config=0xc,name=cci400_si_wrq_hs_any/ \
>
> From the look of those configs, you'll be counting events on slave interface
> 0, which may not even have anything connected anyway. The CPU clusters on a
> CCI-400 will be on slave interfaces 3 and 4, so try something like '-e
> CCI_400/cci400_si_rrq_hs_any,source=4/'.
>
> The interrupts only matter for counter overflow, so confirming those could
> be done by picking a sufficiently frequent event, counting for long enough
> to capture slightly more than 2^32 of those, then seeing whether the
> overflow accumulates correctly or the count appears to go backwards (and/or
> checking what fired in /proc/interrupts). I believe the cycle counter is
> also 32-bit on CCI, so that should be relatively easy; for the other
> counters beyond the first one it should be feasible to schedule additional
> dummy events before the event of interest in order to trick
> pmu_get_event_idx() into allocating the desired counter for it.
>
> Robin.