[EXT] Re: [PATCH v2 1/3] perf/imx_ddr: speed up overflow frequency of cycle counter

[Xu Yang]

Hi Mark,

> On Thu, Jul 13, 2023 at 06:37:56PM +0800, Xu Yang wrote:
> > For i.MX8MP, we cannot ensure that cycle counter overflow occurs at least
> > 4 times as often as other events. Due to byte counters will count for any
> > event configured, it will overflow more often. And if byte counters
> > overflow that related counters would stop since they share the COUNTER_CNTL
> > We can speed up cycle counter overflow frequency by setting counter
> > parameter (CP) field of cycle counter. In this way, we can avoid stop
> > counting byte counters when interrupt didn't come and the byte counters
> > can be fetched or updated from each cycle counter overflow interrupt.
> >
> > Signed-off-by: Xu Yang <xu.yang_2 at nxp.com>
> >
> > ---
> > Changes in v2:
> >  - improve if condition
> > ---
> >  drivers/perf/fsl_imx8_ddr_perf.c | 15 +++++++++++++++
> >  1 file changed, 15 insertions(+)
> >
> > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> > index 5222ba1e79d0..039069756bbc 100644
> > --- a/drivers/perf/fsl_imx8_ddr_perf.c
> > +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> > @@ -28,6 +28,8 @@
> >  #define CNTL_CLEAR_MASK              0xFFFFFFFD
> >  #define CNTL_OVER_MASK               0xFFFFFFFE
> >
> > +#define CNTL_CP_SHIFT                16
> > +#define CNTL_CP_MASK         (0xFF << CNTL_CP_SHIFT)
> >  #define CNTL_CSV_SHIFT               24
> >  #define CNTL_CSV_MASK                (0xFFU << CNTL_CSV_SHIFT)
> >
> > @@ -427,6 +429,19 @@ static void ddr_perf_counter_enable(struct ddr_pmu *pmu, int config,
> >               writel(0, pmu->base + reg);
> >               val = CNTL_EN | CNTL_CLEAR;
> >               val |= FIELD_PREP(CNTL_CSV_MASK, config);
> > +
> > +             /*
> > +              * Workaround for i.MX8MP:
> > +              * Common counters and byte counters share the same COUNTER_CNTL,
> > +              * and byte counters could overflow before cycle counter. Need set
> > +              * counter parameter(CP) of cycle counter to give it initial value
> > +              * which can speed up cycle counter overflow frequency.
> > +              */
> 
> From the comments on path 2, it sounds like this "counter parameter" sets bits
> [31..28] of the counter value, is that correct?

Correct.

> 
> Assuming so, could we please update this comment to say:
> 
>         /*
>          * On i.MX8MP we need to bias the cycle counter to overflow more often.
>          * We do this by initializing bits [31:28] of the counter value via the
>          * COUNTER_CTRL Counter Parameter (CP) field.
>          *
>          * See ddr_perf_counter_enable() for more details.
>          */

Sure, I will update the comment like this.

Thanks,
Xu Yang

> 
> Thanks,
> Mark.
> 
> > +             if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER_ENHANCED) {
> > +                     if (counter == EVENT_CYCLES_COUNTER)
> > +                             val |= FIELD_PREP(CNTL_CP_MASK, 0xf0);
> > +             }
> > +
> >               writel(val, pmu->base + reg);
> >       } else {
> >               /* Disable counter */
> > --
> > 2.34.1
> >