* [PATCH] powerpc/perf: Freeze PMC5/6 if we're not using them on Power8
@ 2013-06-13 1:16 Michael Ellerman
2013-06-13 6:39 ` Anshuman Khandual
0 siblings, 1 reply; 3+ messages in thread
From: Michael Ellerman @ 2013-06-13 1:16 UTC (permalink / raw)
To: linuxppc-dev
On Power8 we can freeze PMC5 and 6 if we're not using them. Normally they
run all the time.
Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
---
arch/powerpc/include/asm/reg.h | 1 +
arch/powerpc/perf/power8-pmu.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
index 4a9e408..362142b 100644
--- a/arch/powerpc/include/asm/reg.h
+++ b/arch/powerpc/include/asm/reg.h
@@ -626,6 +626,7 @@
#define MMCR0_TRIGGER 0x00002000UL /* TRIGGER enable */
#define MMCR0_PMAO 0x00000080UL /* performance monitor alert has occurred, set to 0 after handling exception */
#define MMCR0_SHRFC 0x00000040UL /* SHRre freeze conditions between threads */
+#define MMCR0_FC56 0x00000010UL /* freeze counters 5 and 6 */
#define MMCR0_FCTI 0x00000008UL /* freeze counters in tags inactive mode */
#define MMCR0_FCTA 0x00000004UL /* freeze counters in tags active mode */
#define MMCR0_FCWAIT 0x00000002UL /* freeze counter in WAIT state */
diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
index f7d1c4f..e791c68 100644
--- a/arch/powerpc/perf/power8-pmu.c
+++ b/arch/powerpc/perf/power8-pmu.c
@@ -378,6 +378,10 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
if (pmc_inuse & 0x7c)
mmcr[0] |= MMCR0_PMCjCE;
+ /* If we're not using PMC 5 or 6, freeze them */
+ if (!(pmc_inuse & 0x60))
+ mmcr[0] |= MMCR0_FC56;
+
mmcr[1] = mmcr1;
mmcr[2] = mmcra;
--
1.7.10.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc/perf: Freeze PMC5/6 if we're not using them on Power8
2013-06-13 1:16 [PATCH] powerpc/perf: Freeze PMC5/6 if we're not using them on Power8 Michael Ellerman
@ 2013-06-13 6:39 ` Anshuman Khandual
2013-06-25 1:36 ` Michael Ellerman
0 siblings, 1 reply; 3+ messages in thread
From: Anshuman Khandual @ 2013-06-13 6:39 UTC (permalink / raw)
To: Michael Ellerman; +Cc: linuxppc-dev
On 06/13/2013 06:46 AM, Michael Ellerman wrote:
> On Power8 we can freeze PMC5 and 6 if we're not using them. Normally they
> run all the time.
>
> Signed-off-by: Michael Ellerman <michael@ellerman.id.au>
> ---
> arch/powerpc/include/asm/reg.h | 1 +
> arch/powerpc/perf/power8-pmu.c | 4 ++++
> 2 files changed, 5 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/reg.h b/arch/powerpc/include/asm/reg.h
> index 4a9e408..362142b 100644
> --- a/arch/powerpc/include/asm/reg.h
> +++ b/arch/powerpc/include/asm/reg.h
> @@ -626,6 +626,7 @@
> #define MMCR0_TRIGGER 0x00002000UL /* TRIGGER enable */
> #define MMCR0_PMAO 0x00000080UL /* performance monitor alert has occurred, set to 0 after handling exception */
> #define MMCR0_SHRFC 0x00000040UL /* SHRre freeze conditions between threads */
> +#define MMCR0_FC56 0x00000010UL /* freeze counters 5 and 6 */
> #define MMCR0_FCTI 0x00000008UL /* freeze counters in tags inactive mode */
> #define MMCR0_FCTA 0x00000004UL /* freeze counters in tags active mode */
> #define MMCR0_FCWAIT 0x00000002UL /* freeze counter in WAIT state */
> diff --git a/arch/powerpc/perf/power8-pmu.c b/arch/powerpc/perf/power8-pmu.c
> index f7d1c4f..e791c68 100644
> --- a/arch/powerpc/perf/power8-pmu.c
> +++ b/arch/powerpc/perf/power8-pmu.c
> @@ -378,6 +378,10 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
> if (pmc_inuse & 0x7c)
> mmcr[0] |= MMCR0_PMCjCE;
>
> + /* If we're not using PMC 5 or 6, freeze them */
> + if (!(pmc_inuse & 0x60))
> + mmcr[0] |= MMCR0_FC56;
> +
> mmcr[1] = mmcr1;
> mmcr[2] = mmcra;
>
Hey Michael,
This looks good. But we need to undo this changes when we terminate the perf session.
That way user would be able to continue reading PMC5 and PMC6 through /sys interface
as before (which may not be ideal). Adding the following changes along with this patch
would keep the status quo as it is.
diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
index 29c6482..141756a 100644
--- a/arch/powerpc/perf/core-book3s.c
+++ b/arch/powerpc/perf/core-book3s.c
@@ -881,6 +881,12 @@ static void power_pmu_disable(struct pmu *pmu)
}
/*
+ * Undo PMC5/PMC6 freeze if already applied
+ */
+ if (mfspr(SPRN_MMCR0) & MMCR0_FC56)
+ mtspr(SPRN_MMCR0, mfspr(SPRN_MMCR0) & ~PMCR0_FC56)
+
+ /*
* Set the 'freeze counters' bit.
* The barrier is to make sure the mtspr has been
* executed and the PMU has frozen the events
Regards
Anshuman
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] powerpc/perf: Freeze PMC5/6 if we're not using them on Power8
2013-06-13 6:39 ` Anshuman Khandual
@ 2013-06-25 1:36 ` Michael Ellerman
0 siblings, 0 replies; 3+ messages in thread
From: Michael Ellerman @ 2013-06-25 1:36 UTC (permalink / raw)
To: Anshuman Khandual; +Cc: linuxppc-dev
On Thu, Jun 13, 2013 at 12:09:47PM +0530, Anshuman Khandual wrote:
> On 06/13/2013 06:46 AM, Michael Ellerman wrote:
> > On Power8 we can freeze PMC5 and 6 if we're not using them. Normally they
> > run all the time.
> >
> > index f7d1c4f..e791c68 100644
> > --- a/arch/powerpc/perf/power8-pmu.c
> > +++ b/arch/powerpc/perf/power8-pmu.c
> > @@ -378,6 +378,10 @@ static int power8_compute_mmcr(u64 event[], int n_ev,
> > if (pmc_inuse & 0x7c)
> > mmcr[0] |= MMCR0_PMCjCE;
> >
> > + /* If we're not using PMC 5 or 6, freeze them */
> > + if (!(pmc_inuse & 0x60))
> > + mmcr[0] |= MMCR0_FC56;
> > +
> > mmcr[1] = mmcr1;
> > mmcr[2] = mmcra;
> >
>
> Hey Michael,
>
> This looks good. But we need to undo this changes when we terminate the perf session.
> That way user would be able to continue reading PMC5 and PMC6 through /sys interface
> as before (which may not be ideal). Adding the following changes along with this patch
> would keep the status quo as it is.
Yep.
> diff --git a/arch/powerpc/perf/core-book3s.c b/arch/powerpc/perf/core-book3s.c
> index 29c6482..141756a 100644
> --- a/arch/powerpc/perf/core-book3s.c
> +++ b/arch/powerpc/perf/core-book3s.c
> @@ -881,6 +881,12 @@ static void power_pmu_disable(struct pmu *pmu)
> }
>
> /*
> + * Undo PMC5/PMC6 freeze if already applied
> + */
> + if (mfspr(SPRN_MMCR0) & MMCR0_FC56)
> + mtspr(SPRN_MMCR0, mfspr(SPRN_MMCR0) & ~PMCR0_FC56)
The intent here is correct. But you've added two mfsprs() and an mtspr()
when the surrounding code has already read MMCR0, and will soon write
it. They may not be that expensive but still.
See my updated patch.
cheers
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2013-06-25 1:36 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-06-13 1:16 [PATCH] powerpc/perf: Freeze PMC5/6 if we're not using them on Power8 Michael Ellerman
2013-06-13 6:39 ` Anshuman Khandual
2013-06-25 1:36 ` Michael Ellerman
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.