[PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

This adds core support for saving and restoring CPU PMU registers
for suspend/resume support i.e. deeper C-states in cpuidle terms.
This patch adds support only to ARMv7 PMU registers save/restore.
It needs to be extended to xscale and ARMv6 if needed.

I made this patch because DS-5 is not working on Marvell's CA7 based SoCs.
And it has consulted Sudeep KarkadaNagesha's patch set for multiple PMUs.

Thanks Will and Sudeep's suggestion to only save / restore used events.

Cc: Sudeep KarkadaNagesha <sudeep.karkadanagesha@arm.com>
Signed-off-by: Neil Zhang <zhangwm@marvell.com>
---
 arch/arm/include/asm/pmu.h       |    4 +++
 arch/arm/kernel/perf_event.c     |    2 ++
 arch/arm/kernel/perf_event_cpu.c |   28 +++++++++++++++
 arch/arm/kernel/perf_event_v7.c  |   74 ++++++++++++++++++++++++++++++++++++++
 4 files changed, 108 insertions(+)

diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
index ae1919b..3de3db7 100644
--- a/arch/arm/include/asm/pmu.h
+++ b/arch/arm/include/asm/pmu.h
@@ -83,6 +83,10 @@ struct arm_pmu {
 	int		(*request_irq)(struct arm_pmu *, irq_handler_t handler);
 	void		(*free_irq)(struct arm_pmu *);
 	int		(*map_event)(struct perf_event *event);
+	int		(*register_pm_notifier)(struct arm_pmu *);
+	void		(*unregister_pm_notifier)(struct arm_pmu *);
+	void		(*save_regs)(struct arm_pmu *);
+	void		(*restore_regs)(struct arm_pmu *);
 	int		num_events;
 	atomic_t	active_events;
 	struct mutex	reserve_mutex;
diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
index a6bc431..08822de 100644
--- a/arch/arm/kernel/perf_event.c
+++ b/arch/arm/kernel/perf_event.c
@@ -326,6 +326,7 @@ static void
 armpmu_release_hardware(struct arm_pmu *armpmu)
 {
 	armpmu->free_irq(armpmu);
+	armpmu->unregister_pm_notifier(armpmu);
 	pm_runtime_put_sync(&armpmu->plat_device->dev);
 }
 
@@ -339,6 +340,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
 		return -ENODEV;
 
 	pm_runtime_get_sync(&pmu_device->dev);
+	armpmu->register_pm_notifier(armpmu);
 	err = armpmu->request_irq(armpmu, armpmu_dispatch_irq);
 	if (err) {
 		armpmu_release_hardware(armpmu);
diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
index 51798d7..79e1c06 100644
--- a/arch/arm/kernel/perf_event_cpu.c
+++ b/arch/arm/kernel/perf_event_cpu.c
@@ -19,6 +19,7 @@
 #define pr_fmt(fmt) "CPU PMU: " fmt
 
 #include <linux/bitmap.h>
+#include <linux/cpu_pm.h>
 #include <linux/export.h>
 #include <linux/kernel.h>
 #include <linux/of.h>
@@ -173,6 +174,31 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
 	return 0;
 }
 
+static int cpu_pmu_pm_notify(struct notifier_block *b,
+					unsigned long action, void *v)
+{
+	if (action == CPU_PM_ENTER && cpu_pmu->save_regs)
+		cpu_pmu->save_regs(cpu_pmu);
+	else if (action == CPU_PM_EXIT && cpu_pmu->restore_regs)
+		cpu_pmu->restore_regs(cpu_pmu);
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block cpu_pmu_pm_notifier = {
+	.notifier_call = cpu_pmu_pm_notify,
+};
+
+static int cpu_pmu_register_pm_notifier(struct arm_pmu *cpu_pmu)
+{
+	return cpu_pm_register_notifier(&cpu_pmu_pm_notifier);
+}
+
+static void cpu_pmu_unregister_pm_notifier(struct arm_pmu *cpu_pmu)
+{
+	cpu_pm_unregister_notifier(&cpu_pmu_pm_notifier);
+}
+
 static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 {
 	int cpu;
@@ -187,6 +213,8 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
 	cpu_pmu->get_hw_events	= cpu_pmu_get_cpu_events;
 	cpu_pmu->request_irq	= cpu_pmu_request_irq;
 	cpu_pmu->free_irq	= cpu_pmu_free_irq;
+	cpu_pmu->register_pm_notifier	= cpu_pmu_register_pm_notifier;
+	cpu_pmu->unregister_pm_notifier	= cpu_pmu_unregister_pm_notifier;
 
 	/* Ensure the PMU has sane values out of reset. */
 	if (cpu_pmu->reset)
diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
index f4ef398..9069310 100644
--- a/arch/arm/kernel/perf_event_v7.c
+++ b/arch/arm/kernel/perf_event_v7.c
@@ -1237,6 +1237,78 @@ static void armv7_pmnc_dump_regs(struct arm_pmu *cpu_pmu)
 }
 #endif
 
+struct armv7_pmuregs {
+	u32 pmc;
+	u32 pmcntenset;
+	u32 pmintenset;
+	u32 pmxevttype[8];
+	u32 pmxevtcnt[8];
+};
+
+static DEFINE_PER_CPU(struct armv7_pmuregs, pmu_regs);
+
+static void armv7pmu_reset(void *info);
+
+static void armv7pmu_save_regs(struct arm_pmu *cpu_pmu)
+{
+	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct armv7_pmuregs *regs;
+	int bit;
+
+	/* Check whether there are events used */
+	bit = find_first_bit(events->used_mask, cpu_pmu->num_events);
+	if (bit >= cpu_pmu->num_events)
+		return;
+
+	regs = this_cpu_ptr(&pmu_regs);
+	for_each_set_bit(bit, events->used_mask, cpu_pmu->num_events) {
+		if (bit) {
+			armv7_pmnc_select_counter(bit);
+			asm volatile("mrc p15, 0, %0, c9, c13, 1"
+					: "=r"(regs->pmxevttype[bit]));
+			asm volatile("mrc p15, 0, %0, c9, c13, 2"
+					: "=r"(regs->pmxevtcnt[bit]));
+		} else
+			asm volatile("mrc p15, 0, %0, c9, c13, 0"
+					: "=r" (regs->pmxevtcnt[0]));
+	}
+
+	asm volatile("mrc p15, 0, %0, c9, c12, 1" : "=r" (regs->pmcntenset));
+	asm volatile("mrc p15, 0, %0, c9, c14, 1" : "=r" (regs->pmintenset));
+	asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (regs->pmc));
+}
+
+static void armv7pmu_restore_regs(struct arm_pmu *cpu_pmu)
+{
+	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
+	struct armv7_pmuregs *regs;
+	int bit;
+
+	/* Check whether there are events used */
+	bit = find_first_bit(events->used_mask, cpu_pmu->num_events);
+	if (bit >= cpu_pmu->num_events)
+		return;
+
+	armv7pmu_reset(cpu_pmu);
+
+	regs = this_cpu_ptr(&pmu_regs);
+	for_each_set_bit(bit, events->used_mask, cpu_pmu->num_events) {
+		if (bit) {
+			armv7_pmnc_select_counter(bit);
+			asm volatile("mcr p15, 0, %0, c9, c13, 1"
+					: : "r"(regs->pmxevttype[bit]));
+			asm volatile("mcr p15, 0, %0, c9, c13, 2"
+					: : "r"(regs->pmxevtcnt[bit]));
+		} else
+			asm volatile("mcr p15, 0, %0, c9, c13, 0"
+					: : "r" (regs->pmxevtcnt[0]));
+	}
+
+	asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (regs->pmcntenset));
+	asm volatile("mcr p15, 0, %0, c9, c14, 1" : : "r" (regs->pmintenset));
+	asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (regs->pmc));
+}
+
 static void armv7pmu_enable_event(struct perf_event *event)
 {
 	unsigned long flags;
@@ -1528,6 +1600,8 @@ static void armv7pmu_init(struct arm_pmu *cpu_pmu)
 	cpu_pmu->start		= armv7pmu_start;
 	cpu_pmu->stop		= armv7pmu_stop;
 	cpu_pmu->reset		= armv7pmu_reset;
+	cpu_pmu->save_regs	= armv7pmu_save_regs;
+	cpu_pmu->restore_regs	= armv7pmu_restore_regs;
 	cpu_pmu->max_period	= (1LLU << 32) - 1;
 };
 
-- 
1.7.9.5

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Will Deacon]

Hi Neil,

On Tue, Apr 22, 2014 at 03:26:36AM +0100, Neil Zhang wrote:
> This adds core support for saving and restoring CPU PMU registers
> for suspend/resume support i.e. deeper C-states in cpuidle terms.
> This patch adds support only to ARMv7 PMU registers save/restore.
> It needs to be extended to xscale and ARMv6 if needed.
> 
> I made this patch because DS-5 is not working on Marvell's CA7 based SoCs.
> And it has consulted Sudeep KarkadaNagesha's patch set for multiple PMUs.
> 
> Thanks Will and Sudeep's suggestion to only save / restore used events.

Whilst this is a step in the right direction, I'd still like to see the
save/restore predicated on something in the device-tree or otherwise. Most
SoCs *don't* require these registers to be preserved by software, so we need
a way to describe that the PMU is in a power-domain where its state is lost
when the CPU goes idle.

This doesn't sound like a PMU-specific problem, so there's a possibility
that this has been discussed elsewhere, in the context of other IP blocks

[adding the devicetree list in case somebody there is aware of any work in
 this area]

Will

> diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
> index ae1919b..3de3db7 100644
> --- a/arch/arm/include/asm/pmu.h
> +++ b/arch/arm/include/asm/pmu.h
> @@ -83,6 +83,10 @@ struct arm_pmu {
>  	int		(*request_irq)(struct arm_pmu *, irq_handler_t handler);
>  	void		(*free_irq)(struct arm_pmu *);
>  	int		(*map_event)(struct perf_event *event);
> +	int		(*register_pm_notifier)(struct arm_pmu *);
> +	void		(*unregister_pm_notifier)(struct arm_pmu *);
> +	void		(*save_regs)(struct arm_pmu *);
> +	void		(*restore_regs)(struct arm_pmu *);
>  	int		num_events;
>  	atomic_t	active_events;
>  	struct mutex	reserve_mutex;
> diff --git a/arch/arm/kernel/perf_event.c b/arch/arm/kernel/perf_event.c
> index a6bc431..08822de 100644
> --- a/arch/arm/kernel/perf_event.c
> +++ b/arch/arm/kernel/perf_event.c
> @@ -326,6 +326,7 @@ static void
>  armpmu_release_hardware(struct arm_pmu *armpmu)
>  {
>  	armpmu->free_irq(armpmu);
> +	armpmu->unregister_pm_notifier(armpmu);
>  	pm_runtime_put_sync(&armpmu->plat_device->dev);
>  }
>  
> @@ -339,6 +340,7 @@ armpmu_reserve_hardware(struct arm_pmu *armpmu)
>  		return -ENODEV;
>  
>  	pm_runtime_get_sync(&pmu_device->dev);
> +	armpmu->register_pm_notifier(armpmu);
>  	err = armpmu->request_irq(armpmu, armpmu_dispatch_irq);
>  	if (err) {
>  		armpmu_release_hardware(armpmu);
> diff --git a/arch/arm/kernel/perf_event_cpu.c b/arch/arm/kernel/perf_event_cpu.c
> index 51798d7..79e1c06 100644
> --- a/arch/arm/kernel/perf_event_cpu.c
> +++ b/arch/arm/kernel/perf_event_cpu.c
> @@ -19,6 +19,7 @@
>  #define pr_fmt(fmt) "CPU PMU: " fmt
>  
>  #include <linux/bitmap.h>
> +#include <linux/cpu_pm.h>
>  #include <linux/export.h>
>  #include <linux/kernel.h>
>  #include <linux/of.h>
> @@ -173,6 +174,31 @@ static int cpu_pmu_request_irq(struct arm_pmu *cpu_pmu, irq_handler_t handler)
>  	return 0;
>  }
>  
> +static int cpu_pmu_pm_notify(struct notifier_block *b,
> +					unsigned long action, void *v)
> +{
> +	if (action == CPU_PM_ENTER && cpu_pmu->save_regs)
> +		cpu_pmu->save_regs(cpu_pmu);
> +	else if (action == CPU_PM_EXIT && cpu_pmu->restore_regs)
> +		cpu_pmu->restore_regs(cpu_pmu);
> +
> +	return NOTIFY_OK;
> +}
> +
> +static struct notifier_block cpu_pmu_pm_notifier = {
> +	.notifier_call = cpu_pmu_pm_notify,
> +};
> +
> +static int cpu_pmu_register_pm_notifier(struct arm_pmu *cpu_pmu)
> +{
> +	return cpu_pm_register_notifier(&cpu_pmu_pm_notifier);
> +}
> +
> +static void cpu_pmu_unregister_pm_notifier(struct arm_pmu *cpu_pmu)
> +{
> +	cpu_pm_unregister_notifier(&cpu_pmu_pm_notifier);
> +}
> +
>  static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
>  {
>  	int cpu;
> @@ -187,6 +213,8 @@ static void cpu_pmu_init(struct arm_pmu *cpu_pmu)
>  	cpu_pmu->get_hw_events	= cpu_pmu_get_cpu_events;
>  	cpu_pmu->request_irq	= cpu_pmu_request_irq;
>  	cpu_pmu->free_irq	= cpu_pmu_free_irq;
> +	cpu_pmu->register_pm_notifier	= cpu_pmu_register_pm_notifier;
> +	cpu_pmu->unregister_pm_notifier	= cpu_pmu_unregister_pm_notifier;
>  
>  	/* Ensure the PMU has sane values out of reset. */
>  	if (cpu_pmu->reset)
> diff --git a/arch/arm/kernel/perf_event_v7.c b/arch/arm/kernel/perf_event_v7.c
> index f4ef398..9069310 100644
> --- a/arch/arm/kernel/perf_event_v7.c
> +++ b/arch/arm/kernel/perf_event_v7.c
> @@ -1237,6 +1237,78 @@ static void armv7_pmnc_dump_regs(struct arm_pmu *cpu_pmu)
>  }
>  #endif
>  
> +struct armv7_pmuregs {
> +	u32 pmc;
> +	u32 pmcntenset;
> +	u32 pmintenset;
> +	u32 pmxevttype[8];
> +	u32 pmxevtcnt[8];
> +};
> +
> +static DEFINE_PER_CPU(struct armv7_pmuregs, pmu_regs);
> +
> +static void armv7pmu_reset(void *info);
> +
> +static void armv7pmu_save_regs(struct arm_pmu *cpu_pmu)
> +{
> +	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
> +	struct armv7_pmuregs *regs;
> +	int bit;
> +
> +	/* Check whether there are events used */
> +	bit = find_first_bit(events->used_mask, cpu_pmu->num_events);
> +	if (bit >= cpu_pmu->num_events)
> +		return;
> +
> +	regs = this_cpu_ptr(&pmu_regs);
> +	for_each_set_bit(bit, events->used_mask, cpu_pmu->num_events) {
> +		if (bit) {
> +			armv7_pmnc_select_counter(bit);
> +			asm volatile("mrc p15, 0, %0, c9, c13, 1"
> +					: "=r"(regs->pmxevttype[bit]));
> +			asm volatile("mrc p15, 0, %0, c9, c13, 2"
> +					: "=r"(regs->pmxevtcnt[bit]));
> +		} else
> +			asm volatile("mrc p15, 0, %0, c9, c13, 0"
> +					: "=r" (regs->pmxevtcnt[0]));
> +	}
> +
> +	asm volatile("mrc p15, 0, %0, c9, c12, 1" : "=r" (regs->pmcntenset));
> +	asm volatile("mrc p15, 0, %0, c9, c14, 1" : "=r" (regs->pmintenset));
> +	asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (regs->pmc));
> +}
> +
> +static void armv7pmu_restore_regs(struct arm_pmu *cpu_pmu)
> +{
> +	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
> +	struct armv7_pmuregs *regs;
> +	int bit;
> +
> +	/* Check whether there are events used */
> +	bit = find_first_bit(events->used_mask, cpu_pmu->num_events);
> +	if (bit >= cpu_pmu->num_events)
> +		return;
> +
> +	armv7pmu_reset(cpu_pmu);
> +
> +	regs = this_cpu_ptr(&pmu_regs);
> +	for_each_set_bit(bit, events->used_mask, cpu_pmu->num_events) {
> +		if (bit) {
> +			armv7_pmnc_select_counter(bit);
> +			asm volatile("mcr p15, 0, %0, c9, c13, 1"
> +					: : "r"(regs->pmxevttype[bit]));
> +			asm volatile("mcr p15, 0, %0, c9, c13, 2"
> +					: : "r"(regs->pmxevtcnt[bit]));
> +		} else
> +			asm volatile("mcr p15, 0, %0, c9, c13, 0"
> +					: : "r" (regs->pmxevtcnt[0]));
> +	}
> +
> +	asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (regs->pmcntenset));
> +	asm volatile("mcr p15, 0, %0, c9, c14, 1" : : "r" (regs->pmintenset));
> +	asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (regs->pmc));
> +}
> +
>  static void armv7pmu_enable_event(struct perf_event *event)
>  {
>  	unsigned long flags;
> @@ -1528,6 +1600,8 @@ static void armv7pmu_init(struct arm_pmu *cpu_pmu)
>  	cpu_pmu->start		= armv7pmu_start;
>  	cpu_pmu->stop		= armv7pmu_stop;
>  	cpu_pmu->reset		= armv7pmu_reset;
> +	cpu_pmu->save_regs	= armv7pmu_save_regs;
> +	cpu_pmu->restore_regs	= armv7pmu_restore_regs;
>  	cpu_pmu->max_period	= (1LLU << 32) - 1;
>  };
>  
> -- 
> 1.7.9.5
> 
> 

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 8305 bytes --]


> -----Original Message-----
> From: Will Deacon [mailto:will.deacon@arm.com]
> Sent: 2014Äê4ÔÂ22ÈÕ 18:37
> To: Neil Zhang
> Cc: linux@arm.linux.org.uk; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; Sudeep Holla; devicetree@vger.kernel.org
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> Hi Neil,
> 
> On Tue, Apr 22, 2014 at 03:26:36AM +0100, Neil Zhang wrote:
> > This adds core support for saving and restoring CPU PMU registers for
> > suspend/resume support i.e. deeper C-states in cpuidle terms.
> > This patch adds support only to ARMv7 PMU registers save/restore.
> > It needs to be extended to xscale and ARMv6 if needed.
> >
> > I made this patch because DS-5 is not working on Marvell's CA7 based SoCs.
> > And it has consulted Sudeep KarkadaNagesha's patch set for multiple PMUs.
> >
> > Thanks Will and Sudeep's suggestion to only save / restore used events.
> 
> Whilst this is a step in the right direction, I'd still like to see the save/restore
> predicated on something in the device-tree or otherwise. Most SoCs *don't*
> require these registers to be preserved by software, so we need a way to
> describe that the PMU is in a power-domain where its state is lost when the
> CPU goes idle.
> 
> This doesn't sound like a PMU-specific problem, so there's a possibility that
> this has been discussed elsewhere, in the context of other IP blocks
> 
> [adding the devicetree list in case somebody there is aware of any work in
> this area]
> 

Thanks Will.
What should I do now?
Add a filed under PMU or waiting for somebody whether there are general supporting for power domain maintain.

> Will
> 
> > diff --git a/arch/arm/include/asm/pmu.h b/arch/arm/include/asm/pmu.h
> > index ae1919b..3de3db7 100644
> > --- a/arch/arm/include/asm/pmu.h
> > +++ b/arch/arm/include/asm/pmu.h
> > @@ -83,6 +83,10 @@ struct arm_pmu {
> >  	int		(*request_irq)(struct arm_pmu *, irq_handler_t handler);
> >  	void		(*free_irq)(struct arm_pmu *);
> >  	int		(*map_event)(struct perf_event *event);
> > +	int		(*register_pm_notifier)(struct arm_pmu *);
> > +	void		(*unregister_pm_notifier)(struct arm_pmu *);
> > +	void		(*save_regs)(struct arm_pmu *);
> > +	void		(*restore_regs)(struct arm_pmu *);
> >  	int		num_events;
> >  	atomic_t	active_events;
> >  	struct mutex	reserve_mutex;
> > diff --git a/arch/arm/kernel/perf_event.c
> > b/arch/arm/kernel/perf_event.c index a6bc431..08822de 100644
> > --- a/arch/arm/kernel/perf_event.c
> > +++ b/arch/arm/kernel/perf_event.c
> > @@ -326,6 +326,7 @@ static void
> >  armpmu_release_hardware(struct arm_pmu *armpmu)  {
> >  	armpmu->free_irq(armpmu);
> > +	armpmu->unregister_pm_notifier(armpmu);
> >  	pm_runtime_put_sync(&armpmu->plat_device->dev);
> >  }
> >
> > @@ -339,6 +340,7 @@ armpmu_reserve_hardware(struct arm_pmu
> *armpmu)
> >  		return -ENODEV;
> >
> >  	pm_runtime_get_sync(&pmu_device->dev);
> > +	armpmu->register_pm_notifier(armpmu);
> >  	err = armpmu->request_irq(armpmu, armpmu_dispatch_irq);
> >  	if (err) {
> >  		armpmu_release_hardware(armpmu);
> > diff --git a/arch/arm/kernel/perf_event_cpu.c
> > b/arch/arm/kernel/perf_event_cpu.c
> > index 51798d7..79e1c06 100644
> > --- a/arch/arm/kernel/perf_event_cpu.c
> > +++ b/arch/arm/kernel/perf_event_cpu.c
> > @@ -19,6 +19,7 @@
> >  #define pr_fmt(fmt) "CPU PMU: " fmt
> >
> >  #include <linux/bitmap.h>
> > +#include <linux/cpu_pm.h>
> >  #include <linux/export.h>
> >  #include <linux/kernel.h>
> >  #include <linux/of.h>
> > @@ -173,6 +174,31 @@ static int cpu_pmu_request_irq(struct arm_pmu
> *cpu_pmu, irq_handler_t handler)
> >  	return 0;
> >  }
> >
> > +static int cpu_pmu_pm_notify(struct notifier_block *b,
> > +					unsigned long action, void *v)
> > +{
> > +	if (action == CPU_PM_ENTER && cpu_pmu->save_regs)
> > +		cpu_pmu->save_regs(cpu_pmu);
> > +	else if (action == CPU_PM_EXIT && cpu_pmu->restore_regs)
> > +		cpu_pmu->restore_regs(cpu_pmu);
> > +
> > +	return NOTIFY_OK;
> > +}
> > +
> > +static struct notifier_block cpu_pmu_pm_notifier = {
> > +	.notifier_call = cpu_pmu_pm_notify,
> > +};
> > +
> > +static int cpu_pmu_register_pm_notifier(struct arm_pmu *cpu_pmu) {
> > +	return cpu_pm_register_notifier(&cpu_pmu_pm_notifier);
> > +}
> > +
> > +static void cpu_pmu_unregister_pm_notifier(struct arm_pmu *cpu_pmu) {
> > +	cpu_pm_unregister_notifier(&cpu_pmu_pm_notifier);
> > +}
> > +
> >  static void cpu_pmu_init(struct arm_pmu *cpu_pmu)  {
> >  	int cpu;
> > @@ -187,6 +213,8 @@ static void cpu_pmu_init(struct arm_pmu
> *cpu_pmu)
> >  	cpu_pmu->get_hw_events	= cpu_pmu_get_cpu_events;
> >  	cpu_pmu->request_irq	= cpu_pmu_request_irq;
> >  	cpu_pmu->free_irq	= cpu_pmu_free_irq;
> > +	cpu_pmu->register_pm_notifier	= cpu_pmu_register_pm_notifier;
> > +	cpu_pmu->unregister_pm_notifier	=
> cpu_pmu_unregister_pm_notifier;
> >
> >  	/* Ensure the PMU has sane values out of reset. */
> >  	if (cpu_pmu->reset)
> > diff --git a/arch/arm/kernel/perf_event_v7.c
> > b/arch/arm/kernel/perf_event_v7.c index f4ef398..9069310 100644
> > --- a/arch/arm/kernel/perf_event_v7.c
> > +++ b/arch/arm/kernel/perf_event_v7.c
> > @@ -1237,6 +1237,78 @@ static void armv7_pmnc_dump_regs(struct
> arm_pmu
> > *cpu_pmu)  }  #endif
> >
> > +struct armv7_pmuregs {
> > +	u32 pmc;
> > +	u32 pmcntenset;
> > +	u32 pmintenset;
> > +	u32 pmxevttype[8];
> > +	u32 pmxevtcnt[8];
> > +};
> > +
> > +static DEFINE_PER_CPU(struct armv7_pmuregs, pmu_regs);
> > +
> > +static void armv7pmu_reset(void *info);
> > +
> > +static void armv7pmu_save_regs(struct arm_pmu *cpu_pmu) {
> > +	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
> > +	struct armv7_pmuregs *regs;
> > +	int bit;
> > +
> > +	/* Check whether there are events used */
> > +	bit = find_first_bit(events->used_mask, cpu_pmu->num_events);
> > +	if (bit >= cpu_pmu->num_events)
> > +		return;
> > +
> > +	regs = this_cpu_ptr(&pmu_regs);
> > +	for_each_set_bit(bit, events->used_mask, cpu_pmu->num_events) {
> > +		if (bit) {
> > +			armv7_pmnc_select_counter(bit);
> > +			asm volatile("mrc p15, 0, %0, c9, c13, 1"
> > +					: "=r"(regs->pmxevttype[bit]));
> > +			asm volatile("mrc p15, 0, %0, c9, c13, 2"
> > +					: "=r"(regs->pmxevtcnt[bit]));
> > +		} else
> > +			asm volatile("mrc p15, 0, %0, c9, c13, 0"
> > +					: "=r" (regs->pmxevtcnt[0]));
> > +	}
> > +
> > +	asm volatile("mrc p15, 0, %0, c9, c12, 1" : "=r" (regs->pmcntenset));
> > +	asm volatile("mrc p15, 0, %0, c9, c14, 1" : "=r" (regs->pmintenset));
> > +	asm volatile("mrc p15, 0, %0, c9, c12, 0" : "=r" (regs->pmc)); }
> > +
> > +static void armv7pmu_restore_regs(struct arm_pmu *cpu_pmu) {
> > +	struct pmu_hw_events *events = cpu_pmu->get_hw_events();
> > +	struct armv7_pmuregs *regs;
> > +	int bit;
> > +
> > +	/* Check whether there are events used */
> > +	bit = find_first_bit(events->used_mask, cpu_pmu->num_events);
> > +	if (bit >= cpu_pmu->num_events)
> > +		return;
> > +
> > +	armv7pmu_reset(cpu_pmu);
> > +
> > +	regs = this_cpu_ptr(&pmu_regs);
> > +	for_each_set_bit(bit, events->used_mask, cpu_pmu->num_events) {
> > +		if (bit) {
> > +			armv7_pmnc_select_counter(bit);
> > +			asm volatile("mcr p15, 0, %0, c9, c13, 1"
> > +					: : "r"(regs->pmxevttype[bit]));
> > +			asm volatile("mcr p15, 0, %0, c9, c13, 2"
> > +					: : "r"(regs->pmxevtcnt[bit]));
> > +		} else
> > +			asm volatile("mcr p15, 0, %0, c9, c13, 0"
> > +					: : "r" (regs->pmxevtcnt[0]));
> > +	}
> > +
> > +	asm volatile("mcr p15, 0, %0, c9, c12, 1" : : "r" (regs->pmcntenset));
> > +	asm volatile("mcr p15, 0, %0, c9, c14, 1" : : "r" (regs->pmintenset));
> > +	asm volatile("mcr p15, 0, %0, c9, c12, 0" : : "r" (regs->pmc)); }
> > +
> >  static void armv7pmu_enable_event(struct perf_event *event)  {
> >  	unsigned long flags;
> > @@ -1528,6 +1600,8 @@ static void armv7pmu_init(struct arm_pmu
> *cpu_pmu)
> >  	cpu_pmu->start		= armv7pmu_start;
> >  	cpu_pmu->stop		= armv7pmu_stop;
> >  	cpu_pmu->reset		= armv7pmu_reset;
> > +	cpu_pmu->save_regs	= armv7pmu_save_regs;
> > +	cpu_pmu->restore_regs	= armv7pmu_restore_regs;
> >  	cpu_pmu->max_period	= (1LLU << 32) - 1;
> >  };
> >
> > --
> > 1.7.9.5
> >
> >

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Will Deacon]

On Wed, Apr 23, 2014 at 11:31:09AM +0100, Neil Zhang wrote:
> 
> > -----Original Message-----
> > From: Will Deacon [mailto:will.deacon@arm.com]
> > Sent: 2014年4月22日 18:37
> > To: Neil Zhang
> > Cc: linux@arm.linux.org.uk; linux-arm-kernel@lists.infradead.org;
> > linux-kernel@vger.kernel.org; Sudeep Holla; devicetree@vger.kernel.org
> > Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> > 
> > Hi Neil,
> > 
> > On Tue, Apr 22, 2014 at 03:26:36AM +0100, Neil Zhang wrote:
> > > This adds core support for saving and restoring CPU PMU registers for
> > > suspend/resume support i.e. deeper C-states in cpuidle terms.
> > > This patch adds support only to ARMv7 PMU registers save/restore.
> > > It needs to be extended to xscale and ARMv6 if needed.
> > >
> > > I made this patch because DS-5 is not working on Marvell's CA7 based SoCs.
> > > And it has consulted Sudeep KarkadaNagesha's patch set for multiple PMUs.
> > >
> > > Thanks Will and Sudeep's suggestion to only save / restore used events.
> > 
> > Whilst this is a step in the right direction, I'd still like to see the save/restore
> > predicated on something in the device-tree or otherwise. Most SoCs *don't*
> > require these registers to be preserved by software, so we need a way to
> > describe that the PMU is in a power-domain where its state is lost when the
> > CPU goes idle.
> > 
> > This doesn't sound like a PMU-specific problem, so there's a possibility that
> > this has been discussed elsewhere, in the context of other IP blocks
> > 
> > [adding the devicetree list in case somebody there is aware of any work in
> > this area]
> > 
> 
> Thanks Will.
> What should I do now?
> Add a filed under PMU or waiting for somebody whether there are general
> supporting for power domain maintain.

I think we need some input from the device-tree guys to see whether they
would object to us solving this locally (in the PMU node) or not.
Personally, I'd much prefer a general way to describe the need for
pm-notifiers, but if that's not being looked at then we can cook something
specifically for our needs.

Will

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2804 bytes --]


> -----Original Message-----
> From: Will Deacon [mailto:will.deacon@arm.com]
> Sent: 2014年4月24日 1:08
> To: Neil Zhang
> Cc: linux@arm.linux.org.uk; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; Sudeep Holla; devicetree@vger.kernel.org
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> On Wed, Apr 23, 2014 at 11:31:09AM +0100, Neil Zhang wrote:
> >
> > > -----Original Message-----
> > > From: Will Deacon [mailto:will.deacon@arm.com]
> > > Sent: 2014年4月22日 18:37
> > > To: Neil Zhang
> > > Cc: linux@arm.linux.org.uk; linux-arm-kernel@lists.infradead.org;
> > > linux-kernel@vger.kernel.org; Sudeep Holla;
> > > devicetree@vger.kernel.org
> > > Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> > > notifier
> > >
> > > Hi Neil,
> > >
> > > On Tue, Apr 22, 2014 at 03:26:36AM +0100, Neil Zhang wrote:
> > > > This adds core support for saving and restoring CPU PMU registers
> > > > for suspend/resume support i.e. deeper C-states in cpuidle terms.
> > > > This patch adds support only to ARMv7 PMU registers save/restore.
> > > > It needs to be extended to xscale and ARMv6 if needed.
> > > >
> > > > I made this patch because DS-5 is not working on Marvell's CA7 based
> SoCs.
> > > > And it has consulted Sudeep KarkadaNagesha's patch set for multiple
> PMUs.
> > > >
> > > > Thanks Will and Sudeep's suggestion to only save / restore used events.
> > >
> > > Whilst this is a step in the right direction, I'd still like to see
> > > the save/restore predicated on something in the device-tree or
> > > otherwise. Most SoCs *don't* require these registers to be preserved
> > > by software, so we need a way to describe that the PMU is in a
> > > power-domain where its state is lost when the CPU goes idle.
> > >
> > > This doesn't sound like a PMU-specific problem, so there's a
> > > possibility that this has been discussed elsewhere, in the context
> > > of other IP blocks
> > >
> > > [adding the devicetree list in case somebody there is aware of any
> > > work in this area]
> > >
> >
> > Thanks Will.
> > What should I do now?
> > Add a filed under PMU or waiting for somebody whether there are
> > general supporting for power domain maintain.
> 
> I think we need some input from the device-tree guys to see whether they
> would object to us solving this locally (in the PMU node) or not.
> Personally, I'd much prefer a general way to describe the need for pm-notifiers,
> but if that's not being looked at then we can cook something specifically for
> our needs.
> 

No input from device-tree guys :(

> Will

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Sudeep Holla]

Hi Neil,

On 30/04/14 03:21, Neil Zhang wrote:
> 
>> -----Original Message-----
>> From: Will Deacon [mailto:will.deacon@arm.com]

[...]

>>
>> I think we need some input from the device-tree guys to see whether they
>> would object to us solving this locally (in the PMU node) or not.
>> Personally, I'd much prefer a general way to describe the need for pm-notifiers,
>> but if that's not being looked at then we can cook something specifically for
>> our needs.
>>
> 
> No input from device-tree guys :(

The device tree bindings for power domains is under discussion [1]

Regards,
Sudeep

[1] https://lkml.org/lkml/2014/4/23/755

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1436 bytes --]

Sudeep,

> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: 2014年4月30日 21:31
> To: Neil Zhang; Will Deacon
> Cc: Sudeep Holla; linux@arm.linux.org.uk;
> linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> devicetree@vger.kernel.org
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> Hi Neil,
> 
> On 30/04/14 03:21, Neil Zhang wrote:
> >
> >> -----Original Message-----
> >> From: Will Deacon [mailto:will.deacon@arm.com]
> 
> [...]
> 
> >>
> >> I think we need some input from the device-tree guys to see whether
> >> they would object to us solving this locally (in the PMU node) or not.
> >> Personally, I'd much prefer a general way to describe the need for
> >> pm-notifiers, but if that's not being looked at then we can cook
> >> something specifically for our needs.
> >>
> >
> > No input from device-tree guys :(
> 
> The device tree bindings for power domains is under discussion [1]

Thanks for the information.
But it currently for device only, core related stuff are not supported.
And is it really good to register power provider for core and let vfp / pmu etc to get it?

> 
> Regards,
> Sudeep
> 
> [1] https://lkml.org/lkml/2014/4/23/755


Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 1964 bytes --]

Will & Sudeep,

> -----Original Message-----
> From: Neil Zhang
> Sent: 2014年5月5日 14:28
> To: Sudeep Holla; Will Deacon
> Cc: linux@arm.linux.org.uk; linux-arm-kernel@lists.infradead.org;
> linux-kernel@vger.kernel.org; devicetree@vger.kernel.org
> Subject: RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> Sudeep,
> 
> > -----Original Message-----
> > From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> > Sent: 2014年4月30日 21:31
> > To: Neil Zhang; Will Deacon
> > Cc: Sudeep Holla; linux@arm.linux.org.uk;
> > linux-arm-kernel@lists.infradead.org; linux-kernel@vger.kernel.org;
> > devicetree@vger.kernel.org
> > Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> > notifier
> >
> > Hi Neil,
> >
> > On 30/04/14 03:21, Neil Zhang wrote:
> > >
> > >> -----Original Message-----
> > >> From: Will Deacon [mailto:will.deacon@arm.com]
> >
> > [...]
> >
> > >>
> > >> I think we need some input from the device-tree guys to see whether
> > >> they would object to us solving this locally (in the PMU node) or not.
> > >> Personally, I'd much prefer a general way to describe the need for
> > >> pm-notifiers, but if that's not being looked at then we can cook
> > >> something specifically for our needs.
> > >>
> > >
> > > No input from device-tree guys :(
> >
> > The device tree bindings for power domains is under discussion [1]
> 
> Thanks for the information.
> But it currently for device only, core related stuff are not supported.
> And is it really good to register power provider for core and let vfp / pmu etc
> to get it?
> 

What's your suggestion about it?
Is it OK that I add it under the PMU node?

> >
> > Regards,
> > Sudeep
> >
> > [1] https://lkml.org/lkml/2014/4/23/755
> 
> 
> Best Regards,
> Neil Zhang

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Will Deacon]

On Mon, May 12, 2014 at 11:22:09AM +0100, Neil Zhang wrote:
> > > The device tree bindings for power domains is under discussion [1]
> > 
> > Thanks for the information.
> > But it currently for device only, core related stuff are not supported.
> > And is it really good to register power provider for core and let vfp / pmu etc
> > to get it?
> > 
> 
> What's your suggestion about it?
> Is it OK that I add it under the PMU node?

I don't really mind. I just want to avoid re-inventing the wheel in a
PMU-specific way and having to maintain that code forever because it ended
up in our DT description.

Will

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1428 bytes --]

> -----Original Message-----
> From: Will Deacon [mailto:will.deacon@arm.com]
> Sent: 2014Äê5ÔÂ14ÈÕ 2:45
> To: Neil Zhang
> Cc: Sudeep Holla; 'linux@arm.linux.org.uk';
> 'linux-arm-kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org';
> 'devicetree@vger.kernel.org'
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> On Mon, May 12, 2014 at 11:22:09AM +0100, Neil Zhang wrote:
> > > > The device tree bindings for power domains is under discussion [1]
> > >
> > > Thanks for the information.
> > > But it currently for device only, core related stuff are not supported.
> > > And is it really good to register power provider for core and let
> > > vfp / pmu etc to get it?
> > >
> >
> > What's your suggestion about it?
> > Is it OK that I add it under the PMU node?
> 
> I don't really mind. I just want to avoid re-inventing the wheel in a
> PMU-specific way and having to maintain that code forever because it ended
> up in our DT description.
> 
> Will

I will prepare another patch to add DT description under PMU since there is 
no generic power domain support for pm notifier if no other concerns.
We can change the manner if there is generic power domain support for pm notifier later.
Thanks.

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Sudeep Holla]

On 14/05/14 03:28, Neil Zhang wrote:
>> -----Original Message-----
>> From: Will Deacon [mailto:will.deacon@arm.com]
>> Sent: 2014年5月14日 2:45
>> To: Neil Zhang
>> Cc: Sudeep Holla; 'linux@arm.linux.org.uk';
>> 'linux-arm-kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org';
>> 'devicetree@vger.kernel.org'
>> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
>>
>> On Mon, May 12, 2014 at 11:22:09AM +0100, Neil Zhang wrote:
>>>>> The device tree bindings for power domains is under discussion [1]
>>>>
>>>> Thanks for the information.
>>>> But it currently for device only, core related stuff are not supported.
>>>> And is it really good to register power provider for core and let
>>>> vfp / pmu etc to get it?
>>>>
>>>
>>> What's your suggestion about it?
>>> Is it OK that I add it under the PMU node?
>>
>> I don't really mind. I just want to avoid re-inventing the wheel in a
>> PMU-specific way and having to maintain that code forever because it ended
>> up in our DT description.
>>
>> Will
>
> I will prepare another patch to add DT description under PMU since there is
> no generic power domain support for pm notifier if no other concerns.
> We can change the manner if there is generic power domain support for pm notifier later.
> Thanks.

No, please don't add any DT bindings for power domains specific to PMU node.
We can't change the DT bindings once added.

As I pointed out the DT bindings for generic power domains are under discussion.
See if you can reuse it, if not help in extending it so that it can be used.

Regards,
Sudeep

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 2623 bytes --]

Sudeep，

> -----Original Message-----
> From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> Sent: 2014年5月14日 17:32
> To: Neil Zhang; Will Deacon
> Cc: Sudeep Holla; 'linux@arm.linux.org.uk';
> 'linux-arm-kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org';
> 'devicetree@vger.kernel.org'
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> 
> 
> On 14/05/14 03:28, Neil Zhang wrote:
> >> -----Original Message-----
> >> From: Will Deacon [mailto:will.deacon@arm.com]
> >> Sent: 2014年5月14日 2:45
> >> To: Neil Zhang
> >> Cc: Sudeep Holla; 'linux@arm.linux.org.uk';
> >> 'linux-arm-kernel@lists.infradead.org';
> >> 'linux-kernel@vger.kernel.org'; 'devicetree@vger.kernel.org'
> >> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> >> notifier
> >>
> >> On Mon, May 12, 2014 at 11:22:09AM +0100, Neil Zhang wrote:
> >>>>> The device tree bindings for power domains is under discussion [1]
> >>>>
> >>>> Thanks for the information.
> >>>> But it currently for device only, core related stuff are not supported.
> >>>> And is it really good to register power provider for core and let
> >>>> vfp / pmu etc to get it?
> >>>>
> >>>
> >>> What's your suggestion about it?
> >>> Is it OK that I add it under the PMU node?
> >>
> >> I don't really mind. I just want to avoid re-inventing the wheel in a
> >> PMU-specific way and having to maintain that code forever because it
> >> ended up in our DT description.
> >>
> >> Will
> >
> > I will prepare another patch to add DT description under PMU since
> > there is no generic power domain support for pm notifier if no other
> concerns.
> > We can change the manner if there is generic power domain support for
> pm notifier later.
> > Thanks.
> 
> No, please don't add any DT bindings for power domains specific to PMU
> node.
> We can't change the DT bindings once added.
> 
> As I pointed out the DT bindings for generic power domains are under
> discussion.
> See if you can reuse it, if not help in extending it so that it can be used.
> 

Sorry for reply later.
As I said before the under discussed generic power domain is not suitable for
CPU peripherals since they are all known belong to CPU or cluster power domain.
If we want to follow the way they are discussion, we need to register core 
and cluster power provider, and need vfp/gic/pmu etc to require them.
Is it really suitable?

> Regards,
> Sudeep
> 
> 

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="utf-8", Size: 3227 bytes --]

Sudeep & Will，

> -----Original Message-----
> From: Neil Zhang
> Sent: 2014年5月21日 19:47
> To: 'Sudeep Holla'; Will Deacon
> Cc: 'linux@arm.linux.org.uk'; 'linux-arm-kernel@lists.infradead.org';
> 'linux-kernel@vger.kernel.org'; 'devicetree@vger.kernel.org'
> Subject: RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier
> 
> Sudeep，
> 
> > -----Original Message-----
> > From: Sudeep Holla [mailto:sudeep.holla@arm.com]
> > Sent: 2014年5月14日 17:32
> > To: Neil Zhang; Will Deacon
> > Cc: Sudeep Holla; 'linux@arm.linux.org.uk';
> > 'linux-arm-kernel@lists.infradead.org';
> > 'linux-kernel@vger.kernel.org'; 'devicetree@vger.kernel.org'
> > Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> > notifier
> >
> >
> >
> > On 14/05/14 03:28, Neil Zhang wrote:
> > >> -----Original Message-----
> > >> From: Will Deacon [mailto:will.deacon@arm.com]
> > >> Sent: 2014年5月14日 2:45
> > >> To: Neil Zhang
> > >> Cc: Sudeep Holla; 'linux@arm.linux.org.uk';
> > >> 'linux-arm-kernel@lists.infradead.org';
> > >> 'linux-kernel@vger.kernel.org'; 'devicetree@vger.kernel.org'
> > >> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> > >> notifier
> > >>
> > >> On Mon, May 12, 2014 at 11:22:09AM +0100, Neil Zhang wrote:
> > >>>>> The device tree bindings for power domains is under discussion
> > >>>>> [1]
> > >>>>
> > >>>> Thanks for the information.
> > >>>> But it currently for device only, core related stuff are not supported.
> > >>>> And is it really good to register power provider for core and let
> > >>>> vfp / pmu etc to get it?
> > >>>>
> > >>>
> > >>> What's your suggestion about it?
> > >>> Is it OK that I add it under the PMU node?
> > >>
> > >> I don't really mind. I just want to avoid re-inventing the wheel in
> > >> a PMU-specific way and having to maintain that code forever because
> > >> it ended up in our DT description.
> > >>
> > >> Will
> > >
> > > I will prepare another patch to add DT description under PMU since
> > > there is no generic power domain support for pm notifier if no other
> > concerns.
> > > We can change the manner if there is generic power domain support
> > > for
> > pm notifier later.
> > > Thanks.
> >
> > No, please don't add any DT bindings for power domains specific to PMU
> > node.
> > We can't change the DT bindings once added.
> >
> > As I pointed out the DT bindings for generic power domains are under
> > discussion.
> > See if you can reuse it, if not help in extending it so that it can be used.
> >
> 
> Sorry for reply later.
> As I said before the under discussed generic power domain is not suitable for
> CPU peripherals since they are all known belong to CPU or cluster power
> domain.
> If we want to follow the way they are discussion, we need to register core
> and cluster power provider, and need vfp/gic/pmu etc to require them.
> Is it really suitable?
> 
Do you have any comments?
If no, I would like to put it under PMU node.

> > Regards,
> > Sudeep
> >
> >
> 
> Best Regards,
> Neil Zhang

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Will Deacon]

On Mon, Jun 30, 2014 at 11:39:15AM +0100, Neil Zhang wrote:
> > > > I will prepare another patch to add DT description under PMU since
> > > > there is no generic power domain support for pm notifier if no other
> > > concerns.
> > > > We can change the manner if there is generic power domain support
> > > > for
> > > pm notifier later.
> > > > Thanks.
> > >
> > > No, please don't add any DT bindings for power domains specific to PMU
> > > node.
> > > We can't change the DT bindings once added.
> > >
> > > As I pointed out the DT bindings for generic power domains are under
> > > discussion.
> > > See if you can reuse it, if not help in extending it so that it can be used.
> > >
> > 
> > Sorry for reply later.
> > As I said before the under discussed generic power domain is not suitable for
> > CPU peripherals since they are all known belong to CPU or cluster power
> > domain.
> > If we want to follow the way they are discussion, we need to register core
> > and cluster power provider, and need vfp/gic/pmu etc to require them.
> > Is it really suitable?
> > 
> Do you have any comments?
> If no, I would like to put it under PMU node.

Sudeep is a better person to comment than me, but I'd still rather this was
handled more generically as opposed to a PMU-specific hack. I don't see a
problem including GIC and VFP here, but only when we actually need to
save/restore them (i.e. what the hardware guys went crazy with the power
domains).

Will

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 2568 bytes --]



> -----Original Message-----
> From: Will Deacon [mailto:will.deacon@arm.com]
> Sent: 2014Äê7ÔÂ4ÈÕ 1:57
> To: Neil Zhang
> Cc: Sudeep Holla; 'linux@arm.linux.org.uk'; 'linux-arm-
> kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org';
> 'devicetree@vger.kernel.org'
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> notifier
> 
> On Mon, Jun 30, 2014 at 11:39:15AM +0100, Neil Zhang wrote:
> > > > > I will prepare another patch to add DT description under PMU
> > > > > since there is no generic power domain support for pm notifier
> > > > > if no other
> > > > concerns.
> > > > > We can change the manner if there is generic power domain
> > > > > support for
> > > > pm notifier later.
> > > > > Thanks.
> > > >
> > > > No, please don't add any DT bindings for power domains specific
> to
> > > > PMU node.
> > > > We can't change the DT bindings once added.
> > > >
> > > > As I pointed out the DT bindings for generic power domains are
> > > > under discussion.
> > > > See if you can reuse it, if not help in extending it so that it
> can be used.
> > > >
> > >
> > > Sorry for reply later.
> > > As I said before the under discussed generic power domain is not
> > > suitable for CPU peripherals since they are all known belong to CPU
> > > or cluster power domain.
> > > If we want to follow the way they are discussion, we need to
> > > register core and cluster power provider, and need vfp/gic/pmu etc
> to require them.
> > > Is it really suitable?
> > >
> > Do you have any comments?
> > If no, I would like to put it under PMU node.
> 
> Sudeep is a better person to comment than me, but I'd still rather this
> was handled more generically as opposed to a PMU-specific hack. I don't
> see a problem including GIC and VFP here, but only when we actually
> need to save/restore them (i.e. what the hardware guys went crazy with
> the power domains).
> 

Long time no follow up for this loop.
Sorry that I will pick it again.

Will,
I prefer to check always-on field under PMU node to check whether we need
Save/restore them.

Here is a sample for arch timer which also add under itself.
What do you think?

commit 82a5619410d4c4df65c04272db198eca5a867c18
Author: Lorenzo Pieralisi <lorenzo.pieralisi@arm.com>
Date:   Tue Apr 8 10:04:32 2014 +0100

    clocksource: arch_arm_timer: Fix age-old arch timer C3STOP detection issue


> Will

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Sudeep Holla]

Hi Neil,

On 20/10/14 09:46, Neil Zhang wrote:
>
>
>> -----Original Message----- From: Will Deacon
>> [mailto:will.deacon@arm.com] Sent: 2014年7月4日 1:57 To: Neil Zhang
>> Cc: Sudeep Holla; 'linux@arm.linux.org.uk'; 'linux-arm-
>> kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org';
>> 'devicetree@vger.kernel.org' Subject: Re: [PATCH v4] ARM: perf:
>> save/restore pmu registers in pm notifier
>>
>> On Mon, Jun 30, 2014 at 11:39:15AM +0100, Neil Zhang wrote:
>>>>>> I will prepare another patch to add DT description under
>>>>>> PMU since there is no generic power domain support for pm
>>>>>> notifier if no other concerns. We can change the manner if
>>>>>> there is generic power domain support for pm notifier
>>>>>> later. Thanks.
>>>>>
>>>>> No, please don't add any DT bindings for power domains
>>>>> specific to PMU node. We can't change the DT bindings once
>>>>> added.
>>>>>
>>>>> As I pointed out the DT bindings for generic power domains
>>>>> are under discussion. See if you can reuse it, if not help in
>>>>> extending it so that it can be used.
>>>>>
>>>> Sorry for reply later. As I said before the under discussed
>>>> generic power domain is not suitable for CPU peripherals since
>>>>  they are all known belong to CPU or cluster power domain. If
>>>> we want to follow the way they are discussion, we need to
>>>> register core and cluster power provider, and need vfp/gic/pmu
>>>> etc to require them.
>>>> Is it really suitable?
>>>>
>>> Do you have any comments? If no, I would like to put it under PMU
>>> node.
>>
>> Sudeep is a better person to comment than me, but I'd still rather
>>  this was handled more generically as opposed to a PMU-specific
>> hack. I don't see a problem including GIC and VFP here, but only
>> when we actually need to save/restore them (i.e. what the hardware
>>  guys went crazy with the power domains).
>>
>
> Long time no follow up for this loop. Sorry that I will pick it
> again.
>
Yes, the generic PD got added in v3.18-rc1, it's better to check if we
can reuse it. I will also have a look at that and think about how we can
use it.

> Will, I prefer to check always-on field under PMU node to check
> whether we need Save/restore them.
>
But how do you handle it for different idle states. e.g. if CPU is in
retention, PMU's *might be* retained. Also I don't think PMUs will be
placed in "always-on" power domain like timers. So using "always-on"
sounds incorrect to me.

Regards,
Sudeep

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Will Deacon]

On Mon, Oct 20, 2014 at 10:16:16AM +0100, Sudeep Holla wrote:
> On 20/10/14 09:46, Neil Zhang wrote:
> > Will, I prefer to check always-on field under PMU node to check
> > whether we need Save/restore them.
> >
> But how do you handle it for different idle states. e.g. if CPU is in
> retention, PMU's *might be* retained. Also I don't think PMUs will be
> placed in "always-on" power domain like timers. So using "always-on"
> sounds incorrect to me.

Adding Mathieu to CC, since I spoke to him at LPC about this and he was
talking about implementing proper PM domain descriptions for coresight
components.

Will

RE: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Neil Zhang]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset="gb2312", Size: 1286 bytes --]


> -----Original Message-----
> From: Will Deacon [mailto:will.deacon@arm.com]
> Sent: 2014Äê10ÔÂ20ÈÕ 17:20
> To: Sudeep Holla
> Cc: Neil Zhang; 'linux@arm.linux.org.uk'; 'linux-arm-
> kernel@lists.infradead.org'; 'linux-kernel@vger.kernel.org';
> 'devicetree@vger.kernel.org'; mathieu.poirier@linaro.org
> Subject: Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm
> notifier
> 
> On Mon, Oct 20, 2014 at 10:16:16AM +0100, Sudeep Holla wrote:
> > On 20/10/14 09:46, Neil Zhang wrote:
> > > Will, I prefer to check always-on field under PMU node to check
> > > whether we need Save/restore them.
> > >
> > But how do you handle it for different idle states. e.g. if CPU is in
> > retention, PMU's *might be* retained. Also I don't think PMUs will be
> > placed in "always-on" power domain like timers. So using "always-on"
> > sounds incorrect to me.
> 
> Adding Mathieu to CC, since I spoke to him at LPC about this and he was
> talking about implementing proper PM domain descriptions for coresight
> components.
> 

Good to know that!
Hope we can figure out it with a proper way.

> Will

Best Regards,
Neil Zhang
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Geert Uytterhoeven]

On Mon, Oct 20, 2014 at 11:20 AM, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 20, 2014 at 10:16:16AM +0100, Sudeep Holla wrote:
>> On 20/10/14 09:46, Neil Zhang wrote:
>> > Will, I prefer to check always-on field under PMU node to check
>> > whether we need Save/restore them.
>> >
>> But how do you handle it for different idle states. e.g. if CPU is in
>> retention, PMU's *might be* retained. Also I don't think PMUs will be
>> placed in "always-on" power domain like timers. So using "always-on"
>> sounds incorrect to me.
>
> Adding Mathieu to CC, since I spoke to him at LPC about this and he was
> talking about implementing proper PM domain descriptions for coresight
> components.

Good to hear that!

Gr{oetje,eeting}s,

                        Geert (who skipped LPC)

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

Re: [PATCH v4] ARM: perf: save/restore pmu registers in pm notifier

[Mathieu Poirier]

On 20 October 2014 11:20, Will Deacon <will.deacon@arm.com> wrote:
> On Mon, Oct 20, 2014 at 10:16:16AM +0100, Sudeep Holla wrote:
>> On 20/10/14 09:46, Neil Zhang wrote:
>> > Will, I prefer to check always-on field under PMU node to check
>> > whether we need Save/restore them.
>> >
>> But how do you handle it for different idle states. e.g. if CPU is in
>> retention, PMU's *might be* retained. Also I don't think PMUs will be
>> placed in "always-on" power domain like timers. So using "always-on"
>> sounds incorrect to me.
>
> Adding Mathieu to CC, since I spoke to him at LPC about this and he was
> talking about implementing proper PM domain descriptions for coresight
> components.
>
> Will

Will is correct - it's in the pipeline now.  Just hang tight.