linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/7] Support Krait CPU PMUs
@ 2014-01-08 22:59 Stephen Boyd
  2014-01-08 22:59 ` [PATCH 1/7] ARM: perf_event: Silence sparse warning Stephen Boyd
                   ` (6 more replies)
  0 siblings, 7 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

This patchset adds support for the Krait CPU PMUs. I split the main patch
up into two parts: first the basic support that gets us just the architected
events and second the full support patch that tackles the PMRESR interface.
I'm willing to squash things together if desired.

Please note, this patchset relies on the per-cpu irq patch from Vinayak Kale,
7f4a8e7b1943 (genirq: Add an accessor for IRQ_PER_CPU flag, 2013-12-04),
that's sitting in linux-next. Patches are based on commit 21dea6695134 
(ARM: msm_defconfig: Enable restart driver, 2013-12-20) in linux-next.

Stephen Boyd (7):
  ARM: perf_event: Silence sparse warning
  ARM: perf_event: Support percpu irqs for the CPU PMU
  ARM: perf_event: Add basic support for Krait CPU PMUs
  ARM: perf_event: Add hook for event index clearing
  ARM: perf_event: Fully support Krait CPU PMU events
  devicetree: bindings: Document Krait performance monitor units (PMU)
  ARM: dts: msm: Add krait-pmu to platforms with Krait CPUs

 Documentation/devicetree/bindings/arm/pmu.txt |   1 +
 arch/arm/boot/dts/qcom-msm8960-cdp.dts        |   5 +
 arch/arm/boot/dts/qcom-msm8974.dtsi           |   5 +
 arch/arm/include/asm/pmu.h                    |   1 +
 arch/arm/kernel/perf_event.c                  |   2 +
 arch/arm/kernel/perf_event_cpu.c              | 110 ++++--
 arch/arm/kernel/perf_event_v7.c               | 545 ++++++++++++++++++++++++++
 7 files changed, 640 insertions(+), 29 deletions(-)

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/7] ARM: perf_event: Silence sparse warning
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  2014-01-09 10:45   ` Will Deacon
  2014-01-08 22:59 ` [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU Stephen Boyd
                   ` (5 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

arch/arm/kernel/perf_event_cpu.c:274:25: warning: incorrect type in assignment (different modifiers)
arch/arm/kernel/perf_event_cpu.c:274:25: expected int ( *init_fn )( ... )
arch/arm/kernel/perf_event_cpu.c:274:25: got void const *const data

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/perf_event_cpu.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index d85055cd24ba..20d553c9f5e2 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -254,7 +254,7 @@ static int probe_current_pmu(struct arm_pmu *pmu)
 static int cpu_pmu_device_probe(struct platform_device *pdev)
 {
 	const struct of_device_id *of_id;
-	int (*init_fn)(struct arm_pmu *);
+	const int (*init_fn)(struct arm_pmu *);
 	struct device_node *node = pdev->dev.of_node;
 	struct arm_pmu *pmu;
 	int ret = -ENODEV;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
  2014-01-08 22:59 ` [PATCH 1/7] ARM: perf_event: Silence sparse warning Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  2014-01-09 10:49   ` Will Deacon
  2014-01-08 22:59 ` [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs Stephen Boyd
                   ` (4 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

Some CPU PMUs are wired up with one PPI for all the CPUs instead
of with a different SPI for each CPU. Add support for these
devices.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/perf_event_cpu.c | 107 +++++++++++++++++++++++++++++----------
 1 file changed, 79 insertions(+), 28 deletions(-)

diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index 20d553c9f5e2..58916ffa61e5 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -25,6 +25,8 @@
 #include <linux/platform_device.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/irq.h>
+#include <linux/irqdesc.h>
 
 #include <asm/cputype.h>
 #include <asm/irq_regs.h>
@@ -33,6 +35,7 @@
 /* Set at runtime when we know what CPU type we are. */
 static struct arm_pmu *cpu_pmu;
 
+static DEFINE_PER_CPU(struct arm_pmu *, percpu_pmu);
 static DEFINE_PER_CPU(struct perf_event * [ARMPMU_MAX_HWEVENTS], hw_events);
 static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(ARMPMU_MAX_HWEVENTS)], used_mask);
 static DEFINE_PER_CPU(struct pmu_hw_events, cpu_hw_events);
@@ -71,6 +74,26 @@ static struct pmu_hw_events *cpu_pmu_get_cpu_events(void)
 	return this_cpu_ptr(&cpu_hw_events);
 }
 
+static void cpu_pmu_enable_percpu_irq(void *data)
+{
+	struct arm_pmu *cpu_pmu = data;
+	struct platform_device *pmu_device = cpu_pmu->plat_device;
+	int irq = platform_get_irq(pmu_device, 0);
+
+	enable_percpu_irq(irq, IRQ_TYPE_NONE);
+	cpumask_set_cpu(smp_processor_id(), &cpu_pmu->active_irqs);
+}
+
+static void cpu_pmu_disable_percpu_irq(void *data)
+{
+	struct arm_pmu *cpu_pmu = data;
+	struct platform_device *pmu_device = cpu_pmu->plat_device;
+	int irq = platform_get_irq(pmu_device, 0);
+
+	cpumask_clear_cpu(smp_processor_id(), &cpu_pmu->active_irqs);
+	disable_percpu_irq(irq);
+}
+
 static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu)
 {
 	int i, irq, irqs;
@@ -78,15 +101,29 @@ static void cpu_pmu_free_irq(struct arm_pmu *cpu_pmu)
 
 	irqs = min(pmu_device->num_resources, num_possible_cpus());
 
-	for (i = 0; i < irqs; ++i) {
-		if (!cpumask_test_and_clear_cpu(i, &cpu_pmu->active_irqs))
-			continue;
-		irq = platform_get_irq(pmu_device, i);
-		if (irq >= 0)
-			free_irq(irq, cpu_pmu);
+	irq = platform_get_irq(pmu_device, 0);
+	if (irq >= 0 && irq_is_percpu(irq)) {
+		on_each_cpu(cpu_pmu_disable_percpu_irq, cpu_pmu, 1);
+		free_percpu_irq(irq, &percpu_pmu);
+	} else {
+		for (i = 0; i < irqs; ++i) {
+			if (!cpumask_test_and_clear_cpu(i, &cpu_pmu->active_irqs))
+				continue;
+			irq = platform_get_irq(pmu_device, i);
+			if (irq >= 0)
+				free_irq(irq, cpu_pmu);
+		}
 	}
 }
 
+static irq_handler_t cpu_handler;
+
+static irqreturn_t cpu_pmu_dispatch_irq(int irq, void *dev)
+{
+	struct arm_pmu *arm_pmu = *(struct arm_pmu **)dev;
+	return cpu_handler(irq, arm_pmu);
+}
+
 static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 {
 	int i, err, irq, irqs;
@@ -101,33 +138,46 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 		return -ENODEV;
 	}
 
-	for (i = 0; i < irqs; ++i) {
-		err = 0;
-		irq = platform_get_irq(pmu_device, i);
-		if (irq < 0)
-			continue;
-
-		/*
-		 * If we have a single PMU interrupt that we can't shift,
-		 * assume that we're running on a uniprocessor machine and
-		 * continue. Otherwise, continue without this interrupt.
-		 */
-		if (irq_set_affinity(irq, cpumask_of(i)) && irqs > 1) {
-			pr_warning("unable to set irq affinity (irq=%d, cpu=%u)\n",
-				    irq, i);
-			continue;
-		}
-
-		err = request_irq(irq, handler,
-				  IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu",
-				  cpu_pmu);
+	irq = platform_get_irq(pmu_device, 0);
+	if (irq >= 0 && irq_is_percpu(irq)) {
+		cpu_handler = handler;
+		err = request_percpu_irq(irq, cpu_pmu_dispatch_irq, "arm-pmu",
+				&percpu_pmu);
 		if (err) {
 			pr_err("unable to request IRQ%d for ARM PMU counters\n",
 				irq);
 			return err;
 		}
-
-		cpumask_set_cpu(i, &cpu_pmu->active_irqs);
+		on_each_cpu(cpu_pmu_enable_percpu_irq, cpu_pmu, 1);
+	} else {
+		for (i = 0; i < irqs; ++i) {
+			err = 0;
+			irq = platform_get_irq(pmu_device, i);
+			if (irq < 0)
+				continue;
+
+			/*
+			 * If we have a single PMU interrupt that we can't shift,
+			 * assume that we're running on a uniprocessor machine and
+			 * continue. Otherwise, continue without this interrupt.
+			 */
+			if (irq_set_affinity(irq, cpumask_of(i)) && irqs > 1) {
+				pr_warning("unable to set irq affinity (irq=%d, cpu=%u)\n",
+					    irq, i);
+				continue;
+			}
+
+			err = request_irq(irq, handler,
+					  IRQF_NOBALANCING | IRQF_NO_THREAD, "arm-pmu",
+					  cpu_pmu);
+			if (err) {
+				pr_err("unable to request IRQ%d for ARM PMU counters\n",
+					irq);
+				return err;
+			}
+
+			cpumask_set_cpu(i, &cpu_pmu->active_irqs);
+		}
 	}
 
 	return 0;
@@ -141,6 +191,7 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 		events->events = per_cpu(hw_events, cpu);
 		events->used_mask = per_cpu(used_mask, cpu);
 		raw_spin_lock_init(&events->pmu_lock);
+		per_cpu(percpu_pmu, cpu) = cpu_pmu;
 	}
 
 	cpu_pmu->get_hw_events	= cpu_pmu_get_cpu_events;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
  2014-01-08 22:59 ` [PATCH 1/7] ARM: perf_event: Silence sparse warning Stephen Boyd
  2014-01-08 22:59 ` [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  2014-01-09 11:04   ` Will Deacon
  2014-01-08 22:59 ` [PATCH 4/7] ARM: perf_event: Add hook for event index clearing Stephen Boyd
                   ` (3 subsequent siblings)
  6 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

Add basic support for the Krait CPU PMU. This allows us to use
the architected functionality of the PMU.

This is based on code originally written by Ashwin Chaugule and
Neil Leeder [1].

[1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/arch/arm/kernel/perf_event_msm_krait.c?h=msm-3.4

Cc: Neil Leeder <nleeder@codeaurora.org>
Cc: Ashwin Chaugule <ashwinc@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/perf_event_cpu.c |   1 +
 arch/arm/kernel/perf_event_v7.c  | 165 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 166 insertions(+)

diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index 58916ffa61e5..ab69abf14efd 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -239,6 +239,7 @@ static struct of_device_id cpu_pmu_of_device_ids[] = {
 	{.compatible = "arm,arm11mpcore-pmu",	.data = armv6mpcore_pmu_init},
 	{.compatible = "arm,arm1176-pmu",	.data = armv6pmu_init},
 	{.compatible = "arm,arm1136-pmu",	.data = armv6pmu_init},
+	{.compatible = "qcom,krait-pmu",	.data = krait_pmu_init},
 	{},
 };
 
diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index 039cffb053a7..ec86cf42d2ba 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -732,6 +732,138 @@ static const unsigned armv7_a7_perf_cache_map[PERF_COUNT_HW_CACHE_MAX]
 };
 
 /*
+ * Krait HW events mapping
+ */
+static const unsigned krait_perf_map[PERF_COUNT_HW_MAX] = {
+	[PERF_COUNT_HW_CPU_CYCLES]	    = ARMV7_PERFCTR_CPU_CYCLES,
+	[PERF_COUNT_HW_INSTRUCTIONS]	    = ARMV7_PERFCTR_INSTR_EXECUTED,
+	[PERF_COUNT_HW_CACHE_REFERENCES]    = HW_OP_UNSUPPORTED,
+	[PERF_COUNT_HW_CACHE_MISSES]	    = HW_OP_UNSUPPORTED,
+	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = ARMV7_PERFCTR_PC_WRITE,
+	[PERF_COUNT_HW_BRANCH_MISSES]	    = ARMV7_PERFCTR_PC_BRANCH_MIS_PRED,
+	[PERF_COUNT_HW_BUS_CYCLES]	    = ARMV7_PERFCTR_CLOCK_CYCLES,
+};
+
+static const unsigned krait_perf_map_no_branch[PERF_COUNT_HW_MAX] = {
+	[PERF_COUNT_HW_CPU_CYCLES]	    = ARMV7_PERFCTR_CPU_CYCLES,
+	[PERF_COUNT_HW_INSTRUCTIONS]	    = ARMV7_PERFCTR_INSTR_EXECUTED,
+	[PERF_COUNT_HW_CACHE_REFERENCES]    = HW_OP_UNSUPPORTED,
+	[PERF_COUNT_HW_CACHE_MISSES]	    = HW_OP_UNSUPPORTED,
+	[PERF_COUNT_HW_BRANCH_INSTRUCTIONS] = HW_OP_UNSUPPORTED,
+	[PERF_COUNT_HW_BRANCH_MISSES]	    = ARMV7_PERFCTR_PC_BRANCH_MIS_PRED,
+	[PERF_COUNT_HW_BUS_CYCLES]	    = ARMV7_PERFCTR_CLOCK_CYCLES,
+};
+
+static const unsigned krait_perf_cache_map[PERF_COUNT_HW_CACHE_MAX]
+					  [PERF_COUNT_HW_CACHE_OP_MAX]
+					  [PERF_COUNT_HW_CACHE_RESULT_MAX] = {
+	[C(L1D)] = {
+		/*
+		 * The performance counters don't differentiate between read
+		 * and write accesses/misses so this isn't strictly correct,
+		 * but it's the best we can do. Writes and reads get
+		 * combined.
+		 */
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= ARMV7_PERFCTR_L1_DCACHE_ACCESS,
+			[C(RESULT_MISS)]	= ARMV7_PERFCTR_L1_DCACHE_REFILL,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= ARMV7_PERFCTR_L1_DCACHE_ACCESS,
+			[C(RESULT_MISS)]	= ARMV7_PERFCTR_L1_DCACHE_REFILL,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+	[C(L1I)] = {
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+	[C(LL)] = {
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+	[C(DTLB)] = {
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+	[C(ITLB)] = {
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+	[C(BPU)] = {
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= ARMV7_PERFCTR_PC_BRANCH_PRED,
+			[C(RESULT_MISS)]	= ARMV7_PERFCTR_PC_BRANCH_MIS_PRED,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= ARMV7_PERFCTR_PC_BRANCH_PRED,
+			[C(RESULT_MISS)]	= ARMV7_PERFCTR_PC_BRANCH_MIS_PRED,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+	[C(NODE)] = {
+		[C(OP_READ)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_WRITE)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+		[C(OP_PREFETCH)] = {
+			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+		},
+	},
+};
+
+/*
  * Perf Events' indices
  */
 #define	ARMV7_IDX_CYCLE_COUNTER	0
@@ -1212,6 +1344,18 @@ static int armv7_a7_map_event(struct perf_event *event)
 				&armv7_a7_perf_cache_map, 0xFF);
 }
 
+static int krait_map_event(struct perf_event *event)
+{
+	return armpmu_map_event(event, &krait_perf_map,
+				&krait_perf_cache_map, 0xFFFFF);
+}
+
+static int krait_map_event_no_branch(struct perf_event *event)
+{
+	return armpmu_map_event(event, &krait_perf_map_no_branch,
+				&krait_perf_cache_map, 0xFFFFF);
+}
+
 static void armv7pmu_init(struct arm_pmu *cpu_pmu)
 {
 	cpu_pmu->handle_irq	= armv7pmu_handle_irq;
@@ -1283,6 +1427,22 @@ static int armv7_a7_pmu_init(struct arm_pmu *cpu_pmu)
 	cpu_pmu->set_event_filter = armv7pmu_set_event_filter;
 	return 0;
 }
+
+static int krait_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	u32 id = read_cpuid_id() & 0xffffff00;
+
+	armv7pmu_init(cpu_pmu);
+	cpu_pmu->name		= "ARMv7 Krait";
+	/* Some early versions of Krait don't support PC write events */
+	if (id == 0x511f0400 || id == 0x510f0600)
+		cpu_pmu->map_event	= krait_map_event_no_branch;
+	else
+		cpu_pmu->map_event	= krait_map_event;
+	cpu_pmu->num_events	= armv7_read_num_pmnc_events();
+	cpu_pmu->set_event_filter = armv7pmu_set_event_filter;
+	return 0;
+}
 #else
 static inline int armv7_a8_pmu_init(struct arm_pmu *cpu_pmu)
 {
@@ -1308,4 +1468,9 @@ static inline int armv7_a7_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	return -ENODEV;
 }
+
+static inline int krait_pmu_init(struct arm_pmu *cpu_pmu)
+{
+	return -ENODEV;
+}
 #endif	/* CONFIG_CPU_V7 */
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 4/7] ARM: perf_event: Add hook for event index clearing
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
                   ` (2 preceding siblings ...)
  2014-01-08 22:59 ` [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  2014-01-08 22:59 ` [PATCH 5/7] ARM: perf_event: Fully support Krait CPU PMU events Stephen Boyd
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

On Krait processors we have a many-to-one relationship between
raw CPU events and the event programmed into the PMNx counter.
Two raw CPU events could map to the same value programmed in the
PMNx counter. To avoid this problem, we check for collisions
during the get_event_idx() callback by setting a bit in a bitmap
whenever a certain event is used in a PMNx counter (see the next
patch). Unfortunately, we don't have a hook to clear this bit in
the bitmap when the event is deleted so let's add an optional
clear_event_idx() callback for this purpose.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/include/asm/pmu.h   | 1 +
 arch/arm/kernel/perf_event.c | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index f24edad26c70..994ac445e792 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h
@@ -71,6 +71,7 @@ struct arm_pmu {
 	void		(*disable)(struct perf_event *event);
 	int		(*get_event_idx)(struct pmu_hw_events *hw_events,
 					 struct perf_event *event);
+	void		(*clear_event_idx)(struct perf_event *event);
 	int		(*set_event_filter)(struct hw_perf_event *evt,
 					    struct perf_event_attr *attr);
 	u32		(*read_counter)(struct perf_event *event);
diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index bc3f2efa0d86..083904e4dc12 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -209,6 +209,8 @@ armpmu_del(struct perf_event *event, int flags)
 	armpmu_stop(event, PERF_EF_UPDATE);
 	hw_events->events[idx] = NULL;
 	clear_bit(idx, hw_events->used_mask);
+	if (armpmu->clear_event_idx)
+		armpmu->clear_event_idx(event);
 
 	perf_event_update_userpage(event);
 }
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 5/7] ARM: perf_event: Fully support Krait CPU PMU events
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
                   ` (3 preceding siblings ...)
  2014-01-08 22:59 ` [PATCH 4/7] ARM: perf_event: Add hook for event index clearing Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  2014-01-08 22:59 ` [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU) Stephen Boyd
  2014-01-08 22:59 ` [PATCH 7/7] ARM: dts: msm: Add krait-pmu to platforms with Krait CPUs Stephen Boyd
  6 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

Krait supports a set of performance monitor region event
selection registers (PMRESR) sitting behind a cp15 based
interface that extend the architected PMU events to include Krait
CPU and Venum VFP specific events. To use these events the user
is expected to program the region register (PMRESRn) with the
event code shifted into the group they care about and then point
the PMNx event at that region+group combo by writing a
PMRESRn_GROUPx event. Add support for this hardware.

Note: the raw event number is a pure software construct that
allows us to map the multi-dimensional number space of regions,
groups, and event codes into a flat event number space suitable
for use by the perf framework.

This is based on code originally written by Ashwin Chaugule and
Neil Leeder [1].

[1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/arch/arm/kernel/perf_event_msm_krait.c?h=msm-3.4

Cc: Neil Leeder <nleeder@codeaurora.org>
Cc: Ashwin Chaugule <ashwinc@codeaurora.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 arch/arm/kernel/perf_event_v7.c | 392 +++++++++++++++++++++++++++++++++++++++-
 1 file changed, 386 insertions(+), 6 deletions(-)

diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index ec86cf42d2ba..bb06d9f3510b 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -18,6 +18,10 @@
 
 #ifdef CONFIG_CPU_V7
 
+#include <asm/cp15.h>
+#include <asm/vfp.h>
+#include "../vfp/vfpinstr.h"
+
 /*
  * Common ARMv7 event types
  *
@@ -109,6 +113,20 @@ enum armv7_a15_perf_types {
 	ARMV7_A15_PERFCTR_PC_WRITE_SPEC			= 0x76,
 };
 
+/* ARMv7 Krait specific event types */
+enum krait_perf_types {
+	KRAIT_PMRESR0_GROUP0				= 0xcc,
+	KRAIT_PMRESR1_GROUP0				= 0xd0,
+	KRAIT_PMRESR2_GROUP0				= 0xd4,
+	KRAIT_VPMRESR0_GROUP0				= 0xd8,
+
+	KRAIT_PERFCTR_L1_ICACHE_ACCESS			= 0x10011,
+	KRAIT_PERFCTR_L1_ICACHE_MISS			= 0x10010,
+
+	KRAIT_PERFCTR_L1_ITLB_ACCESS			= 0x12222,
+	KRAIT_PERFCTR_L1_DTLB_ACCESS			= 0x12210,
+};
+
 /*
  * Cortex-A8 HW events mapping
  *
@@ -779,8 +797,8 @@ static const unsigned krait_perf_cache_map[PERF_COUNT_HW_CACHE_MAX]
 	},
 	[C(L1I)] = {
 		[C(OP_READ)] = {
-			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
-			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_ACCESS)]	= KRAIT_PERFCTR_L1_ICACHE_ACCESS,
+			[C(RESULT_MISS)]	= KRAIT_PERFCTR_L1_ICACHE_MISS,
 		},
 		[C(OP_WRITE)] = {
 			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
@@ -807,11 +825,11 @@ static const unsigned krait_perf_cache_map[PERF_COUNT_HW_CACHE_MAX]
 	},
 	[C(DTLB)] = {
 		[C(OP_READ)] = {
-			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_ACCESS)]	= KRAIT_PERFCTR_L1_DTLB_ACCESS,
 			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
 		},
 		[C(OP_WRITE)] = {
-			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_ACCESS)]	= KRAIT_PERFCTR_L1_DTLB_ACCESS,
 			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
 		},
 		[C(OP_PREFETCH)] = {
@@ -821,11 +839,11 @@ static const unsigned krait_perf_cache_map[PERF_COUNT_HW_CACHE_MAX]
 	},
 	[C(ITLB)] = {
 		[C(OP_READ)] = {
-			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_ACCESS)]	= KRAIT_PERFCTR_L1_ITLB_ACCESS,
 			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
 		},
 		[C(OP_WRITE)] = {
-			[C(RESULT_ACCESS)]	= CACHE_OP_UNSUPPORTED,
+			[C(RESULT_ACCESS)]	= KRAIT_PERFCTR_L1_ITLB_ACCESS,
 			[C(RESULT_MISS)]	= CACHE_OP_UNSUPPORTED,
 		},
 		[C(OP_PREFETCH)] = {
@@ -1428,6 +1446,363 @@ static int armv7_a7_pmu_init(struct arm_pmu *cpu_pmu)
 	return 0;
 }
 
+/*
+ * Krait Performance Monitor Region Event Selection Register (PMRESRn)
+ *
+ *            31   30     24     16     8      0
+ *            +--------------------------------+
+ *  PMRESR0   | EN |  CC  |  CC  |  CC  |  CC  |   N = 1, R = 0
+ *            +--------------------------------+
+ *  PMRESR1   | EN |  CC  |  CC  |  CC  |  CC  |   N = 1, R = 1
+ *            +--------------------------------+
+ *  PMRESR2   | EN |  CC  |  CC  |  CC  |  CC  |   N = 1, R = 2
+ *            +--------------------------------+
+ *  VPMRESR0  | EN |  CC  |  CC  |  CC  |  CC  |   N = 2, R = ?
+ *            +--------------------------------+
+ *              EN | G=3  | G=2  | G=1  | G=0
+ *
+ *  Event Encoding:
+ *
+ *      hwc->config_base = 0xNRCCG
+ *
+ *      N  = prefix, 1 for Krait CPU (PMRESRn), 2 for Venum VFP (VPMRESR)
+ *      R  = region register
+ *      CC = event selection within group G
+ *      G  = group
+ *
+ *  Example: 0x12021 is a Krait CPU event in PMRESR2's group 1 with code 2
+ */
+
+#define KRAIT_EVENT		(1 << 16)
+#define VENUM_EVENT		(2 << 16)
+#define KRAIT_EVENT_MASK	(KRAIT_EVENT | VENUM_EVENT)
+#define PMRESRn_EN		BIT(31)
+#define NUM_PMRESR	(KRAIT_VPMRESR0_GROUP0 + 4 - KRAIT_PMRESR0_GROUP0)
+
+static DEFINE_PER_CPU(unsigned long [BITS_TO_LONGS(NUM_PMRESR)], pmresrn_used);
+
+static u32 krait_read_pmresrn(int n)
+{
+	u32 val;
+
+	switch (n) {
+	case 0:
+		asm volatile("mrc p15, 1, %0, c9, c15, 0" : "=r" (val));
+		break;
+	case 1:
+		asm volatile("mrc p15, 1, %0, c9, c15, 1" : "=r" (val));
+		break;
+	case 2:
+		asm volatile("mrc p15, 1, %0, c9, c15, 2" : "=r" (val));
+		break;
+	default:
+		BUG(); /* Should be validated in krait_pmu_get_event_idx() */
+	}
+
+	return val;
+}
+
+static void krait_write_pmresrn(int n, u32 val)
+{
+	switch (n) {
+	case 0:
+		asm volatile("mcr p15, 1, %0, c9, c15, 0" : : "r" (val));
+		break;
+	case 1:
+		asm volatile("mcr p15, 1, %0, c9, c15, 1" : : "r" (val));
+		break;
+	case 2:
+		asm volatile("mcr p15, 1, %0, c9, c15, 2" : : "r" (val));
+		break;
+	default:
+		BUG(); /* Should be validated in krait_pmu_get_event_idx() */
+	}
+}
+
+static u32 krait_read_vpmresr0(void)
+{
+	u32 val;
+	asm volatile("mrc p10, 7, %0, c11, c0, 0" : "=r" (val));
+	return val;
+}
+
+static void krait_write_vpmresr0(u32 val)
+{
+	asm volatile("mcr p10, 7, %0, c11, c0, 0" : : "r" (val));
+}
+
+static void krait_pre_vpmresr0(u32 *venum_orig_val, u32 *fp_orig_val)
+{
+	u32 venum_new_val;
+	u32 fp_new_val;
+
+	/* CPACR Enable CP10 and CP11 access */
+	*venum_orig_val = get_copro_access();
+	venum_new_val = *venum_orig_val | CPACC_SVC(10) | CPACC_SVC(11);
+	set_copro_access(venum_new_val);
+
+	/* Enable FPEXC */
+	*fp_orig_val = fmrx(FPEXC);
+	fp_new_val = *fp_orig_val | FPEXC_EN;
+	fmxr(FPEXC, fp_new_val);
+}
+
+static void krait_post_vpmresr0(u32 venum_orig_val, u32 fp_orig_val)
+{
+	/* Restore FPEXC */
+	fmxr(FPEXC, fp_orig_val);
+	isb();
+	/* Restore CPACR */
+	set_copro_access(venum_orig_val);
+}
+
+static u32 krait_get_pmresrn_event(int region)
+{
+	static const u32 pmresrn_table[] = { KRAIT_PMRESR0_GROUP0,
+					     KRAIT_PMRESR1_GROUP0,
+					     KRAIT_PMRESR2_GROUP0 };
+	return pmresrn_table[region];
+}
+
+static void krait_evt_setup(int idx, u32 config_base)
+{
+	u32 val;
+	u32 mask;
+	u32 vval, fval;
+	unsigned int region;
+	unsigned int group;
+	unsigned int code;
+	unsigned int group_shift;
+	bool venum_event;
+
+	venum_event = !!(config_base & VENUM_EVENT);
+	region = (config_base >> 12) & 0xf;
+	code   = (config_base >> 4) & 0xff;
+	group  = (config_base >> 0)  & 0xf;
+
+	group_shift = group * 8;
+	mask = 0xff << group_shift;
+
+	/* Configure evtsel for the region and group */
+	if (venum_event)
+		val = KRAIT_VPMRESR0_GROUP0;
+	else
+		val = krait_get_pmresrn_event(region);
+	val += group;
+	/* Mix in mode-exclusion bits */
+	val |= config_base & (ARMV7_EXCLUDE_USER | ARMV7_EXCLUDE_PL1);
+	armv7_pmnc_write_evtsel(idx, val);
+
+	asm volatile("mcr p15, 0, %0, c9, c15, 0" : : "r" (0));
+
+	if (venum_event) {
+		krait_pre_vpmresr0(&vval, &fval);
+		val = krait_read_vpmresr0();
+		val &= ~mask;
+		val |= code << group_shift;
+		val |= PMRESRn_EN;
+		krait_write_vpmresr0(val);
+		krait_post_vpmresr0(vval, fval);
+	} else {
+		val = krait_read_pmresrn(region);
+		val &= ~mask;
+		val |= code << group_shift;
+		val |= PMRESRn_EN;
+		krait_write_pmresrn(region, val);
+	}
+}
+
+static u32 krait_clear_pmresrn_group(u32 val, int group)
+{
+	u32 mask;
+	int group_shift;
+
+	group_shift = group * 8;
+	mask = 0xff << group_shift;
+	val &= ~mask;
+
+	/* Don't clear enable bit if entire region isn't disabled */
+	if (val & ~PMRESRn_EN)
+		return val |= PMRESRn_EN;
+
+	return 0;
+}
+
+static void krait_clearpmu(u32 config_base)
+{
+	u32 val;
+	u32 vval, fval;
+	unsigned int region;
+	unsigned int group;
+	bool venum_event;
+
+	venum_event = !!(config_base & VENUM_EVENT);
+	region = (config_base >> 12) & 0xf;
+	group  = (config_base >> 0)  & 0xf;
+
+	if (venum_event) {
+		krait_pre_vpmresr0(&vval, &fval);
+		val = krait_read_vpmresr0();
+		val = krait_clear_pmresrn_group(val, group);
+		krait_write_vpmresr0(val);
+		krait_post_vpmresr0(vval, fval);
+	} else {
+		val = krait_read_pmresrn(region);
+		val = krait_clear_pmresrn_group(val, group);
+		krait_write_pmresrn(region, val);
+	}
+}
+
+static void krait_pmu_disable_event(struct perf_event *event)
+{
+	unsigned long flags;
+	struct hw_perf_event *hwc = &event->hw;
+	int idx = hwc->idx;
+	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+
+	/* Disable counter and interrupt */
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+
+	/* Disable counter */
+	armv7_pmnc_disable_counter(idx);
+
+	/*
+	 * Clear pmresr code (if destined for PMNx counters)
+	 */
+	if (hwc->config_base & KRAIT_EVENT_MASK)
+		krait_clearpmu(hwc->config_base);
+
+	/* Disable interrupt for this counter */
+	armv7_pmnc_disable_intens(idx);
+
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
+static void krait_pmu_enable_event(struct perf_event *event)
+{
+	unsigned long flags;
+	struct hw_perf_event *hwc = &event->hw;
+	int idx = hwc->idx;
+	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+
+	/*
+	 * Enable counter and interrupt, and set the counter to count
+	 * the event that we're interested in.
+	 */
+	raw_spin_lock_irqsave(&events->pmu_lock, flags);
+
+	/* Disable counter */
+	armv7_pmnc_disable_counter(idx);
+
+	/*
+	 * Set event (if destined for PMNx counters)
+	 * We set the event for the cycle counter because we
+	 * have the ability to perform event filtering.
+	 */
+	if (hwc->config_base & KRAIT_EVENT_MASK)
+		krait_evt_setup(idx, hwc->config_base);
+	else
+		armv7_pmnc_write_evtsel(idx, hwc->config_base);
+
+	/* Enable interrupt for this counter */
+	armv7_pmnc_enable_intens(idx);
+
+	/* Enable counter */
+	armv7_pmnc_enable_counter(idx);
+
+	raw_spin_unlock_irqrestore(&events->pmu_lock, flags);
+}
+
+static void krait_pmu_reset(void *info)
+{
+	u32 vval, fval;
+
+	armv7pmu_reset(info);
+
+	/* Clear all pmresrs */
+	krait_write_pmresrn(0, 0);
+	krait_write_pmresrn(1, 0);
+	krait_write_pmresrn(2, 0);
+
+	krait_pre_vpmresr0(&vval, &fval);
+	krait_write_vpmresr0(0);
+	krait_post_vpmresr0(vval, fval);
+}
+
+/*
+ * We check for column exclusion constraints here.
+ * Two events cant use the same group within a pmresr register.
+ */
+static int krait_pmu_get_event_idx(struct pmu_hw_events *cpuc,
+				   struct perf_event *event)
+{
+	int idx;
+	int bit;
+	unsigned int prefix;
+	unsigned int region;
+	unsigned int code;
+	unsigned int group;
+	bool krait_event;
+	struct hw_perf_event *hwc = &event->hw;
+	unsigned long *bitmap = this_cpu_ptr(pmresrn_used);
+
+	region = (hwc->config_base >> 12) & 0xf;
+	code   = (hwc->config_base >> 4) & 0xff;
+	group  = (hwc->config_base >> 0) & 0xf;
+	krait_event = !!(hwc->config_base & KRAIT_EVENT_MASK);
+
+	if (krait_event) {
+		/* Ignore invalid events */
+		if (group > 3 || region > 2)
+			return -EINVAL;
+		prefix = hwc->config_base & KRAIT_EVENT_MASK;
+		if (prefix != KRAIT_EVENT && prefix != VENUM_EVENT)
+			return -EINVAL;
+		if (prefix == VENUM_EVENT && (code & 0xe0))
+			return -EINVAL;
+
+		if (prefix == VENUM_EVENT)
+			bit = KRAIT_VPMRESR0_GROUP0;
+		else
+			bit = krait_get_pmresrn_event(region);
+		bit -= krait_get_pmresrn_event(0);
+		bit += group;
+
+		if (test_and_set_bit(bit, bitmap))
+			return -EAGAIN;
+	}
+
+	idx = armv7pmu_get_event_idx(cpuc, event);
+	if (idx < 0 && krait_event)
+		clear_bit(bit, bitmap);
+
+	return idx;
+}
+
+static void krait_pmu_clear_event_idx(struct perf_event *event)
+{
+	int bit;
+	struct hw_perf_event *hwc = &event->hw;
+	unsigned int region;
+	unsigned int group;
+	bool krait_event;
+	unsigned long *bitmap = this_cpu_ptr(pmresrn_used);
+
+	region = (hwc->config_base >> 12) & 0xf;
+	group  = (hwc->config_base >> 0) & 0xf;
+	krait_event = !!(hwc->config_base & KRAIT_EVENT_MASK);
+
+	if (krait_event) {
+		if (hwc->config_base & VENUM_EVENT)
+			bit = KRAIT_VPMRESR0_GROUP0;
+		else
+			bit = krait_get_pmresrn_event(region);
+		bit -= krait_get_pmresrn_event(0);
+		bit += group;
+		clear_bit(bit, bitmap);
+	}
+}
+
 static int krait_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	u32 id = read_cpuid_id() & 0xffffff00;
@@ -1441,6 +1816,11 @@ static int krait_pmu_init(struct arm_pmu *cpu_pmu)
 		cpu_pmu->map_event	= krait_map_event;
 	cpu_pmu->num_events	= armv7_read_num_pmnc_events();
 	cpu_pmu->set_event_filter = armv7pmu_set_event_filter;
+	cpu_pmu->reset		= krait_pmu_reset;
+	cpu_pmu->enable		= krait_pmu_enable_event;
+	cpu_pmu->disable	= krait_pmu_disable_event;
+	cpu_pmu->get_event_idx	= krait_pmu_get_event_idx;
+	cpu_pmu->clear_event_idx = krait_pmu_clear_event_idx;
 	return 0;
 }
 #else
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU)
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
                   ` (4 preceding siblings ...)
  2014-01-08 22:59 ` [PATCH 5/7] ARM: perf_event: Fully support Krait CPU PMU events Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  2014-01-09 18:14   ` Will Deacon
  2014-01-08 22:59 ` [PATCH 7/7] ARM: dts: msm: Add krait-pmu to platforms with Krait CPUs Stephen Boyd
  6 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

Document the Krait PMU compatible string.

Cc: <devicetree@vger.kernel.org>
Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---
 Documentation/devicetree/bindings/arm/pmu.txt | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt
index 3e1e498fea96..ce7cccb99d87 100644
--- a/Documentation/devicetree/bindings/arm/pmu.txt
+++ b/Documentation/devicetree/bindings/arm/pmu.txt
@@ -16,6 +16,7 @@ Required properties:
 	"arm,arm11mpcore-pmu"
 	"arm,arm1176-pmu"
 	"arm,arm1136-pmu"
+	"qcom,krait-pmu"
 - interrupts : 1 combined interrupt or 1 per core.
 
 Example:
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 7/7] ARM: dts: msm: Add krait-pmu to platforms with Krait CPUs
  2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
                   ` (5 preceding siblings ...)
  2014-01-08 22:59 ` [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU) Stephen Boyd
@ 2014-01-08 22:59 ` Stephen Boyd
  6 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-08 22:59 UTC (permalink / raw)
  To: linux-arm-kernel

Allows us to probe the performance counters on Krait CPUs.

Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
---

This one is piled on top of the SMP patches I sent a few weeks ago. Applying
it directly on -next should cause minor conflicts.

 arch/arm/boot/dts/qcom-msm8960-cdp.dts | 5 +++++
 arch/arm/boot/dts/qcom-msm8974.dtsi    | 5 +++++
 2 files changed, 10 insertions(+)

diff --git a/arch/arm/boot/dts/qcom-msm8960-cdp.dts b/arch/arm/boot/dts/qcom-msm8960-cdp.dts
index ea24fdfbdd06..138d4760ece6 100644
--- a/arch/arm/boot/dts/qcom-msm8960-cdp.dts
+++ b/arch/arm/boot/dts/qcom-msm8960-cdp.dts
@@ -39,6 +39,11 @@
 		};
 	};
 
+	cpu-pmu {
+		compatible = "qcom,krait-pmu";
+		interrupts = <1 10 0x304>;
+	};
+
 	intc: interrupt-controller at 2000000 {
 		compatible = "qcom,msm-qgic2";
 		interrupt-controller;
diff --git a/arch/arm/boot/dts/qcom-msm8974.dtsi b/arch/arm/boot/dts/qcom-msm8974.dtsi
index f607cf4a11d9..24d63c82a15a 100644
--- a/arch/arm/boot/dts/qcom-msm8974.dtsi
+++ b/arch/arm/boot/dts/qcom-msm8974.dtsi
@@ -52,6 +52,11 @@
 		};
 	};
 
+	cpu-pmu {
+		compatible = "qcom,krait-pmu";
+		interrupts = <1 7 0xf04>;
+	};
+
 	soc: soc {
 		#address-cells = <1>;
 		#size-cells = <1>;
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 1/7] ARM: perf_event: Silence sparse warning
  2014-01-08 22:59 ` [PATCH 1/7] ARM: perf_event: Silence sparse warning Stephen Boyd
@ 2014-01-09 10:45   ` Will Deacon
  2014-01-09 23:59     ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-09 10:45 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

On Wed, Jan 08, 2014 at 10:59:38PM +0000, Stephen Boyd wrote:
> arch/arm/kernel/perf_event_cpu.c:274:25: warning: incorrect type in assignment (different modifiers)
> arch/arm/kernel/perf_event_cpu.c:274:25: expected int ( *init_fn )( ... )
> arch/arm/kernel/perf_event_cpu.c:274:25: got void const *const data
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---

Given that the rest of the series depends on the change to the percpu IRQ
stuff, I think you can just send this patch to Russell's patch system with
my ack:

  Acked-by: Will Deacon <will.deacon@arm.com>

Cheers,

Will

>  arch/arm/kernel/perf_event_cpu.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> index d85055cd24ba..20d553c9f5e2 100644
> --- a/arch/arm/kernel/perf_event_cpu.c
> +++ b/arch/arm/kernel/perf_event_cpu.c
> @@ -254,7 +254,7 @@ static int probe_current_pmu(struct arm_pmu *pmu)
>  static int cpu_pmu_device_probe(struct platform_device *pdev)
>  {
>  	const struct of_device_id *of_id;
> -	int (*init_fn)(struct arm_pmu *);
> +	const int (*init_fn)(struct arm_pmu *);
>  	struct device_node *node = pdev->dev.of_node;
>  	struct arm_pmu *pmu;
>  	int ret = -ENODEV;
> -- 
> The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
> hosted by The Linux Foundation
> 
> 

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-08 22:59 ` [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU Stephen Boyd
@ 2014-01-09 10:49   ` Will Deacon
  2014-01-09 19:17     ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-09 10:49 UTC (permalink / raw)
  To: linux-arm-kernel

Hello,

On Wed, Jan 08, 2014 at 10:59:39PM +0000, Stephen Boyd wrote:
> Some CPU PMUs are wired up with one PPI for all the CPUs instead
> of with a different SPI for each CPU. Add support for these
> devices.
> 
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm/kernel/perf_event_cpu.c | 107 +++++++++++++++++++++++++++++----------
>  1 file changed, 79 insertions(+), 28 deletions(-)

[...]

> +static irq_handler_t cpu_handler;
> +
> +static irqreturn_t cpu_pmu_dispatch_irq(int irq, void *dev)
> +{
> +	struct arm_pmu *arm_pmu = *(struct arm_pmu **)dev;
> +	return cpu_handler(irq, arm_pmu);
> +}

I don't like this bit -- having a global cpu_handler field is going to
interfere with the big.LITTLE work and casting the per-cpu dev token is also
pretty hacky.

However, you're forced down this route by the need to invoke the armpmu IRQ
dispatcher. Now, that only exists as a workaround for the braindead
interrupt routing on the u8500 (they OR'd all the PMU SPIs together) -- it's
not a problem that will affect a system using PPIs. If you look, there is
only one use of the thing in: arch/arm/mach-ux500/cpu-db8500.c.

So, we could rename that callback to make it clear that it's not so much an
IRQ handler wrapper as a specific hack to deal with broken SPIs. Then the
cpu_pmu code can neglect to make the callback if it's using PPI.

What do you think?

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs
  2014-01-08 22:59 ` [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs Stephen Boyd
@ 2014-01-09 11:04   ` Will Deacon
  2014-01-09 19:57     ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-09 11:04 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 08, 2014 at 10:59:40PM +0000, Stephen Boyd wrote:
> Add basic support for the Krait CPU PMU. This allows us to use
> the architected functionality of the PMU.
> 
> This is based on code originally written by Ashwin Chaugule and
> Neil Leeder [1].
> 
> [1] https://www.codeaurora.org/cgit/quic/la/kernel/msm/tree/arch/arm/kernel/perf_event_msm_krait.c?h=msm-3.4
> 
> Cc: Neil Leeder <nleeder@codeaurora.org>
> Cc: Ashwin Chaugule <ashwinc@codeaurora.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  arch/arm/kernel/perf_event_cpu.c |   1 +
>  arch/arm/kernel/perf_event_v7.c  | 165 +++++++++++++++++++++++++++++++++++++++
>  2 files changed, 166 insertions(+)

[...]

> +static int krait_pmu_init(struct arm_pmu *cpu_pmu)
> +{
> +	u32 id = read_cpuid_id() & 0xffffff00;
> +
> +	armv7pmu_init(cpu_pmu);
> +	cpu_pmu->name		= "ARMv7 Krait";
> +	/* Some early versions of Krait don't support PC write events */
> +	if (id == 0x511f0400 || id == 0x510f0600)
> +		cpu_pmu->map_event	= krait_map_event_no_branch;

Hmm, I'd really rather this information came via the DT. In fact, you could
just drop the branch event from your main map_event_function and keep things
simple. It depends how badly you want to advertise it in perf list :)

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU)
  2014-01-08 22:59 ` [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU) Stephen Boyd
@ 2014-01-09 18:14   ` Will Deacon
  2014-01-09 19:57     ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-09 18:14 UTC (permalink / raw)
  To: linux-arm-kernel

On Wed, Jan 08, 2014 at 10:59:43PM +0000, Stephen Boyd wrote:
> Document the Krait PMU compatible string.
> 
> Cc: <devicetree@vger.kernel.org>
> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
> ---
>  Documentation/devicetree/bindings/arm/pmu.txt | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt
> index 3e1e498fea96..ce7cccb99d87 100644
> --- a/Documentation/devicetree/bindings/arm/pmu.txt
> +++ b/Documentation/devicetree/bindings/arm/pmu.txt
> @@ -16,6 +16,7 @@ Required properties:
>  	"arm,arm11mpcore-pmu"
>  	"arm,arm1176-pmu"
>  	"arm,arm1136-pmu"
> +	"qcom,krait-pmu"
>  - interrupts : 1 combined interrupt or 1 per core.

Might be worth updating the part about interrupts too, given that you've
added PPI support.

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-09 10:49   ` Will Deacon
@ 2014-01-09 19:17     ` Stephen Boyd
  2014-01-10 10:58       ` Will Deacon
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-09 19:17 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/09/14 02:49, Will Deacon wrote:
>
>> +static irq_handler_t cpu_handler;
>> +
>> +static irqreturn_t cpu_pmu_dispatch_irq(int irq, void *dev)
>> +{
>> +	struct arm_pmu *arm_pmu = *(struct arm_pmu **)dev;
>> +	return cpu_handler(irq, arm_pmu);
>> +}
> I don't like this bit -- having a global cpu_handler field is going to
> interfere with the big.LITTLE work and casting the per-cpu dev token is also
> pretty hacky.
>
> However, you're forced down this route by the need to invoke the armpmu IRQ
> dispatcher. Now, that only exists as a workaround for the braindead
> interrupt routing on the u8500 (they OR'd all the PMU SPIs together) -- it's
> not a problem that will affect a system using PPIs. If you look, there is
> only one use of the thing in: arch/arm/mach-ux500/cpu-db8500.c.
>
> So, we could rename that callback to make it clear that it's not so much an
> IRQ handler wrapper as a specific hack to deal with broken SPIs. Then the
> cpu_pmu code can neglect to make the callback if it's using PPI.
>
> What do you think?

Yeah I hate this bouncing layer too but it was the best I could come up
with. I'll rename it to 'armpmu_dispatch_spi_irq' (bikeshedding welcome).

We can avoid the hacky cast of the per-cpu dev token by using the
cpu_pmu pointer directly, but we'll still need to pass something to the
percpu interrupt handler otherwise the genirq layer doesn't allow us to
request the PPI. I can pass hw_events I guess. Is that what you're
thinking? Or were you thinking that we could just use
cpu_pmu->handle_irq as the handler argument in request_percpu_irq()? I
can't figure out how that is supposed to work.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs
  2014-01-09 11:04   ` Will Deacon
@ 2014-01-09 19:57     ` Stephen Boyd
  2014-01-10 11:01       ` Will Deacon
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-09 19:57 UTC (permalink / raw)
  To: linux-arm-kernel

(Adding DT reviewers)

On 01/09/14 03:04, Will Deacon wrote:
> On Wed, Jan 08, 2014 at 10:59:40PM +0000, Stephen Boyd wrote:
>
>> +static int krait_pmu_init(struct arm_pmu *cpu_pmu)
>> +{
>> +	u32 id = read_cpuid_id() & 0xffffff00;
>> +
>> +	armv7pmu_init(cpu_pmu);
>> +	cpu_pmu->name		= "ARMv7 Krait";
>> +	/* Some early versions of Krait don't support PC write events */
>> +	if (id == 0x511f0400 || id == 0x510f0600)
>> +		cpu_pmu->map_event	= krait_map_event_no_branch;
> Hmm, I'd really rather this information came via the DT. In fact, you could
> just drop the branch event from your main map_event_function and keep things
> simple. It depends how badly you want to advertise it in perf list :)
>

Not every version of Krait is missing support for this event, so I'd
like to keep it so things like perf stat show branch counts. How about I
add a bool property to the pmu node indicating that this PMU is missing
support for the PC write events? Something like "no-pc-write"?

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU)
  2014-01-09 18:14   ` Will Deacon
@ 2014-01-09 19:57     ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-09 19:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/09/14 10:14, Will Deacon wrote:
> On Wed, Jan 08, 2014 at 10:59:43PM +0000, Stephen Boyd wrote:
>> Document the Krait PMU compatible string.
>>
>> Cc: <devicetree@vger.kernel.org>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
>>  Documentation/devicetree/bindings/arm/pmu.txt | 1 +
>>  1 file changed, 1 insertion(+)
>>
>> diff --git a/Documentation/devicetree/bindings/arm/pmu.txt b/Documentation/devicetree/bindings/arm/pmu.txt
>> index 3e1e498fea96..ce7cccb99d87 100644
>> --- a/Documentation/devicetree/bindings/arm/pmu.txt
>> +++ b/Documentation/devicetree/bindings/arm/pmu.txt
>> @@ -16,6 +16,7 @@ Required properties:
>>  	"arm,arm11mpcore-pmu"
>>  	"arm,arm1176-pmu"
>>  	"arm,arm1136-pmu"
>> +	"qcom,krait-pmu"
>>  - interrupts : 1 combined interrupt or 1 per core.
> Might be worth updating the part about interrupts too, given that you've
> added PPI support.

Done.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/7] ARM: perf_event: Silence sparse warning
  2014-01-09 10:45   ` Will Deacon
@ 2014-01-09 23:59     ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-09 23:59 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/09/14 02:45, Will Deacon wrote:
> Hi Stephen,
>
> On Wed, Jan 08, 2014 at 10:59:38PM +0000, Stephen Boyd wrote:
>> arch/arm/kernel/perf_event_cpu.c:274:25: warning: incorrect type in assignment (different modifiers)
>> arch/arm/kernel/perf_event_cpu.c:274:25: expected int ( *init_fn )( ... )
>> arch/arm/kernel/perf_event_cpu.c:274:25: got void const *const data
>>
>> Signed-off-by: Stephen Boyd <sboyd@codeaurora.org>
>> ---
> Given that the rest of the series depends on the change to the percpu IRQ
> stuff, I think you can just send this patch to Russell's patch system with
> my ack:
>
>   Acked-by: Will Deacon <will.deacon@arm.com>
>

Thanks. It's 7937/1.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-09 19:17     ` Stephen Boyd
@ 2014-01-10 10:58       ` Will Deacon
  2014-01-10 19:36         ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-10 10:58 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 09, 2014 at 07:17:29PM +0000, Stephen Boyd wrote:
> On 01/09/14 02:49, Will Deacon wrote:
> >
> >> +static irq_handler_t cpu_handler;
> >> +
> >> +static irqreturn_t cpu_pmu_dispatch_irq(int irq, void *dev)
> >> +{
> >> +	struct arm_pmu *arm_pmu = *(struct arm_pmu **)dev;
> >> +	return cpu_handler(irq, arm_pmu);
> >> +}
> > I don't like this bit -- having a global cpu_handler field is going to
> > interfere with the big.LITTLE work and casting the per-cpu dev token is also
> > pretty hacky.
> >
> > However, you're forced down this route by the need to invoke the armpmu IRQ
> > dispatcher. Now, that only exists as a workaround for the braindead
> > interrupt routing on the u8500 (they OR'd all the PMU SPIs together) -- it's
> > not a problem that will affect a system using PPIs. If you look, there is
> > only one use of the thing in: arch/arm/mach-ux500/cpu-db8500.c.
> >
> > So, we could rename that callback to make it clear that it's not so much an
> > IRQ handler wrapper as a specific hack to deal with broken SPIs. Then the
> > cpu_pmu code can neglect to make the callback if it's using PPI.
> >
> > What do you think?
> 
> Yeah I hate this bouncing layer too but it was the best I could come up
> with. I'll rename it to 'armpmu_dispatch_spi_irq' (bikeshedding welcome).

That sounds fine.

> We can avoid the hacky cast of the per-cpu dev token by using the
> cpu_pmu pointer directly, but we'll still need to pass something to the
> percpu interrupt handler otherwise the genirq layer doesn't allow us to
> request the PPI. I can pass hw_events I guess. Is that what you're
> thinking? Or were you thinking that we could just use
> cpu_pmu->handle_irq as the handler argument in request_percpu_irq()? I
> can't figure out how that is supposed to work.

Actually, I was thinking you could remove cpu_pmu_dispatch_irq completely
and just pass the actual handler straight through to request_percpu_irq. On
arm64 we pass the hw_events as the pcpu token, so I'd be inclined to do the
same here unless there's a good reason not to.

Cheers,

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs
  2014-01-09 19:57     ` Stephen Boyd
@ 2014-01-10 11:01       ` Will Deacon
  2014-01-10 18:57         ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-10 11:01 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jan 09, 2014 at 07:57:12PM +0000, Stephen Boyd wrote:
> (Adding DT reviewers)
> 
> On 01/09/14 03:04, Will Deacon wrote:
> > On Wed, Jan 08, 2014 at 10:59:40PM +0000, Stephen Boyd wrote:
> >
> >> +static int krait_pmu_init(struct arm_pmu *cpu_pmu)
> >> +{
> >> +	u32 id = read_cpuid_id() & 0xffffff00;
> >> +
> >> +	armv7pmu_init(cpu_pmu);
> >> +	cpu_pmu->name		= "ARMv7 Krait";
> >> +	/* Some early versions of Krait don't support PC write events */
> >> +	if (id == 0x511f0400 || id == 0x510f0600)
> >> +		cpu_pmu->map_event	= krait_map_event_no_branch;
> > Hmm, I'd really rather this information came via the DT. In fact, you could
> > just drop the branch event from your main map_event_function and keep things
> > simple. It depends how badly you want to advertise it in perf list :)
> >
> 
> Not every version of Krait is missing support for this event, so I'd
> like to keep it so things like perf stat show branch counts. How about I
> add a bool property to the pmu node indicating that this PMU is missing
> support for the PC write events? Something like "no-pc-write"?

Perhaps, although I think it should be qualcomm-specific, so something like
"qcom,krait-no-pc-write"? Again, I'd be glad to hear something from a DT
reviewer on this.

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs
  2014-01-10 11:01       ` Will Deacon
@ 2014-01-10 18:57         ` Stephen Boyd
  0 siblings, 0 replies; 23+ messages in thread
From: Stephen Boyd @ 2014-01-10 18:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/10, Will Deacon wrote:
> On Thu, Jan 09, 2014 at 07:57:12PM +0000, Stephen Boyd wrote:
> > (Adding DT reviewers)
> > 
> > On 01/09/14 03:04, Will Deacon wrote:
> > > On Wed, Jan 08, 2014 at 10:59:40PM +0000, Stephen Boyd wrote:
> > >
> > >> +static int krait_pmu_init(struct arm_pmu *cpu_pmu)
> > >> +{
> > >> +	u32 id = read_cpuid_id() & 0xffffff00;
> > >> +
> > >> +	armv7pmu_init(cpu_pmu);
> > >> +	cpu_pmu->name		= "ARMv7 Krait";
> > >> +	/* Some early versions of Krait don't support PC write events */
> > >> +	if (id == 0x511f0400 || id == 0x510f0600)
> > >> +		cpu_pmu->map_event	= krait_map_event_no_branch;
> > > Hmm, I'd really rather this information came via the DT. In fact, you could
> > > just drop the branch event from your main map_event_function and keep things
> > > simple. It depends how badly you want to advertise it in perf list :)
> > >
> > 
> > Not every version of Krait is missing support for this event, so I'd
> > like to keep it so things like perf stat show branch counts. How about I
> > add a bool property to the pmu node indicating that this PMU is missing
> > support for the PC write events? Something like "no-pc-write"?
> 
> Perhaps, although I think it should be qualcomm-specific, so something like
> "qcom,krait-no-pc-write"? Again, I'd be glad to hear something from a DT
> reviewer on this.
> 

Yes a vendor prefix is probably a good idea. I'll add that in.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-10 10:58       ` Will Deacon
@ 2014-01-10 19:36         ` Stephen Boyd
  2014-01-13 11:52           ` Will Deacon
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-10 19:36 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/10, Will Deacon wrote:
> On Thu, Jan 09, 2014 at 07:17:29PM +0000, Stephen Boyd wrote:
> 
> > We can avoid the hacky cast of the per-cpu dev token by using the
> > cpu_pmu pointer directly, but we'll still need to pass something to the
> > percpu interrupt handler otherwise the genirq layer doesn't allow us to
> > request the PPI. I can pass hw_events I guess. Is that what you're
> > thinking? Or were you thinking that we could just use
> > cpu_pmu->handle_irq as the handler argument in request_percpu_irq()? I
> > can't figure out how that is supposed to work.
> 
> Actually, I was thinking you could remove cpu_pmu_dispatch_irq completely
> and just pass the actual handler straight through to request_percpu_irq. On
> arm64 we pass the hw_events as the pcpu token, so I'd be inclined to do the
> same here unless there's a good reason not to.
> 

Passing the hw_events as the pcpu token here is kind of hacky.
The reason is because the token is dereferenced into cpu_pmu in
armv7pmu_handle_irq() like so:

	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;

It would be great if we could pass cpu_pmu directly to the
request call like so:

	request_percpu_irq(irq, cpu_pmu->handle_irq, "arm-pmu", &cpu_pmu);

but no. request_percpu_irq() wants a percpu pointer so this won't
work. If cpu_pmu was declared as DEFINE_PER_CPU, this would work
out just fine.

Should the cpu_pmu become a per-cpu variable? That sounds rather
invasive.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-10 19:36         ` Stephen Boyd
@ 2014-01-13 11:52           ` Will Deacon
  2014-01-14 20:57             ` Stephen Boyd
  0 siblings, 1 reply; 23+ messages in thread
From: Will Deacon @ 2014-01-13 11:52 UTC (permalink / raw)
  To: linux-arm-kernel

On Fri, Jan 10, 2014 at 07:36:57PM +0000, Stephen Boyd wrote:
> On 01/10, Will Deacon wrote:
> > On Thu, Jan 09, 2014 at 07:17:29PM +0000, Stephen Boyd wrote:
> > 
> > > We can avoid the hacky cast of the per-cpu dev token by using the
> > > cpu_pmu pointer directly, but we'll still need to pass something to the
> > > percpu interrupt handler otherwise the genirq layer doesn't allow us to
> > > request the PPI. I can pass hw_events I guess. Is that what you're
> > > thinking? Or were you thinking that we could just use
> > > cpu_pmu->handle_irq as the handler argument in request_percpu_irq()? I
> > > can't figure out how that is supposed to work.
> > 
> > Actually, I was thinking you could remove cpu_pmu_dispatch_irq completely
> > and just pass the actual handler straight through to request_percpu_irq. On
> > arm64 we pass the hw_events as the pcpu token, so I'd be inclined to do the
> > same here unless there's a good reason not to.
> > 
> 
> Passing the hw_events as the pcpu token here is kind of hacky.
> The reason is because the token is dereferenced into cpu_pmu in
> armv7pmu_handle_irq() like so:
> 
> 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;
> 
> It would be great if we could pass cpu_pmu directly to the
> request call like so:
> 
> 	request_percpu_irq(irq, cpu_pmu->handle_irq, "arm-pmu", &cpu_pmu);
> 
> but no. request_percpu_irq() wants a percpu pointer so this won't
> work. If cpu_pmu was declared as DEFINE_PER_CPU, this would work
> out just fine.

That feels really broken though, since we rely on the cpu_pmu being a
container for the struct pmu that was registered with perf core.

> Should the cpu_pmu become a per-cpu variable? That sounds rather
> invasive.

I also don't think that's the right solution, based on the above. It's
actually pretty hard to work out what's the right thing to do here...

We *could* have a per-cpu pointer to the cpu_pmu_pointer, but then we'd
need to update the IRQ handlers, including things like the CCI PMU which
really doesn't care about per-cpu stuff. So after all this, the shim we have
around the IRQ handler for the U8500 SPI workarounds might be the right
thing after all -- it allows us to consolidate the conversion of a pcpu
pointer into the relevant instance (actually any instance, since they'd all
point at the same thing) for the current CPU.

What do you think to having that shim throw away the second level pcpu
pointer in the case of a PPI? (probably means we need to revisit that
renaming again).

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-13 11:52           ` Will Deacon
@ 2014-01-14 20:57             ` Stephen Boyd
  2014-01-15 10:33               ` Will Deacon
  0 siblings, 1 reply; 23+ messages in thread
From: Stephen Boyd @ 2014-01-14 20:57 UTC (permalink / raw)
  To: linux-arm-kernel

On 01/13/14 03:52, Will Deacon wrote:
> On Fri, Jan 10, 2014 at 07:36:57PM +0000, Stephen Boyd wrote:
>>
>> Passing the hw_events as the pcpu token here is kind of hacky.
>> The reason is because the token is dereferenced into cpu_pmu in
>> armv7pmu_handle_irq() like so:
>>
>> 	struct arm_pmu *cpu_pmu = (struct arm_pmu *)dev;
>>
>> It would be great if we could pass cpu_pmu directly to the
>> request call like so:
>>
>> 	request_percpu_irq(irq, cpu_pmu->handle_irq, "arm-pmu", &cpu_pmu);
>>
>> but no. request_percpu_irq() wants a percpu pointer so this won't
>> work. If cpu_pmu was declared as DEFINE_PER_CPU, this would work
>> out just fine.
> That feels really broken though, since we rely on the cpu_pmu being a
> container for the struct pmu that was registered with perf core.
>
>> Should the cpu_pmu become a per-cpu variable? That sounds rather
>> invasive.
> I also don't think that's the right solution, based on the above. It's
> actually pretty hard to work out what's the right thing to do here...

Yes it doesn't seem like the right solution.

>
> We *could* have a per-cpu pointer to the cpu_pmu_pointer, but then we'd
> need to update the IRQ handlers, including things like the CCI PMU which
> really doesn't care about per-cpu stuff. So after all this, the shim we have
> around the IRQ handler for the U8500 SPI workarounds might be the right
> thing after all -- it allows us to consolidate the conversion of a pcpu
> pointer into the relevant instance (actually any instance, since they'd all
> point at the same thing) for the current CPU.
>
> What do you think to having that shim throw away the second level pcpu
> pointer in the case of a PPI? (probably means we need to revisit that
> renaming again).

Ok I think I understand what you're getting at. We pass a per-cpu
pointer to the cpu_pmu pointer as the dev_id argument to the PPI irq
handler, and then we check to see if the irq is per-cpu inside the
armpmu_dispatch_irq() function and throw away the second level of
pointer, i.e.

static irqreturn_t armpmu_dispatch_irq(int irq, void *dev)
{
        struct arm_pmu *armpmu;
        struct platform_device *plat_device;
        struct arm_pmu_platdata *plat;

        if (irq_is_percpu(irq))
                dev = *(struct arm_pmu_cpu **)dev;
        armpmu = dev;
        plat_device = armpmu->plat_device;
        plat = dev_get_platdata(&plat_device->dev);

        if (plat && plat->handle_irq)
                return plat->handle_irq(irq, dev, armpmu->handle_irq);
        else
                return armpmu->handle_irq(irq, dev);
}


We still need to make a per-cpu variable to hold the pointer, and assign
it during cpu_pmu_init like this patch does. Hopefully that is ok.

-- 
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
hosted by The Linux Foundation

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU
  2014-01-14 20:57             ` Stephen Boyd
@ 2014-01-15 10:33               ` Will Deacon
  0 siblings, 0 replies; 23+ messages in thread
From: Will Deacon @ 2014-01-15 10:33 UTC (permalink / raw)
  To: linux-arm-kernel

Hi Stephen,

On Tue, Jan 14, 2014 at 08:57:53PM +0000, Stephen Boyd wrote:
> On 01/13/14 03:52, Will Deacon wrote:
> > I also don't think that's the right solution, based on the above. It's
> > actually pretty hard to work out what's the right thing to do here...
> 
> Yes it doesn't seem like the right solution.
> 
> >
> > We *could* have a per-cpu pointer to the cpu_pmu_pointer, but then we'd
> > need to update the IRQ handlers, including things like the CCI PMU which
> > really doesn't care about per-cpu stuff. So after all this, the shim we have
> > around the IRQ handler for the U8500 SPI workarounds might be the right
> > thing after all -- it allows us to consolidate the conversion of a pcpu
> > pointer into the relevant instance (actually any instance, since they'd all
> > point at the same thing) for the current CPU.
> >
> > What do you think to having that shim throw away the second level pcpu
> > pointer in the case of a PPI? (probably means we need to revisit that
> > renaming again).
> 
> Ok I think I understand what you're getting at. We pass a per-cpu
> pointer to the cpu_pmu pointer as the dev_id argument to the PPI irq
> handler, and then we check to see if the irq is per-cpu inside the
> armpmu_dispatch_irq() function and throw away the second level of
> pointer, i.e.
> 
> static irqreturn_t armpmu_dispatch_irq(int irq, void *dev)
> {
>         struct arm_pmu *armpmu;
>         struct platform_device *plat_device;
>         struct arm_pmu_platdata *plat;
> 
>         if (irq_is_percpu(irq))
>                 dev = *(struct arm_pmu_cpu **)dev;
>         armpmu = dev;
>         plat_device = armpmu->plat_device;
>         plat = dev_get_platdata(&plat_device->dev);
> 
>         if (plat && plat->handle_irq)
>                 return plat->handle_irq(irq, dev, armpmu->handle_irq);
>         else
>                 return armpmu->handle_irq(irq, dev);
> }

Yup, that's what I was trying to explain (badly). Thanks.

> We still need to make a per-cpu variable to hold the pointer, and assign
> it during cpu_pmu_init like this patch does. Hopefully that is ok.

I think that's ok. The percpu code in genirq requires a pcpu token (for good
reason) and the irq handler needs to get at the pmu structure. The
alternative is adding pointers from something like the pmu_hw_events to the
arm_pmu, but I think that's more ugly.

Will

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2014-01-15 10:33 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-08 22:59 [PATCH 0/7] Support Krait CPU PMUs Stephen Boyd
2014-01-08 22:59 ` [PATCH 1/7] ARM: perf_event: Silence sparse warning Stephen Boyd
2014-01-09 10:45   ` Will Deacon
2014-01-09 23:59     ` Stephen Boyd
2014-01-08 22:59 ` [PATCH 2/7] ARM: perf_event: Support percpu irqs for the CPU PMU Stephen Boyd
2014-01-09 10:49   ` Will Deacon
2014-01-09 19:17     ` Stephen Boyd
2014-01-10 10:58       ` Will Deacon
2014-01-10 19:36         ` Stephen Boyd
2014-01-13 11:52           ` Will Deacon
2014-01-14 20:57             ` Stephen Boyd
2014-01-15 10:33               ` Will Deacon
2014-01-08 22:59 ` [PATCH 3/7] ARM: perf_event: Add basic support for Krait CPU PMUs Stephen Boyd
2014-01-09 11:04   ` Will Deacon
2014-01-09 19:57     ` Stephen Boyd
2014-01-10 11:01       ` Will Deacon
2014-01-10 18:57         ` Stephen Boyd
2014-01-08 22:59 ` [PATCH 4/7] ARM: perf_event: Add hook for event index clearing Stephen Boyd
2014-01-08 22:59 ` [PATCH 5/7] ARM: perf_event: Fully support Krait CPU PMU events Stephen Boyd
2014-01-08 22:59 ` [PATCH 6/7] devicetree: bindings: Document Krait performance monitor units (PMU) Stephen Boyd
2014-01-09 18:14   ` Will Deacon
2014-01-09 19:57     ` Stephen Boyd
2014-01-08 22:59 ` [PATCH 7/7] ARM: dts: msm: Add krait-pmu to platforms with Krait CPUs Stephen Boyd

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).