Re: [PATCH V2 3/4] perf/imx_ddr: Add enhanced AXI ID filter support

[Will Deacon <will@kernel.org>]

On Tue, Oct 29, 2019 at 07:06:19AM +0000, Joakim Zhang wrote:
> With DDR_CAP_AXI_ID_FILTER quirk, indicating HW supports AXI ID filter
> which only can get bursts from DDR transaction, i.e. DDR read/write
> requests.
> 
> This patch add DDR_CAP_AXI_ID_ENHANCED_FILTER quirk, indicating HW
> supports AXI ID filter which can get bursts and bytes from DDR
> transaction at the same time. We hope PMU always return bytes in the
> driver due to it is more meaningful for users.
> 
> DDR_CAP_AXI_ID_ENHANCED_FILTER is based on DDR_CAP_AXI_ID_FILTER and
> extend it a bit. So need select both above two qiurks together when
> HW supports enhanced AXI ID filter.
> 
> Signed-off-by: Joakim Zhang <qiangqing.zhang@nxp.com>
> ---
> Changelog:
> V1->V2:
> 	* use ddr_perf_is_filtered() helper to simply the code.
> 	* improve the commit message.
> ---
>  drivers/perf/fsl_imx8_ddr_perf.c | 55 ++++++++++++++++++++------------
>  1 file changed, 34 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
> index ce7345745b42..17c817d89222 100644
> --- a/drivers/perf/fsl_imx8_ddr_perf.c
> +++ b/drivers/perf/fsl_imx8_ddr_perf.c
> @@ -45,7 +45,8 @@
>  static DEFINE_IDA(ddr_ida);
>  
>  /* DDR Perf hardware feature */
> -#define DDR_CAP_AXI_ID_FILTER          0x1     /* support AXI ID filter */
> +#define DDR_CAP_AXI_ID_FILTER			BIT(1)     /* support AXI ID filter */
> +#define DDR_CAP_AXI_ID_FILTER_ENHANCED		BIT(2)     /* support enhanced AXI ID filter */

I still don't understand why you don't do something like this:

#define DDR_CAP_AXI_ID_FILTER		0x1 /* support AXI ID filter */
#define DDR_CAP_AXI_ID_FILTER_ENHANCED	0x3 /* support enhanced AXI ID filter */


static bool ddr_perf_is_enhanced_filtered(struct perf_event *event)
{
	unsigned int filt;
	struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);

	filt = pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER_ENHANCED;
	return (filt == DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
		ddr_perf_is_filtered(event);
}


and then:

> +	/*
> +	 * return bytes instead of bursts from ddr transaction for
> +	 * axid-read and axid-write event if PMU core supports enhanced
> +	 * filter.
> +	 */
> +	if ((pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) &&
> +	    (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER_ENHANCED) &&
> +	    ddr_perf_is_filtered(event)) {

static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter)
{
	struct perf_event *event = pmu->events[counter];
	void __iomem *base = pmu->base;

	base += ddr_perf_is_enhanced_filtered(event) ? COUNTER_DPCR1 :
						       COUNTER_READ;
	return readl_relaxed(base + counter * 4);
}


In patch 4 you can then do:

.quirks = DDR_CAP_AXI_ID_FILTER_ENHANCED;

Make sense?

Will

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