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

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

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@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?

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.
	 */

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
> 

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