* [PATCH V5 1/4] dt-bindings: perf: imx8-ddr: add imx8qxp ddr performance monitor
@ 2019-04-16 20:39 ` Frank Li
0 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Added binding doc for imx8qxp ddr performance monitor
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No change from v4 to v5
Change from v4 to v4
* remove "standard xxx"
Change from v2 to v3
* ddr_pmu0 -> ddr-pmu
.../devicetree/bindings/perf/fsl-imx-ddr.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
diff --git a/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
new file mode 100644
index 0000000..c667b87
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
@@ -0,0 +1,22 @@
+* Freescale(NXP) IMX8 DDR performance monitor
+
+Required properties:
+
+- compatible: should be one of:
+ "fsl,imx8-ddr-pmu"
+ "fsl,imx8m-ddr-pmu"
+
+- reg: physical address and size
+
+- interrupts: single interrupt
+ generated by the control block
+
+Example:
+
+ ddr-pmu@5c020000 {
+ compatible = "fsl,imx8-ddr-pmu";
+ reg = <0x0 0x5c020000 0x0 0x10000>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
--
2.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 1/4] dt-bindings: perf: imx8-ddr: add imx8qxp ddr performance monitor
@ 2019-04-16 20:39 ` Frank Li
0 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Added binding doc for imx8qxp ddr performance monitor
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Rob Herring <robh@kernel.org>
---
No change from v4 to v5
Change from v4 to v4
* remove "standard xxx"
Change from v2 to v3
* ddr_pmu0 -> ddr-pmu
.../devicetree/bindings/perf/fsl-imx-ddr.txt | 22 ++++++++++++++++++++++
1 file changed, 22 insertions(+)
create mode 100644 Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
diff --git a/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
new file mode 100644
index 0000000..c667b87
--- /dev/null
+++ b/Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
@@ -0,0 +1,22 @@
+* Freescale(NXP) IMX8 DDR performance monitor
+
+Required properties:
+
+- compatible: should be one of:
+ "fsl,imx8-ddr-pmu"
+ "fsl,imx8m-ddr-pmu"
+
+- reg: physical address and size
+
+- interrupts: single interrupt
+ generated by the control block
+
+Example:
+
+ ddr-pmu@5c020000 {
+ compatible = "fsl,imx8-ddr-pmu";
+ reg = <0x0 0x5c020000 0x0 0x10000>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
--
2.5.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
2019-04-16 20:39 ` Frank Li
@ 2019-04-16 20:39 ` Frank Li
-1 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Add ddr performance monitor support for iMX8QXP
There are 4 counters for ddr perfomance events.
counter 0 is dedicated for cycles.
you choose any up to 3 no cycles events.
for example:
perf stat -a -e ddr0/read-cycles/,ddr0/write-cycles/,ddr0/precharge/ ls
perf stat -a -e ddr0/cycles/,ddr0/read-access/,ddr0/write-access/ ls
Support below events.
ddr0/activate/ [Kernel PMU event]
ddr0/cycles/ [Kernel PMU event]
ddr0/hp-read-credit-cnt/ [Kernel PMU event]
ddr0/hp-read/ [Kernel PMU event]
ddr0/hp-req-nodcredit/ [Kernel PMU event]
ddr0/hp-xact-credit/ [Kernel PMU event]
ddr0/load-mode/ [Kernel PMU event]
ddr0/lp-read-credit-cnt/ [Kernel PMU event]
ddr0/lp-req-nocredit/ [Kernel PMU event]
ddr0/lp-xact-credit/ [Kernel PMU event]
ddr0/mwr/ [Kernel PMU event]
ddr0/precharge/ [Kernel PMU event]
ddr0/raw-hazard/ [Kernel PMU event]
ddr0/read-access/ [Kernel PMU event]
ddr0/read-activate/ [Kernel PMU event]
ddr0/read-command/ [Kernel PMU event]
ddr0/read-cycles/ [Kernel PMU event]
ddr0/read-modify-write-command/ [Kernel PMU event]
ddr0/read-queue-depth/ [Kernel PMU event]
ddr0/read-write-transition/ [Kernel PMU event]
ddr0/read/ [Kernel PMU event]
ddr0/refresh/ [Kernel PMU event]
ddr0/selfresh/ [Kernel PMU event]
ddr0/wr-xact-credit/ [Kernel PMU event]
ddr0/write-access/ [Kernel PMU event]
ddr0/write-command/ [Kernel PMU event]
ddr0/write-credit-cnt/ [Kernel PMU event]
ddr0/write-cycles/ [Kernel PMU event]
ddr0/write-queue-depth/ [Kernel PMU event]
ddr0/write/
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v4 to v5
* Remove AXI ID filter function
Change from v3 to v4
* Change FSL_IMX8_DDR_PERF to FSL_IMX8_DDR_PMU
* sort include
* remove struct fsl_ddr_devtype_data
* Added comment need disable control first
* Added comment about must enable cycle counter
* Added macro for EVENT_AXI_READ, remove hardcode 0x41 and 0x42
* Added comment about cycle counter is fastest one
Change from v2 to v3
* remove kfree
Change from V1 to V2
* update Kconfig by use i.MX8 instead of i.MX8 QXP
* remove gpl statememnt since SPDX tag
* use dev_kzalloc
* use dev_err
* commit message show axi_read 0x41\axi_write 0x42
* commit message show cycles must be enabled
* Irq only issue at cycles overflow
* use NUM_COUNTER
* use devm_request_irq
* add hotplug callback to handle context migration
drivers/perf/Kconfig | 7 +
drivers/perf/Makefile | 1 +
drivers/perf/fsl_imx8_ddr_perf.c | 553 +++++++++++++++++++++++++++++++++++++++
3 files changed, 561 insertions(+)
create mode 100644 drivers/perf/fsl_imx8_ddr_perf.c
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index a94e586..9bc3785 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -70,6 +70,13 @@ config ARM_DSU_PMU
system, control logic. The PMU allows counting various events related
to DSU.
+config FSL_IMX8_DDR_PMU
+ tristate "Freescale i.MX8 DDR perf monitor"
+ depends on ARCH_MXC
+ help
+ Provides support for ddr perfomance monitor in i.MX8. Provide memory
+ througput information.
+
config HISI_PMU
bool "HiSilicon SoC PMU"
depends on ARM64 && ACPI
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index 3048994..2ebb4de 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ARM_DSU_PMU) += arm_dsu_pmu.o
obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o
obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
obj-$(CONFIG_ARM_SMMU_V3_PMU) += arm_smmuv3_pmu.o
+obj-$(CONFIG_FSL_IMX8_DDR_PMU) += fsl_imx8_ddr_perf.o
obj-$(CONFIG_HISI_PMU) += hisilicon/
obj-$(CONFIG_QCOM_L2_PMU) += qcom_l2_pmu.o
obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
new file mode 100644
index 0000000..aed9414
--- /dev/null
+++ b/drivers/perf/fsl_imx8_ddr_perf.c
@@ -0,0 +1,553 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2017 NXP
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ */
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/perf_event.h>
+#include <linux/slab.h>
+
+#define COUNTER_CNTL 0x0
+#define COUNTER_READ 0x20
+
+#define COUNTER_DPCR1 0x30
+
+#define CNTL_OVER 0x1
+#define CNTL_CLEAR 0x2
+#define CNTL_EN 0x4
+#define CNTL_EN_MASK 0xFFFFFFFB
+#define CNTL_CLEAR_MASK 0xFFFFFFFD
+#define CNTL_OVER_MASK 0xFFFFFFFE
+
+#define CNTL_CSV_SHIFT 24
+#define CNTL_CSV_MASK (0xFF << CNTL_CSV_SHIFT)
+
+#define EVENT_CYCLES_ID 0
+#define EVENT_CYCLES_COUNTER 0
+#define NUM_COUNTER 4
+#define MAX_EVENT 3
+#define EVENT_AXI_READ 0x41
+#define EVENT_AXI_WRITE 0x42
+
+#define to_ddr_pmu(p) container_of(p, struct ddr_pmu, pmu)
+
+#define DDR_PERF_DEV_NAME "ddr_perf"
+
+static DEFINE_IDA(ddr_ida);
+
+PMU_EVENT_ATTR_STRING(cycles, ddr_perf_cycles, "event=0x00");
+PMU_EVENT_ATTR_STRING(selfresh, ddr_perf_selfresh, "event=0x01");
+PMU_EVENT_ATTR_STRING(read-access, ddr_perf_read_accesses, "event=0x04");
+PMU_EVENT_ATTR_STRING(write-access, ddr_perf_write_accesses, "event=0x05");
+PMU_EVENT_ATTR_STRING(read-queue-depth, ddr_perf_read_queue_depth,
+ "event=0x08");
+PMU_EVENT_ATTR_STRING(write-queue-depth, ddr_perf_write_queue_depth,
+ "event=0x09");
+PMU_EVENT_ATTR_STRING(lp-read-credit-cnt, ddr_perf_lp_read_credit_cnt,
+ "event=0x10");
+PMU_EVENT_ATTR_STRING(hp-read-credit-cnt, ddr_perf_hp_read_credit_cnt,
+ "event=0x11");
+PMU_EVENT_ATTR_STRING(write-credit-cnt, ddr_perf_write_credit_cnt,
+ "event=0x12");
+PMU_EVENT_ATTR_STRING(read-command, ddr_perf_read_command, "event=0x20");
+PMU_EVENT_ATTR_STRING(write-command, ddr_perf_write_command, "event=0x21");
+PMU_EVENT_ATTR_STRING(read-modify-write-command,
+ ddr_perf_read_modify_write_command, "event=0x22");
+PMU_EVENT_ATTR_STRING(hp-read, ddr_perf_hp_read, "event=0x23");
+PMU_EVENT_ATTR_STRING(hp-req-nodcredit, ddr_perf_hp_req_nocredit, "event=0x24");
+PMU_EVENT_ATTR_STRING(hp-xact-credit, ddr_perf_hp_xact_credit, "event=0x25");
+PMU_EVENT_ATTR_STRING(lp-req-nocredit, ddr_perf_lp_req_nocredit, "event=0x26");
+PMU_EVENT_ATTR_STRING(lp-xact-credit, ddr_perf_lp_xact_credit, "event=0x27");
+PMU_EVENT_ATTR_STRING(wr-xact-credit, ddr_perf_wr_xact_credit, "event=0x29");
+PMU_EVENT_ATTR_STRING(read-cycles, ddr_perf_read_cycles, "event=0x2a");
+PMU_EVENT_ATTR_STRING(write-cycles, ddr_perf_write_cycles, "event=0x2b");
+PMU_EVENT_ATTR_STRING(read-write-transition, ddr_perf_read_write_transition,
+ "event=0x30");
+PMU_EVENT_ATTR_STRING(precharge, ddr_perf_precharge, "event=0x31");
+PMU_EVENT_ATTR_STRING(activate, ddr_perf_activate, "event=0x32");
+PMU_EVENT_ATTR_STRING(load-mode, ddr_perf_load_mode, "event=0x33");
+PMU_EVENT_ATTR_STRING(mwr, ddr_perf_mwr, "event=0x34");
+PMU_EVENT_ATTR_STRING(read, ddr_perf_read, "event=0x35");
+PMU_EVENT_ATTR_STRING(read-activate, ddr_perf_read_activate, "event=0x36");
+PMU_EVENT_ATTR_STRING(refresh, ddr_perf_refresh, "event=0x37");
+PMU_EVENT_ATTR_STRING(write, ddr_perf_write, "event=0x38");
+PMU_EVENT_ATTR_STRING(raw-hazard, ddr_perf_raw_hazard, "event=0x39");
+
+static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
+ { .compatible = "fsl,imx8-ddr-pmu",},
+ { .compatible = "fsl,imx8m-ddr-pmu",},
+ { /* sentinel */ }
+};
+
+struct ddr_pmu {
+ struct pmu pmu;
+ void __iomem *base;
+ cpumask_t cpu;
+ struct hlist_node node;
+ struct device *dev;
+ struct perf_event *active_events[NUM_COUNTER];
+ int total_events;
+ enum cpuhp_state cpuhp_state;
+ bool cycles_active;
+ uintptr_t flags;
+};
+
+static ssize_t ddr_perf_cpumask_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ddr_pmu *pmu = dev_get_drvdata(dev);
+
+ return cpumap_print_to_pagebuf(true, buf, &pmu->cpu);
+}
+
+static struct device_attribute ddr_perf_cpumask_attr =
+ __ATTR(cpumask, 0444, ddr_perf_cpumask_show, NULL);
+
+static struct attribute *ddr_perf_cpumask_attrs[] = {
+ &ddr_perf_cpumask_attr.attr,
+ NULL,
+};
+
+static struct attribute_group ddr_perf_cpumask_attr_group = {
+ .attrs = ddr_perf_cpumask_attrs,
+};
+
+static struct attribute *ddr_perf_events_attrs[] = {
+ &ddr_perf_cycles.attr.attr,
+ &ddr_perf_selfresh.attr.attr,
+ &ddr_perf_read_accesses.attr.attr,
+ &ddr_perf_write_accesses.attr.attr,
+ &ddr_perf_read_queue_depth.attr.attr,
+ &ddr_perf_write_queue_depth.attr.attr,
+ &ddr_perf_lp_read_credit_cnt.attr.attr,
+ &ddr_perf_hp_read_credit_cnt.attr.attr,
+ &ddr_perf_write_credit_cnt.attr.attr,
+ &ddr_perf_read_command.attr.attr,
+ &ddr_perf_write_command.attr.attr,
+ &ddr_perf_read_modify_write_command.attr.attr,
+ &ddr_perf_hp_read.attr.attr,
+ &ddr_perf_hp_req_nocredit.attr.attr,
+ &ddr_perf_hp_xact_credit.attr.attr,
+ &ddr_perf_lp_req_nocredit.attr.attr,
+ &ddr_perf_lp_xact_credit.attr.attr,
+ &ddr_perf_wr_xact_credit.attr.attr,
+ &ddr_perf_read_cycles.attr.attr,
+ &ddr_perf_write_cycles.attr.attr,
+ &ddr_perf_read_write_transition.attr.attr,
+ &ddr_perf_precharge.attr.attr,
+ &ddr_perf_activate.attr.attr,
+ &ddr_perf_load_mode.attr.attr,
+ &ddr_perf_mwr.attr.attr,
+ &ddr_perf_read.attr.attr,
+ &ddr_perf_read_activate.attr.attr,
+ &ddr_perf_refresh.attr.attr,
+ &ddr_perf_write.attr.attr,
+ &ddr_perf_raw_hazard.attr.attr,
+ NULL,
+};
+
+static struct attribute_group ddr_perf_events_attr_group = {
+ .name = "events",
+ .attrs = ddr_perf_events_attrs,
+};
+
+PMU_FORMAT_ATTR(event, "config:0-63");
+
+static struct attribute *ddr_perf_format_attrs[] = {
+ &format_attr_event.attr,
+ NULL,
+};
+
+static struct attribute_group ddr_perf_format_attr_group = {
+ .name = "format",
+ .attrs = ddr_perf_format_attrs,
+};
+
+static const struct attribute_group *attr_groups[] = {
+ &ddr_perf_events_attr_group,
+ &ddr_perf_format_attr_group,
+ &ddr_perf_cpumask_attr_group,
+ NULL,
+};
+
+static u32 ddr_perf_alloc_counter(struct ddr_pmu *pmu, int event)
+{
+ int i;
+
+ /* Always map cycle event to counter 0
+ Cycles counter is dedicated for cycle event
+ can't used for the other events
+ */
+ if (event == EVENT_CYCLES_ID)
+ return EVENT_CYCLES_COUNTER;
+
+ for (i = 1; i < NUM_COUNTER; i++)
+ if (pmu->active_events[i] == NULL)
+ return i;
+
+ return -ENOENT;
+}
+
+static u32 ddr_perf_free_counter(struct ddr_pmu *pmu, int counter)
+{
+ if (counter < 0 || counter >= NUM_COUNTER)
+ return -ENOENT;
+
+ pmu->active_events[counter] = NULL;
+
+ return 0;
+}
+
+static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter)
+{
+ return readl(pmu->base + COUNTER_READ + counter * 4);
+}
+
+static int ddr_perf_event_init(struct perf_event *event)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+ return -EOPNOTSUPP;
+
+ if (event->cpu < 0) {
+ dev_warn(pmu->dev, "Can't provide per-task data!\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (event->attr.exclude_user ||
+ event->attr.exclude_kernel ||
+ event->attr.exclude_hv ||
+ event->attr.exclude_idle ||
+ event->attr.exclude_host ||
+ event->attr.exclude_guest ||
+ event->attr.sample_period)
+ return -EINVAL;
+
+ event->cpu = cpumask_first(&pmu->cpu);
+ hwc->idx = -1;
+
+ return 0;
+}
+
+
+static void ddr_perf_event_update(struct perf_event *event)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ u64 delta, prev_raw_count, new_raw_count;
+ int counter = hwc->idx;
+
+ do {
+ prev_raw_count = local64_read(&hwc->prev_count);
+ new_raw_count = ddr_perf_read_counter(pmu, counter);
+ } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
+ new_raw_count) != prev_raw_count);
+
+ delta = (new_raw_count - prev_raw_count) & 0xFFFFFFFF;
+
+ local64_add(delta, &event->count);
+}
+
+static void ddr_perf_event_enable(struct ddr_pmu *pmu, int config,
+ int counter, bool enable)
+{
+ u8 reg = counter * 4 + COUNTER_CNTL;
+ int val;
+
+ if (enable) {
+ /* must disable first, then enable again
+ * otherwise, cycle counter will not work
+ * if previous state is enabled.
+ */
+ writel(0, pmu->base + reg);
+ val = CNTL_EN | CNTL_CLEAR;
+ val |= (config << CNTL_CSV_SHIFT) & CNTL_CSV_MASK;
+ } else {
+ /* Disable counter */
+ val = readl(pmu->base + reg) & CNTL_EN_MASK;
+ }
+
+ writel(val, pmu->base + reg);
+
+ if (config == EVENT_CYCLES_ID)
+ pmu->cycles_active = enable;
+}
+
+static void ddr_perf_event_start(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter = hwc->idx;
+
+ local64_set(&hwc->prev_count, 0);
+
+ ddr_perf_event_enable(pmu, event->attr.config, counter, true);
+ /*
+ * If the cycles counter wasn't explicitly selected,
+ * we will enable it now.
+ * cycles counter must be enabled otherwise other counters will
+ * stopped.
+ */
+ if (counter > 0 && !pmu->cycles_active)
+ ddr_perf_event_enable(pmu, EVENT_CYCLES_ID,
+ EVENT_CYCLES_COUNTER, true);
+}
+
+static int ddr_perf_event_add(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter;
+ int cfg = event->attr.config;
+
+ counter = ddr_perf_alloc_counter(pmu, cfg);
+ if (counter < 0) {
+ dev_dbg(pmu->dev, "There are not enough counters\n");
+ return -EOPNOTSUPP;
+ }
+
+ pmu->active_events[counter] = event;
+ pmu->total_events++;
+ hwc->idx = counter;
+
+ if (flags & PERF_EF_START)
+ ddr_perf_event_start(event, flags);
+
+ local64_set(&hwc->prev_count, ddr_perf_read_counter(pmu, counter));
+
+ return 0;
+}
+
+static void ddr_perf_event_stop(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter = hwc->idx;
+
+ ddr_perf_event_enable(pmu, event->attr.config, counter, false);
+ ddr_perf_event_update(event);
+}
+
+static void ddr_perf_event_del(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter = hwc->idx;
+
+ ddr_perf_event_stop(event, PERF_EF_UPDATE);
+
+ ddr_perf_free_counter(pmu, counter);
+ pmu->total_events--;
+ hwc->idx = -1;
+
+ /* If all events have stopped, stop the cycles counter as well */
+ if ((pmu->total_events == 0) && pmu->cycles_active)
+ ddr_perf_event_enable(pmu, EVENT_CYCLES_ID,
+ EVENT_CYCLES_COUNTER, false);
+}
+
+static int ddr_perf_init(struct ddr_pmu *pmu, void __iomem *base,
+ struct device *dev)
+{
+ *pmu = (struct ddr_pmu) {
+ .pmu = (struct pmu) {
+ .task_ctx_nr = perf_invalid_context,
+ .attr_groups = attr_groups,
+ .event_init = ddr_perf_event_init,
+ .add = ddr_perf_event_add,
+ .del = ddr_perf_event_del,
+ .start = ddr_perf_event_start,
+ .stop = ddr_perf_event_stop,
+ .read = ddr_perf_event_update,
+ },
+ .base = base,
+ .dev = dev,
+ };
+
+ return ida_simple_get(&ddr_ida, 0, 0, GFP_KERNEL);
+}
+
+static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
+{
+ int i;
+ u8 reg;
+ int val;
+ int counter;
+ struct ddr_pmu *pmu = (struct ddr_pmu *) p;
+ struct perf_event *event;
+
+ /* Only cycles counter overflowed can issue irq. all counters will
+ * be stopped when cycles counter overflow. but other counter don't stop
+ * when overflow happen. Update all of the local counter values,
+ * then reset the cycles counter, so the others can continue
+ * counting. cycles counter is fastest counter in all events. at last
+ * 4 times than other counters.
+ */
+ for (i = 0; i < NUM_COUNTER; i++) {
+
+ if (!pmu->active_events[i])
+ continue;
+
+ event = pmu->active_events[i];
+ counter = event->hw.idx;
+ reg = counter * 4 + COUNTER_CNTL;
+ val = readl(pmu->base + reg);
+
+ ddr_perf_event_update(event);
+
+ if (counter == EVENT_CYCLES_COUNTER) {
+ ddr_perf_event_enable(pmu,
+ EVENT_CYCLES_ID,
+ EVENT_CYCLES_COUNTER,
+ true);
+ ddr_perf_event_update(event);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int ddr_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
+{
+ struct ddr_pmu *pmu = hlist_entry_safe(node, struct ddr_pmu, node);
+ int target;
+
+ if (!cpumask_test_and_clear_cpu(cpu, &pmu->cpu))
+ return 0;
+
+ target = cpumask_any_but(cpu_online_mask, cpu);
+ if (target >= nr_cpu_ids)
+ return 0;
+
+ perf_pmu_migrate_context(&pmu->pmu, cpu, target);
+ cpumask_set_cpu(target, &pmu->cpu);
+
+ return 0;
+}
+
+static int ddr_perf_probe(struct platform_device *pdev)
+{
+ struct ddr_pmu *pmu;
+ struct device_node *np;
+ void __iomem *base;
+ struct resource *iomem;
+ char *name;
+ char *hpname;
+ int num;
+ int ret;
+ u32 irq;
+ const struct of_device_id *of_id =
+ of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
+
+ iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, iomem);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ np = pdev->dev.of_node;
+
+ pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
+ if (!pmu)
+ return -ENOMEM;
+
+ num = ddr_perf_init(pmu, base, &pdev->dev);
+
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
+ if (!name)
+ return -ENOMEM;
+
+ hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+ "perf/imx/ddr%d:online", num);
+ if (!hpname)
+ return -ENOMEM;
+
+ pmu->flags = (uintptr_t) of_id->data;
+
+ cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
+ ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
+ ddr_perf_offline_cpu);
+
+ if (ret < 0) {
+ dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
+ goto ddr_perf_err;
+ }
+
+ pmu->cpuhp_state = ret;
+
+ /* Register the pmu instance for cpu hotplug */
+ cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+
+ ret = perf_pmu_register(&(pmu->pmu), name, -1);
+ if (ret)
+ goto ddr_perf_err;
+
+ /* Request irq */
+ irq = of_irq_get(np, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "Failed to get irq: %d", irq);
+ ret = irq;
+ goto ddr_perf_irq_err;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq,
+ ddr_perf_irq_handler,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ DDR_PERF_DEV_NAME,
+ pmu);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Request irq failed: %d", ret);
+ goto ddr_perf_irq_err;
+ }
+
+ return 0;
+
+ddr_perf_irq_err:
+ perf_pmu_unregister(&(pmu->pmu));
+
+ddr_perf_err:
+ if (pmu->cpuhp_state)
+ cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+
+ dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret);
+ return ret;
+}
+
+static int ddr_perf_remove(struct platform_device *pdev)
+{
+ struct ddr_pmu *pmu = platform_get_drvdata(pdev);
+
+ cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+ perf_pmu_unregister(&pmu->pmu);
+
+ return 0;
+}
+
+static struct platform_driver imx_ddr_pmu_driver = {
+ .driver = {
+ .name = "imx-ddr-pmu",
+ .of_match_table = imx_ddr_pmu_dt_ids,
+ },
+ .probe = ddr_perf_probe,
+ .remove = ddr_perf_remove,
+};
+
+static int __init imx_ddr_pmu_init(void)
+{
+ return platform_driver_register(&imx_ddr_pmu_driver);
+}
+
+module_init(imx_ddr_pmu_init);
+
--
2.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
@ 2019-04-16 20:39 ` Frank Li
0 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Add ddr performance monitor support for iMX8QXP
There are 4 counters for ddr perfomance events.
counter 0 is dedicated for cycles.
you choose any up to 3 no cycles events.
for example:
perf stat -a -e ddr0/read-cycles/,ddr0/write-cycles/,ddr0/precharge/ ls
perf stat -a -e ddr0/cycles/,ddr0/read-access/,ddr0/write-access/ ls
Support below events.
ddr0/activate/ [Kernel PMU event]
ddr0/cycles/ [Kernel PMU event]
ddr0/hp-read-credit-cnt/ [Kernel PMU event]
ddr0/hp-read/ [Kernel PMU event]
ddr0/hp-req-nodcredit/ [Kernel PMU event]
ddr0/hp-xact-credit/ [Kernel PMU event]
ddr0/load-mode/ [Kernel PMU event]
ddr0/lp-read-credit-cnt/ [Kernel PMU event]
ddr0/lp-req-nocredit/ [Kernel PMU event]
ddr0/lp-xact-credit/ [Kernel PMU event]
ddr0/mwr/ [Kernel PMU event]
ddr0/precharge/ [Kernel PMU event]
ddr0/raw-hazard/ [Kernel PMU event]
ddr0/read-access/ [Kernel PMU event]
ddr0/read-activate/ [Kernel PMU event]
ddr0/read-command/ [Kernel PMU event]
ddr0/read-cycles/ [Kernel PMU event]
ddr0/read-modify-write-command/ [Kernel PMU event]
ddr0/read-queue-depth/ [Kernel PMU event]
ddr0/read-write-transition/ [Kernel PMU event]
ddr0/read/ [Kernel PMU event]
ddr0/refresh/ [Kernel PMU event]
ddr0/selfresh/ [Kernel PMU event]
ddr0/wr-xact-credit/ [Kernel PMU event]
ddr0/write-access/ [Kernel PMU event]
ddr0/write-command/ [Kernel PMU event]
ddr0/write-credit-cnt/ [Kernel PMU event]
ddr0/write-cycles/ [Kernel PMU event]
ddr0/write-queue-depth/ [Kernel PMU event]
ddr0/write/
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
Change from v4 to v5
* Remove AXI ID filter function
Change from v3 to v4
* Change FSL_IMX8_DDR_PERF to FSL_IMX8_DDR_PMU
* sort include
* remove struct fsl_ddr_devtype_data
* Added comment need disable control first
* Added comment about must enable cycle counter
* Added macro for EVENT_AXI_READ, remove hardcode 0x41 and 0x42
* Added comment about cycle counter is fastest one
Change from v2 to v3
* remove kfree
Change from V1 to V2
* update Kconfig by use i.MX8 instead of i.MX8 QXP
* remove gpl statememnt since SPDX tag
* use dev_kzalloc
* use dev_err
* commit message show axi_read 0x41\axi_write 0x42
* commit message show cycles must be enabled
* Irq only issue at cycles overflow
* use NUM_COUNTER
* use devm_request_irq
* add hotplug callback to handle context migration
drivers/perf/Kconfig | 7 +
drivers/perf/Makefile | 1 +
drivers/perf/fsl_imx8_ddr_perf.c | 553 +++++++++++++++++++++++++++++++++++++++
3 files changed, 561 insertions(+)
create mode 100644 drivers/perf/fsl_imx8_ddr_perf.c
diff --git a/drivers/perf/Kconfig b/drivers/perf/Kconfig
index a94e586..9bc3785 100644
--- a/drivers/perf/Kconfig
+++ b/drivers/perf/Kconfig
@@ -70,6 +70,13 @@ config ARM_DSU_PMU
system, control logic. The PMU allows counting various events related
to DSU.
+config FSL_IMX8_DDR_PMU
+ tristate "Freescale i.MX8 DDR perf monitor"
+ depends on ARCH_MXC
+ help
+ Provides support for ddr perfomance monitor in i.MX8. Provide memory
+ througput information.
+
config HISI_PMU
bool "HiSilicon SoC PMU"
depends on ARM64 && ACPI
diff --git a/drivers/perf/Makefile b/drivers/perf/Makefile
index 3048994..2ebb4de 100644
--- a/drivers/perf/Makefile
+++ b/drivers/perf/Makefile
@@ -5,6 +5,7 @@ obj-$(CONFIG_ARM_DSU_PMU) += arm_dsu_pmu.o
obj-$(CONFIG_ARM_PMU) += arm_pmu.o arm_pmu_platform.o
obj-$(CONFIG_ARM_PMU_ACPI) += arm_pmu_acpi.o
obj-$(CONFIG_ARM_SMMU_V3_PMU) += arm_smmuv3_pmu.o
+obj-$(CONFIG_FSL_IMX8_DDR_PMU) += fsl_imx8_ddr_perf.o
obj-$(CONFIG_HISI_PMU) += hisilicon/
obj-$(CONFIG_QCOM_L2_PMU) += qcom_l2_pmu.o
obj-$(CONFIG_QCOM_L3_PMU) += qcom_l3_pmu.o
diff --git a/drivers/perf/fsl_imx8_ddr_perf.c b/drivers/perf/fsl_imx8_ddr_perf.c
new file mode 100644
index 0000000..aed9414
--- /dev/null
+++ b/drivers/perf/fsl_imx8_ddr_perf.c
@@ -0,0 +1,553 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright 2017 NXP
+ * Copyright 2016 Freescale Semiconductor, Inc.
+ */
+
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/io.h>
+#include <linux/module.h>
+#include <linux/of.h>
+#include <linux/of_address.h>
+#include <linux/of_device.h>
+#include <linux/of_irq.h>
+#include <linux/perf_event.h>
+#include <linux/slab.h>
+
+#define COUNTER_CNTL 0x0
+#define COUNTER_READ 0x20
+
+#define COUNTER_DPCR1 0x30
+
+#define CNTL_OVER 0x1
+#define CNTL_CLEAR 0x2
+#define CNTL_EN 0x4
+#define CNTL_EN_MASK 0xFFFFFFFB
+#define CNTL_CLEAR_MASK 0xFFFFFFFD
+#define CNTL_OVER_MASK 0xFFFFFFFE
+
+#define CNTL_CSV_SHIFT 24
+#define CNTL_CSV_MASK (0xFF << CNTL_CSV_SHIFT)
+
+#define EVENT_CYCLES_ID 0
+#define EVENT_CYCLES_COUNTER 0
+#define NUM_COUNTER 4
+#define MAX_EVENT 3
+#define EVENT_AXI_READ 0x41
+#define EVENT_AXI_WRITE 0x42
+
+#define to_ddr_pmu(p) container_of(p, struct ddr_pmu, pmu)
+
+#define DDR_PERF_DEV_NAME "ddr_perf"
+
+static DEFINE_IDA(ddr_ida);
+
+PMU_EVENT_ATTR_STRING(cycles, ddr_perf_cycles, "event=0x00");
+PMU_EVENT_ATTR_STRING(selfresh, ddr_perf_selfresh, "event=0x01");
+PMU_EVENT_ATTR_STRING(read-access, ddr_perf_read_accesses, "event=0x04");
+PMU_EVENT_ATTR_STRING(write-access, ddr_perf_write_accesses, "event=0x05");
+PMU_EVENT_ATTR_STRING(read-queue-depth, ddr_perf_read_queue_depth,
+ "event=0x08");
+PMU_EVENT_ATTR_STRING(write-queue-depth, ddr_perf_write_queue_depth,
+ "event=0x09");
+PMU_EVENT_ATTR_STRING(lp-read-credit-cnt, ddr_perf_lp_read_credit_cnt,
+ "event=0x10");
+PMU_EVENT_ATTR_STRING(hp-read-credit-cnt, ddr_perf_hp_read_credit_cnt,
+ "event=0x11");
+PMU_EVENT_ATTR_STRING(write-credit-cnt, ddr_perf_write_credit_cnt,
+ "event=0x12");
+PMU_EVENT_ATTR_STRING(read-command, ddr_perf_read_command, "event=0x20");
+PMU_EVENT_ATTR_STRING(write-command, ddr_perf_write_command, "event=0x21");
+PMU_EVENT_ATTR_STRING(read-modify-write-command,
+ ddr_perf_read_modify_write_command, "event=0x22");
+PMU_EVENT_ATTR_STRING(hp-read, ddr_perf_hp_read, "event=0x23");
+PMU_EVENT_ATTR_STRING(hp-req-nodcredit, ddr_perf_hp_req_nocredit, "event=0x24");
+PMU_EVENT_ATTR_STRING(hp-xact-credit, ddr_perf_hp_xact_credit, "event=0x25");
+PMU_EVENT_ATTR_STRING(lp-req-nocredit, ddr_perf_lp_req_nocredit, "event=0x26");
+PMU_EVENT_ATTR_STRING(lp-xact-credit, ddr_perf_lp_xact_credit, "event=0x27");
+PMU_EVENT_ATTR_STRING(wr-xact-credit, ddr_perf_wr_xact_credit, "event=0x29");
+PMU_EVENT_ATTR_STRING(read-cycles, ddr_perf_read_cycles, "event=0x2a");
+PMU_EVENT_ATTR_STRING(write-cycles, ddr_perf_write_cycles, "event=0x2b");
+PMU_EVENT_ATTR_STRING(read-write-transition, ddr_perf_read_write_transition,
+ "event=0x30");
+PMU_EVENT_ATTR_STRING(precharge, ddr_perf_precharge, "event=0x31");
+PMU_EVENT_ATTR_STRING(activate, ddr_perf_activate, "event=0x32");
+PMU_EVENT_ATTR_STRING(load-mode, ddr_perf_load_mode, "event=0x33");
+PMU_EVENT_ATTR_STRING(mwr, ddr_perf_mwr, "event=0x34");
+PMU_EVENT_ATTR_STRING(read, ddr_perf_read, "event=0x35");
+PMU_EVENT_ATTR_STRING(read-activate, ddr_perf_read_activate, "event=0x36");
+PMU_EVENT_ATTR_STRING(refresh, ddr_perf_refresh, "event=0x37");
+PMU_EVENT_ATTR_STRING(write, ddr_perf_write, "event=0x38");
+PMU_EVENT_ATTR_STRING(raw-hazard, ddr_perf_raw_hazard, "event=0x39");
+
+static const struct of_device_id imx_ddr_pmu_dt_ids[] = {
+ { .compatible = "fsl,imx8-ddr-pmu",},
+ { .compatible = "fsl,imx8m-ddr-pmu",},
+ { /* sentinel */ }
+};
+
+struct ddr_pmu {
+ struct pmu pmu;
+ void __iomem *base;
+ cpumask_t cpu;
+ struct hlist_node node;
+ struct device *dev;
+ struct perf_event *active_events[NUM_COUNTER];
+ int total_events;
+ enum cpuhp_state cpuhp_state;
+ bool cycles_active;
+ uintptr_t flags;
+};
+
+static ssize_t ddr_perf_cpumask_show(struct device *dev,
+ struct device_attribute *attr, char *buf)
+{
+ struct ddr_pmu *pmu = dev_get_drvdata(dev);
+
+ return cpumap_print_to_pagebuf(true, buf, &pmu->cpu);
+}
+
+static struct device_attribute ddr_perf_cpumask_attr =
+ __ATTR(cpumask, 0444, ddr_perf_cpumask_show, NULL);
+
+static struct attribute *ddr_perf_cpumask_attrs[] = {
+ &ddr_perf_cpumask_attr.attr,
+ NULL,
+};
+
+static struct attribute_group ddr_perf_cpumask_attr_group = {
+ .attrs = ddr_perf_cpumask_attrs,
+};
+
+static struct attribute *ddr_perf_events_attrs[] = {
+ &ddr_perf_cycles.attr.attr,
+ &ddr_perf_selfresh.attr.attr,
+ &ddr_perf_read_accesses.attr.attr,
+ &ddr_perf_write_accesses.attr.attr,
+ &ddr_perf_read_queue_depth.attr.attr,
+ &ddr_perf_write_queue_depth.attr.attr,
+ &ddr_perf_lp_read_credit_cnt.attr.attr,
+ &ddr_perf_hp_read_credit_cnt.attr.attr,
+ &ddr_perf_write_credit_cnt.attr.attr,
+ &ddr_perf_read_command.attr.attr,
+ &ddr_perf_write_command.attr.attr,
+ &ddr_perf_read_modify_write_command.attr.attr,
+ &ddr_perf_hp_read.attr.attr,
+ &ddr_perf_hp_req_nocredit.attr.attr,
+ &ddr_perf_hp_xact_credit.attr.attr,
+ &ddr_perf_lp_req_nocredit.attr.attr,
+ &ddr_perf_lp_xact_credit.attr.attr,
+ &ddr_perf_wr_xact_credit.attr.attr,
+ &ddr_perf_read_cycles.attr.attr,
+ &ddr_perf_write_cycles.attr.attr,
+ &ddr_perf_read_write_transition.attr.attr,
+ &ddr_perf_precharge.attr.attr,
+ &ddr_perf_activate.attr.attr,
+ &ddr_perf_load_mode.attr.attr,
+ &ddr_perf_mwr.attr.attr,
+ &ddr_perf_read.attr.attr,
+ &ddr_perf_read_activate.attr.attr,
+ &ddr_perf_refresh.attr.attr,
+ &ddr_perf_write.attr.attr,
+ &ddr_perf_raw_hazard.attr.attr,
+ NULL,
+};
+
+static struct attribute_group ddr_perf_events_attr_group = {
+ .name = "events",
+ .attrs = ddr_perf_events_attrs,
+};
+
+PMU_FORMAT_ATTR(event, "config:0-63");
+
+static struct attribute *ddr_perf_format_attrs[] = {
+ &format_attr_event.attr,
+ NULL,
+};
+
+static struct attribute_group ddr_perf_format_attr_group = {
+ .name = "format",
+ .attrs = ddr_perf_format_attrs,
+};
+
+static const struct attribute_group *attr_groups[] = {
+ &ddr_perf_events_attr_group,
+ &ddr_perf_format_attr_group,
+ &ddr_perf_cpumask_attr_group,
+ NULL,
+};
+
+static u32 ddr_perf_alloc_counter(struct ddr_pmu *pmu, int event)
+{
+ int i;
+
+ /* Always map cycle event to counter 0
+ Cycles counter is dedicated for cycle event
+ can't used for the other events
+ */
+ if (event == EVENT_CYCLES_ID)
+ return EVENT_CYCLES_COUNTER;
+
+ for (i = 1; i < NUM_COUNTER; i++)
+ if (pmu->active_events[i] == NULL)
+ return i;
+
+ return -ENOENT;
+}
+
+static u32 ddr_perf_free_counter(struct ddr_pmu *pmu, int counter)
+{
+ if (counter < 0 || counter >= NUM_COUNTER)
+ return -ENOENT;
+
+ pmu->active_events[counter] = NULL;
+
+ return 0;
+}
+
+static u32 ddr_perf_read_counter(struct ddr_pmu *pmu, int counter)
+{
+ return readl(pmu->base + COUNTER_READ + counter * 4);
+}
+
+static int ddr_perf_event_init(struct perf_event *event)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+
+ if (event->attr.type != event->pmu->type)
+ return -ENOENT;
+
+ if (is_sampling_event(event) || event->attach_state & PERF_ATTACH_TASK)
+ return -EOPNOTSUPP;
+
+ if (event->cpu < 0) {
+ dev_warn(pmu->dev, "Can't provide per-task data!\n");
+ return -EOPNOTSUPP;
+ }
+
+ if (event->attr.exclude_user ||
+ event->attr.exclude_kernel ||
+ event->attr.exclude_hv ||
+ event->attr.exclude_idle ||
+ event->attr.exclude_host ||
+ event->attr.exclude_guest ||
+ event->attr.sample_period)
+ return -EINVAL;
+
+ event->cpu = cpumask_first(&pmu->cpu);
+ hwc->idx = -1;
+
+ return 0;
+}
+
+
+static void ddr_perf_event_update(struct perf_event *event)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ u64 delta, prev_raw_count, new_raw_count;
+ int counter = hwc->idx;
+
+ do {
+ prev_raw_count = local64_read(&hwc->prev_count);
+ new_raw_count = ddr_perf_read_counter(pmu, counter);
+ } while (local64_cmpxchg(&hwc->prev_count, prev_raw_count,
+ new_raw_count) != prev_raw_count);
+
+ delta = (new_raw_count - prev_raw_count) & 0xFFFFFFFF;
+
+ local64_add(delta, &event->count);
+}
+
+static void ddr_perf_event_enable(struct ddr_pmu *pmu, int config,
+ int counter, bool enable)
+{
+ u8 reg = counter * 4 + COUNTER_CNTL;
+ int val;
+
+ if (enable) {
+ /* must disable first, then enable again
+ * otherwise, cycle counter will not work
+ * if previous state is enabled.
+ */
+ writel(0, pmu->base + reg);
+ val = CNTL_EN | CNTL_CLEAR;
+ val |= (config << CNTL_CSV_SHIFT) & CNTL_CSV_MASK;
+ } else {
+ /* Disable counter */
+ val = readl(pmu->base + reg) & CNTL_EN_MASK;
+ }
+
+ writel(val, pmu->base + reg);
+
+ if (config == EVENT_CYCLES_ID)
+ pmu->cycles_active = enable;
+}
+
+static void ddr_perf_event_start(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter = hwc->idx;
+
+ local64_set(&hwc->prev_count, 0);
+
+ ddr_perf_event_enable(pmu, event->attr.config, counter, true);
+ /*
+ * If the cycles counter wasn't explicitly selected,
+ * we will enable it now.
+ * cycles counter must be enabled otherwise other counters will
+ * stopped.
+ */
+ if (counter > 0 && !pmu->cycles_active)
+ ddr_perf_event_enable(pmu, EVENT_CYCLES_ID,
+ EVENT_CYCLES_COUNTER, true);
+}
+
+static int ddr_perf_event_add(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter;
+ int cfg = event->attr.config;
+
+ counter = ddr_perf_alloc_counter(pmu, cfg);
+ if (counter < 0) {
+ dev_dbg(pmu->dev, "There are not enough counters\n");
+ return -EOPNOTSUPP;
+ }
+
+ pmu->active_events[counter] = event;
+ pmu->total_events++;
+ hwc->idx = counter;
+
+ if (flags & PERF_EF_START)
+ ddr_perf_event_start(event, flags);
+
+ local64_set(&hwc->prev_count, ddr_perf_read_counter(pmu, counter));
+
+ return 0;
+}
+
+static void ddr_perf_event_stop(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter = hwc->idx;
+
+ ddr_perf_event_enable(pmu, event->attr.config, counter, false);
+ ddr_perf_event_update(event);
+}
+
+static void ddr_perf_event_del(struct perf_event *event, int flags)
+{
+ struct ddr_pmu *pmu = to_ddr_pmu(event->pmu);
+ struct hw_perf_event *hwc = &event->hw;
+ int counter = hwc->idx;
+
+ ddr_perf_event_stop(event, PERF_EF_UPDATE);
+
+ ddr_perf_free_counter(pmu, counter);
+ pmu->total_events--;
+ hwc->idx = -1;
+
+ /* If all events have stopped, stop the cycles counter as well */
+ if ((pmu->total_events == 0) && pmu->cycles_active)
+ ddr_perf_event_enable(pmu, EVENT_CYCLES_ID,
+ EVENT_CYCLES_COUNTER, false);
+}
+
+static int ddr_perf_init(struct ddr_pmu *pmu, void __iomem *base,
+ struct device *dev)
+{
+ *pmu = (struct ddr_pmu) {
+ .pmu = (struct pmu) {
+ .task_ctx_nr = perf_invalid_context,
+ .attr_groups = attr_groups,
+ .event_init = ddr_perf_event_init,
+ .add = ddr_perf_event_add,
+ .del = ddr_perf_event_del,
+ .start = ddr_perf_event_start,
+ .stop = ddr_perf_event_stop,
+ .read = ddr_perf_event_update,
+ },
+ .base = base,
+ .dev = dev,
+ };
+
+ return ida_simple_get(&ddr_ida, 0, 0, GFP_KERNEL);
+}
+
+static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
+{
+ int i;
+ u8 reg;
+ int val;
+ int counter;
+ struct ddr_pmu *pmu = (struct ddr_pmu *) p;
+ struct perf_event *event;
+
+ /* Only cycles counter overflowed can issue irq. all counters will
+ * be stopped when cycles counter overflow. but other counter don't stop
+ * when overflow happen. Update all of the local counter values,
+ * then reset the cycles counter, so the others can continue
+ * counting. cycles counter is fastest counter in all events. at last
+ * 4 times than other counters.
+ */
+ for (i = 0; i < NUM_COUNTER; i++) {
+
+ if (!pmu->active_events[i])
+ continue;
+
+ event = pmu->active_events[i];
+ counter = event->hw.idx;
+ reg = counter * 4 + COUNTER_CNTL;
+ val = readl(pmu->base + reg);
+
+ ddr_perf_event_update(event);
+
+ if (counter == EVENT_CYCLES_COUNTER) {
+ ddr_perf_event_enable(pmu,
+ EVENT_CYCLES_ID,
+ EVENT_CYCLES_COUNTER,
+ true);
+ ddr_perf_event_update(event);
+ }
+ }
+
+ return IRQ_HANDLED;
+}
+
+static int ddr_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
+{
+ struct ddr_pmu *pmu = hlist_entry_safe(node, struct ddr_pmu, node);
+ int target;
+
+ if (!cpumask_test_and_clear_cpu(cpu, &pmu->cpu))
+ return 0;
+
+ target = cpumask_any_but(cpu_online_mask, cpu);
+ if (target >= nr_cpu_ids)
+ return 0;
+
+ perf_pmu_migrate_context(&pmu->pmu, cpu, target);
+ cpumask_set_cpu(target, &pmu->cpu);
+
+ return 0;
+}
+
+static int ddr_perf_probe(struct platform_device *pdev)
+{
+ struct ddr_pmu *pmu;
+ struct device_node *np;
+ void __iomem *base;
+ struct resource *iomem;
+ char *name;
+ char *hpname;
+ int num;
+ int ret;
+ u32 irq;
+ const struct of_device_id *of_id =
+ of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
+
+ iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
+ base = devm_ioremap_resource(&pdev->dev, iomem);
+ if (IS_ERR(base))
+ return PTR_ERR(base);
+
+ np = pdev->dev.of_node;
+
+ pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
+ if (!pmu)
+ return -ENOMEM;
+
+ num = ddr_perf_init(pmu, base, &pdev->dev);
+
+ name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
+ if (!name)
+ return -ENOMEM;
+
+ hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
+ "perf/imx/ddr%d:online", num);
+ if (!hpname)
+ return -ENOMEM;
+
+ pmu->flags = (uintptr_t) of_id->data;
+
+ cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
+ ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
+ ddr_perf_offline_cpu);
+
+ if (ret < 0) {
+ dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
+ goto ddr_perf_err;
+ }
+
+ pmu->cpuhp_state = ret;
+
+ /* Register the pmu instance for cpu hotplug */
+ cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+
+ ret = perf_pmu_register(&(pmu->pmu), name, -1);
+ if (ret)
+ goto ddr_perf_err;
+
+ /* Request irq */
+ irq = of_irq_get(np, 0);
+ if (irq < 0) {
+ dev_err(&pdev->dev, "Failed to get irq: %d", irq);
+ ret = irq;
+ goto ddr_perf_irq_err;
+ }
+
+ ret = devm_request_irq(&pdev->dev, irq,
+ ddr_perf_irq_handler,
+ IRQF_TRIGGER_RISING | IRQF_ONESHOT,
+ DDR_PERF_DEV_NAME,
+ pmu);
+ if (ret < 0) {
+ dev_err(&pdev->dev, "Request irq failed: %d", ret);
+ goto ddr_perf_irq_err;
+ }
+
+ return 0;
+
+ddr_perf_irq_err:
+ perf_pmu_unregister(&(pmu->pmu));
+
+ddr_perf_err:
+ if (pmu->cpuhp_state)
+ cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+
+ dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret);
+ return ret;
+}
+
+static int ddr_perf_remove(struct platform_device *pdev)
+{
+ struct ddr_pmu *pmu = platform_get_drvdata(pdev);
+
+ cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
+ perf_pmu_unregister(&pmu->pmu);
+
+ return 0;
+}
+
+static struct platform_driver imx_ddr_pmu_driver = {
+ .driver = {
+ .name = "imx-ddr-pmu",
+ .of_match_table = imx_ddr_pmu_dt_ids,
+ },
+ .probe = ddr_perf_probe,
+ .remove = ddr_perf_remove,
+};
+
+static int __init imx_ddr_pmu_init(void)
+{
+ return platform_driver_register(&imx_ddr_pmu_driver);
+}
+
+module_init(imx_ddr_pmu_init);
+
--
2.5.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 3/4] arm64: dts: imx8qxp: added ddr performance monitor nodes
2019-04-16 20:39 ` Frank Li
@ 2019-04-16 20:39 ` Frank Li
-1 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Add ddr performance monitor
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
---
Change from v3 to v5
* none
Change from v2 to v3
* ddr_pmu0 -> ddr-pmu
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 0683ee2..16f2588 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -110,6 +110,13 @@
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
};
+ ddr-pmu@5c020000 {
+ compatible = "fsl,imx8-ddr-pmu";
+ reg = <0x0 0x5c020000 0x0 0x10000>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
psci {
compatible = "arm,psci-1.0";
method = "smc";
--
2.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 3/4] arm64: dts: imx8qxp: added ddr performance monitor nodes
@ 2019-04-16 20:39 ` Frank Li
0 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Add ddr performance monitor
Signed-off-by: Frank Li <Frank.Li@nxp.com>
Reviewed-by: Fabio Estevam <festevam@gmail.com>
---
Change from v3 to v5
* none
Change from v2 to v3
* ddr_pmu0 -> ddr-pmu
arch/arm64/boot/dts/freescale/imx8qxp.dtsi | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
index 0683ee2..16f2588 100644
--- a/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
+++ b/arch/arm64/boot/dts/freescale/imx8qxp.dtsi
@@ -110,6 +110,13 @@
interrupts = <GIC_PPI 7 IRQ_TYPE_LEVEL_HIGH>;
};
+ ddr-pmu@5c020000 {
+ compatible = "fsl,imx8-ddr-pmu";
+ reg = <0x0 0x5c020000 0x0 0x10000>;
+ interrupt-parent = <&gic>;
+ interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH>;
+ };
+
psci {
compatible = "arm,psci-1.0";
method = "smc";
--
2.5.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 4/4] MAINTAINERS: Added imx DDR performonitor driver maintainer information
2019-04-16 20:39 ` Frank Li
@ 2019-04-16 20:39 ` Frank Li
-1 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Add DDR perf counter driver maintainer information
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
No change from v1 to v5
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b4a972f..332faba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6263,6 +6263,13 @@ L: linux-i2c@vger.kernel.org
S: Maintained
F: drivers/i2c/busses/i2c-cpm.c
+FREESCALE IMX DDR Performance Monitor DRIVER
+M: Frank Li <Frank.li@nxp.com>
+L: linux-arm-kernel@lists.infradead.org
+S: Maintained
+F: drivers/perf/fsl_imx8_ddr_perf.c
+F: Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
+
FREESCALE IMX LPI2C DRIVER
M: Dong Aisheng <aisheng.dong@nxp.com>
L: linux-i2c@vger.kernel.org
--
2.5.2
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH V5 4/4] MAINTAINERS: Added imx DDR performonitor driver maintainer information
@ 2019-04-16 20:39 ` Frank Li
0 siblings, 0 replies; 16+ messages in thread
From: Frank Li @ 2019-04-16 20:39 UTC (permalink / raw)
To: mark.rutland, will.deacon, shawnguo, s.hauer, kernel, festevam,
dl-linux-imx, robh+dt, Aisheng Dong, devicetree, lznuaa,
linux-arm-kernel
Cc: Frank Li
Add DDR perf counter driver maintainer information
Signed-off-by: Frank Li <Frank.Li@nxp.com>
---
No change from v1 to v5
MAINTAINERS | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index b4a972f..332faba 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -6263,6 +6263,13 @@ L: linux-i2c@vger.kernel.org
S: Maintained
F: drivers/i2c/busses/i2c-cpm.c
+FREESCALE IMX DDR Performance Monitor DRIVER
+M: Frank Li <Frank.li@nxp.com>
+L: linux-arm-kernel@lists.infradead.org
+S: Maintained
+F: drivers/perf/fsl_imx8_ddr_perf.c
+F: Documentation/devicetree/bindings/perf/fsl-imx-ddr.txt
+
FREESCALE IMX LPI2C DRIVER
M: Dong Aisheng <aisheng.dong@nxp.com>
L: linux-i2c@vger.kernel.org
--
2.5.2
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
2019-04-16 20:39 ` Frank Li
@ 2019-04-24 10:58 ` Will Deacon
-1 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2019-04-24 10:58 UTC (permalink / raw)
To: Frank Li
Cc: mark.rutland, Aisheng Dong, devicetree, shawnguo, s.hauer,
robh+dt, dl-linux-imx, kernel, lznuaa, festevam,
linux-arm-kernel
Hi Frank,
On Tue, Apr 16, 2019 at 08:39:33PM +0000, Frank Li wrote:
> Add ddr performance monitor support for iMX8QXP
>
> There are 4 counters for ddr perfomance events.
> counter 0 is dedicated for cycles.
> you choose any up to 3 no cycles events.
I was about to pick this up, but I still have a few questions/comments
on the code:
> +static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
> +{
> + int i;
> + u8 reg;
> + int val;
> + int counter;
> + struct ddr_pmu *pmu = (struct ddr_pmu *) p;
> + struct perf_event *event;
> +
> + /* Only cycles counter overflowed can issue irq. all counters will
> + * be stopped when cycles counter overflow. but other counter don't stop
> + * when overflow happen. Update all of the local counter values,
> + * then reset the cycles counter, so the others can continue
> + * counting. cycles counter is fastest counter in all events. at last
> + * 4 times than other counters.
> + */
> + for (i = 0; i < NUM_COUNTER; i++) {
> +
> + if (!pmu->active_events[i])
> + continue;
> +
> + event = pmu->active_events[i];
> + counter = event->hw.idx;
> + reg = counter * 4 + COUNTER_CNTL;
> + val = readl(pmu->base + reg);
Does this read have a side-effect, or can it be removed given that you don't
use val for anything else?
> + ddr_perf_event_update(event);
> +
> + if (counter == EVENT_CYCLES_COUNTER) {
> + ddr_perf_event_enable(pmu,
> + EVENT_CYCLES_ID,
> + EVENT_CYCLES_COUNTER,
> + true);
> + ddr_perf_event_update(event);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
What stops the IRQ handler running concurrently with perf callbacks? I can't
see any locking here, or is the IRQ supposed to be affine to the same CPU
that's handling the perf context?
> +static int ddr_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct ddr_pmu *pmu = hlist_entry_safe(node, struct ddr_pmu, node);
> + int target;
> +
> + if (!cpumask_test_and_clear_cpu(cpu, &pmu->cpu))
> + return 0;
> +
> + target = cpumask_any_but(cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> + return 0;
> +
> + perf_pmu_migrate_context(&pmu->pmu, cpu, target);
> + cpumask_set_cpu(target, &pmu->cpu);
> +
> + return 0;
> +}
> +
> +static int ddr_perf_probe(struct platform_device *pdev)
> +{
> + struct ddr_pmu *pmu;
> + struct device_node *np;
> + void __iomem *base;
> + struct resource *iomem;
> + char *name;
> + char *hpname;
> + int num;
> + int ret;
> + u32 irq;
> + const struct of_device_id *of_id =
> + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
> +
> + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, iomem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + np = pdev->dev.of_node;
> +
> + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + num = ddr_perf_init(pmu, base, &pdev->dev);
> +
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
> + if (!name)
> + return -ENOMEM;
> +
> + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> + "perf/imx/ddr%d:online", num);
> + if (!hpname)
> + return -ENOMEM;
> +
> + pmu->flags = (uintptr_t) of_id->data;
> +
> + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
> + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
> + ddr_perf_offline_cpu);
> +
> + if (ret < 0) {
> + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
> + goto ddr_perf_err;
> + }
> +
> + pmu->cpuhp_state = ret;
> +
> + /* Register the pmu instance for cpu hotplug */
> + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> +
> + ret = perf_pmu_register(&(pmu->pmu), name, -1);
> + if (ret)
> + goto ddr_perf_err;
> +
> + /* Request irq */
> + irq = of_irq_get(np, 0);
> + if (irq < 0) {
irq is a u32 so this will never execute. You need to make it an int.
> + dev_err(&pdev->dev, "Failed to get irq: %d", irq);
> + ret = irq;
> + goto ddr_perf_irq_err;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq,
> + ddr_perf_irq_handler,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
Can you explain why you need to pass these specific IRQ flags, please? I'm
struggling to see what the ONESHOT is buying you.
Will
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
@ 2019-04-24 10:58 ` Will Deacon
0 siblings, 0 replies; 16+ messages in thread
From: Will Deacon @ 2019-04-24 10:58 UTC (permalink / raw)
To: Frank Li
Cc: mark.rutland, Aisheng Dong, devicetree, shawnguo, s.hauer,
robh+dt, dl-linux-imx, kernel, lznuaa, festevam,
linux-arm-kernel
Hi Frank,
On Tue, Apr 16, 2019 at 08:39:33PM +0000, Frank Li wrote:
> Add ddr performance monitor support for iMX8QXP
>
> There are 4 counters for ddr perfomance events.
> counter 0 is dedicated for cycles.
> you choose any up to 3 no cycles events.
I was about to pick this up, but I still have a few questions/comments
on the code:
> +static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
> +{
> + int i;
> + u8 reg;
> + int val;
> + int counter;
> + struct ddr_pmu *pmu = (struct ddr_pmu *) p;
> + struct perf_event *event;
> +
> + /* Only cycles counter overflowed can issue irq. all counters will
> + * be stopped when cycles counter overflow. but other counter don't stop
> + * when overflow happen. Update all of the local counter values,
> + * then reset the cycles counter, so the others can continue
> + * counting. cycles counter is fastest counter in all events. at last
> + * 4 times than other counters.
> + */
> + for (i = 0; i < NUM_COUNTER; i++) {
> +
> + if (!pmu->active_events[i])
> + continue;
> +
> + event = pmu->active_events[i];
> + counter = event->hw.idx;
> + reg = counter * 4 + COUNTER_CNTL;
> + val = readl(pmu->base + reg);
Does this read have a side-effect, or can it be removed given that you don't
use val for anything else?
> + ddr_perf_event_update(event);
> +
> + if (counter == EVENT_CYCLES_COUNTER) {
> + ddr_perf_event_enable(pmu,
> + EVENT_CYCLES_ID,
> + EVENT_CYCLES_COUNTER,
> + true);
> + ddr_perf_event_update(event);
> + }
> + }
> +
> + return IRQ_HANDLED;
> +}
What stops the IRQ handler running concurrently with perf callbacks? I can't
see any locking here, or is the IRQ supposed to be affine to the same CPU
that's handling the perf context?
> +static int ddr_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
> +{
> + struct ddr_pmu *pmu = hlist_entry_safe(node, struct ddr_pmu, node);
> + int target;
> +
> + if (!cpumask_test_and_clear_cpu(cpu, &pmu->cpu))
> + return 0;
> +
> + target = cpumask_any_but(cpu_online_mask, cpu);
> + if (target >= nr_cpu_ids)
> + return 0;
> +
> + perf_pmu_migrate_context(&pmu->pmu, cpu, target);
> + cpumask_set_cpu(target, &pmu->cpu);
> +
> + return 0;
> +}
> +
> +static int ddr_perf_probe(struct platform_device *pdev)
> +{
> + struct ddr_pmu *pmu;
> + struct device_node *np;
> + void __iomem *base;
> + struct resource *iomem;
> + char *name;
> + char *hpname;
> + int num;
> + int ret;
> + u32 irq;
> + const struct of_device_id *of_id =
> + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
> +
> + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, iomem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + np = pdev->dev.of_node;
> +
> + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + num = ddr_perf_init(pmu, base, &pdev->dev);
> +
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
> + if (!name)
> + return -ENOMEM;
> +
> + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> + "perf/imx/ddr%d:online", num);
> + if (!hpname)
> + return -ENOMEM;
> +
> + pmu->flags = (uintptr_t) of_id->data;
> +
> + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
> + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
> + ddr_perf_offline_cpu);
> +
> + if (ret < 0) {
> + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
> + goto ddr_perf_err;
> + }
> +
> + pmu->cpuhp_state = ret;
> +
> + /* Register the pmu instance for cpu hotplug */
> + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> +
> + ret = perf_pmu_register(&(pmu->pmu), name, -1);
> + if (ret)
> + goto ddr_perf_err;
> +
> + /* Request irq */
> + irq = of_irq_get(np, 0);
> + if (irq < 0) {
irq is a u32 so this will never execute. You need to make it an int.
> + dev_err(&pdev->dev, "Failed to get irq: %d", irq);
> + ret = irq;
> + goto ddr_perf_irq_err;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq,
> + ddr_perf_irq_handler,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
Can you explain why you need to pass these specific IRQ flags, please? I'm
struggling to see what the ONESHOT is buying you.
Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
2019-04-16 20:39 ` Frank Li
@ 2019-04-24 11:08 ` Robin Murphy
-1 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2019-04-24 11:08 UTC (permalink / raw)
To: Frank Li, mark.rutland, will.deacon, shawnguo, s.hauer, kernel,
festevam, dl-linux-imx, robh+dt, Aisheng Dong, devicetree,
lznuaa, linux-arm-kernel
On 16/04/2019 21:39, Frank Li wrote:
[...]
> +static int ddr_perf_probe(struct platform_device *pdev)
> +{
> + struct ddr_pmu *pmu;
> + struct device_node *np;
> + void __iomem *base;
> + struct resource *iomem;
> + char *name;
> + char *hpname;
> + int num;
> + int ret;
> + u32 irq;
> + const struct of_device_id *of_id =
> + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
> +
> + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, iomem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + np = pdev->dev.of_node;
> +
> + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + num = ddr_perf_init(pmu, base, &pdev->dev);
> +
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
> + if (!name)
> + return -ENOMEM;
> +
> + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> + "perf/imx/ddr%d:online", num);
> + if (!hpname)
> + return -ENOMEM;
> +
> + pmu->flags = (uintptr_t) of_id->data;
> +
> + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
> + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
> + ddr_perf_offline_cpu);
> +
> + if (ret < 0) {
> + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
> + goto ddr_perf_err;
> + }
Since you don't seem to ever remove this state again, this looks like it
would make subsequent loads of the module always fail after it's been
loaded and unloaded once - have you tried that?
Robin.
> +
> + pmu->cpuhp_state = ret;
> +
> + /* Register the pmu instance for cpu hotplug */
> + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> +
> + ret = perf_pmu_register(&(pmu->pmu), name, -1);
> + if (ret)
> + goto ddr_perf_err;
> +
> + /* Request irq */
> + irq = of_irq_get(np, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "Failed to get irq: %d", irq);
> + ret = irq;
> + goto ddr_perf_irq_err;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq,
> + ddr_perf_irq_handler,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + DDR_PERF_DEV_NAME,
> + pmu);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Request irq failed: %d", ret);
> + goto ddr_perf_irq_err;
> + }
> +
> + return 0;
> +
> +ddr_perf_irq_err:
> + perf_pmu_unregister(&(pmu->pmu));
> +
> +ddr_perf_err:
> + if (pmu->cpuhp_state)
> + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> +
> + dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret);
> + return ret;
> +}
> +
> +static int ddr_perf_remove(struct platform_device *pdev)
> +{
> + struct ddr_pmu *pmu = platform_get_drvdata(pdev);
> +
> + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> + perf_pmu_unregister(&pmu->pmu);
> +
> + return 0;
> +}
> +
> +static struct platform_driver imx_ddr_pmu_driver = {
> + .driver = {
> + .name = "imx-ddr-pmu",
> + .of_match_table = imx_ddr_pmu_dt_ids,
> + },
> + .probe = ddr_perf_probe,
> + .remove = ddr_perf_remove,
> +};
> +
> +static int __init imx_ddr_pmu_init(void)
> +{
> + return platform_driver_register(&imx_ddr_pmu_driver);
> +}
> +
> +module_init(imx_ddr_pmu_init);
> +
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
@ 2019-04-24 11:08 ` Robin Murphy
0 siblings, 0 replies; 16+ messages in thread
From: Robin Murphy @ 2019-04-24 11:08 UTC (permalink / raw)
To: Frank Li, mark.rutland, will.deacon, shawnguo, s.hauer, kernel,
festevam, dl-linux-imx, robh+dt, Aisheng Dong, devicetree,
lznuaa, linux-arm-kernel
On 16/04/2019 21:39, Frank Li wrote:
[...]
> +static int ddr_perf_probe(struct platform_device *pdev)
> +{
> + struct ddr_pmu *pmu;
> + struct device_node *np;
> + void __iomem *base;
> + struct resource *iomem;
> + char *name;
> + char *hpname;
> + int num;
> + int ret;
> + u32 irq;
> + const struct of_device_id *of_id =
> + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
> +
> + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> + base = devm_ioremap_resource(&pdev->dev, iomem);
> + if (IS_ERR(base))
> + return PTR_ERR(base);
> +
> + np = pdev->dev.of_node;
> +
> + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> + if (!pmu)
> + return -ENOMEM;
> +
> + num = ddr_perf_init(pmu, base, &pdev->dev);
> +
> + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
> + if (!name)
> + return -ENOMEM;
> +
> + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> + "perf/imx/ddr%d:online", num);
> + if (!hpname)
> + return -ENOMEM;
> +
> + pmu->flags = (uintptr_t) of_id->data;
> +
> + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
> + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
> + ddr_perf_offline_cpu);
> +
> + if (ret < 0) {
> + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
> + goto ddr_perf_err;
> + }
Since you don't seem to ever remove this state again, this looks like it
would make subsequent loads of the module always fail after it's been
loaded and unloaded once - have you tried that?
Robin.
> +
> + pmu->cpuhp_state = ret;
> +
> + /* Register the pmu instance for cpu hotplug */
> + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> +
> + ret = perf_pmu_register(&(pmu->pmu), name, -1);
> + if (ret)
> + goto ddr_perf_err;
> +
> + /* Request irq */
> + irq = of_irq_get(np, 0);
> + if (irq < 0) {
> + dev_err(&pdev->dev, "Failed to get irq: %d", irq);
> + ret = irq;
> + goto ddr_perf_irq_err;
> + }
> +
> + ret = devm_request_irq(&pdev->dev, irq,
> + ddr_perf_irq_handler,
> + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
> + DDR_PERF_DEV_NAME,
> + pmu);
> + if (ret < 0) {
> + dev_err(&pdev->dev, "Request irq failed: %d", ret);
> + goto ddr_perf_irq_err;
> + }
> +
> + return 0;
> +
> +ddr_perf_irq_err:
> + perf_pmu_unregister(&(pmu->pmu));
> +
> +ddr_perf_err:
> + if (pmu->cpuhp_state)
> + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> +
> + dev_warn(&pdev->dev, "i.MX8 DDR Perf PMU failed (%d), disabled\n", ret);
> + return ret;
> +}
> +
> +static int ddr_perf_remove(struct platform_device *pdev)
> +{
> + struct ddr_pmu *pmu = platform_get_drvdata(pdev);
> +
> + cpuhp_state_remove_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> + perf_pmu_unregister(&pmu->pmu);
> +
> + return 0;
> +}
> +
> +static struct platform_driver imx_ddr_pmu_driver = {
> + .driver = {
> + .name = "imx-ddr-pmu",
> + .of_match_table = imx_ddr_pmu_dt_ids,
> + },
> + .probe = ddr_perf_probe,
> + .remove = ddr_perf_remove,
> +};
> +
> +static int __init imx_ddr_pmu_init(void)
> +{
> + return platform_driver_register(&imx_ddr_pmu_driver);
> +}
> +
> +module_init(imx_ddr_pmu_init);
> +
>
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
2019-04-24 10:58 ` Will Deacon
@ 2019-04-24 19:13 ` Zhi Li
-1 siblings, 0 replies; 16+ messages in thread
From: Zhi Li @ 2019-04-24 19:13 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland, Aisheng Dong, devicetree, festevam, s.hauer,
Frank Li, robh+dt, dl-linux-imx, kernel, shawnguo,
linux-arm-kernel
On Wed, Apr 24, 2019 at 5:58 AM Will Deacon <will.deacon@arm.com> wrote:
>
> Hi Frank,
>
> On Tue, Apr 16, 2019 at 08:39:33PM +0000, Frank Li wrote:
> > Add ddr performance monitor support for iMX8QXP
> >
> > There are 4 counters for ddr perfomance events.
> > counter 0 is dedicated for cycles.
> > you choose any up to 3 no cycles events.
>
> I was about to pick this up, but I still have a few questions/comments
> on the code:
>
> > +static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
> > +{
> > + int i;
> > + u8 reg;
> > + int val;
> > + int counter;
> > + struct ddr_pmu *pmu = (struct ddr_pmu *) p;
> > + struct perf_event *event;
> > +
> > + /* Only cycles counter overflowed can issue irq. all counters will
> > + * be stopped when cycles counter overflow. but other counter don't stop
> > + * when overflow happen. Update all of the local counter values,
> > + * then reset the cycles counter, so the others can continue
> > + * counting. cycles counter is fastest counter in all events. at last
> > + * 4 times than other counters.
> > + */
> > + for (i = 0; i < NUM_COUNTER; i++) {
> > +
> > + if (!pmu->active_events[i])
> > + continue;
> > +
> > + event = pmu->active_events[i];
> > + counter = event->hw.idx;
> > + reg = counter * 4 + COUNTER_CNTL;
> > + val = readl(pmu->base + reg);
>
> Does this read have a side-effect, or can it be removed given that you don't
> use val for anything else?
It can be removed.
>
> > + ddr_perf_event_update(event);
> > +
> > + if (counter == EVENT_CYCLES_COUNTER) {
> > + ddr_perf_event_enable(pmu,
> > + EVENT_CYCLES_ID,
> > + EVENT_CYCLES_COUNTER,
> > + true);
> > + ddr_perf_event_update(event);
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
>
> What stops the IRQ handler running concurrently with perf callbacks? I can't
> see any locking here, or is the IRQ supposed to be affine to the same CPU
> that's handling the perf context?
I don't under your means here.
ddr_perf_event_update is lockless design.
I can't found problem if run ddr_perf_event_enable at both perf
context and IRQ concurrently.
>
> > +static int ddr_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
> > +{
> > + struct ddr_pmu *pmu = hlist_entry_safe(node, struct ddr_pmu, node);
> > + int target;
> > +
> > + if (!cpumask_test_and_clear_cpu(cpu, &pmu->cpu))
> > + return 0;
> > +
> > + target = cpumask_any_but(cpu_online_mask, cpu);
> > + if (target >= nr_cpu_ids)
> > + return 0;
> > +
> > + perf_pmu_migrate_context(&pmu->pmu, cpu, target);
> > + cpumask_set_cpu(target, &pmu->cpu);
> > +
> > + return 0;
> > +}
> > +
> > +static int ddr_perf_probe(struct platform_device *pdev)
> > +{
> > + struct ddr_pmu *pmu;
> > + struct device_node *np;
> > + void __iomem *base;
> > + struct resource *iomem;
> > + char *name;
> > + char *hpname;
> > + int num;
> > + int ret;
> > + u32 irq;
> > + const struct of_device_id *of_id =
> > + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
> > +
> > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, iomem);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + np = pdev->dev.of_node;
> > +
> > + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> > + if (!pmu)
> > + return -ENOMEM;
> > +
> > + num = ddr_perf_init(pmu, base, &pdev->dev);
> > +
> > + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
> > + if (!name)
> > + return -ENOMEM;
> > +
> > + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> > + "perf/imx/ddr%d:online", num);
> > + if (!hpname)
> > + return -ENOMEM;
> > +
> > + pmu->flags = (uintptr_t) of_id->data;
> > +
> > + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
> > + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
> > + ddr_perf_offline_cpu);
> > +
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
> > + goto ddr_perf_err;
> > + }
> > +
> > + pmu->cpuhp_state = ret;
> > +
> > + /* Register the pmu instance for cpu hotplug */
> > + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> > +
> > + ret = perf_pmu_register(&(pmu->pmu), name, -1);
> > + if (ret)
> > + goto ddr_perf_err;
> > +
> > + /* Request irq */
> > + irq = of_irq_get(np, 0);
> > + if (irq < 0) {
>
> irq is a u32 so this will never execute. You need to make it an int.
>
> > + dev_err(&pdev->dev, "Failed to get irq: %d", irq);
> > + ret = irq;
> > + goto ddr_perf_irq_err;
> > + }
> > +
> > + ret = devm_request_irq(&pdev->dev, irq,
> > + ddr_perf_irq_handler,
> > + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>
> Can you explain why you need to pass these specific IRQ flags, please? I'm
> struggling to see what the ONESHOT is buying you.
I think default should be okay. I will update at next version
>
> Will
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
@ 2019-04-24 19:13 ` Zhi Li
0 siblings, 0 replies; 16+ messages in thread
From: Zhi Li @ 2019-04-24 19:13 UTC (permalink / raw)
To: Will Deacon
Cc: mark.rutland, Aisheng Dong, devicetree, festevam, s.hauer,
Frank Li, robh+dt, dl-linux-imx, kernel, shawnguo,
linux-arm-kernel
On Wed, Apr 24, 2019 at 5:58 AM Will Deacon <will.deacon@arm.com> wrote:
>
> Hi Frank,
>
> On Tue, Apr 16, 2019 at 08:39:33PM +0000, Frank Li wrote:
> > Add ddr performance monitor support for iMX8QXP
> >
> > There are 4 counters for ddr perfomance events.
> > counter 0 is dedicated for cycles.
> > you choose any up to 3 no cycles events.
>
> I was about to pick this up, but I still have a few questions/comments
> on the code:
>
> > +static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
> > +{
> > + int i;
> > + u8 reg;
> > + int val;
> > + int counter;
> > + struct ddr_pmu *pmu = (struct ddr_pmu *) p;
> > + struct perf_event *event;
> > +
> > + /* Only cycles counter overflowed can issue irq. all counters will
> > + * be stopped when cycles counter overflow. but other counter don't stop
> > + * when overflow happen. Update all of the local counter values,
> > + * then reset the cycles counter, so the others can continue
> > + * counting. cycles counter is fastest counter in all events. at last
> > + * 4 times than other counters.
> > + */
> > + for (i = 0; i < NUM_COUNTER; i++) {
> > +
> > + if (!pmu->active_events[i])
> > + continue;
> > +
> > + event = pmu->active_events[i];
> > + counter = event->hw.idx;
> > + reg = counter * 4 + COUNTER_CNTL;
> > + val = readl(pmu->base + reg);
>
> Does this read have a side-effect, or can it be removed given that you don't
> use val for anything else?
It can be removed.
>
> > + ddr_perf_event_update(event);
> > +
> > + if (counter == EVENT_CYCLES_COUNTER) {
> > + ddr_perf_event_enable(pmu,
> > + EVENT_CYCLES_ID,
> > + EVENT_CYCLES_COUNTER,
> > + true);
> > + ddr_perf_event_update(event);
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
>
> What stops the IRQ handler running concurrently with perf callbacks? I can't
> see any locking here, or is the IRQ supposed to be affine to the same CPU
> that's handling the perf context?
I don't under your means here.
ddr_perf_event_update is lockless design.
I can't found problem if run ddr_perf_event_enable at both perf
context and IRQ concurrently.
>
> > +static int ddr_perf_offline_cpu(unsigned int cpu, struct hlist_node *node)
> > +{
> > + struct ddr_pmu *pmu = hlist_entry_safe(node, struct ddr_pmu, node);
> > + int target;
> > +
> > + if (!cpumask_test_and_clear_cpu(cpu, &pmu->cpu))
> > + return 0;
> > +
> > + target = cpumask_any_but(cpu_online_mask, cpu);
> > + if (target >= nr_cpu_ids)
> > + return 0;
> > +
> > + perf_pmu_migrate_context(&pmu->pmu, cpu, target);
> > + cpumask_set_cpu(target, &pmu->cpu);
> > +
> > + return 0;
> > +}
> > +
> > +static int ddr_perf_probe(struct platform_device *pdev)
> > +{
> > + struct ddr_pmu *pmu;
> > + struct device_node *np;
> > + void __iomem *base;
> > + struct resource *iomem;
> > + char *name;
> > + char *hpname;
> > + int num;
> > + int ret;
> > + u32 irq;
> > + const struct of_device_id *of_id =
> > + of_match_device(imx_ddr_pmu_dt_ids, &pdev->dev);
> > +
> > + iomem = platform_get_resource(pdev, IORESOURCE_MEM, 0);
> > + base = devm_ioremap_resource(&pdev->dev, iomem);
> > + if (IS_ERR(base))
> > + return PTR_ERR(base);
> > +
> > + np = pdev->dev.of_node;
> > +
> > + pmu = devm_kzalloc(&pdev->dev, sizeof(*pmu), GFP_KERNEL);
> > + if (!pmu)
> > + return -ENOMEM;
> > +
> > + num = ddr_perf_init(pmu, base, &pdev->dev);
> > +
> > + name = devm_kasprintf(&pdev->dev, GFP_KERNEL, "ddr%d", num);
> > + if (!name)
> > + return -ENOMEM;
> > +
> > + hpname = devm_kasprintf(&pdev->dev, GFP_KERNEL,
> > + "perf/imx/ddr%d:online", num);
> > + if (!hpname)
> > + return -ENOMEM;
> > +
> > + pmu->flags = (uintptr_t) of_id->data;
> > +
> > + cpumask_set_cpu(raw_smp_processor_id(), &pmu->cpu);
> > + ret = cpuhp_setup_state_multi(CPUHP_AP_ONLINE_DYN, hpname, NULL,
> > + ddr_perf_offline_cpu);
> > +
> > + if (ret < 0) {
> > + dev_err(&pdev->dev, "cpuhp_setup_state_multi failed\n");
> > + goto ddr_perf_err;
> > + }
> > +
> > + pmu->cpuhp_state = ret;
> > +
> > + /* Register the pmu instance for cpu hotplug */
> > + cpuhp_state_add_instance_nocalls(pmu->cpuhp_state, &pmu->node);
> > +
> > + ret = perf_pmu_register(&(pmu->pmu), name, -1);
> > + if (ret)
> > + goto ddr_perf_err;
> > +
> > + /* Request irq */
> > + irq = of_irq_get(np, 0);
> > + if (irq < 0) {
>
> irq is a u32 so this will never execute. You need to make it an int.
>
> > + dev_err(&pdev->dev, "Failed to get irq: %d", irq);
> > + ret = irq;
> > + goto ddr_perf_irq_err;
> > + }
> > +
> > + ret = devm_request_irq(&pdev->dev, irq,
> > + ddr_perf_irq_handler,
> > + IRQF_TRIGGER_RISING | IRQF_ONESHOT,
>
> Can you explain why you need to pass these specific IRQ flags, please? I'm
> struggling to see what the ONESHOT is buying you.
I think default should be okay. I will update at next version
>
> Will
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
2019-04-24 10:58 ` Will Deacon
@ 2019-04-25 17:16 ` Mark Rutland
-1 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2019-04-25 17:16 UTC (permalink / raw)
To: Will Deacon
Cc: Aisheng Dong, devicetree, festevam, s.hauer, Frank Li, robh+dt,
dl-linux-imx, kernel, lznuaa, shawnguo, linux-arm-kernel
On Wed, Apr 24, 2019 at 11:58:22AM +0100, Will Deacon wrote:
> Hi Frank,
>
> On Tue, Apr 16, 2019 at 08:39:33PM +0000, Frank Li wrote:
> > Add ddr performance monitor support for iMX8QXP
> >
> > There are 4 counters for ddr perfomance events.
> > counter 0 is dedicated for cycles.
> > you choose any up to 3 no cycles events.
>
> I was about to pick this up, but I still have a few questions/comments
> on the code:
>
> > +static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
> > +{
> > + int i;
> > + u8 reg;
> > + int val;
> > + int counter;
> > + struct ddr_pmu *pmu = (struct ddr_pmu *) p;
> > + struct perf_event *event;
> > +
> > + /* Only cycles counter overflowed can issue irq. all counters will
> > + * be stopped when cycles counter overflow. but other counter don't stop
> > + * when overflow happen. Update all of the local counter values,
> > + * then reset the cycles counter, so the others can continue
> > + * counting. cycles counter is fastest counter in all events. at last
> > + * 4 times than other counters.
> > + */
> > + for (i = 0; i < NUM_COUNTER; i++) {
> > +
> > + if (!pmu->active_events[i])
> > + continue;
> > +
> > + event = pmu->active_events[i];
> > + counter = event->hw.idx;
> > + reg = counter * 4 + COUNTER_CNTL;
> > + val = readl(pmu->base + reg);
>
> Does this read have a side-effect, or can it be removed given that you don't
> use val for anything else?
>
> > + ddr_perf_event_update(event);
> > +
> > + if (counter == EVENT_CYCLES_COUNTER) {
> > + ddr_perf_event_enable(pmu,
> > + EVENT_CYCLES_ID,
> > + EVENT_CYCLES_COUNTER,
> > + true);
> > + ddr_perf_event_update(event);
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
>
> What stops the IRQ handler running concurrently with perf callbacks? I can't
> see any locking here, or is the IRQ supposed to be affine to the same CPU
> that's handling the perf context?
To be correct, the IRQ affinity should be set to match the logical
affinity of the PMU, but it looks like that's not done at setup or
migrate time.
Frank, please take a look at what drivers/perf/arm-ccn.c does with teh
IRQ, including the use of IRQF_NOBALANCING | IRQF_NO_THREAD flags when
requesting the IRQ to begin with.
Thanks,
Mark.
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support
@ 2019-04-25 17:16 ` Mark Rutland
0 siblings, 0 replies; 16+ messages in thread
From: Mark Rutland @ 2019-04-25 17:16 UTC (permalink / raw)
To: Will Deacon
Cc: Aisheng Dong, devicetree, festevam, s.hauer, Frank Li, robh+dt,
dl-linux-imx, kernel, lznuaa, shawnguo, linux-arm-kernel
On Wed, Apr 24, 2019 at 11:58:22AM +0100, Will Deacon wrote:
> Hi Frank,
>
> On Tue, Apr 16, 2019 at 08:39:33PM +0000, Frank Li wrote:
> > Add ddr performance monitor support for iMX8QXP
> >
> > There are 4 counters for ddr perfomance events.
> > counter 0 is dedicated for cycles.
> > you choose any up to 3 no cycles events.
>
> I was about to pick this up, but I still have a few questions/comments
> on the code:
>
> > +static irqreturn_t ddr_perf_irq_handler(int irq, void *p)
> > +{
> > + int i;
> > + u8 reg;
> > + int val;
> > + int counter;
> > + struct ddr_pmu *pmu = (struct ddr_pmu *) p;
> > + struct perf_event *event;
> > +
> > + /* Only cycles counter overflowed can issue irq. all counters will
> > + * be stopped when cycles counter overflow. but other counter don't stop
> > + * when overflow happen. Update all of the local counter values,
> > + * then reset the cycles counter, so the others can continue
> > + * counting. cycles counter is fastest counter in all events. at last
> > + * 4 times than other counters.
> > + */
> > + for (i = 0; i < NUM_COUNTER; i++) {
> > +
> > + if (!pmu->active_events[i])
> > + continue;
> > +
> > + event = pmu->active_events[i];
> > + counter = event->hw.idx;
> > + reg = counter * 4 + COUNTER_CNTL;
> > + val = readl(pmu->base + reg);
>
> Does this read have a side-effect, or can it be removed given that you don't
> use val for anything else?
>
> > + ddr_perf_event_update(event);
> > +
> > + if (counter == EVENT_CYCLES_COUNTER) {
> > + ddr_perf_event_enable(pmu,
> > + EVENT_CYCLES_ID,
> > + EVENT_CYCLES_COUNTER,
> > + true);
> > + ddr_perf_event_update(event);
> > + }
> > + }
> > +
> > + return IRQ_HANDLED;
> > +}
>
> What stops the IRQ handler running concurrently with perf callbacks? I can't
> see any locking here, or is the IRQ supposed to be affine to the same CPU
> that's handling the perf context?
To be correct, the IRQ affinity should be set to match the logical
affinity of the PMU, but it looks like that's not done at setup or
migrate time.
Frank, please take a look at what drivers/perf/arm-ccn.c does with teh
IRQ, including the use of IRQF_NOBALANCING | IRQF_NO_THREAD flags when
requesting the IRQ to begin with.
Thanks,
Mark.
_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2019-04-25 17:17 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-04-16 20:39 [PATCH V5 1/4] dt-bindings: perf: imx8-ddr: add imx8qxp ddr performance monitor Frank Li
2019-04-16 20:39 ` Frank Li
2019-04-16 20:39 ` [PATCH V5 2/4] drivers/perf: imx_ddr: Add ddr performance counter support Frank Li
2019-04-16 20:39 ` Frank Li
2019-04-24 10:58 ` Will Deacon
2019-04-24 10:58 ` Will Deacon
2019-04-24 19:13 ` Zhi Li
2019-04-24 19:13 ` Zhi Li
2019-04-25 17:16 ` Mark Rutland
2019-04-25 17:16 ` Mark Rutland
2019-04-24 11:08 ` Robin Murphy
2019-04-24 11:08 ` Robin Murphy
2019-04-16 20:39 ` [PATCH V5 3/4] arm64: dts: imx8qxp: added ddr performance monitor nodes Frank Li
2019-04-16 20:39 ` Frank Li
2019-04-16 20:39 ` [PATCH V5 4/4] MAINTAINERS: Added imx DDR performonitor driver maintainer information Frank Li
2019-04-16 20:39 ` Frank Li
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.