ARM: perf_event: allow platform-specific interrupt handler
Subsystems (5)
PATCH v2 1/2 ARM: perf_event: allow platform-specific interrupt handler
Allow a platform-specific IRQ handler to be specified via platform data. This will be used to implement the single-irq workaround for the DB8500.
--- arch/arm/include/asm/pmu.h | 14 ++++++++++++++ arch/arm/kernel/perf_event.c | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletions(-)
arch/arm/include/asm/pmu.h +14 -0
@@ -12,11 +12,25 @@#ifndef __ARM_PMU_H__#define __ARM_PMU_H__ +#include <linux/interrupt.h> +enum arm_pmu_type {ARM_PMU_DEVICE_CPU =0 ,ARM_NUM_PMU_DEVICES , }; +/* +* struct arm_pmu_platdata - ARM PMU platform data +* +* @handle_irq: an optional handler which will be called from the interrupt and +* passed the address of the low level handler, and can be used to implement +* any platform specific handling before or after calling it. +*/ +struct arm_pmu_platdata { +irqreturn_t (*handle_irq)(int irq,void *dev, +irq_handler_t pmu_handler); +}; +#ifdef CONFIG_CPU_HAS_PMU /**
arch/arm/kernel/perf_event.c +16 -1
@@ -377,9 +377,18 @@ validate_group(struct perf_event *event)return 0 ; } +static irqreturn_t armpmu_platform_irq (int irq,void *dev) +{ +struct arm_pmu_platdata *plat =dev_get_platdata (&pmu_device->dev); + +return plat->handle_irq (irq, dev, armpmu->handle_irq); +} +static int armpmu_reserve_hardware (void ) { +struct arm_pmu_platdata *plat; +irq_handler_t handle_irq;int i, err = -ENODEV , irq; pmu_device =reserve_pmu (ARM_PMU_DEVICE_CPU );
@@ -390,6 +399,12 @@ armpmu_reserve_hardware(void)init_pmu (ARM_PMU_DEVICE_CPU ); + plat =dev_get_platdata (&pmu_device->dev); +if (plat && plat->handle_irq) + handle_irq = armpmu_platform_irq; +else + handle_irq = armpmu->handle_irq; +if (pmu_device->num_resources <1 ) {pr_err ("no irqs for PMUs defined\n" );return -ENODEV ;
@@ -400,7 +415,7 @@ armpmu_reserve_hardware(void)if (irq <0 )continue ; - err =request_irq (irq, armpmu->handle_irq, + err =request_irq (irq, handle_irq,IRQF_DISABLED |IRQF_NOBALANCING ,"armpmu" ,NULL );if (err) {
Signature · 1 line
1.7.2.dirty
PATCH v2 2/2 ux500: DB8500 PMU support
DB8500 has irqs from two cores ORed into one. Implement a workaround to handle this by bouncing the interrupt by setting the affinity to the other core when the interrupt appears to be spurious on the current core.
--- arch/arm/mach-ux500/cpu-db8500.c | 51 +++++++++++++++++++++++++++++++++++--- 1 files changed, 47 insertions(+), 4 deletions(-)
arch/arm/mach-ux500/cpu-db8500.c +47 -4
@@ -12,12 +12,14 @@#include <linux/init.h> #include <linux/device.h> #include <linux/amba/bus.h> +#include <linux/interrupt.h> #include <linux/irq.h> #include <linux/gpio.h> #include <linux/platform_device.h> #include <linux/io.h> #include <asm/mach/map.h> +#include <asm/pmu.h> #include <mach/hardware.h> #include <mach/setup.h> #include <mach/devices.h>
@@ -26,10 +28,6 @@#include "devices-db8500.h" #include "ste-dma40-db8500.h" -static struct platform_device *platform_devs[]__initdata = { - &u8500_dma40_device, -}; -/* minimum static i/o mapping required to boot U8500 platforms */ static struct map_desc u8500_uart_io_desc[]__initdata = {__IO_DEV_DESC (U8500_UART0_BASE ,SZ_4K ),
@@ -91,6 +89,51 @@ void __init u8500_map_io(void)iotable_init (u8500_v2_io_desc,ARRAY_SIZE (u8500_v2_io_desc)); } +static struct resource db8500_pmu_resources[] = { + [0 ] = { + .start =IRQ_DB8500_PMU , + .end =IRQ_DB8500_PMU , + .flags =IORESOURCE_IRQ , + }, +}; + +/* +* The PMU IRQ lines of two cores are wired together into a single interrupt. +* Bounce the interrupt to the other core if it's not ours. +*/ +static irqreturn_t db8500_pmu_handler (int irq,void *dev,irq_handler_t handler) +{ +irqreturn_t ret =handler (irq, dev); +int other = !smp_processor_id (); + +if (ret ==IRQ_NONE &&cpu_online (other)) +irq_set_affinity (irq,cpumask_of (other)); + +/* +* We should be able to get away with the amount of IRQ_NONEs we give, +* while still having the spurious IRQ detection code kick in if the +* interrupt really starts hitting spuriously. +*/ +return ret; +} + +static struct arm_pmu_platdata db8500_pmu_platdata = { + .handle_irq = db8500_pmu_handler, +}; + +static struct platform_device db8500_pmu_device = { + .name ="arm-pmu" , + .id =ARM_PMU_DEVICE_CPU , + .num_resources =ARRAY_SIZE (db8500_pmu_resources), + .resource = db8500_pmu_resources, + .dev.platform_data = &db8500_pmu_platdata, +}; + +static struct platform_device *platform_devs[]__initdata = { + &u8500_dma40_device, + &db8500_pmu_device, +}; +static resource_size_t __initdata db8500_gpio_base[] = {U8500_GPIOBANK0_BASE ,U8500_GPIOBANK1_BASE ,
Signature · 1 line
1.7.2.dirty
PATCH v2 1/2 ARM: perf_event: allow platform-specific interrupt handler
In reply to Rabin VincentHi Rabin,
Quoted 8 lines
Allow a platform-specific IRQ handler to be specified via platform data. This will be used to implement the single-irq workaround for the DB8500.
Signed-off-by Rabin Vincent <[email protected]>--- arch/arm/include/asm/pmu.h | 14 ++++++++++++++ arch/arm/kernel/perf_event.c | 17 ++++++++++++++++- 2 files changed, 30 insertions(+), 1 deletions(-)
If you're happy with this as a workaround for your platform, then it looks alright to me.
One thing you could try is using the GIC patch I posted the other day:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-February/041496.html
If you then do:
ARM: gic: allow per-cpu SPIs to be affine to multiple CPUs
The concept of a per-cpu SPI is somewhat a contradiction, but can occur in systems where SPIs from different CPUs are ORd together into a single line.
An example of this is the PMU interrupt on the u8500 platform.
This patch allows SPIs with the IRQF_PERCPU flag to be affine to multiple CPUs in a CPU mask. This, of course, assumes that the driver knows what it is doing and can handle such a configuration.
arch/arm/common/gic.c +12 -2
@@ -145,7 +145,7 @@ gic_set_cpu(struct irq_data *d, const struct cpumask *mask_val, bool force) {void __iomem *reg =gic_dist_base (d) +GIC_DIST_TARGET + (gic_irq (d) & ~3 );unsigned int shift = (d->irq %4 ) *8 ; -unsigned int cpu =cpumask_first (mask_val); +unsigned int cpu_map, cpu =cpumask_first (mask_val);u32 val;struct irq_desc *desc;
@@ -155,9 +155,19 @@ gic_set_cpu(struct irq_data *d, const struct cpumask *mask_val, bool force)spin_unlock (&irq_controller_lock);return -EINVAL ; } + d->node = cpu; + +if (CHECK_IRQ_PER_CPU (desc->status)) { + cpu_map =0 ; +for_each_cpu (cpu, mask_val) + cpu_map |=1 << (cpu + shift); + }else { + cpu_map =1 << (cpu + shift); + } + val =readl (reg) & ~(0xff << shift); - val |=1 << (cpu + shift); + val |= cpu_map;writel (val, reg);spin_unlock (&irq_controller_lock);
You'll be able to target the PMU IRQ to both CPUs and avoid the need for ping-ponging the affinity. This is a bit weird though as usually you'd have a PPI for a percpu interrupt so this might be better off staying inside platform code and leaving the GIC code alone. I also think this approach is more invasive from the perf point of view.
Unless this approach gives markedly better profiling results than your proposal, I think we should go with what you've got.
Will
A message that is not in the archive (sent off-list or not yet received): -6723806473392693428@unknownmsgid
PATCH v2 1/2 ARM: perf_event: allow platform-specific interrupt handler
Quoted 9 lines · Will Deacon
Quoted 8 lines
Allow a platform-specific IRQ handler to be specified via platform data. ?This will be used to implement the single-irq workaround for the DB8500.
Signed-off-by Rabin Vincent <[email protected]>---
?arch/arm/include/asm/pmu.h ? | ? 14 ++++++++++++++ ?arch/arm/kernel/perf_event.c | ? 17 ++++++++++++++++- ?2 files changed, 30 insertions(+), 1 deletions(-)
Quoted 4 lines
If you're happy with this as a workaround for your platform, then it looks alright to me.
Acked-by Will Deacon <[email protected]>
Thanks.
Quoted 55 lines
One thing you could try is using the GIC patch I posted the other day:
http://lists.infradead.org/pipermail/linux-arm-kernel/2011-February/041496.html
If you then do:
? ?ARM: gic: allow per-cpu SPIs to be affine to multiple CPUs
? ?The concept of a per-cpu SPI is somewhat a contradiction, but can occur in ? ?systems where SPIs from different CPUs are ORd together into a single line.
? ?An example of this is the PMU interrupt on the u8500 platform.
? ?This patch allows SPIs with the
IRQF_PERCPUflag to be affine to multiple ? ?CPUs in a CPU mask. This, of course, assumes that the driver knows what it ? ?is doing and can handle such a configuration.? ?Signed-off-by: Will Deacon <[email protected]>
arch/arm/common/gic.c +12
-2@@ -145,7 +145,7 @@ gic_set_cpu(struct irq_data *d, const struct cpumask *mask_val, bool force) ?{ ? ? ? ?void __iomem *reg = gic_dist_base(d) + GIC_DIST_TARGET + (gic_irq(d) & ~3); ? ? ? ?unsigned int shift = (d->irq % 4) * 8; - ? ? ? unsigned int cpu = cpumask_first(mask_val); + ? ? ? unsigned int cpu_map, cpu = cpumask_first(mask_val); ? ? ? ?u32 val; ? ? ? ?struct irq_desc *desc;@@ -155,9 +155,19 @@ gic_set_cpu(struct irq_data *d, const struct cpumask *mask_val, bool force) ? ? ? ? ? ? ? ?spin_unlock(&irq_controller_lock); ? ? ? ? ? ? ? ?return -EINVAL; ? ? ? ?} + ? ? ? ?d->node = cpu; + + ? ? ? if (CHECK_IRQ_PER_CPU(desc->status)) { + ? ? ? ? ? ? ? cpu_map = 0; + ? ? ? ? ? ? ? for_each_cpu(cpu, mask_val) + ? ? ? ? ? ? ? ? ? ? ? cpu_map |= 1 << (cpu + shift); + ? ? ? } else { + ? ? ? ? ? ? ? cpu_map = 1 << (cpu + shift); + ? ? ? } + ? ? ? ?val = readl(reg) & ~(0xff << shift); - ? ? ? val |= 1 << (cpu + shift); + ? ? ? val |= cpu_map; ? ? ? ?writel(val, reg); ? ? ? ?spin_unlock(&irq_controller_lock);
Quoted 8 lines
You'll be able to target the PMU IRQ to both CPUs and avoid the need for ping-ponging the affinity. This is a bit weird though as usually you'd have a PPI for a percpu interrupt so this might be better off staying inside platform code and leaving the GIC code alone. I also think this approach is more invasive from the perf point of view.
Unless this approach gives markedly better profiling results than your proposal, I think we should go with what you've got.
I gave this a try, along with the modifications to enable IRQ_PER_CPU and have the pmu code use the appropriate flags and set the affinity. Didn't work though; it always ends up triggering the spurious IRQ check.
PATCH v2 1/2 ARM: perf_event: allow platform-specific interrupt handler
Hi Rabin,
Quoted 6 lines
Quoted 6 lines
You'll be able to target the PMU IRQ to both CPUs and avoid the need for ping-ponging the affinity. This is a bit weird though as usually you'd have a PPI for a percpu interrupt so this might be better off staying inside platform code and leaving the GIC code alone. I also think this approach is more invasive from the perf point of view.
Quoted 6 lines
Quoted 2 lines
Unless this approach gives markedly better profiling results than your proposal, I think we should go with what you've got.
I gave this a try, along with the modifications to enable
IRQ_PER_CPUand have the pmu code use the appropriate flags and set the affinity. Didn't work though; it always ends up triggering the spurious IRQ check.
Hmm, that doesn't sound right. Did you have any synchronisation to ensure that the CPU without the overflow didn't return IRQ_NONE until the handling CPU had returned IRQ_HANDLED?
Will
A message that is not in the archive (sent off-list or not yet received): -3663809140750500272@unknownmsgid
PATCH v2 1/2 ARM: perf_event: allow platform-specific interrupt handler
Quoted 7 lines · Will Deacon
Quoted 3 lines
I gave this a try, along with the modifications to enable
IRQ_PER_CPUand have the pmu code use the appropriate flags and set the affinity. Didn't work though; it always ends up triggering the spurious IRQ check.Hmm, that doesn't sound right. Did you have any synchronisation to ensure that the CPU without the overflow didn't return
IRQ_NONEuntil the handling CPU had returnedIRQ_HANDLED?
No. What kind of synchronization do you mean?
PATCH v2 1/2 ARM: perf_event: allow platform-specific interrupt handler
Quoted 10 lines · Will Deacon
Quoted 7 lines
Quoted 3 lines
I gave this a try, along with the modifications to enable
IRQ_PER_CPUand have the pmu code use the appropriate flags and set the affinity. Didn't work though; it always ends up triggering the spurious IRQ check.Hmm, that doesn't sound right. Did you have any synchronisation to ensure that the CPU without the overflow didn't return
IRQ_NONEuntil the handling CPU had returnedIRQ_HANDLED?No. What kind of synchronization do you mean?
Well my guess is that the CPU returning IRQ_NONE is perpetually taking the same interrupt until the other CPU has poked the PMU to stop it asserting the line (otherwise I can't see how the spurious check would trip). You could have a lock in the driver to synchronise between the two CPUs so that if you don't have an overflow, you wait for the other CPU to clear the interrupt before returning.
This has quickly started to become horrible but I was just curious as to whether it provides better results than the affinity-setting approach (which is certainly less invasive).
Cheers,
Will