* [PATCH] oprofile: VR5500 performance counter driver @ 2009-02-23 16:26 Mark Asselstine 2009-02-25 16:59 ` Robert Richter 2009-02-26 17:49 ` [PATCH] " Ralf Baechle 0 siblings, 2 replies; 11+ messages in thread From: Mark Asselstine @ 2009-02-23 16:26 UTC (permalink / raw) To: linux-mips, oprofile-list This is inspired by op_model_mipsxx.c with some modification in regards to register layout and overflow handling. This has been tested on a NEC VR5500 board and shown to produce sane results. Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile index cfd4b60..977a828 100644 --- a/arch/mips/oprofile/Makefile +++ b/arch/mips/oprofile/Makefile @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o +oprofile-$(CONFIG_CPU_VR5500) += op_model_vr5500.o oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c index e1bffab..68aad99 100644 --- a/arch/mips/oprofile/common.c +++ b/arch/mips/oprofile/common.c @@ -17,6 +17,7 @@ extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); +extern struct op_mips_model op_model_vr5500_ops __attribute__((weak)); static struct op_mips_model *model; @@ -94,6 +95,10 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) case CPU_RM9000: lmodel = &op_model_rm9000_ops; break; + + case CPU_R5500: + lmodel = &op_model_vr5500_ops; + break; }; diff --git a/arch/mips/oprofile/op_model_vr5500.c b/arch/mips/oprofile/op_model_vr5500.c new file mode 100644 index 0000000..75fae6a --- /dev/null +++ b/arch/mips/oprofile/op_model_vr5500.c @@ -0,0 +1,179 @@ +/* + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * Copyright (c) 2009 Wind River Systems, Inc. + */ +#include <linux/cpumask.h> +#include <linux/oprofile.h> +#include <linux/interrupt.h> +#include <linux/smp.h> +#include <asm/irq_regs.h> + +#include "op_impl.h" + +#define M_PERFCTL_EXL (1UL << 0) +#define M_PERFCTL_KERNEL (1UL << 1) +#define M_PERFCTL_SUPERVISOR (1UL << 2) +#define M_PERFCTL_USER (1UL << 3) +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) +#define M_PERFCTL_INTERRUPT (1UL << 5) +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) + +#define NUM_COUNTERS 2 + +static int (*save_perf_irq) (void); + +#define __define_perf_accessors(r, n) \ + \ + static inline unsigned int r_c0_ ## r ## n(void) \ + { \ + return read_c0_ ## r ## n(); \ + } \ + \ + static inline void w_c0_ ## r ## n(unsigned int value) \ + { \ + write_c0_ ## r ## n(value); \ + } \ + +__define_perf_accessors(perfcntr, 0) +__define_perf_accessors(perfcntr, 1) + +__define_perf_accessors(perfctrl, 0) +__define_perf_accessors(perfctrl, 1) + +struct op_mips_model op_model_vr5500_ops; + +static struct vr5500_register_config { + unsigned int control[NUM_COUNTERS]; + unsigned int counter[NUM_COUNTERS]; +} reg; + +/* Compute all of the registers in preparation for enabling profiling. */ +static void vr5500_reg_setup(struct op_counter_config *ctr) +{ + int i; + unsigned int counters = NUM_COUNTERS; + + /* Compute the performance counter control word. */ + for (i = 0; i < counters; i++) { + reg.control[i] = 0; + reg.counter[i] = 0; + + if (!ctr[i].enabled) + continue; + + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; + if (ctr[i].kernel) + reg.control[i] |= M_PERFCTL_KERNEL; + if (ctr[i].user) + reg.control[i] |= M_PERFCTL_USER; + if (ctr[i].exl) + reg.control[i] |= M_PERFCTL_EXL; + + reg.counter[i] = 0xffffffff - ctr[i].count + 1; + } +} + +/* Program all of the registers in preparation for enabling profiling. */ +static void vr5500_cpu_setup(void *args) +{ + w_c0_perfctrl1(0); + w_c0_perfcntr1(reg.counter[1]); + + w_c0_perfctrl0(0); + w_c0_perfcntr0(reg.counter[0]); +} + +/* Start all counters on current CPU */ +static void vr5500_cpu_start(void *args) +{ + w_c0_perfctrl1(reg.control[1]); + w_c0_perfctrl0(reg.control[0]); +} + +/* Stop all counters on current CPU */ +static void vr5500_cpu_stop(void *args) +{ + w_c0_perfctrl1(0); + w_c0_perfctrl0(0); +} + +static int vr5500_perfcount_handler(void) +{ + unsigned int control; + unsigned int counter; + int handled = IRQ_NONE; + unsigned int counters = NUM_COUNTERS; + + if (cpu_has_mips_r2 && !(read_c0_cause() & (1 << 26))) + return handled; + + switch (counters) { + #define HANDLE_COUNTER(n) \ + case n + 1: \ + control = r_c0_perfctrl ## n(); \ + counter = r_c0_perfcntr ## n(); \ + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && \ + (control & M_PERFCTL_INTERRUPT)) { \ + oprofile_add_sample(get_irq_regs(), n); \ + w_c0_perfcntr ## n(reg.counter[n]); \ + w_c0_perfctrl ## n(control & ~M_PERFCTL_INTERRUPT); \ + handled = IRQ_HANDLED; \ + } + HANDLE_COUNTER(1) + HANDLE_COUNTER(0) + } + + return handled; +} + +static void reset_counters(void *arg) +{ + w_c0_perfctrl1(0); + w_c0_perfcntr1(0); + + w_c0_perfctrl0(0); + w_c0_perfcntr0(0); +} + +static int __init vr5500_init(void) +{ + on_each_cpu(reset_counters, NULL, 1); + + switch (current_cpu_type()) { + case CPU_R5500: + op_model_vr5500_ops.cpu_type = "mips/vr5500"; + break; + + default: + printk(KERN_ERR "Profiling unsupported for this CPU\n"); + + return -ENODEV; + } + + save_perf_irq = perf_irq; + perf_irq = vr5500_perfcount_handler; + + return 0; +} + +static void vr5500_exit(void) +{ + on_each_cpu(reset_counters, NULL, 1); + + perf_irq = save_perf_irq; +} + +struct op_mips_model op_model_vr5500_ops = { + .reg_setup = vr5500_reg_setup, + .cpu_setup = vr5500_cpu_setup, + .init = vr5500_init, + .exit = vr5500_exit, + .cpu_start = vr5500_cpu_start, + .cpu_stop = vr5500_cpu_stop, + .num_counters = NUM_COUNTERS, +}; ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] oprofile: VR5500 performance counter driver 2009-02-23 16:26 [PATCH] oprofile: VR5500 performance counter driver Mark Asselstine @ 2009-02-25 16:59 ` Robert Richter 2009-02-26 16:30 ` M. Asselstine 2009-02-26 20:49 ` [PATCH V2] " Mark Asselstine 2009-02-26 17:49 ` [PATCH] " Ralf Baechle 1 sibling, 2 replies; 11+ messages in thread From: Robert Richter @ 2009-02-25 16:59 UTC (permalink / raw) To: Mark Asselstine, Ralf Baechle; +Cc: linux-mips, oprofile-list On 23.02.09 11:26:34, Mark Asselstine wrote: > This is inspired by op_model_mipsxx.c with some modification > in regards to register layout and overflow handling. This has > been tested on a NEC VR5500 board and shown to produce sane > results. Mark, I have looked at the differences between the VR5500 code and the generic in op_model_mipsxx.c. If I am not wrong, only the interrupt handling is different. This affects only vr5500_reg_setup() and vr5500_perfcount_handler(). I think it would be better to implement cpu checks in the generic functions or remap to cpu specific functions during mipsxx_init(). This extension of the generic code is much more maintainable. Also, there is less code in the end. See also my comments below. -Robert > > Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> > > diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile > index cfd4b60..977a828 100644 > --- a/arch/mips/oprofile/Makefile > +++ b/arch/mips/oprofile/Makefile > @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o > oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o > oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o > oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o > +oprofile-$(CONFIG_CPU_VR5500) += op_model_vr5500.o I could not find a Kconfig option for this. > oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o > diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c > index e1bffab..68aad99 100644 > --- a/arch/mips/oprofile/common.c > +++ b/arch/mips/oprofile/common.c > @@ -17,6 +17,7 @@ > > extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); > extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); > +extern struct op_mips_model op_model_vr5500_ops __attribute__((weak)); > > static struct op_mips_model *model; > > @@ -94,6 +95,10 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) > case CPU_RM9000: > lmodel = &op_model_rm9000_ops; > break; > + > + case CPU_R5500: > + lmodel = &op_model_vr5500_ops; > + break; Is there a reason for using _vr5500_ instead of _r5500_ for the function and the filename? > }; > > > diff --git a/arch/mips/oprofile/op_model_vr5500.c b/arch/mips/oprofile/op_model_vr5500.c > new file mode 100644 > index 0000000..75fae6a > --- /dev/null > +++ b/arch/mips/oprofile/op_model_vr5500.c > @@ -0,0 +1,179 @@ > +/* > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file "COPYING" in the main directory of this archive > + * for more details. > + * Copyright statements from op_model_mipsxx.c should be added here. > + * Copyright (c) 2009 Wind River Systems, Inc. > + */ > +#include <linux/cpumask.h> > +#include <linux/oprofile.h> > +#include <linux/interrupt.h> > +#include <linux/smp.h> > +#include <asm/irq_regs.h> > + > +#include "op_impl.h" > + > +#define M_PERFCTL_EXL (1UL << 0) > +#define M_PERFCTL_KERNEL (1UL << 1) > +#define M_PERFCTL_SUPERVISOR (1UL << 2) > +#define M_PERFCTL_USER (1UL << 3) > +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) > +#define M_PERFCTL_INTERRUPT (1UL << 5) > +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) > +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) > + > +#define NUM_COUNTERS 2 > + > +static int (*save_perf_irq) (void); > + > +#define __define_perf_accessors(r, n) \ > + \ > + static inline unsigned int r_c0_ ## r ## n(void) \ > + { \ > + return read_c0_ ## r ## n(); \ > + } \ > + \ > + static inline void w_c0_ ## r ## n(unsigned int value) \ > + { \ > + write_c0_ ## r ## n(value); \ > + } \ > + > +__define_perf_accessors(perfcntr, 0) > +__define_perf_accessors(perfcntr, 1) > + > +__define_perf_accessors(perfctrl, 0) > +__define_perf_accessors(perfctrl, 1) I know this code is borrowed, but why not use write/read_c0_perfXXXX() directly? > + > +struct op_mips_model op_model_vr5500_ops; > + > +static struct vr5500_register_config { > + unsigned int control[NUM_COUNTERS]; > + unsigned int counter[NUM_COUNTERS]; > +} reg; > + > +/* Compute all of the registers in preparation for enabling profiling. */ > +static void vr5500_reg_setup(struct op_counter_config *ctr) > +{ > + int i; > + unsigned int counters = NUM_COUNTERS; > + > + /* Compute the performance counter control word. */ > + for (i = 0; i < counters; i++) { > + reg.control[i] = 0; > + reg.counter[i] = 0; > + > + if (!ctr[i].enabled) > + continue; > + > + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | > + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; > + if (ctr[i].kernel) > + reg.control[i] |= M_PERFCTL_KERNEL; > + if (ctr[i].user) > + reg.control[i] |= M_PERFCTL_USER; > + if (ctr[i].exl) > + reg.control[i] |= M_PERFCTL_EXL; > + > + reg.counter[i] = 0xffffffff - ctr[i].count + 1; > + } > +} > + > +/* Program all of the registers in preparation for enabling profiling. */ > +static void vr5500_cpu_setup(void *args) > +{ > + w_c0_perfctrl1(0); > + w_c0_perfcntr1(reg.counter[1]); > + > + w_c0_perfctrl0(0); > + w_c0_perfcntr0(reg.counter[0]); > +} > + > +/* Start all counters on current CPU */ > +static void vr5500_cpu_start(void *args) > +{ > + w_c0_perfctrl1(reg.control[1]); > + w_c0_perfctrl0(reg.control[0]); > +} > + > +/* Stop all counters on current CPU */ > +static void vr5500_cpu_stop(void *args) > +{ > + w_c0_perfctrl1(0); > + w_c0_perfctrl0(0); > +} > + > +static int vr5500_perfcount_handler(void) > +{ > + unsigned int control; > + unsigned int counter; > + int handled = IRQ_NONE; > + unsigned int counters = NUM_COUNTERS; > + > + if (cpu_has_mips_r2 && !(read_c0_cause() & (1 << 26))) Do not use magic numbers. > + return handled; > + > + switch (counters) { Since counters is a fix value the switch/case could be removed. > + #define HANDLE_COUNTER(n) \ > + case n + 1: \ > + control = r_c0_perfctrl ## n(); \ > + counter = r_c0_perfcntr ## n(); \ > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && \ > + (control & M_PERFCTL_INTERRUPT)) { \ > + oprofile_add_sample(get_irq_regs(), n); \ > + w_c0_perfcntr ## n(reg.counter[n]); \ > + w_c0_perfctrl ## n(control & ~M_PERFCTL_INTERRUPT); \ > + handled = IRQ_HANDLED; \ > + } > + HANDLE_COUNTER(1) > + HANDLE_COUNTER(0) > + } It is hard to see a loop here. I would like to prefer programming c instead of macros unless there is a good reason to do so. Also, this causes a checkpatch error. > + > + return handled; > +} > + > +static void reset_counters(void *arg) > +{ > + w_c0_perfctrl1(0); > + w_c0_perfcntr1(0); > + > + w_c0_perfctrl0(0); > + w_c0_perfcntr0(0); > +} > + > +static int __init vr5500_init(void) > +{ > + on_each_cpu(reset_counters, NULL, 1); > + > + switch (current_cpu_type()) { > + case CPU_R5500: > + op_model_vr5500_ops.cpu_type = "mips/vr5500"; > + break; > + > + default: > + printk(KERN_ERR "Profiling unsupported for this CPU\n"); > + > + return -ENODEV; > + } > + > + save_perf_irq = perf_irq; > + perf_irq = vr5500_perfcount_handler; > + > + return 0; > +} > + > +static void vr5500_exit(void) > +{ > + on_each_cpu(reset_counters, NULL, 1); > + > + perf_irq = save_perf_irq; > +} > + > +struct op_mips_model op_model_vr5500_ops = { > + .reg_setup = vr5500_reg_setup, > + .cpu_setup = vr5500_cpu_setup, > + .init = vr5500_init, > + .exit = vr5500_exit, > + .cpu_start = vr5500_cpu_start, > + .cpu_stop = vr5500_cpu_stop, > + .num_counters = NUM_COUNTERS, Please align this vertically. > +}; > > ------------------------------------------------------------------------------ > Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA > -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise > -Strategies to boost innovation and cut costs with open source participation > -Receive a $600 discount off the registration fee with the source code: SFAD > http://p.sf.net/sfu/XcvMzF8H > _______________________________________________ > oprofile-list mailing list > oprofile-list@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/oprofile-list > -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] oprofile: VR5500 performance counter driver 2009-02-25 16:59 ` Robert Richter @ 2009-02-26 16:30 ` M. Asselstine 2009-02-26 17:07 ` Robert Richter 2009-02-26 20:49 ` [PATCH V2] " Mark Asselstine 1 sibling, 1 reply; 11+ messages in thread From: M. Asselstine @ 2009-02-26 16:30 UTC (permalink / raw) To: Robert Richter; +Cc: Ralf Baechle, linux-mips, oprofile-list On Wednesday 25 February 2009, Robert Richter wrote: > On 23.02.09 11:26:34, Mark Asselstine wrote: > > This is inspired by op_model_mipsxx.c with some modification > > in regards to register layout and overflow handling. This has > > been tested on a NEC VR5500 board and shown to produce sane > > results. > > Mark, > > I have looked at the differences between the VR5500 code and the > generic in op_model_mipsxx.c. If I am not wrong, only the interrupt > handling is different. This affects only vr5500_reg_setup() and > vr5500_perfcount_handler(). I think it would be better to implement > cpu checks in the generic functions or remap to cpu specific functions > during mipsxx_init(). This extension of the generic code is much more > maintainable. Also, there is less code in the end. See also my > comments below. > I had original thought the same thing but found there was enough deviation leaving me to create the new file. In addition to what you point out the register layout is also different so I was ending up having to make use of some CPU specific ifdefs which made for ugly code. As well the CPU does not implement MIPS32 nor MIPS64 so didn't necessarily fall under mipsxx (weak yes). It might make sense to name the file to be more generic and cover other NEC MIPS CPUs. Anyways I will have another look and see if it can be done nicely. > -Robert > > > Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> > > > > diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile > > index cfd4b60..977a828 100644 > > --- a/arch/mips/oprofile/Makefile > > +++ b/arch/mips/oprofile/Makefile > > @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o > > +oprofile-$(CONFIG_CPU_VR5500) += op_model_vr5500.o > > I could not find a Kconfig option for this. > Sorry, was working from another tree that had a patch pulling this config in, it is in fact CONFIG_CPU_R5500 as seen in mainline. I will correct this. > > oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o > > diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c > > index e1bffab..68aad99 100644 > > --- a/arch/mips/oprofile/common.c > > +++ b/arch/mips/oprofile/common.c > > @@ -17,6 +17,7 @@ > > > > extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); > > extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); > > +extern struct op_mips_model op_model_vr5500_ops __attribute__((weak)); > > > > static struct op_mips_model *model; > > > > @@ -94,6 +95,10 @@ int __init oprofile_arch_init(struct > > oprofile_operations *ops) case CPU_RM9000: > > lmodel = &op_model_rm9000_ops; > > break; > > + > > + case CPU_R5500: > > + lmodel = &op_model_vr5500_ops; > > + break; > > Is there a reason for using _vr5500_ instead of _r5500_ for the > function and the filename? > If I keep this as a new file I will change to _r5500_ as suggested. > > }; > > > > > > diff --git a/arch/mips/oprofile/op_model_vr5500.c > > b/arch/mips/oprofile/op_model_vr5500.c new file mode 100644 > > index 0000000..75fae6a > > --- /dev/null > > +++ b/arch/mips/oprofile/op_model_vr5500.c > > @@ -0,0 +1,179 @@ > > +/* > > + * This file is subject to the terms and conditions of the GNU General > > Public + * License. See the file "COPYING" in the main directory of this > > archive + * for more details. > > + * > > Copyright statements from op_model_mipsxx.c should be added here. > I will ask Ralf if he want to have his carried over, but will carry the MIPS copyright if it makes legal sense. > > + * Copyright (c) 2009 Wind River Systems, Inc. > > + */ > > +#include <linux/cpumask.h> > > +#include <linux/oprofile.h> > > +#include <linux/interrupt.h> > > +#include <linux/smp.h> > > +#include <asm/irq_regs.h> > > + > > +#include "op_impl.h" > > + > > +#define M_PERFCTL_EXL (1UL << 0) > > +#define M_PERFCTL_KERNEL (1UL << 1) > > +#define M_PERFCTL_SUPERVISOR (1UL << 2) > > +#define M_PERFCTL_USER (1UL << 3) > > +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) > > +#define M_PERFCTL_INTERRUPT (1UL << 5) > > +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) > > +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) > > + > > +#define NUM_COUNTERS 2 > > + > > +static int (*save_perf_irq) (void); > > + > > +#define __define_perf_accessors(r, n) \ > > + \ > > + static inline unsigned int r_c0_ ## r ## n(void) \ > > + { \ > > + return read_c0_ ## r ## n(); \ > > + } \ > > + \ > > + static inline void w_c0_ ## r ## n(unsigned int value) \ > > + { \ > > + write_c0_ ## r ## n(value); \ > > + } \ > > + > > +__define_perf_accessors(perfcntr, 0) > > +__define_perf_accessors(perfcntr, 1) > > + > > +__define_perf_accessors(perfctrl, 0) > > +__define_perf_accessors(perfctrl, 1) > > I know this code is borrowed, but why not use write/read_c0_perfXXXX() > directly? > Sure, will change. > > + > > +struct op_mips_model op_model_vr5500_ops; > > + > > +static struct vr5500_register_config { > > + unsigned int control[NUM_COUNTERS]; > > + unsigned int counter[NUM_COUNTERS]; > > +} reg; > > + > > +/* Compute all of the registers in preparation for enabling profiling. > > */ +static void vr5500_reg_setup(struct op_counter_config *ctr) > > +{ > > + int i; > > + unsigned int counters = NUM_COUNTERS; > > + > > + /* Compute the performance counter control word. */ > > + for (i = 0; i < counters; i++) { > > + reg.control[i] = 0; > > + reg.counter[i] = 0; > > + > > + if (!ctr[i].enabled) > > + continue; > > + > > + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | > > + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; > > + if (ctr[i].kernel) > > + reg.control[i] |= M_PERFCTL_KERNEL; > > + if (ctr[i].user) > > + reg.control[i] |= M_PERFCTL_USER; > > + if (ctr[i].exl) > > + reg.control[i] |= M_PERFCTL_EXL; > > + > > + reg.counter[i] = 0xffffffff - ctr[i].count + 1; > > + } > > +} > > + > > +/* Program all of the registers in preparation for enabling profiling. > > */ +static void vr5500_cpu_setup(void *args) > > +{ > > + w_c0_perfctrl1(0); > > + w_c0_perfcntr1(reg.counter[1]); > > + > > + w_c0_perfctrl0(0); > > + w_c0_perfcntr0(reg.counter[0]); > > +} > > + > > +/* Start all counters on current CPU */ > > +static void vr5500_cpu_start(void *args) > > +{ > > + w_c0_perfctrl1(reg.control[1]); > > + w_c0_perfctrl0(reg.control[0]); > > +} > > + > > +/* Stop all counters on current CPU */ > > +static void vr5500_cpu_stop(void *args) > > +{ > > + w_c0_perfctrl1(0); > > + w_c0_perfctrl0(0); > > +} > > + > > +static int vr5500_perfcount_handler(void) > > +{ > > + unsigned int control; > > + unsigned int counter; > > + int handled = IRQ_NONE; > > + unsigned int counters = NUM_COUNTERS; > > + > > + if (cpu_has_mips_r2 && !(read_c0_cause() & (1 << 26))) > > Do not use magic numbers. > A comment will be added or a well named define. > > + return handled; > > + > > + switch (counters) { > > Since counters is a fix value the switch/case could be removed. > > > + #define HANDLE_COUNTER(n) \ > > + case n + 1: \ > > + control = r_c0_perfctrl ## n(); \ > > + counter = r_c0_perfcntr ## n(); \ > > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && \ > > + (control & M_PERFCTL_INTERRUPT)) { \ > > + oprofile_add_sample(get_irq_regs(), n); \ > > + w_c0_perfcntr ## n(reg.counter[n]); \ > > + w_c0_perfctrl ## n(control & ~M_PERFCTL_INTERRUPT); \ > > + handled = IRQ_HANDLED; \ > > + } > > + HANDLE_COUNTER(1) > > + HANDLE_COUNTER(0) > > + } > > It is hard to see a loop here. I would like to prefer programming c > instead of macros unless there is a good reason to do so. Also, this > causes a checkpatch error. > I will look to get rid of the macros, you don't have to ask me twice. > > + > > + return handled; > > +} > > + > > +static void reset_counters(void *arg) > > +{ > > + w_c0_perfctrl1(0); > > + w_c0_perfcntr1(0); > > + > > + w_c0_perfctrl0(0); > > + w_c0_perfcntr0(0); > > +} > > + > > +static int __init vr5500_init(void) > > +{ > > + on_each_cpu(reset_counters, NULL, 1); > > + > > + switch (current_cpu_type()) { > > + case CPU_R5500: > > + op_model_vr5500_ops.cpu_type = "mips/vr5500"; > > + break; > > + > > + default: > > + printk(KERN_ERR "Profiling unsupported for this CPU\n"); > > + > > + return -ENODEV; > > + } > > + > > + save_perf_irq = perf_irq; > > + perf_irq = vr5500_perfcount_handler; > > + > > + return 0; > > +} > > + > > +static void vr5500_exit(void) > > +{ > > + on_each_cpu(reset_counters, NULL, 1); > > + > > + perf_irq = save_perf_irq; > > +} > > + > > +struct op_mips_model op_model_vr5500_ops = { > > + .reg_setup = vr5500_reg_setup, > > + .cpu_setup = vr5500_cpu_setup, > > + .init = vr5500_init, > > + .exit = vr5500_exit, > > + .cpu_start = vr5500_cpu_start, > > + .cpu_stop = vr5500_cpu_stop, > > + .num_counters = NUM_COUNTERS, > > Please align this vertically. > Sure. Expect a resend soon. Regards, Mark > > +}; > > > > ------------------------------------------------------------------------- > >----- Open Source Business Conference (OSBC), March 24-25, 2009, San > > Francisco, CA -OSBC tackles the biggest issue in open source: Open > > Sourcing the Enterprise -Strategies to boost innovation and cut costs > > with open source participation -Receive a $600 discount off the > > registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H > > _______________________________________________ > > oprofile-list mailing list > > oprofile-list@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/oprofile-list ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] oprofile: VR5500 performance counter driver 2009-02-26 16:30 ` M. Asselstine @ 2009-02-26 17:07 ` Robert Richter 2009-02-26 17:51 ` Ralf Baechle 0 siblings, 1 reply; 11+ messages in thread From: Robert Richter @ 2009-02-26 17:07 UTC (permalink / raw) To: M. Asselstine; +Cc: Ralf Baechle, linux-mips, oprofile-list Thanks Mark. On 26.02.09 11:30:48, M. Asselstine wrote: > > > + if (cpu_has_mips_r2 && !(read_c0_cause() & (1 << 26))) > > > > Do not use magic numbers. > > > > A comment will be added or a well named define. I found this: arch/mips/include/asm/mipsregs.h:#define CAUSEB_CE 28 Not sure if this is the same register. If so, you could add the macro to mipsregs.h too. -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] oprofile: VR5500 performance counter driver 2009-02-26 17:07 ` Robert Richter @ 2009-02-26 17:51 ` Ralf Baechle 0 siblings, 0 replies; 11+ messages in thread From: Ralf Baechle @ 2009-02-26 17:51 UTC (permalink / raw) To: Robert Richter; +Cc: M. Asselstine, linux-mips, oprofile-list On Thu, Feb 26, 2009 at 06:07:25PM +0100, Robert Richter wrote: > I found this: > > arch/mips/include/asm/mipsregs.h:#define CAUSEB_CE 28 > > Not sure if this is the same register. If so, you could add the macro > to mipsregs.h too. It is the same register. Ralf ^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH V2] oprofile: VR5500 performance counter driver 2009-02-25 16:59 ` Robert Richter 2009-02-26 16:30 ` M. Asselstine @ 2009-02-26 20:49 ` Mark Asselstine 2009-03-03 11:07 ` Robert Richter 1 sibling, 1 reply; 11+ messages in thread From: Mark Asselstine @ 2009-02-26 20:49 UTC (permalink / raw) To: linux-mips, oprofile-list; +Cc: mark.asselstine This is inspired by op_model_mipsxx.c with some modification in regards to register layout and overflow handling. This has been tested on a NEC VR5500 board and shown to produce sane results. Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> --- I have left this as a new file as there is enough differences to make combining it combersome. If pushed I would possibly change my mind but I am not convinced yet. The userspace events are seen as mips/vr5500 so if there is a desire to have everything be r5500 some userspace changes would need to be made. arch/mips/oprofile/Makefile | 1 + arch/mips/oprofile/common.c | 5 + arch/mips/oprofile/op_model_r5500.c | 161 +++++++++++++++++++++++++++++++++++ 3 files changed, 167 insertions(+), 0 deletions(-) create mode 100644 arch/mips/oprofile/op_model_r5500.c diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile index bf3be6f..586e64e 100644 --- a/arch/mips/oprofile/Makefile +++ b/arch/mips/oprofile/Makefile @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o +oprofile-$(CONFIG_CPU_R5500) += op_model_r5500.o oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c index 3bf3354..26780c7 100644 --- a/arch/mips/oprofile/common.c +++ b/arch/mips/oprofile/common.c @@ -16,6 +16,7 @@ extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); +extern struct op_mips_model op_model_r5500_ops __attribute__((weak)); static struct op_mips_model *model; @@ -93,6 +94,10 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) case CPU_RM9000: lmodel = &op_model_rm9000_ops; break; + + case CPU_R5500: + lmodel = &op_model_r5500_ops; + break; }; if (!lmodel) diff --git a/arch/mips/oprofile/op_model_r5500.c b/arch/mips/oprofile/op_model_r5500.c new file mode 100644 index 0000000..9b0d20f --- /dev/null +++ b/arch/mips/oprofile/op_model_r5500.c @@ -0,0 +1,161 @@ +/* + * This file is subject to the terms and conditions of the GNU General Public + * License. See the file "COPYING" in the main directory of this archive + * for more details. + * + * Copyright (c) 2009 Wind River Systems, Inc. + * + * Derived from op_model_mipsxx.c Copyright Ralf Baechle, MIPS Technologies Inc + */ +#include <linux/oprofile.h> +#include <linux/interrupt.h> +#include <asm/irq_regs.h> + +#include "op_impl.h" + +#define M_PERFCTL_EXL (1UL << 0) +#define M_PERFCTL_KERNEL (1UL << 1) +#define M_PERFCTL_SUPERVISOR (1UL << 2) +#define M_PERFCTL_USER (1UL << 3) +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) +#define M_PERFCTL_INTERRUPT (1UL << 5) +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) + +#define NUM_COUNTERS 2 + +static int (*save_perf_irq) (void); + +struct op_mips_model op_model_r5500_ops; + +static struct r5500_register_config { + unsigned int control[NUM_COUNTERS]; + unsigned int counter[NUM_COUNTERS]; +} reg; + +/* Compute all of the registers in preparation for enabling profiling. */ +static void r5500_reg_setup(struct op_counter_config *ctr) +{ + int i; + unsigned int counters = NUM_COUNTERS; + + /* Compute the performance counter control word. */ + for (i = 0; i < counters; i++) { + reg.control[i] = 0; + reg.counter[i] = 0; + + if (!ctr[i].enabled) + continue; + + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; + if (ctr[i].kernel) + reg.control[i] |= M_PERFCTL_KERNEL; + if (ctr[i].user) + reg.control[i] |= M_PERFCTL_USER; + if (ctr[i].exl) + reg.control[i] |= M_PERFCTL_EXL; + + reg.counter[i] = 0xffffffff - ctr[i].count + 1; + } +} + +/* Program all of the registers in preparation for enabling profiling. */ +static void r5500_cpu_setup(void *args) +{ + write_c0_perfctrl1(0); + write_c0_perfcntr1(reg.counter[1]); + + write_c0_perfctrl0(0); + write_c0_perfcntr0(reg.counter[0]); +} + +/* Start all counters on current CPU */ +static void r5500_cpu_start(void *args) +{ + write_c0_perfctrl1(reg.control[1]); + write_c0_perfctrl0(reg.control[0]); +} + +/* Stop all counters on current CPU */ +static void r5500_cpu_stop(void *args) +{ + write_c0_perfctrl1(0); + write_c0_perfctrl0(0); +} + +static int r5500_perfcount_handler(void) +{ + unsigned int control; + unsigned int counter; + int handled = IRQ_NONE; + + control = read_c0_perfctrl0(); + counter = read_c0_perfcntr0(); + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && + (control & M_PERFCTL_INTERRUPT)) { + oprofile_add_sample(get_irq_regs(), 0); + write_c0_perfcntr0(reg.counter[0]); + write_c0_perfctrl0(control & ~M_PERFCTL_INTERRUPT); + handled = IRQ_HANDLED; + } + + control = read_c0_perfctrl1(); + counter = read_c0_perfcntr1(); + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && + (control & M_PERFCTL_INTERRUPT)) { + oprofile_add_sample(get_irq_regs(), 1); + write_c0_perfcntr1(reg.counter[1]); + write_c0_perfctrl1(control & ~M_PERFCTL_INTERRUPT); + handled = IRQ_HANDLED; + } + + return handled; +} + +static void reset_counters(void *arg) +{ + write_c0_perfctrl1(0); + write_c0_perfcntr1(0); + + write_c0_perfctrl0(0); + write_c0_perfcntr0(0); +} + +static int __init r5500_init(void) +{ + on_each_cpu(reset_counters, NULL, 1); + + switch (current_cpu_type()) { + case CPU_R5500: + op_model_r5500_ops.cpu_type = "mips/vr5500"; + break; + + default: + printk(KERN_ERR "Profiling unsupported for this CPU\n"); + + return -ENODEV; + } + + save_perf_irq = perf_irq; + perf_irq = r5500_perfcount_handler; + + return 0; +} + +static void r5500_exit(void) +{ + on_each_cpu(reset_counters, NULL, 1); + + perf_irq = save_perf_irq; +} + +struct op_mips_model op_model_r5500_ops = { + .reg_setup = r5500_reg_setup, + .cpu_setup = r5500_cpu_setup, + .init = r5500_init, + .exit = r5500_exit, + .cpu_start = r5500_cpu_start, + .cpu_stop = r5500_cpu_stop, + .num_counters = NUM_COUNTERS, +}; -- 1.6.0.3 ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH V2] oprofile: VR5500 performance counter driver 2009-02-26 20:49 ` [PATCH V2] " Mark Asselstine @ 2009-03-03 11:07 ` Robert Richter 2009-03-04 17:53 ` M. Asselstine 2009-06-23 9:29 ` Robert Richter 0 siblings, 2 replies; 11+ messages in thread From: Robert Richter @ 2009-03-03 11:07 UTC (permalink / raw) To: Mark Asselstine, Ralf Baechle; +Cc: linux-mips, oprofile-list On 26.02.09 15:49:34, Mark Asselstine wrote: > This is inspired by op_model_mipsxx.c with some modification > in regards to register layout and overflow handling. This has > been tested on a NEC VR5500 board and shown to produce sane > results. > > Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> > --- > > I have left this as a new file as there is enough differences > to make combining it combersome. If pushed I would possibly > change my mind but I am not convinced yet. The userspace > events are seen as mips/vr5500 so if there is a desire to > have everything be r5500 some userspace changes would need > to be made. > > arch/mips/oprofile/Makefile | 1 + > arch/mips/oprofile/common.c | 5 + > arch/mips/oprofile/op_model_r5500.c | 161 +++++++++++++++++++++++++++++++++++ Mark, the Kconfig option for CONFIG_CPU_R5500 is still missing otherwise the patch itself looks fine. Ralf, do you agree on introducing a separate file for this cpu model? Please ack. Thanks, -Robert > 3 files changed, 167 insertions(+), 0 deletions(-) > create mode 100644 arch/mips/oprofile/op_model_r5500.c > > diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile > index bf3be6f..586e64e 100644 > --- a/arch/mips/oprofile/Makefile > +++ b/arch/mips/oprofile/Makefile > @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o > oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o > oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o > oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o > +oprofile-$(CONFIG_CPU_R5500) += op_model_r5500.o > oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o > diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c > index 3bf3354..26780c7 100644 > --- a/arch/mips/oprofile/common.c > +++ b/arch/mips/oprofile/common.c > @@ -16,6 +16,7 @@ > > extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); > extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); > +extern struct op_mips_model op_model_r5500_ops __attribute__((weak)); > > static struct op_mips_model *model; > > @@ -93,6 +94,10 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) > case CPU_RM9000: > lmodel = &op_model_rm9000_ops; > break; > + > + case CPU_R5500: > + lmodel = &op_model_r5500_ops; > + break; > }; > > if (!lmodel) > diff --git a/arch/mips/oprofile/op_model_r5500.c b/arch/mips/oprofile/op_model_r5500.c > new file mode 100644 > index 0000000..9b0d20f > --- /dev/null > +++ b/arch/mips/oprofile/op_model_r5500.c > @@ -0,0 +1,161 @@ > +/* > + * This file is subject to the terms and conditions of the GNU General Public > + * License. See the file "COPYING" in the main directory of this archive > + * for more details. > + * > + * Copyright (c) 2009 Wind River Systems, Inc. > + * > + * Derived from op_model_mipsxx.c Copyright Ralf Baechle, MIPS Technologies Inc > + */ > +#include <linux/oprofile.h> > +#include <linux/interrupt.h> > +#include <asm/irq_regs.h> > + > +#include "op_impl.h" > + > +#define M_PERFCTL_EXL (1UL << 0) > +#define M_PERFCTL_KERNEL (1UL << 1) > +#define M_PERFCTL_SUPERVISOR (1UL << 2) > +#define M_PERFCTL_USER (1UL << 3) > +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) > +#define M_PERFCTL_INTERRUPT (1UL << 5) > +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) > +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) > + > +#define NUM_COUNTERS 2 > + > +static int (*save_perf_irq) (void); > + > +struct op_mips_model op_model_r5500_ops; > + > +static struct r5500_register_config { > + unsigned int control[NUM_COUNTERS]; > + unsigned int counter[NUM_COUNTERS]; > +} reg; > + > +/* Compute all of the registers in preparation for enabling profiling. */ > +static void r5500_reg_setup(struct op_counter_config *ctr) > +{ > + int i; > + unsigned int counters = NUM_COUNTERS; > + > + /* Compute the performance counter control word. */ > + for (i = 0; i < counters; i++) { > + reg.control[i] = 0; > + reg.counter[i] = 0; > + > + if (!ctr[i].enabled) > + continue; > + > + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | > + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; > + if (ctr[i].kernel) > + reg.control[i] |= M_PERFCTL_KERNEL; > + if (ctr[i].user) > + reg.control[i] |= M_PERFCTL_USER; > + if (ctr[i].exl) > + reg.control[i] |= M_PERFCTL_EXL; > + > + reg.counter[i] = 0xffffffff - ctr[i].count + 1; > + } > +} > + > +/* Program all of the registers in preparation for enabling profiling. */ > +static void r5500_cpu_setup(void *args) > +{ > + write_c0_perfctrl1(0); > + write_c0_perfcntr1(reg.counter[1]); > + > + write_c0_perfctrl0(0); > + write_c0_perfcntr0(reg.counter[0]); > +} > + > +/* Start all counters on current CPU */ > +static void r5500_cpu_start(void *args) > +{ > + write_c0_perfctrl1(reg.control[1]); > + write_c0_perfctrl0(reg.control[0]); > +} > + > +/* Stop all counters on current CPU */ > +static void r5500_cpu_stop(void *args) > +{ > + write_c0_perfctrl1(0); > + write_c0_perfctrl0(0); > +} > + > +static int r5500_perfcount_handler(void) > +{ > + unsigned int control; > + unsigned int counter; > + int handled = IRQ_NONE; > + > + control = read_c0_perfctrl0(); > + counter = read_c0_perfcntr0(); > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && > + (control & M_PERFCTL_INTERRUPT)) { > + oprofile_add_sample(get_irq_regs(), 0); > + write_c0_perfcntr0(reg.counter[0]); > + write_c0_perfctrl0(control & ~M_PERFCTL_INTERRUPT); > + handled = IRQ_HANDLED; > + } > + > + control = read_c0_perfctrl1(); > + counter = read_c0_perfcntr1(); > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && > + (control & M_PERFCTL_INTERRUPT)) { > + oprofile_add_sample(get_irq_regs(), 1); > + write_c0_perfcntr1(reg.counter[1]); > + write_c0_perfctrl1(control & ~M_PERFCTL_INTERRUPT); > + handled = IRQ_HANDLED; > + } > + > + return handled; > +} > + > +static void reset_counters(void *arg) > +{ > + write_c0_perfctrl1(0); > + write_c0_perfcntr1(0); > + > + write_c0_perfctrl0(0); > + write_c0_perfcntr0(0); > +} > + > +static int __init r5500_init(void) > +{ > + on_each_cpu(reset_counters, NULL, 1); > + > + switch (current_cpu_type()) { > + case CPU_R5500: > + op_model_r5500_ops.cpu_type = "mips/vr5500"; > + break; > + > + default: > + printk(KERN_ERR "Profiling unsupported for this CPU\n"); > + > + return -ENODEV; > + } > + > + save_perf_irq = perf_irq; > + perf_irq = r5500_perfcount_handler; > + > + return 0; > +} > + > +static void r5500_exit(void) > +{ > + on_each_cpu(reset_counters, NULL, 1); > + > + perf_irq = save_perf_irq; > +} > + > +struct op_mips_model op_model_r5500_ops = { > + .reg_setup = r5500_reg_setup, > + .cpu_setup = r5500_cpu_setup, > + .init = r5500_init, > + .exit = r5500_exit, > + .cpu_start = r5500_cpu_start, > + .cpu_stop = r5500_cpu_stop, > + .num_counters = NUM_COUNTERS, > +}; > -- > 1.6.0.3 > > > ------------------------------------------------------------------------------ > Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA > -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise > -Strategies to boost innovation and cut costs with open source participation > -Receive a $600 discount off the registration fee with the source code: SFAD > http://p.sf.net/sfu/XcvMzF8H > _______________________________________________ > oprofile-list mailing list > oprofile-list@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/oprofile-list > -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] oprofile: VR5500 performance counter driver 2009-03-03 11:07 ` Robert Richter @ 2009-03-04 17:53 ` M. Asselstine 2009-03-04 21:50 ` Robert Richter 2009-06-23 9:29 ` Robert Richter 1 sibling, 1 reply; 11+ messages in thread From: M. Asselstine @ 2009-03-04 17:53 UTC (permalink / raw) To: Robert Richter; +Cc: Ralf Baechle, linux-mips, oprofile-list On Tuesday 03 March 2009, Robert Richter wrote: > On 26.02.09 15:49:34, Mark Asselstine wrote: > > This is inspired by op_model_mipsxx.c with some modification > > in regards to register layout and overflow handling. This has > > been tested on a NEC VR5500 board and shown to produce sane > > results. > > > > Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> > > --- > > > > I have left this as a new file as there is enough differences > > to make combining it combersome. If pushed I would possibly > > change my mind but I am not convinced yet. The userspace > > events are seen as mips/vr5500 so if there is a desire to > > have everything be r5500 some userspace changes would need > > to be made. > > > > arch/mips/oprofile/Makefile | 1 + > > arch/mips/oprofile/common.c | 5 + > > arch/mips/oprofile/op_model_r5500.c | 161 > > +++++++++++++++++++++++++++++++++++ > > Mark, > > the Kconfig option for CONFIG_CPU_R5500 is still missing otherwise the > patch itself looks fine. > Hmmm, what tree are you looking at? I see it in Linus' tree as well as Ralf's .29 RCs and master. Mark > Ralf, > > do you agree on introducing a separate file for this cpu model? > Please ack. > > Thanks, > > -Robert > > > 3 files changed, 167 insertions(+), 0 deletions(-) > > create mode 100644 arch/mips/oprofile/op_model_r5500.c > > > > diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile > > index bf3be6f..586e64e 100644 > > --- a/arch/mips/oprofile/Makefile > > +++ b/arch/mips/oprofile/Makefile > > @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o > > +oprofile-$(CONFIG_CPU_R5500) += op_model_r5500.o > > oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o > > diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c > > index 3bf3354..26780c7 100644 > > --- a/arch/mips/oprofile/common.c > > +++ b/arch/mips/oprofile/common.c > > @@ -16,6 +16,7 @@ > > > > extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); > > extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); > > +extern struct op_mips_model op_model_r5500_ops __attribute__((weak)); > > > > static struct op_mips_model *model; > > > > @@ -93,6 +94,10 @@ int __init oprofile_arch_init(struct > > oprofile_operations *ops) case CPU_RM9000: > > lmodel = &op_model_rm9000_ops; > > break; > > + > > + case CPU_R5500: > > + lmodel = &op_model_r5500_ops; > > + break; > > }; > > > > if (!lmodel) > > diff --git a/arch/mips/oprofile/op_model_r5500.c > > b/arch/mips/oprofile/op_model_r5500.c new file mode 100644 > > index 0000000..9b0d20f > > --- /dev/null > > +++ b/arch/mips/oprofile/op_model_r5500.c > > @@ -0,0 +1,161 @@ > > +/* > > + * This file is subject to the terms and conditions of the GNU General > > Public + * License. See the file "COPYING" in the main directory of this > > archive + * for more details. > > + * > > + * Copyright (c) 2009 Wind River Systems, Inc. > > + * > > + * Derived from op_model_mipsxx.c Copyright Ralf Baechle, MIPS > > Technologies Inc + */ > > +#include <linux/oprofile.h> > > +#include <linux/interrupt.h> > > +#include <asm/irq_regs.h> > > + > > +#include "op_impl.h" > > + > > +#define M_PERFCTL_EXL (1UL << 0) > > +#define M_PERFCTL_KERNEL (1UL << 1) > > +#define M_PERFCTL_SUPERVISOR (1UL << 2) > > +#define M_PERFCTL_USER (1UL << 3) > > +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) > > +#define M_PERFCTL_INTERRUPT (1UL << 5) > > +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) > > +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) > > + > > +#define NUM_COUNTERS 2 > > + > > +static int (*save_perf_irq) (void); > > + > > +struct op_mips_model op_model_r5500_ops; > > + > > +static struct r5500_register_config { > > + unsigned int control[NUM_COUNTERS]; > > + unsigned int counter[NUM_COUNTERS]; > > +} reg; > > + > > +/* Compute all of the registers in preparation for enabling profiling. > > */ +static void r5500_reg_setup(struct op_counter_config *ctr) > > +{ > > + int i; > > + unsigned int counters = NUM_COUNTERS; > > + > > + /* Compute the performance counter control word. */ > > + for (i = 0; i < counters; i++) { > > + reg.control[i] = 0; > > + reg.counter[i] = 0; > > + > > + if (!ctr[i].enabled) > > + continue; > > + > > + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | > > + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; > > + if (ctr[i].kernel) > > + reg.control[i] |= M_PERFCTL_KERNEL; > > + if (ctr[i].user) > > + reg.control[i] |= M_PERFCTL_USER; > > + if (ctr[i].exl) > > + reg.control[i] |= M_PERFCTL_EXL; > > + > > + reg.counter[i] = 0xffffffff - ctr[i].count + 1; > > + } > > +} > > + > > +/* Program all of the registers in preparation for enabling profiling. > > */ +static void r5500_cpu_setup(void *args) > > +{ > > + write_c0_perfctrl1(0); > > + write_c0_perfcntr1(reg.counter[1]); > > + > > + write_c0_perfctrl0(0); > > + write_c0_perfcntr0(reg.counter[0]); > > +} > > + > > +/* Start all counters on current CPU */ > > +static void r5500_cpu_start(void *args) > > +{ > > + write_c0_perfctrl1(reg.control[1]); > > + write_c0_perfctrl0(reg.control[0]); > > +} > > + > > +/* Stop all counters on current CPU */ > > +static void r5500_cpu_stop(void *args) > > +{ > > + write_c0_perfctrl1(0); > > + write_c0_perfctrl0(0); > > +} > > + > > +static int r5500_perfcount_handler(void) > > +{ > > + unsigned int control; > > + unsigned int counter; > > + int handled = IRQ_NONE; > > + > > + control = read_c0_perfctrl0(); > > + counter = read_c0_perfcntr0(); > > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && > > + (control & M_PERFCTL_INTERRUPT)) { > > + oprofile_add_sample(get_irq_regs(), 0); > > + write_c0_perfcntr0(reg.counter[0]); > > + write_c0_perfctrl0(control & ~M_PERFCTL_INTERRUPT); > > + handled = IRQ_HANDLED; > > + } > > + > > + control = read_c0_perfctrl1(); > > + counter = read_c0_perfcntr1(); > > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && > > + (control & M_PERFCTL_INTERRUPT)) { > > + oprofile_add_sample(get_irq_regs(), 1); > > + write_c0_perfcntr1(reg.counter[1]); > > + write_c0_perfctrl1(control & ~M_PERFCTL_INTERRUPT); > > + handled = IRQ_HANDLED; > > + } > > + > > + return handled; > > +} > > + > > +static void reset_counters(void *arg) > > +{ > > + write_c0_perfctrl1(0); > > + write_c0_perfcntr1(0); > > + > > + write_c0_perfctrl0(0); > > + write_c0_perfcntr0(0); > > +} > > + > > +static int __init r5500_init(void) > > +{ > > + on_each_cpu(reset_counters, NULL, 1); > > + > > + switch (current_cpu_type()) { > > + case CPU_R5500: > > + op_model_r5500_ops.cpu_type = "mips/vr5500"; > > + break; > > + > > + default: > > + printk(KERN_ERR "Profiling unsupported for this CPU\n"); > > + > > + return -ENODEV; > > + } > > + > > + save_perf_irq = perf_irq; > > + perf_irq = r5500_perfcount_handler; > > + > > + return 0; > > +} > > + > > +static void r5500_exit(void) > > +{ > > + on_each_cpu(reset_counters, NULL, 1); > > + > > + perf_irq = save_perf_irq; > > +} > > + > > +struct op_mips_model op_model_r5500_ops = { > > + .reg_setup = r5500_reg_setup, > > + .cpu_setup = r5500_cpu_setup, > > + .init = r5500_init, > > + .exit = r5500_exit, > > + .cpu_start = r5500_cpu_start, > > + .cpu_stop = r5500_cpu_stop, > > + .num_counters = NUM_COUNTERS, > > +}; > > -- > > 1.6.0.3 > > > > > > ------------------------------------------------------------------------- > >----- Open Source Business Conference (OSBC), March 24-25, 2009, San > > Francisco, CA -OSBC tackles the biggest issue in open source: Open > > Sourcing the Enterprise -Strategies to boost innovation and cut costs > > with open source participation -Receive a $600 discount off the > > registration fee with the source code: SFAD http://p.sf.net/sfu/XcvMzF8H > > _______________________________________________ > > oprofile-list mailing list > > oprofile-list@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/oprofile-list ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] oprofile: VR5500 performance counter driver 2009-03-04 17:53 ` M. Asselstine @ 2009-03-04 21:50 ` Robert Richter 0 siblings, 0 replies; 11+ messages in thread From: Robert Richter @ 2009-03-04 21:50 UTC (permalink / raw) To: M. Asselstine; +Cc: Ralf Baechle, linux-mips, oprofile-list On 04.03.09 12:53:03, M. Asselstine wrote: > > the Kconfig option for CONFIG_CPU_R5500 is still missing otherwise the > > patch itself looks fine. > > > > Hmmm, what tree are you looking at? I see it in Linus' tree as well as Ralf's > .29 RCs and master. Ahh, sorry. Was accidentally greping for CPU_VR5500. It is already upstream. Thanks Mark, -Robert -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH V2] oprofile: VR5500 performance counter driver 2009-03-03 11:07 ` Robert Richter 2009-03-04 17:53 ` M. Asselstine @ 2009-06-23 9:29 ` Robert Richter 1 sibling, 0 replies; 11+ messages in thread From: Robert Richter @ 2009-06-23 9:29 UTC (permalink / raw) To: Ralf Baechle; +Cc: Mark Asselstine, linux-mips, oprofile-list On 03.03.09 12:07:55, Robert Richter wrote: > On 26.02.09 15:49:34, Mark Asselstine wrote: > > This is inspired by op_model_mipsxx.c with some modification > > in regards to register layout and overflow handling. This has > > been tested on a NEC VR5500 board and shown to produce sane > > results. > > > > Signed-off-by: Mark Asselstine <mark.asselstine@windriver.com> > > --- > > > > I have left this as a new file as there is enough differences > > to make combining it combersome. If pushed I would possibly > > change my mind but I am not convinced yet. The userspace > > events are seen as mips/vr5500 so if there is a desire to > > have everything be r5500 some userspace changes would need > > to be made. > > > > arch/mips/oprofile/Makefile | 1 + > > arch/mips/oprofile/common.c | 5 + > > arch/mips/oprofile/op_model_r5500.c | 161 +++++++++++++++++++++++++++++++++++ > > > Mark, > > the Kconfig option for CONFIG_CPU_R5500 is still missing otherwise the > patch itself looks fine. > > Ralf, > > do you agree on introducing a separate file for this cpu model? > Please ack. Ralf, I still have this patch in my queue. Do you have any opinions here? op_model_r5500.c is very similar to op_model_mipsxx.c, it partly introduces duplicate code. Thanks, -Robert > > 3 files changed, 167 insertions(+), 0 deletions(-) > > create mode 100644 arch/mips/oprofile/op_model_r5500.c > > > > diff --git a/arch/mips/oprofile/Makefile b/arch/mips/oprofile/Makefile > > index bf3be6f..586e64e 100644 > > --- a/arch/mips/oprofile/Makefile > > +++ b/arch/mips/oprofile/Makefile > > @@ -14,4 +14,5 @@ oprofile-$(CONFIG_CPU_MIPS32) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_MIPS64) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_R10000) += op_model_mipsxx.o > > oprofile-$(CONFIG_CPU_SB1) += op_model_mipsxx.o > > +oprofile-$(CONFIG_CPU_R5500) += op_model_r5500.o > > oprofile-$(CONFIG_CPU_RM9000) += op_model_rm9000.o > > diff --git a/arch/mips/oprofile/common.c b/arch/mips/oprofile/common.c > > index 3bf3354..26780c7 100644 > > --- a/arch/mips/oprofile/common.c > > +++ b/arch/mips/oprofile/common.c > > @@ -16,6 +16,7 @@ > > > > extern struct op_mips_model op_model_mipsxx_ops __attribute__((weak)); > > extern struct op_mips_model op_model_rm9000_ops __attribute__((weak)); > > +extern struct op_mips_model op_model_r5500_ops __attribute__((weak)); > > > > static struct op_mips_model *model; > > > > @@ -93,6 +94,10 @@ int __init oprofile_arch_init(struct oprofile_operations *ops) > > case CPU_RM9000: > > lmodel = &op_model_rm9000_ops; > > break; > > + > > + case CPU_R5500: > > + lmodel = &op_model_r5500_ops; > > + break; > > }; > > > > if (!lmodel) > > diff --git a/arch/mips/oprofile/op_model_r5500.c b/arch/mips/oprofile/op_model_r5500.c > > new file mode 100644 > > index 0000000..9b0d20f > > --- /dev/null > > +++ b/arch/mips/oprofile/op_model_r5500.c > > @@ -0,0 +1,161 @@ > > +/* > > + * This file is subject to the terms and conditions of the GNU General Public > > + * License. See the file "COPYING" in the main directory of this archive > > + * for more details. > > + * > > + * Copyright (c) 2009 Wind River Systems, Inc. > > + * > > + * Derived from op_model_mipsxx.c Copyright Ralf Baechle, MIPS Technologies Inc > > + */ > > +#include <linux/oprofile.h> > > +#include <linux/interrupt.h> > > +#include <asm/irq_regs.h> > > + > > +#include "op_impl.h" > > + > > +#define M_PERFCTL_EXL (1UL << 0) > > +#define M_PERFCTL_KERNEL (1UL << 1) > > +#define M_PERFCTL_SUPERVISOR (1UL << 2) > > +#define M_PERFCTL_USER (1UL << 3) > > +#define M_PERFCTL_INTERRUPT_ENABLE (1UL << 4) > > +#define M_PERFCTL_INTERRUPT (1UL << 5) > > +#define M_PERFCTL_EVENT(event) (((event) & 0xf) << 6) > > +#define M_PERFCTL_COUNT_ENABLE (1UL << 10) > > + > > +#define NUM_COUNTERS 2 > > + > > +static int (*save_perf_irq) (void); > > + > > +struct op_mips_model op_model_r5500_ops; > > + > > +static struct r5500_register_config { > > + unsigned int control[NUM_COUNTERS]; > > + unsigned int counter[NUM_COUNTERS]; > > +} reg; > > + > > +/* Compute all of the registers in preparation for enabling profiling. */ > > +static void r5500_reg_setup(struct op_counter_config *ctr) > > +{ > > + int i; > > + unsigned int counters = NUM_COUNTERS; > > + > > + /* Compute the performance counter control word. */ > > + for (i = 0; i < counters; i++) { > > + reg.control[i] = 0; > > + reg.counter[i] = 0; > > + > > + if (!ctr[i].enabled) > > + continue; > > + > > + reg.control[i] = M_PERFCTL_EVENT(ctr[i].event) | > > + M_PERFCTL_INTERRUPT_ENABLE | M_PERFCTL_COUNT_ENABLE; > > + if (ctr[i].kernel) > > + reg.control[i] |= M_PERFCTL_KERNEL; > > + if (ctr[i].user) > > + reg.control[i] |= M_PERFCTL_USER; > > + if (ctr[i].exl) > > + reg.control[i] |= M_PERFCTL_EXL; > > + > > + reg.counter[i] = 0xffffffff - ctr[i].count + 1; > > + } > > +} > > + > > +/* Program all of the registers in preparation for enabling profiling. */ > > +static void r5500_cpu_setup(void *args) > > +{ > > + write_c0_perfctrl1(0); > > + write_c0_perfcntr1(reg.counter[1]); > > + > > + write_c0_perfctrl0(0); > > + write_c0_perfcntr0(reg.counter[0]); > > +} > > + > > +/* Start all counters on current CPU */ > > +static void r5500_cpu_start(void *args) > > +{ > > + write_c0_perfctrl1(reg.control[1]); > > + write_c0_perfctrl0(reg.control[0]); > > +} > > + > > +/* Stop all counters on current CPU */ > > +static void r5500_cpu_stop(void *args) > > +{ > > + write_c0_perfctrl1(0); > > + write_c0_perfctrl0(0); > > +} > > + > > +static int r5500_perfcount_handler(void) > > +{ > > + unsigned int control; > > + unsigned int counter; > > + int handled = IRQ_NONE; > > + > > + control = read_c0_perfctrl0(); > > + counter = read_c0_perfcntr0(); > > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && > > + (control & M_PERFCTL_INTERRUPT)) { > > + oprofile_add_sample(get_irq_regs(), 0); > > + write_c0_perfcntr0(reg.counter[0]); > > + write_c0_perfctrl0(control & ~M_PERFCTL_INTERRUPT); > > + handled = IRQ_HANDLED; > > + } > > + > > + control = read_c0_perfctrl1(); > > + counter = read_c0_perfcntr1(); > > + if ((control & M_PERFCTL_INTERRUPT_ENABLE) && > > + (control & M_PERFCTL_INTERRUPT)) { > > + oprofile_add_sample(get_irq_regs(), 1); > > + write_c0_perfcntr1(reg.counter[1]); > > + write_c0_perfctrl1(control & ~M_PERFCTL_INTERRUPT); > > + handled = IRQ_HANDLED; > > + } > > + > > + return handled; > > +} > > + > > +static void reset_counters(void *arg) > > +{ > > + write_c0_perfctrl1(0); > > + write_c0_perfcntr1(0); > > + > > + write_c0_perfctrl0(0); > > + write_c0_perfcntr0(0); > > +} > > + > > +static int __init r5500_init(void) > > +{ > > + on_each_cpu(reset_counters, NULL, 1); > > + > > + switch (current_cpu_type()) { > > + case CPU_R5500: > > + op_model_r5500_ops.cpu_type = "mips/vr5500"; > > + break; > > + > > + default: > > + printk(KERN_ERR "Profiling unsupported for this CPU\n"); > > + > > + return -ENODEV; > > + } > > + > > + save_perf_irq = perf_irq; > > + perf_irq = r5500_perfcount_handler; > > + > > + return 0; > > +} > > + > > +static void r5500_exit(void) > > +{ > > + on_each_cpu(reset_counters, NULL, 1); > > + > > + perf_irq = save_perf_irq; > > +} > > + > > +struct op_mips_model op_model_r5500_ops = { > > + .reg_setup = r5500_reg_setup, > > + .cpu_setup = r5500_cpu_setup, > > + .init = r5500_init, > > + .exit = r5500_exit, > > + .cpu_start = r5500_cpu_start, > > + .cpu_stop = r5500_cpu_stop, > > + .num_counters = NUM_COUNTERS, > > +}; > > -- > > 1.6.0.3 > > > > > > ------------------------------------------------------------------------------ > > Open Source Business Conference (OSBC), March 24-25, 2009, San Francisco, CA > > -OSBC tackles the biggest issue in open source: Open Sourcing the Enterprise > > -Strategies to boost innovation and cut costs with open source participation > > -Receive a $600 discount off the registration fee with the source code: SFAD > > http://p.sf.net/sfu/XcvMzF8H > > _______________________________________________ > > oprofile-list mailing list > > oprofile-list@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/oprofile-list > > > > -- > Advanced Micro Devices, Inc. > Operating System Research Center > email: robert.richter@amd.com -- Advanced Micro Devices, Inc. Operating System Research Center email: robert.richter@amd.com ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] oprofile: VR5500 performance counter driver 2009-02-23 16:26 [PATCH] oprofile: VR5500 performance counter driver Mark Asselstine 2009-02-25 16:59 ` Robert Richter @ 2009-02-26 17:49 ` Ralf Baechle 1 sibling, 0 replies; 11+ messages in thread From: Ralf Baechle @ 2009-02-26 17:49 UTC (permalink / raw) To: Mark Asselstine; +Cc: linux-mips, oprofile-list On Mon, Feb 23, 2009 at 11:26:34AM -0500, Mark Asselstine wrote: > +static int vr5500_perfcount_handler(void) > +{ > + unsigned int control; > + unsigned int counter; > + int handled = IRQ_NONE; > + unsigned int counters = NUM_COUNTERS; > + > + if (cpu_has_mips_r2 && !(read_c0_cause() & (1 << 26))) > + return handled; The Vr5500 is no R2 processor so these two lines are dead code. Ralf ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2009-06-23 9:32 UTC | newest] Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2009-02-23 16:26 [PATCH] oprofile: VR5500 performance counter driver Mark Asselstine 2009-02-25 16:59 ` Robert Richter 2009-02-26 16:30 ` M. Asselstine 2009-02-26 17:07 ` Robert Richter 2009-02-26 17:51 ` Ralf Baechle 2009-02-26 20:49 ` [PATCH V2] " Mark Asselstine 2009-03-03 11:07 ` Robert Richter 2009-03-04 17:53 ` M. Asselstine 2009-03-04 21:50 ` Robert Richter 2009-06-23 9:29 ` Robert Richter 2009-02-26 17:49 ` [PATCH] " Ralf Baechle
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.