[PATCH v4 2/2] perf/imx_ddr: don't enable counter0 if none of 4 counters
Frank Li
frank.li at nxp.com
Thu Aug 10 11:46:26 PDT 2023
> -----Original Message-----
> From: Xu Yang <xu.yang_2 at nxp.com>
> Sent: Thursday, August 10, 2023 6:00 AM
> To: Frank Li <frank.li at nxp.com>
> Cc: will at kernel.org; mark.rutland at arm.com; shawnguo at kernel.org;
> s.hauer at pengutronix.de; kernel at pengutronix.de; dl-linux-imx <linux-
> imx at nxp.com>; linux-arm-kernel at lists.infradead.org
> Subject: [PATCH v4 2/2] perf/imx_ddr: don't enable counter0 if none of 4
> counters
>
> In current driver, counter0 will be enabled after ddr_perf_pmu_enable()
> is called even though none of the 4 counters are used. This will cause
> counter0 continue to count until ddr_perf_pmu_disabled() is called. If
> pmu is not disabled all the time, the pmu interrupt will be asserted
> from time to time due to counter0 will overflow and irq handler will
> clear it. It's not an expected behavior. This patch will not enable
> counter0 if none of 4 counters are used.
>
> Fixes: 9a66d36cc7ac ("drivers/perf: imx_ddr: Add DDR performance counter
> support to perf")
> Signed-off-by: Xu Yang <xu.yang_2 at nxp.com>
Reviewed-by: Frank Li <Frank.li at nxp.com>
>
> ---
> Changes in v4:
> - enable cycle counter when active_counter from 0 -> 1
> - disable cycle counter when active_counter from 1 -> 0
> - make ddr_perf_pmu_enable/disable() an empty function
> Changes in v3:
> - don't differentiate cycle counter and other counters
> - modify logic in pmu_enable()/pmu_disable()
> Changes in v2:
> - add active counter count as suggested from Frank
> ---
> drivers/perf/fsl_imx8_ddr_perf.c | 24 +++++++++---------------
> 1 file changed, 9 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c
> b/drivers/perf/fsl_imx8_ddr_perf.c
> index e59e4fc6378d..ed51b1a99f60 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -105,6 +105,7 @@ struct ddr_pmu {
> const struct fsl_ddr_devtype_data *devtype_data;
> int irq;
> int id;
> + int active_counter;
> };
>
> static ssize_t ddr_perf_identifier_show(struct device *dev,
> @@ -518,6 +519,10 @@ static void ddr_perf_event_start(struct perf_event
> *event, int flags)
>
> ddr_perf_counter_enable(pmu, event->attr.config, counter, true);
>
> + if (!pmu->active_counter++)
> + ddr_perf_counter_enable(pmu, EVENT_CYCLES_ID,
> + EVENT_CYCLES_COUNTER, true);
> +
> hwc->state = 0;
> }
>
> @@ -571,6 +576,10 @@ static void ddr_perf_event_stop(struct perf_event
> *event, int flags)
> ddr_perf_counter_enable(pmu, event->attr.config, counter, false);
> ddr_perf_event_update(event);
>
> + if (!--pmu->active_counter)
> + ddr_perf_counter_enable(pmu, EVENT_CYCLES_ID,
> + EVENT_CYCLES_COUNTER, false);
> +
> hwc->state |= PERF_HES_STOPPED;
> }
>
> @@ -588,25 +597,10 @@ static void ddr_perf_event_del(struct perf_event
> *event, int flags)
>
> static void ddr_perf_pmu_enable(struct pmu *pmu)
> {
> - struct ddr_pmu *ddr_pmu = to_ddr_pmu(pmu);
> -
> - /* enable cycle counter if cycle is not active event list */
> - if (ddr_pmu->events[EVENT_CYCLES_COUNTER] == NULL)
> - ddr_perf_counter_enable(ddr_pmu,
> - EVENT_CYCLES_ID,
> - EVENT_CYCLES_COUNTER,
> - true);
> }
>
> static void ddr_perf_pmu_disable(struct pmu *pmu)
> {
> - struct ddr_pmu *ddr_pmu = to_ddr_pmu(pmu);
> -
> - if (ddr_pmu->events[EVENT_CYCLES_COUNTER] == NULL)
> - ddr_perf_counter_enable(ddr_pmu,
> - EVENT_CYCLES_ID,
> - EVENT_CYCLES_COUNTER,
> - false);
> }
>
> static int ddr_perf_init(struct ddr_pmu *pmu, void __iomem *base,
> --
> 2.34.1
More information about the linux-arm-kernel
mailing list