Re: [PATCH] perf: ampere: Add support for Ampere SoC PMUs

[Robin Murphy]

On 2023-04-27 22:53, Ilkka Koskinen wrote:
> Add support for Ampere SoC PMUs. This driver supports MCU PMU
> available in the AmpereOne SoC.
>
> Signed-off-by: Ilkka Koskinen <ilkka@xxxxxxxxxxxxxxxxxxxxxx>
> ---
>   .../admin-guide/perf/ampere-soc-pmu.rst       |  36 +
>   Documentation/admin-guide/perf/index.rst      |   1 +
>   drivers/perf/Kconfig                          |  10 +
>   drivers/perf/Makefile                         |   1 +
>   drivers/perf/ampere_soc_pmu.c                 | 724 ++++++++++++++++++
>   5 files changed, 772 insertions(+)
>   create mode 100644 Documentation/admin-guide/perf/ampere-soc-pmu.rst
>   create mode 100644 drivers/perf/ampere_soc_pmu.c
>
> diff --git a/Documentation/admin-guide/perf/ampere-soc-pmu.rst b/Documentation/admin-guide/perf/ampere-soc-pmu.rst
> new file mode 100644
> index 000000000000..5161fbd1c548
> --- /dev/null
> +++ b/Documentation/admin-guide/perf/ampere-soc-pmu.rst
> @@ -0,0 +1,36 @@
> +.. SPDX-License-Identifier: GPL-2.0
> +
> +============================================
> +Ampere SoC Performance Monitoring Unit (PMU)
> +============================================
> +
> +Ampere SoC PMU is a generic PMU IP. At the first phase it's used for counting
> +MCU events on AmpereOne.
> +
> +MCU PMU events
> +--------------
> +
> +The PMU driver registers a PMU device for every supported PMU instance on each
> +SoC. See /sys/devices/AMPC0100:<nn>/.
> +
> +The PMU driver supports setting filters for "rank", "bank", and "threshold". The
> +filter settings are device specific and shared between all the relevant events.
> +The default value for all the filters is zero. The filters can be modified by
> +setting them with the last event of the particular device. All the previous
> +settings are overwritten.

Yeah, that doesn't really work... what is the "last event" relative to? 
Order of arguments to arbitrary tools? Order of perf_event_open 
syscalls? Order in which events are actually scheduled on the PMU? 
(which users can't even control - think event rotation)

To be practical I think you'll have to handle this the same way as 
arm_smmuv3_pmu's global filtering, and only allow events with matching 
filter configs to be scheduled together.

[...]
> +#define SOC_PMEVCNTR0_LO	0x000
> +#define SOC_PMEVCNTR0_HI	0x004
> +#define SOC_PMCCNTR_LO		0x0F8
> +#define SOC_PMCCNTR_HI		0x0FC
> +
> +#define SOC_PMEVTYPER0		0x400
> +
> +#define SOC_PMELCSR		0xA10
> +
> +#define SOC_PMCNTENSET		0xC00
> +#define SOC_PMCNTENCLR		0xC20
> +#define SOC_PMINTENSET		0xC40
> +#define SOC_PMINTENCLR		0xC60
> +#define SOC_PMOVSCLR		0xC80
> +#define SOC_PMOVSSET		0xCC0
> +
> +#define SOC_PMAUXR0		0xD80
> +#define SOC_PMAUXR1		0xD84
> +#define SOC_PMAUXR2		0xD88
> +#define SOC_PMAUXR3		0xD8C
> +
> +#define SOC_PMCFGR		0xE00
> +#define SOC_PMCR		0xE04
> +#define PMU_PMCR_E		BIT(0)
> +#define PMU_PMCR_P		BIT(1)
> +
> +#define SOC_PMAUTHSTATUS	0xFB8
> +#define SOC_PMDEVARCH		0xFBC
> +#define SOC_PMDEVTYPE		0xFCC
> +#define SOC_PMPIDR4		0xFD0
> +#define SOC_PMPIDR0		0xFE0
> +#define SOC_PMPIDR1		0xFE4
> +#define SOC_PMPIDR2		0xFE8
> +#define SOC_PMPIDR3		0xFEC
> +#define SOC_PMCIDR0		0xFF0
> +#define SOC_PMCIDR1		0xFF4
> +#define SOC_PMCIDR2		0xFF8
> +#define SOC_PMCIDR3		0xFFC

This register map quite clearly follows the Arm CoreSight PMU 
architecture. Nice to see it being used, but would you mind having a go 
at hooking up your imp-def bits to the existing arm_cspmu driver?

Thanks,
Robin.