[PATCHv2 0/3] Improved perf support for imx53/ppd

[PATCHv2 0/3] Improved perf support for imx53/ppd

[Sebastian Reichel]

Hi,

This improves perf on imx53 by adding support for enabling the Secure
Debug Enable Register (SDER) SUNIDEN bit. This unlocks new
functionality:

ppd before patchset# perf stat -e cycles,instructions sleep 1 2>&1 | grep instructions
                 0      instructions              #    0.00  insn per cycle

ppd  after patchset# perf stat -e cycles,instructions sleep 1 2>&1 | grep instructions
            177864      instructions              #    0.05  insn per cycle

Changes since PATCHv1:
 * Update DTS patch to reference imx53.dtsi's pmu node
 * Remove a superfluous newline in first patch

-- Sebastian

Peter Senna Tschudin (2):
  ARM: imx53: add SoC specific PMU setup
  ARM: dts: imx53: PPD: Enable secure-reg-access

Sebastian Reichel (1):
  drivers/perf: arm_pmu: Add platform hardware setup hooks.

 arch/arm/boot/dts/imx53-ppd.dts |  4 +++
 arch/arm/boot/dts/imx53.dtsi    |  2 +-
 arch/arm/mach-imx/mach-imx53.c  | 68 +++++++++++++++++++++++++++++++++++++++++
 drivers/perf/arm_pmu.c          | 12 ++++++++
 include/linux/perf/arm_pmu.h    |  5 +++
 5 files changed, 90 insertions(+), 1 deletion(-)

-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 1/3] drivers/perf: arm_pmu: Add platform hardware setup hooks.

[Sebastian Reichel]

Allow platform specific code to be called when enabling or
disabling the hardware.

Signed-off-by: Martin Fuzzey <mfuzzey@parkeon.com>
Signed-off-by: Peter Senna Tschudin <peter.senna@collabora.com>
[rebased to v4.12]
Signed-off-by: Nandor Han <nandor.han@ge.com>
[Simplify and cleanup for upstreaming]
Signed-off-by: Sebastian Reichel <sebastian.reichel@collabora.co.uk>
---
 drivers/perf/arm_pmu.c       | 12 ++++++++++++
 include/linux/perf/arm_pmu.h |  5 +++++
 2 files changed, 17 insertions(+)

diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
index 7bc5eee96b31..9c282914ee67 100644
--- a/drivers/perf/arm_pmu.c
+++ b/drivers/perf/arm_pmu.c
@@ -452,8 +452,10 @@ static int armpmu_event_init(struct perf_event *event)
 static void armpmu_enable(struct pmu *pmu)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(pmu);
+	struct arm_pmu_platdata *platdata = armpmu_get_platdata(armpmu);
 	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
 	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
+	int err;
 
 	/* For task-bound events we may be called on other CPUs */
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
@@ -461,17 +463,27 @@ static void armpmu_enable(struct pmu *pmu)
 
 	if (enabled)
 		armpmu->start(armpmu);
+
+	if (platdata && platdata->reserve_hardware) {
+		err = platdata->reserve_hardware(armpmu);
+		if (err)
+			dev_warn(&armpmu->plat_device->dev, "Could not reserve PMU hardware!\n");
+	}
 }
 
 static void armpmu_disable(struct pmu *pmu)
 {
 	struct arm_pmu *armpmu = to_arm_pmu(pmu);
+	struct arm_pmu_platdata *platdata = armpmu_get_platdata(armpmu);
 
 	/* For task-bound events we may be called on other CPUs */
 	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
 		return;
 
 	armpmu->stop(armpmu);
+
+	if (platdata && platdata->release_hardware)
+		platdata->release_hardware(armpmu);
 }
 
 /*
diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
index af0f44effd44..942ae9bfe971 100644
--- a/include/linux/perf/arm_pmu.h
+++ b/include/linux/perf/arm_pmu.h
@@ -17,6 +17,8 @@
 #include <linux/sysfs.h>
 #include <asm/cputype.h>
 
+struct arm_pmu;
+
 /*
  * struct arm_pmu_platdata - ARM PMU platform data
  *
@@ -31,6 +33,8 @@
 struct arm_pmu_platdata {
 	irqreturn_t (*handle_irq)(int irq, void *dev,
 				  irq_handler_t pmu_handler);
+	int (*reserve_hardware)(struct arm_pmu *arm_pmu);
+	void (*release_hardware)(struct arm_pmu *arm_pmu);
 	unsigned long irq_flags;
 };
 
@@ -112,6 +116,7 @@ struct arm_pmu {
 	int		(*map_event)(struct perf_event *event);
 	int		num_events;
 	u64		max_period;
+	bool		reserved_hardware;
 	bool		secure_access; /* 32-bit ARM only */
 #define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40
 	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
-- 
2.15.1

[PATCHv2 2/3] ARM: imx53: add SoC specific PMU setup

[Sebastian Reichel]

From: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

On i.MX53 it is necessary to set the DBG_EN bit in the
platform GPC register to enable access to PMU counters
other than the cycle counter.

Signed-off-by: Martin Fuzzey <mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ@public.gmane.org>
Signed-off-by: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 arch/arm/mach-imx/mach-imx53.c | 68 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 68 insertions(+)

diff --git a/arch/arm/mach-imx/mach-imx53.c b/arch/arm/mach-imx/mach-imx53.c
index 07c2e8dca494..da7c80371f32 100644
--- a/arch/arm/mach-imx/mach-imx53.c
+++ b/arch/arm/mach-imx/mach-imx53.c
@@ -17,6 +17,7 @@
 #include <linux/irq.h>
 #include <linux/of_irq.h>
 #include <linux/of_platform.h>
+#include <linux/perf/arm_pmu.h>
 #include <asm/mach/arch.h>
 #include <asm/mach/time.h>
 
@@ -28,10 +29,77 @@ static void __init imx53_init_early(void)
 	mxc_set_cpu_type(MXC_CPU_MX53);
 }
 
+/* Hard code as this is i.Mx53 only file */
+#define MXC_CORTEXA8_PLAT_GPC 0x63fa0004
+#define GPC_DBG_EN (1 << 16)
+
+static void __iomem *imx53_pmu_get_gpc(void)
+{
+	static void __iomem *gpc;
+
+	if (!gpc) {
+		gpc = ioremap(MXC_CORTEXA8_PLAT_GPC, 4);
+		if (!gpc)
+			printk_once(KERN_INFO "unable to map GPC to enable perf\n");
+	}
+
+	return gpc;
+}
+
+static int imx53_pmu_reserve(struct arm_pmu *arm_pmu)
+{
+	void __iomem *gpc_reg;
+	u32 gpc;
+
+	gpc_reg = imx53_pmu_get_gpc();
+	if (!gpc_reg)
+		return 0;
+
+	gpc = __raw_readl(gpc_reg);
+	if (gpc & GPC_DBG_EN) {
+		arm_pmu->reserved_hardware = false;
+	} else {
+		gpc |= GPC_DBG_EN;
+		__raw_writel(gpc, gpc_reg);
+		arm_pmu->reserved_hardware = true;
+	}
+
+	return 0;
+}
+
+static void imx53_pmu_release(struct arm_pmu *arm_pmu)
+{
+	void __iomem *gpc_reg;
+	u32 gpc;
+
+	gpc_reg = imx53_pmu_get_gpc();
+	if (!gpc_reg)
+		return;
+
+	if (arm_pmu->reserved_hardware) {
+		gpc = __raw_readl(gpc_reg);
+		gpc &= ~GPC_DBG_EN;
+		__raw_writel(gpc, gpc_reg);
+		arm_pmu->reserved_hardware = false;
+	}
+}
+
+static struct arm_pmu_platdata imx53_pmu_platdata = {
+	.reserve_hardware = imx53_pmu_reserve,
+	.release_hardware = imx53_pmu_release,
+};
+
+static struct of_dev_auxdata imx53_auxdata_lookup[] __initdata = {
+	OF_DEV_AUXDATA("arm,cortex-a8-pmu", 0, "arm-pmu", &imx53_pmu_platdata),
+	{}
+};
+
 static void __init imx53_dt_init(void)
 {
 	imx_src_init();
 
+	of_platform_populate(NULL, of_default_bus_match_table,
+					imx53_auxdata_lookup, NULL);
 	imx_aips_allow_unprivileged_access("fsl,imx53-aipstz");
 }
 
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCHv2 3/3] ARM: dts: imx53: PPD: Enable secure-reg-access

[Sebastian Reichel]

From: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

Add secure-reg-access on PPD device tree to enable PMU and
hardware counters for perf.

Signed-off-by: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
---
 arch/arm/boot/dts/imx53-ppd.dts | 4 ++++
 arch/arm/boot/dts/imx53.dtsi    | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/arch/arm/boot/dts/imx53-ppd.dts b/arch/arm/boot/dts/imx53-ppd.dts
index cce959438a79..4ecbe6e4eecb 100644
--- a/arch/arm/boot/dts/imx53-ppd.dts
+++ b/arch/arm/boot/dts/imx53-ppd.dts
@@ -556,6 +556,10 @@
 	};
 };
 
+&pmu {
+	secure-reg-access;
+};
+
 &pwm1 {
 	pinctrl-names = "default";
 	pinctrl-0 = <&pinctrl_pwm1>;
diff --git a/arch/arm/boot/dts/imx53.dtsi b/arch/arm/boot/dts/imx53.dtsi
index 1040251f2951..7df4853dc771 100644
--- a/arch/arm/boot/dts/imx53.dtsi
+++ b/arch/arm/boot/dts/imx53.dtsi
@@ -116,7 +116,7 @@
 		};
 	};
 
-	pmu {
+	pmu: pmu {
 		compatible = "arm,cortex-a8-pmu";
 		interrupt-parent = <&tzic>;
 		interrupts = <77>;
-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 1/3] drivers/perf: arm_pmu: Add platform hardware setup hooks.

[Robin Murphy]

Hi Sebastian,

On 06/02/18 14:26, Sebastian Reichel wrote:
> Allow platform specific code to be called when enabling or
> disabling the hardware.

Since arm_pmu_platdata wants to go away[1], does i.MX53 actually need 
all this machinery, or would it suffice to set DBG_EN from platform code 
at boot time and simply leave it that way?

Robin.

[1] 
http://lists.infradead.org/pipermail/linux-arm-kernel/2018-February/557838.html

> Signed-off-by: Martin Fuzzey <mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> [rebased to v4.12]
> Signed-off-by: Nandor Han <nandor.han-JJi787mZWgc@public.gmane.org>
> [Simplify and cleanup for upstreaming]
> Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
> ---
>   drivers/perf/arm_pmu.c       | 12 ++++++++++++
>   include/linux/perf/arm_pmu.h |  5 +++++
>   2 files changed, 17 insertions(+)
> 
> diff --git a/drivers/perf/arm_pmu.c b/drivers/perf/arm_pmu.c
> index 7bc5eee96b31..9c282914ee67 100644
> --- a/drivers/perf/arm_pmu.c
> +++ b/drivers/perf/arm_pmu.c
> @@ -452,8 +452,10 @@ static int armpmu_event_init(struct perf_event *event)
>   static void armpmu_enable(struct pmu *pmu)
>   {
>   	struct arm_pmu *armpmu = to_arm_pmu(pmu);
> +	struct arm_pmu_platdata *platdata = armpmu_get_platdata(armpmu);
>   	struct pmu_hw_events *hw_events = this_cpu_ptr(armpmu->hw_events);
>   	int enabled = bitmap_weight(hw_events->used_mask, armpmu->num_events);
> +	int err;
>   
>   	/* For task-bound events we may be called on other CPUs */
>   	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
> @@ -461,17 +463,27 @@ static void armpmu_enable(struct pmu *pmu)
>   
>   	if (enabled)
>   		armpmu->start(armpmu);
> +
> +	if (platdata && platdata->reserve_hardware) {
> +		err = platdata->reserve_hardware(armpmu);
> +		if (err)
> +			dev_warn(&armpmu->plat_device->dev, "Could not reserve PMU hardware!\n");
> +	}
>   }
>   
>   static void armpmu_disable(struct pmu *pmu)
>   {
>   	struct arm_pmu *armpmu = to_arm_pmu(pmu);
> +	struct arm_pmu_platdata *platdata = armpmu_get_platdata(armpmu);
>   
>   	/* For task-bound events we may be called on other CPUs */
>   	if (!cpumask_test_cpu(smp_processor_id(), &armpmu->supported_cpus))
>   		return;
>   
>   	armpmu->stop(armpmu);
> +
> +	if (platdata && platdata->release_hardware)
> +		platdata->release_hardware(armpmu);
>   }
>   
>   /*
> diff --git a/include/linux/perf/arm_pmu.h b/include/linux/perf/arm_pmu.h
> index af0f44effd44..942ae9bfe971 100644
> --- a/include/linux/perf/arm_pmu.h
> +++ b/include/linux/perf/arm_pmu.h
> @@ -17,6 +17,8 @@
>   #include <linux/sysfs.h>
>   #include <asm/cputype.h>
>   
> +struct arm_pmu;
> +
>   /*
>    * struct arm_pmu_platdata - ARM PMU platform data
>    *
> @@ -31,6 +33,8 @@
>   struct arm_pmu_platdata {
>   	irqreturn_t (*handle_irq)(int irq, void *dev,
>   				  irq_handler_t pmu_handler);
> +	int (*reserve_hardware)(struct arm_pmu *arm_pmu);
> +	void (*release_hardware)(struct arm_pmu *arm_pmu);
>   	unsigned long irq_flags;
>   };
>   
> @@ -112,6 +116,7 @@ struct arm_pmu {
>   	int		(*map_event)(struct perf_event *event);
>   	int		num_events;
>   	u64		max_period;
> +	bool		reserved_hardware;
>   	bool		secure_access; /* 32-bit ARM only */
>   #define ARMV8_PMUV3_MAX_COMMON_EVENTS 0x40
>   	DECLARE_BITMAP(pmceid_bitmap, ARMV8_PMUV3_MAX_COMMON_EVENTS);
> 
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2 2/3] ARM: imx53: add SoC specific PMU setup

[Fabio Estevam]

On Tue, Feb 6, 2018 at 12:26 PM, Sebastian Reichel
<sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org> wrote:
> From: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>

Looking at http://lists.infradead.org/pipermail/linux-arm-kernel/2014-August/277931.html
this patch seems to be originally from Martin Fuzzey.

In this case I would expect to see his name in the From field.

Please check the same on the other patches.

> On i.MX53 it is necessary to set the DBG_EN bit in the
> platform GPC register to enable access to PMU counters
> other than the cycle counter.
>
> Signed-off-by: Martin Fuzzey <mfuzzey-mB3Nsq4MPf1BDgjK7y7TUQ@public.gmane.org>
> Signed-off-by: Peter Senna Tschudin <peter.senna-ZGY8ohtN/8qB+jHODAdFcQ@public.gmane.org>
> Signed-off-by: Sebastian Reichel <sebastian.reichel-ZGY8ohtN/8pPYcu2f3hruQ@public.gmane.org>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html