| Message ID | 20191101083317.29510-3-qiangqing.zhang@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [V3,1/4] docs/perf: Add explanation for DDR_CAP_AXI_ID_FILTER_ENHANCED quirk | expand |
On Fri, Nov 01, 2019 at 08:36:16AM +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. Thanks. I've queued the series locally, but the part I'm still wondering about is how we advertise the enhanced filter. For example, how does userspace know whether or not it will get bursts or bytes back when specifying an AXI filter? Should we create something like caps/enhanced_filter which reads as 0/1 depending on whether or not the quirk is set? You can look at intel-pt.c and arm_spe_pmu.c for examples of this sort of thing. If you agree, please send a patch on top to implement this. Will
> -----Original Message----- > From: Will Deacon <will@kernel.org> > Sent: 2019年11月1日 23:17 > To: Joakim Zhang <qiangqing.zhang@nxp.com> > Cc: mark.rutland@arm.com; robin.murphy@arm.com; dl-linux-imx > <linux-imx@nxp.com>; Frank Li <frank.li@nxp.com>; > linux-arm-kernel@lists.infradead.org > Subject: Re: [PATCH V3 3/4] perf/imx_ddr: Add enhanced AXI ID filter support > > On Fri, Nov 01, 2019 at 08:36:16AM +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. > > Thanks. I've queued the series locally, but the part I'm still wondering about is > how we advertise the enhanced filter. > > For example, how does userspace know whether or not it will get bursts or > bytes back when specifying an AXI filter? Should we create something like > caps/enhanced_filter which reads as 0/1 depending on whether or not the quirk > is set? You can look at intel-pt.c and arm_spe_pmu.c for examples of this sort > of thing. > > If you agree, please send a patch on top to implement this. Thanks Will, agree, I will implement this later. Best Regards, Joakim Zhang > Will
On Mon, Nov 04, 2019 at 01:21:51AM +0000, Joakim Zhang wrote: > > > -----Original Message----- > > From: Will Deacon <will@kernel.org> > > Sent: 2019年11月1日 23:17 > > To: Joakim Zhang <qiangqing.zhang@nxp.com> > > Cc: mark.rutland@arm.com; robin.murphy@arm.com; dl-linux-imx > > <linux-imx@nxp.com>; Frank Li <frank.li@nxp.com>; > > linux-arm-kernel@lists.infradead.org > > Subject: Re: [PATCH V3 3/4] perf/imx_ddr: Add enhanced AXI ID filter support > > > > On Fri, Nov 01, 2019 at 08:36:16AM +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. > > > > Thanks. I've queued the series locally, but the part I'm still wondering about is > > how we advertise the enhanced filter. > > > > For example, how does userspace know whether or not it will get bursts or > > bytes back when specifying an AXI filter? Should we create something like > > caps/enhanced_filter which reads as 0/1 depending on whether or not the quirk > > is set? You can look at intel-pt.c and arm_spe_pmu.c for examples of this sort > > of thing. > > > > If you agree, please send a patch on top to implement this. > > Thanks Will, agree, I will implement this later. Given that it's ABI, please can you do it this week so that we have something consistent for 5.5? Will
On Mon, Nov 04, 2019 at 12:32:26PM +0000, Will Deacon wrote: > On Mon, Nov 04, 2019 at 01:21:51AM +0000, Joakim Zhang wrote: > > > > > -----Original Message----- > > > From: Will Deacon <will@kernel.org> > > > Sent: 2019年11月1日 23:17 > > > To: Joakim Zhang <qiangqing.zhang@nxp.com> > > > Cc: mark.rutland@arm.com; robin.murphy@arm.com; dl-linux-imx > > > <linux-imx@nxp.com>; Frank Li <frank.li@nxp.com>; > > > linux-arm-kernel@lists.infradead.org > > > Subject: Re: [PATCH V3 3/4] perf/imx_ddr: Add enhanced AXI ID filter support > > > > > > On Fri, Nov 01, 2019 at 08:36:16AM +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. > > > > > > Thanks. I've queued the series locally, but the part I'm still wondering about is > > > how we advertise the enhanced filter. > > > > > > For example, how does userspace know whether or not it will get bursts or > > > bytes back when specifying an AXI filter? Should we create something like > > > caps/enhanced_filter which reads as 0/1 depending on whether or not the quirk > > > is set? You can look at intel-pt.c and arm_spe_pmu.c for examples of this sort > > > of thing. > > > > > > If you agree, please send a patch on top to implement this. > > > > Thanks Will, agree, I will implement this later. > > Given that it's ABI, please can you do it this week so that we have > something consistent for 5.5? Sorry, I notice you've already sent them. Thanks! Will
diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c index ce7345745b42..2a3966d059e7 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 0x1 /* support AXI ID filter */ +#define DDR_CAP_AXI_ID_FILTER_ENHANCED 0x3 /* support enhanced AXI ID filter */ struct fsl_ddr_devtype_data { unsigned int quirks; /* quirks needed for different DDR Perf core */ @@ -178,6 +179,36 @@ static const struct attribute_group *attr_groups[] = { NULL, }; +static bool ddr_perf_is_filtered(struct perf_event *event) +{ + return event->attr.config == 0x41 || event->attr.config == 0x42; +} + +static u32 ddr_perf_filter_val(struct perf_event *event) +{ + return event->attr.config1; +} + +static bool ddr_perf_filters_compatible(struct perf_event *a, + struct perf_event *b) +{ + if (!ddr_perf_is_filtered(a)) + return true; + if (!ddr_perf_is_filtered(b)) + return true; + return ddr_perf_filter_val(a) == ddr_perf_filter_val(b); +} + +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); +} + static u32 ddr_perf_alloc_counter(struct ddr_pmu *pmu, int event) { int i; @@ -209,27 +240,17 @@ static void ddr_perf_free_counter(struct ddr_pmu *pmu, int counter) static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter) { - return readl_relaxed(pmu->base + COUNTER_READ + counter * 4); -} - -static bool ddr_perf_is_filtered(struct perf_event *event) -{ - return event->attr.config == 0x41 || event->attr.config == 0x42; -} + struct perf_event *event = pmu->events[counter]; + void __iomem *base = pmu->base; -static u32 ddr_perf_filter_val(struct perf_event *event) -{ - return event->attr.config1; -} - -static bool ddr_perf_filters_compatible(struct perf_event *a, - struct perf_event *b) -{ - if (!ddr_perf_is_filtered(a)) - return true; - if (!ddr_perf_is_filtered(b)) - return true; - return ddr_perf_filter_val(a) == ddr_perf_filter_val(b); + /* + * return bytes instead of bursts from ddr transaction for + * axid-read and axid-write event if PMU core supports enhanced + * filter. + */ + base += ddr_perf_is_enhanced_filtered(event) ? COUNTER_DPCR1 : + COUNTER_READ; + return readl_relaxed(base + counter * 4); } static int ddr_perf_event_init(struct perf_event *event)
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. 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. V2->V3: * change definition of DDR_CAP_AXI_ID_FILTER_ENHANCED quirk. * add ddr_perf_is_enhanced_filtered() helper. --- drivers/perf/fsl_imx8_ddr_perf.c | 63 +++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 21 deletions(-)