| Message ID | 20231011060838.3413621-1-xu.yang_2@nxp.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2,1/5] perf: fsl_imx8_ddr: Add AXI ID PORT CHANNEL filter support | expand |
> -----Original Message----- > From: Xu Yang <xu.yang_2@nxp.com> > Sent: Wednesday, October 11, 2023 1:09 AM > To: Frank Li <frank.li@nxp.com>; corbet@lwn.net; shawnguo@kernel.org; > s.hauer@pengutronix.de; kernel@pengutronix.de; will@kernel.org; > mark.rutland@arm.com; robh+dt@kernel.org; > krzysztof.kozlowski+dt@linaro.org > Cc: festevam@gmail.com; conor+dt@kernel.org; dl-linux-imx <linux- > imx@nxp.com>; linux-arm-kernel@lists.infradead.org; linux- > doc@vger.kernel.org; devicetree@vger.kernel.org; Xu Yang > <xu.yang_2@nxp.com> > Subject: [PATCH v2 1/5] perf: fsl_imx8_ddr: Add AXI ID PORT CHANNEL filter > support > > This is the extension of AXI ID filter. > > Filter is defined with 2 configuration registers per counter 1-3 (counter > 0 is not used for filtering and lacks these registers). > * Counter N MASK COMP register - AXI_ID and AXI_MASKING. > * Counter N MUX CNTL register - AXI CHANNEL and AXI PORT. > -- 0: address channel > -- 1: data channel > > This filter is exposed to userspace as an additional (channel, port) pair. > The definition of axi_channel is inverted in userspace, and it will be > reverted in driver automatically. > > AXI filter of Perf Monitor in DDR Subsystem, only a single port0 exist, so > axi_port is reserved which should be 0. > > e.g. > perf stat -a -e imx8_ddr0/axid- > read,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > perf stat -a -e imx8_ddr0/axid- > write,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> Reviewed-by: Frank Li <Frank.Li@nxp.com> > > --- > Changes since v2: > - no changes > --- > drivers/perf/fsl_imx8_ddr_perf.c | 39 > ++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c > b/drivers/perf/fsl_imx8_ddr_perf.c > index 92611c98120f..d0eae2d7e64b 100644 > --- a/drivers/perf/fsl_imx8_ddr_perf.c > +++ b/drivers/perf/fsl_imx8_ddr_perf.c > @@ -19,6 +19,8 @@ > #define COUNTER_READ 0x20 > > #define COUNTER_DPCR1 0x30 > +#define COUNTER_MUX_CNTL 0x50 > +#define COUNTER_MASK_COMP 0x54 > > #define CNTL_OVER 0x1 > #define CNTL_CLEAR 0x2 > @@ -32,6 +34,13 @@ > #define CNTL_CSV_SHIFT 24 > #define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT) > > +#define READ_PORT_SHIFT 0 > +#define READ_PORT_MASK (0x7 << READ_PORT_SHIFT) > +#define READ_CHANNEL_REVERT 0x00000008 /* bit 3 for read > channel select */ > +#define WRITE_PORT_SHIFT 8 > +#define WRITE_PORT_MASK (0x7 << WRITE_PORT_SHIFT) > +#define WRITE_CHANNEL_REVERT 0x00000800 /* bit 11 for write > channel select */ > + > #define EVENT_CYCLES_ID 0 > #define EVENT_CYCLES_COUNTER 0 > #define NUM_COUNTERS 4 > @@ -50,6 +59,7 @@ 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_ENHANCED 0x3 /* support > enhanced AXI ID filter */ > +#define DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER 0x4 /* support > AXI ID PORT CHANNEL filter */ > > struct fsl_ddr_devtype_data { > unsigned int quirks; /* quirks needed for different DDR Perf core */ > @@ -144,6 +154,7 @@ static const struct attribute_group > ddr_perf_identifier_attr_group = { > enum ddr_perf_filter_capabilities { > PERF_CAP_AXI_ID_FILTER = 0, > PERF_CAP_AXI_ID_FILTER_ENHANCED, > + PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER, > PERF_CAP_AXI_ID_FEAT_MAX, > }; > > @@ -157,6 +168,8 @@ static u32 ddr_perf_filter_cap_get(struct ddr_pmu > *pmu, int cap) > case PERF_CAP_AXI_ID_FILTER_ENHANCED: > quirks &= DDR_CAP_AXI_ID_FILTER_ENHANCED; > return quirks == DDR_CAP_AXI_ID_FILTER_ENHANCED; > + case PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER: > + return !!(quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER); > default: > WARN(1, "unknown filter cap %d\n", cap); > } > @@ -187,6 +200,7 @@ static ssize_t ddr_perf_filter_cap_show(struct device > *dev, > static struct attribute *ddr_perf_filter_cap_attr[] = { > PERF_FILTER_EXT_ATTR_ENTRY(filter, PERF_CAP_AXI_ID_FILTER), > PERF_FILTER_EXT_ATTR_ENTRY(enhanced_filter, > PERF_CAP_AXI_ID_FILTER_ENHANCED), > + PERF_FILTER_EXT_ATTR_ENTRY(super_filter, > PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER), > NULL, > }; > > @@ -272,11 +286,15 @@ static const struct attribute_group > ddr_perf_events_attr_group = { > PMU_FORMAT_ATTR(event, "config:0-7"); > PMU_FORMAT_ATTR(axi_id, "config1:0-15"); > PMU_FORMAT_ATTR(axi_mask, "config1:16-31"); > +PMU_FORMAT_ATTR(axi_port, "config2:0-2"); > +PMU_FORMAT_ATTR(axi_channel, "config2:3-3"); > > static struct attribute *ddr_perf_format_attrs[] = { > &format_attr_event.attr, > &format_attr_axi_id.attr, > &format_attr_axi_mask.attr, > + &format_attr_axi_port.attr, > + &format_attr_axi_channel.attr, > NULL, > }; > > @@ -530,6 +548,7 @@ static int ddr_perf_event_add(struct perf_event > *event, int flags) > int counter; > int cfg = event->attr.config; > int cfg1 = event->attr.config1; > + int cfg2 = event->attr.config2; > > if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) { > int i; > @@ -553,6 +572,26 @@ static int ddr_perf_event_add(struct perf_event > *event, int flags) > return -EOPNOTSUPP; > } > > + if (pmu->devtype_data->quirks & > DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER) { > + if (ddr_perf_is_filtered(event)) { > + /* revert axi id masking(axi_mask) value */ > + cfg1 ^= AXI_MASKING_REVERT; > + writel(cfg1, pmu->base + COUNTER_MASK_COMP + > ((counter - 1) << 4)); > + > + if (cfg == 0x41) { > + /* revert axi read channel(axi_channel) value > */ > + cfg2 ^= READ_CHANNEL_REVERT; > + cfg2 |= FIELD_PREP(READ_PORT_MASK, cfg2); > + } else { > + /* revert axi write channel(axi_channel) value > */ > + cfg2 ^= WRITE_CHANNEL_REVERT; > + cfg2 |= FIELD_PREP(WRITE_PORT_MASK, > cfg2); > + } > + > + writel(cfg2, pmu->base + COUNTER_MUX_CNTL + > ((counter - 1) << 4)); > + } > + } > + > pmu->events[counter] = event; > hwc->idx = counter; > > -- > 2.34.1
On Wed, Oct 11, 2023 at 02:08:34PM +0800, Xu Yang wrote: > This is the extension of AXI ID filter. > > Filter is defined with 2 configuration registers per counter 1-3 (counter > 0 is not used for filtering and lacks these registers). > * Counter N MASK COMP register - AXI_ID and AXI_MASKING. > * Counter N MUX CNTL register - AXI CHANNEL and AXI PORT. > -- 0: address channel > -- 1: data channel > > This filter is exposed to userspace as an additional (channel, port) pair. > The definition of axi_channel is inverted in userspace, and it will be > reverted in driver automatically. > > AXI filter of Perf Monitor in DDR Subsystem, only a single port0 exist, so > axi_port is reserved which should be 0. > > e.g. > perf stat -a -e imx8_ddr0/axid-read,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > perf stat -a -e imx8_ddr0/axid-write,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > --- > Changes since v2: > - no changes > --- > drivers/perf/fsl_imx8_ddr_perf.c | 39 ++++++++++++++++++++++++++++++++ > 1 file changed, 39 insertions(+) > > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c > index 92611c98120f..d0eae2d7e64b 100644 > --- a/drivers/perf/fsl_imx8_ddr_perf.c > +++ b/drivers/perf/fsl_imx8_ddr_perf.c > @@ -19,6 +19,8 @@ > #define COUNTER_READ 0x20 > > #define COUNTER_DPCR1 0x30 > +#define COUNTER_MUX_CNTL 0x50 > +#define COUNTER_MASK_COMP 0x54 > > #define CNTL_OVER 0x1 > #define CNTL_CLEAR 0x2 > @@ -32,6 +34,13 @@ > #define CNTL_CSV_SHIFT 24 > #define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT) > > +#define READ_PORT_SHIFT 0 > +#define READ_PORT_MASK (0x7 << READ_PORT_SHIFT) > +#define READ_CHANNEL_REVERT 0x00000008 /* bit 3 for read channel select */ > +#define WRITE_PORT_SHIFT 8 > +#define WRITE_PORT_MASK (0x7 << WRITE_PORT_SHIFT) > +#define WRITE_CHANNEL_REVERT 0x00000800 /* bit 11 for write channel select */ > + > #define EVENT_CYCLES_ID 0 > #define EVENT_CYCLES_COUNTER 0 > #define NUM_COUNTERS 4 > @@ -50,6 +59,7 @@ 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_ENHANCED 0x3 /* support enhanced AXI ID filter */ > +#define DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER 0x4 /* support AXI ID PORT CHANNEL filter */ > > struct fsl_ddr_devtype_data { > unsigned int quirks; /* quirks needed for different DDR Perf core */ > @@ -144,6 +154,7 @@ static const struct attribute_group ddr_perf_identifier_attr_group = { > enum ddr_perf_filter_capabilities { > PERF_CAP_AXI_ID_FILTER = 0, > PERF_CAP_AXI_ID_FILTER_ENHANCED, > + PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER, > PERF_CAP_AXI_ID_FEAT_MAX, > }; > > @@ -157,6 +168,8 @@ static u32 ddr_perf_filter_cap_get(struct ddr_pmu *pmu, int cap) > case PERF_CAP_AXI_ID_FILTER_ENHANCED: > quirks &= DDR_CAP_AXI_ID_FILTER_ENHANCED; > return quirks == DDR_CAP_AXI_ID_FILTER_ENHANCED; > + case PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER: > + return !!(quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER); > default: > WARN(1, "unknown filter cap %d\n", cap); > } > @@ -187,6 +200,7 @@ static ssize_t ddr_perf_filter_cap_show(struct device *dev, > static struct attribute *ddr_perf_filter_cap_attr[] = { > PERF_FILTER_EXT_ATTR_ENTRY(filter, PERF_CAP_AXI_ID_FILTER), > PERF_FILTER_EXT_ATTR_ENTRY(enhanced_filter, PERF_CAP_AXI_ID_FILTER_ENHANCED), > + PERF_FILTER_EXT_ATTR_ENTRY(super_filter, PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER), > NULL, > }; > > @@ -272,11 +286,15 @@ static const struct attribute_group ddr_perf_events_attr_group = { > PMU_FORMAT_ATTR(event, "config:0-7"); > PMU_FORMAT_ATTR(axi_id, "config1:0-15"); > PMU_FORMAT_ATTR(axi_mask, "config1:16-31"); > +PMU_FORMAT_ATTR(axi_port, "config2:0-2"); > +PMU_FORMAT_ATTR(axi_channel, "config2:3-3"); Any specific reason to allocate from config2, rather than the upper 32 bits of config1? > @@ -553,6 +572,26 @@ static int ddr_perf_event_add(struct perf_event *event, int flags) > return -EOPNOTSUPP; > } > > + if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER) { > + if (ddr_perf_is_filtered(event)) { > + /* revert axi id masking(axi_mask) value */ > + cfg1 ^= AXI_MASKING_REVERT; > + writel(cfg1, pmu->base + COUNTER_MASK_COMP + ((counter - 1) << 4)); Please can you explain what this "reverting" is doing? It looks like a user-visible change in behaviour to me, or are you saying that the driver currently does the wrong thing on hardware that supports the new filter? > + > + if (cfg == 0x41) { What is this 0x41 for? Will
Hi Will, > On Wed, Oct 11, 2023 at 02:08:34PM +0800, Xu Yang wrote: > > This is the extension of AXI ID filter. > > > > Filter is defined with 2 configuration registers per counter 1-3 (counter > > 0 is not used for filtering and lacks these registers). > > * Counter N MASK COMP register - AXI_ID and AXI_MASKING. > > * Counter N MUX CNTL register - AXI CHANNEL and AXI PORT. > > -- 0: address channel > > -- 1: data channel > > > > This filter is exposed to userspace as an additional (channel, port) pair. > > The definition of axi_channel is inverted in userspace, and it will be > > reverted in driver automatically. > > > > AXI filter of Perf Monitor in DDR Subsystem, only a single port0 exist, so > > axi_port is reserved which should be 0. > > > > e.g. > > perf stat -a -e imx8_ddr0/axid-read,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > > perf stat -a -e imx8_ddr0/axid-write,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > > > --- > > Changes since v2: > > - no changes > > --- > > drivers/perf/fsl_imx8_ddr_perf.c | 39 ++++++++++++++++++++++++++++++++ > > 1 file changed, 39 insertions(+) > > > > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c > > index 92611c98120f..d0eae2d7e64b 100644 > > --- a/drivers/perf/fsl_imx8_ddr_perf.c > > +++ b/drivers/perf/fsl_imx8_ddr_perf.c > > @@ -19,6 +19,8 @@ > > #define COUNTER_READ 0x20 > > > > #define COUNTER_DPCR1 0x30 > > +#define COUNTER_MUX_CNTL 0x50 > > +#define COUNTER_MASK_COMP 0x54 > > > > #define CNTL_OVER 0x1 > > #define CNTL_CLEAR 0x2 > > @@ -32,6 +34,13 @@ > > #define CNTL_CSV_SHIFT 24 > > #define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT) > > > > +#define READ_PORT_SHIFT 0 > > +#define READ_PORT_MASK (0x7 << READ_PORT_SHIFT) > > +#define READ_CHANNEL_REVERT 0x00000008 /* bit 3 for read channel select */ > > +#define WRITE_PORT_SHIFT 8 > > +#define WRITE_PORT_MASK (0x7 << WRITE_PORT_SHIFT) > > +#define WRITE_CHANNEL_REVERT 0x00000800 /* bit 11 for write channel select */ > > + > > #define EVENT_CYCLES_ID 0 > > #define EVENT_CYCLES_COUNTER 0 > > #define NUM_COUNTERS 4 > > @@ -50,6 +59,7 @@ 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_ENHANCED 0x3 /* support enhanced AXI ID filter */ > > +#define DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER 0x4 /* support AXI ID PORT CHANNEL filter */ > > > > struct fsl_ddr_devtype_data { > > unsigned int quirks; /* quirks needed for different DDR Perf core */ > > @@ -144,6 +154,7 @@ static const struct attribute_group ddr_perf_identifier_attr_group = { > > enum ddr_perf_filter_capabilities { > > PERF_CAP_AXI_ID_FILTER = 0, > > PERF_CAP_AXI_ID_FILTER_ENHANCED, > > + PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER, > > PERF_CAP_AXI_ID_FEAT_MAX, > > }; > > > > @@ -157,6 +168,8 @@ static u32 ddr_perf_filter_cap_get(struct ddr_pmu *pmu, int cap) > > case PERF_CAP_AXI_ID_FILTER_ENHANCED: > > quirks &= DDR_CAP_AXI_ID_FILTER_ENHANCED; > > return quirks == DDR_CAP_AXI_ID_FILTER_ENHANCED; > > + case PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER: > > + return !!(quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER); > > default: > > WARN(1, "unknown filter cap %d\n", cap); > > } > > @@ -187,6 +200,7 @@ static ssize_t ddr_perf_filter_cap_show(struct device *dev, > > static struct attribute *ddr_perf_filter_cap_attr[] = { > > PERF_FILTER_EXT_ATTR_ENTRY(filter, PERF_CAP_AXI_ID_FILTER), > > PERF_FILTER_EXT_ATTR_ENTRY(enhanced_filter, PERF_CAP_AXI_ID_FILTER_ENHANCED), > > + PERF_FILTER_EXT_ATTR_ENTRY(super_filter, PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER), > > NULL, > > }; > > > > @@ -272,11 +286,15 @@ static const struct attribute_group ddr_perf_events_attr_group = { > > PMU_FORMAT_ATTR(event, "config:0-7"); > > PMU_FORMAT_ATTR(axi_id, "config1:0-15"); > > PMU_FORMAT_ATTR(axi_mask, "config1:16-31"); > > +PMU_FORMAT_ATTR(axi_port, "config2:0-2"); > > +PMU_FORMAT_ATTR(axi_channel, "config2:3-3"); > > Any specific reason to allocate from config2, rather than the upper 32 > bits of config1? No. Either way is ok for me. > > > @@ -553,6 +572,26 @@ static int ddr_perf_event_add(struct perf_event *event, int flags) > > return -EOPNOTSUPP; > > } > > > > + if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER) { > > + if (ddr_perf_is_filtered(event)) { > > + /* revert axi id masking(axi_mask) value */ > > + cfg1 ^= AXI_MASKING_REVERT; > > + writel(cfg1, pmu->base + COUNTER_MASK_COMP + ((counter - 1) << 4)); > > Please can you explain what this "reverting" is doing? It looks like a > user-visible change in behaviour to me, or are you saying that the driver > currently does the wrong thing on hardware that supports the new filter? In sys/metrics.json table as below, "MetricExpr": "imx8_ddr0@axid\\-read\\,axi_mask\\=0x0000\\,axi_id\\=0x0065@", We have set axi_mask to 0x0000 for most of the metics and assume that bit 0 is used for masking. In hardware register, bit 1 is used for masking axi id. So there exists a reverting operation. It also works for me to set actual axi mask value in sys/metrics.json. But, because this patch is just a supplement for filter, and previous axi filter already use a reverting operation for filter, so I did the same thing there. > > > + > > + if (cfg == 0x41) { > > What is this 0x41 for? IMX8_DDR_PMU_EVENT_ATTR(axid-read, 0x41), This 0x41 means axi read filter event. Thanks, Xu Yang > > Will
Hi Will, > > Hi Will, > > > On Wed, Oct 11, 2023 at 02:08:34PM +0800, Xu Yang wrote: > > > This is the extension of AXI ID filter. > > > > > > Filter is defined with 2 configuration registers per counter 1-3 (counter > > > 0 is not used for filtering and lacks these registers). > > > * Counter N MASK COMP register - AXI_ID and AXI_MASKING. > > > * Counter N MUX CNTL register - AXI CHANNEL and AXI PORT. > > > -- 0: address channel > > > -- 1: data channel > > > > > > This filter is exposed to userspace as an additional (channel, port) pair. > > > The definition of axi_channel is inverted in userspace, and it will be > > > reverted in driver automatically. > > > > > > AXI filter of Perf Monitor in DDR Subsystem, only a single port0 exist, so > > > axi_port is reserved which should be 0. > > > > > > e.g. > > > perf stat -a -e imx8_ddr0/axid-read,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > > > perf stat -a -e imx8_ddr0/axid-write,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd > > > > > > Signed-off-by: Xu Yang <xu.yang_2@nxp.com> > > > > > > --- > > > Changes since v2: > > > - no changes > > > --- > > > drivers/perf/fsl_imx8_ddr_perf.c | 39 ++++++++++++++++++++++++++++++++ > > > 1 file changed, 39 insertions(+) > > > > > > diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c > > > index 92611c98120f..d0eae2d7e64b 100644 > > > --- a/drivers/perf/fsl_imx8_ddr_perf.c > > > +++ b/drivers/perf/fsl_imx8_ddr_perf.c > > > @@ -19,6 +19,8 @@ > > > #define COUNTER_READ 0x20 > > > > > > #define COUNTER_DPCR1 0x30 > > > +#define COUNTER_MUX_CNTL 0x50 > > > +#define COUNTER_MASK_COMP 0x54 > > > > > > #define CNTL_OVER 0x1 > > > #define CNTL_CLEAR 0x2 > > > @@ -32,6 +34,13 @@ > > > #define CNTL_CSV_SHIFT 24 > > > #define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT) > > > > > > +#define READ_PORT_SHIFT 0 > > > +#define READ_PORT_MASK (0x7 << READ_PORT_SHIFT) > > > +#define READ_CHANNEL_REVERT 0x00000008 /* bit 3 for read channel select */ > > > +#define WRITE_PORT_SHIFT 8 > > > +#define WRITE_PORT_MASK (0x7 << WRITE_PORT_SHIFT) > > > +#define WRITE_CHANNEL_REVERT 0x00000800 /* bit 11 for write channel select */ > > > + > > > #define EVENT_CYCLES_ID 0 > > > #define EVENT_CYCLES_COUNTER 0 > > > #define NUM_COUNTERS 4 > > > @@ -50,6 +59,7 @@ 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_ENHANCED 0x3 /* support enhanced AXI ID filter */ > > > +#define DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER 0x4 /* support AXI ID PORT CHANNEL filter */ > > > > > > struct fsl_ddr_devtype_data { > > > unsigned int quirks; /* quirks needed for different DDR Perf core */ > > > @@ -144,6 +154,7 @@ static const struct attribute_group ddr_perf_identifier_attr_group = { > > > enum ddr_perf_filter_capabilities { > > > PERF_CAP_AXI_ID_FILTER = 0, > > > PERF_CAP_AXI_ID_FILTER_ENHANCED, > > > + PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER, > > > PERF_CAP_AXI_ID_FEAT_MAX, > > > }; > > > > > > @@ -157,6 +168,8 @@ static u32 ddr_perf_filter_cap_get(struct ddr_pmu *pmu, int cap) > > > case PERF_CAP_AXI_ID_FILTER_ENHANCED: > > > quirks &= DDR_CAP_AXI_ID_FILTER_ENHANCED; > > > return quirks == DDR_CAP_AXI_ID_FILTER_ENHANCED; > > > + case PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER: > > > + return !!(quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER); > > > default: > > > WARN(1, "unknown filter cap %d\n", cap); > > > } > > > @@ -187,6 +200,7 @@ static ssize_t ddr_perf_filter_cap_show(struct device *dev, > > > static struct attribute *ddr_perf_filter_cap_attr[] = { > > > PERF_FILTER_EXT_ATTR_ENTRY(filter, PERF_CAP_AXI_ID_FILTER), > > > PERF_FILTER_EXT_ATTR_ENTRY(enhanced_filter, PERF_CAP_AXI_ID_FILTER_ENHANCED), > > > + PERF_FILTER_EXT_ATTR_ENTRY(super_filter, PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER), > > > NULL, > > > }; > > > > > > @@ -272,11 +286,15 @@ static const struct attribute_group ddr_perf_events_attr_group = { > > > PMU_FORMAT_ATTR(event, "config:0-7"); > > > PMU_FORMAT_ATTR(axi_id, "config1:0-15"); > > > PMU_FORMAT_ATTR(axi_mask, "config1:16-31"); > > > +PMU_FORMAT_ATTR(axi_port, "config2:0-2"); > > > +PMU_FORMAT_ATTR(axi_channel, "config2:3-3"); > > > > Any specific reason to allocate from config2, rather than the upper 32 > > bits of config1? > > No. Either way is ok for me. > > > > > > @@ -553,6 +572,26 @@ static int ddr_perf_event_add(struct perf_event *event, int flags) > > > return -EOPNOTSUPP; > > > } > > > > > > + if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER) { > > > + if (ddr_perf_is_filtered(event)) { > > > + /* revert axi id masking(axi_mask) value */ > > > + cfg1 ^= AXI_MASKING_REVERT; > > > + writel(cfg1, pmu->base + COUNTER_MASK_COMP + ((counter - 1) << 4)); > > > > Please can you explain what this "reverting" is doing? It looks like a > > user-visible change in behaviour to me, or are you saying that the driver > > currently does the wrong thing on hardware that supports the new filter? > > In sys/metrics.json table as below, > > "MetricExpr": "imx8_ddr0@axid\\-read\\,axi_mask\\=0x0000\\,axi_id\\=0x0065@", > > We have set axi_mask to 0x0000 for most of the metics and assume that > bit 0 is used for masking. In hardware register, bit 1 is used for masking > axi id. So there exists a reverting operation. It also works for me to > set actual axi mask value in sys/metrics.json. But, because this patch > is just a supplement for filter, and previous axi filter already use a > reverting operation for filter, so I did the same thing there. > > > > > > + > > > + if (cfg == 0x41) { > > > > What is this 0x41 for? > > IMX8_DDR_PMU_EVENT_ATTR(axid-read, 0x41), > This 0x41 means axi read filter event. > > Thanks, > Xu Yang > > > > > Will Do these patches still need optimization? Thanks, Xu Yang
diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c index 92611c98120f..d0eae2d7e64b 100644 --- a/drivers/perf/fsl_imx8_ddr_perf.c +++ b/drivers/perf/fsl_imx8_ddr_perf.c @@ -19,6 +19,8 @@ #define COUNTER_READ 0x20 #define COUNTER_DPCR1 0x30 +#define COUNTER_MUX_CNTL 0x50 +#define COUNTER_MASK_COMP 0x54 #define CNTL_OVER 0x1 #define CNTL_CLEAR 0x2 @@ -32,6 +34,13 @@ #define CNTL_CSV_SHIFT 24 #define CNTL_CSV_MASK (0xFFU << CNTL_CSV_SHIFT) +#define READ_PORT_SHIFT 0 +#define READ_PORT_MASK (0x7 << READ_PORT_SHIFT) +#define READ_CHANNEL_REVERT 0x00000008 /* bit 3 for read channel select */ +#define WRITE_PORT_SHIFT 8 +#define WRITE_PORT_MASK (0x7 << WRITE_PORT_SHIFT) +#define WRITE_CHANNEL_REVERT 0x00000800 /* bit 11 for write channel select */ + #define EVENT_CYCLES_ID 0 #define EVENT_CYCLES_COUNTER 0 #define NUM_COUNTERS 4 @@ -50,6 +59,7 @@ 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_ENHANCED 0x3 /* support enhanced AXI ID filter */ +#define DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER 0x4 /* support AXI ID PORT CHANNEL filter */ struct fsl_ddr_devtype_data { unsigned int quirks; /* quirks needed for different DDR Perf core */ @@ -144,6 +154,7 @@ static const struct attribute_group ddr_perf_identifier_attr_group = { enum ddr_perf_filter_capabilities { PERF_CAP_AXI_ID_FILTER = 0, PERF_CAP_AXI_ID_FILTER_ENHANCED, + PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER, PERF_CAP_AXI_ID_FEAT_MAX, }; @@ -157,6 +168,8 @@ static u32 ddr_perf_filter_cap_get(struct ddr_pmu *pmu, int cap) case PERF_CAP_AXI_ID_FILTER_ENHANCED: quirks &= DDR_CAP_AXI_ID_FILTER_ENHANCED; return quirks == DDR_CAP_AXI_ID_FILTER_ENHANCED; + case PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER: + return !!(quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER); default: WARN(1, "unknown filter cap %d\n", cap); } @@ -187,6 +200,7 @@ static ssize_t ddr_perf_filter_cap_show(struct device *dev, static struct attribute *ddr_perf_filter_cap_attr[] = { PERF_FILTER_EXT_ATTR_ENTRY(filter, PERF_CAP_AXI_ID_FILTER), PERF_FILTER_EXT_ATTR_ENTRY(enhanced_filter, PERF_CAP_AXI_ID_FILTER_ENHANCED), + PERF_FILTER_EXT_ATTR_ENTRY(super_filter, PERF_CAP_AXI_ID_PORT_CHANNEL_FILTER), NULL, }; @@ -272,11 +286,15 @@ static const struct attribute_group ddr_perf_events_attr_group = { PMU_FORMAT_ATTR(event, "config:0-7"); PMU_FORMAT_ATTR(axi_id, "config1:0-15"); PMU_FORMAT_ATTR(axi_mask, "config1:16-31"); +PMU_FORMAT_ATTR(axi_port, "config2:0-2"); +PMU_FORMAT_ATTR(axi_channel, "config2:3-3"); static struct attribute *ddr_perf_format_attrs[] = { &format_attr_event.attr, &format_attr_axi_id.attr, &format_attr_axi_mask.attr, + &format_attr_axi_port.attr, + &format_attr_axi_channel.attr, NULL, }; @@ -530,6 +548,7 @@ static int ddr_perf_event_add(struct perf_event *event, int flags) int counter; int cfg = event->attr.config; int cfg1 = event->attr.config1; + int cfg2 = event->attr.config2; if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_FILTER) { int i; @@ -553,6 +572,26 @@ static int ddr_perf_event_add(struct perf_event *event, int flags) return -EOPNOTSUPP; } + if (pmu->devtype_data->quirks & DDR_CAP_AXI_ID_PORT_CHANNEL_FILTER) { + if (ddr_perf_is_filtered(event)) { + /* revert axi id masking(axi_mask) value */ + cfg1 ^= AXI_MASKING_REVERT; + writel(cfg1, pmu->base + COUNTER_MASK_COMP + ((counter - 1) << 4)); + + if (cfg == 0x41) { + /* revert axi read channel(axi_channel) value */ + cfg2 ^= READ_CHANNEL_REVERT; + cfg2 |= FIELD_PREP(READ_PORT_MASK, cfg2); + } else { + /* revert axi write channel(axi_channel) value */ + cfg2 ^= WRITE_CHANNEL_REVERT; + cfg2 |= FIELD_PREP(WRITE_PORT_MASK, cfg2); + } + + writel(cfg2, pmu->base + COUNTER_MUX_CNTL + ((counter - 1) << 4)); + } + } + pmu->events[counter] = event; hwc->idx = counter;
This is the extension of AXI ID filter. Filter is defined with 2 configuration registers per counter 1-3 (counter 0 is not used for filtering and lacks these registers). * Counter N MASK COMP register - AXI_ID and AXI_MASKING. * Counter N MUX CNTL register - AXI CHANNEL and AXI PORT. -- 0: address channel -- 1: data channel This filter is exposed to userspace as an additional (channel, port) pair. The definition of axi_channel is inverted in userspace, and it will be reverted in driver automatically. AXI filter of Perf Monitor in DDR Subsystem, only a single port0 exist, so axi_port is reserved which should be 0. e.g. perf stat -a -e imx8_ddr0/axid-read,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd perf stat -a -e imx8_ddr0/axid-write,axi_mask=0xMMMM,axi_id=0xDDDD,axi_channel=0xH/ cmd Signed-off-by: Xu Yang <xu.yang_2@nxp.com> --- Changes since v2: - no changes --- drivers/perf/fsl_imx8_ddr_perf.c | 39 ++++++++++++++++++++++++++++++++ 1 file changed, 39 insertions(+)