Re: [PATCH v2 3/3] perf/imx_ddr: don't enable counter0 if none of 4 counters are used

[Mark Rutland <mark.rutland@arm.com>]

On Thu, Jul 13, 2023 at 06:37:58PM +0800, Xu Yang wrote:
> 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@nxp.com>

Acked-by: Mark Rutland <mark.rutland@arm.com>

I assume that you'll send this as part of a v3 given the comments on patches 1
and 2.

Thanks,
Mark.

> 
> ---
> Changes in v2:
>  - add active events count as suggested from Frank
> ---
>  drivers/perf/fsl_imx8_ddr_perf.c | 13 +++++++++++--
>  1 file changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index d65200d4e96e..761e45f21092 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -103,6 +103,7 @@ struct ddr_pmu {
>  	const struct fsl_ddr_devtype_data *devtype_data;
>  	int irq;
>  	int id;
> +	int active_count;
>  };
>  
>  static ssize_t ddr_perf_identifier_show(struct device *dev,
> @@ -516,6 +517,9 @@ static void ddr_perf_event_start(struct perf_event *event, int flags)
>  
>  	ddr_perf_counter_enable(pmu, event->attr.config, counter, true);
>  
> +	if (counter != EVENT_CYCLES_COUNTER)
> +		pmu->active_count++;
> +
>  	hwc->state = 0;
>  }
>  
> @@ -569,6 +573,9 @@ 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 (counter != EVENT_CYCLES_COUNTER)
> +		pmu->active_count--;
> +
>  	hwc->state |= PERF_HES_STOPPED;
>  }
>  
> @@ -589,7 +596,8 @@ 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)
> +	if ((ddr_pmu->events[EVENT_CYCLES_COUNTER] == NULL)
> +		&& ddr_pmu->active_count > 0)
>  		ddr_perf_counter_enable(ddr_pmu,
>  				      EVENT_CYCLES_ID,
>  				      EVENT_CYCLES_COUNTER,
> @@ -600,7 +608,8 @@ 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)
> +	if ((ddr_pmu->events[EVENT_CYCLES_COUNTER] == NULL)
> +		&& ddr_pmu->active_count > 0)
>  		ddr_perf_counter_enable(ddr_pmu,
>  				      EVENT_CYCLES_ID,
>  				      EVENT_CYCLES_COUNTER,
> -- 
> 2.34.1
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel