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

[Xu Yang]

Hi Frank,

> -----Original Message-----
> From: Frank Li <frank.li at nxp.com>
> Sent: Wednesday, July 5, 2023 10:51 PM
> To: Xu Yang <xu.yang_2 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: RE: [PATCH 3/3] perf/imx_ddr: don't enable counter0 if none of 4 counters are used
> 
> >
> > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c
> > b/drivers/perf/fsl_imx8_ddr_perf.c
> > index 4e6cbfc48429..f7c832611713 100644
> > --- a/drivers/perf/fsl_imx8_ddr_perf.c
> > +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> > @@ -591,7 +591,9 @@ 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 &&
> 
> EVENT_CYCLES_COUNTER is 0,  if only enable cycles counter,  below logic
> Will failure.   Then counter will not work.

Yes, cycle counter will not be enabled for counting at this point.
However, cycle counter will be enabled after perf_pmu_enable() is
called. My understanding is that ddr_perf_pmu_enable() is used to
enable cycle counter when other counters is used for counting since
other counters depends on cycle counter.

> 
> > +	   (ddr_pmu->events[0] || ddr_pmu->events[1] ||
> > +	    ddr_pmu->events[2] || ddr_pmu->events[3]))
> 
>  I suggest maintain a enabled_event counter,
> 
> 	If(ddr_pmu->active_num==0)
> 		ddr_perf_counter_enable(..., true);
> 	ddr_pmu->active_num++;
> 
> At disable function()
> {
>        ddr_pmu->active_num --;
>        if( ddr_pmu->active_num == 0)
> 	ddr_perf_counter_enable(.., false);
> }

Will try to maintain a enabled_event counter.

Thanks,
Xu Yang

> 
> 
> >  		ddr_perf_counter_enable(ddr_pmu,
> >  				      EVENT_CYCLES_ID,
> >  				      EVENT_CYCLES_COUNTER,
> > @@ -602,7 +604,9 @@ 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->events[0] || ddr_pmu->events[1] ||
> > +	    ddr_pmu->events[2] || ddr_pmu->events[3]))
> >  		ddr_perf_counter_enable(ddr_pmu,
> >  				      EVENT_CYCLES_ID,
> >  				      EVENT_CYCLES_COUNTER,
> > --
> > 2.34.1