interconnect: intel: Add Keem Bay noc driver
PATCH v6 1/1 interconnect: intel: Add Keem Bay noc driver
From: Pandith N <[email protected]>
Add support for Network on Chip(NOC) counters. Enable features to configure and capture NOC probe counters, needed for DDR bandwidth measurement. NOC driver is specific to Intel Keem Bay SOC. NOC hardware counters are used for DDR statistics profiling, it is not related to timers. Interface details are provided in include/uapi/linux/noc_uapi.h
---
Changes in V2 :
NOC is class driver, platform driver not needed. Removed LINUX syscall note from noc_driver.c. Order variable from longest to shortest.
Using varaible in sizeof parameter, rather than data type. Consistent project naming as Keem Bay
Changes in V3 :
Free noc device in failure case and module exit.
Remove platform device references.
Changes in V4 :
Rephrased commit description, added missing punctuation in Kconfig.
Changes in V5 :
Local variables are always initialized before use in all cases of capture function
No need of counter 3 definiton in enumeration
Changes in V6 :
Renaming noc driver as keembay-bwmon, moved to interconnect/intel. Added more description about DSS, NOC
--- MAINTAINERS | 6 + drivers/interconnect/Kconfig | 1 + drivers/interconnect/Makefile | 1 + drivers/interconnect/intel/Kconfig | 19 ++ drivers/interconnect/intel/Makefile | 5 + drivers/interconnect/intel/keembay-bwmon.c | 305 +++++++++++++++++++++ drivers/interconnect/intel/keembay-bwmon.h | 110 ++++++++ include/uapi/linux/noc_uapi.h | 42 +++ 8 files changed, 489 insertions(+) create mode 100644 drivers/interconnect/intel/Kconfig create mode 100644 drivers/interconnect/intel/Makefile create mode 100644 drivers/interconnect/intel/keembay-bwmon.c create mode 100644 drivers/interconnect/intel/keembay-bwmon.h create mode 100644 include/uapi/linux/noc_uapi.h
MAINTAINERS +6 -0
@@ -9045,6 +9045,12 @@ M: Deepak Saxena <[email protected]> S: Maintained F: drivers/char/hw_random/ixp4xx-rng.c +INTEL KEEM BAY DDR SUBSYSTEM, NETWORK ON CHIP INTERCONNECT DRIVER +M: Pandith N <[email protected]> +S: Supported +F: drivers/interconnect/intel/ +F: include/uapi/linux/noc_uapi.h + INTEL KEEM BAY DRM DRIVER M: Anitha Chrisanthus <[email protected]> M: Edmund Dea <[email protected]>
drivers/interconnect/Kconfig +1 -0
@@ -12,6 +12,7 @@ menuconfig INTERCONNECT if INTERCONNECT source "drivers/interconnect/imx/Kconfig" +source "drivers/interconnect/intel/Kconfig" source "drivers/interconnect/qcom/Kconfig" source "drivers/interconnect/samsung/Kconfig"
drivers/interconnect/Makefile +1 -0
@@ -5,5 +5,6 @@ icc-core-objs := core.o bulk.o obj-$(CONFIG_INTERCONNECT) += icc-core.o obj-$(CONFIG_INTERCONNECT_IMX) += imx/ +obj-$(CONFIG_INTERCONNECT_INTEL) += intel/ obj-$(CONFIG_INTERCONNECT_QCOM) += qcom/ obj-$(CONFIG_INTERCONNECT_SAMSUNG) += samsung/
drivers/interconnect/intel/Kconfig new file +19 -0
@@ -0,0 +1,19 @@ +# Copyright (C) 2019 Intel Corporation +# SPDX-License-Identifier: GPL-2.0-only +config INTERCONNECT_INTEL + bool "Intel Network-on-chip interconnect drivers" + help + Support for Intel's Network on chip interconnect hardware. + +config INTERCONNECT_INTEL_KEEMBAY + tristate "Intel Keem Bay Enable DDR profiling using NOC" + depends on INTERCONNECT_INTEL || ARCH_KEEMBAY || COMPILE_TEST + help + Enable this option for DDR bandwidth measurements using NOC + + Add support for Network-on-chip (NOC) in DDR Subsystem(DSS). + DSS NOC has capabilities to enable and get statistics profiling. + NOC driver enables features to configure and capture NOC probe + counters, needed for DSS bandwidth measurement. + Say Y if using a processor that includes the Intel VPU such as + Keem Bay. If unsure, say N.
drivers/interconnect/intel/Makefile new file +5 -0
@@ -0,0 +1,5 @@ +# Copyright (C) 2019 Intel Corporation +# SPDX-License-Identifier: GPL-2.0-only +# Makefile for Keem Bay NOC +# +obj-$(CONFIG_INTERCONNECT_INTEL_KEEMBAY) += keembay-bwmon.o
drivers/interconnect/intel/keembay-bwmon.c new file +305 -0
305 lines not on this page · raw message
drivers/interconnect/intel/keembay-bwmon.h new file +110 -0
110 lines not on this page · raw message
include/uapi/linux/noc_uapi.h new file +42 -0
42 lines not on this page · raw message
Signature · 1 line
2.17.1
Quoted 7 lines · On Tue, 2021-08-31 at 12:06 +0530, [email protected] wrote:
From: Pandith N <[email protected]>
Add support for Network on Chip(NOC) counters. Enable features to configure and capture NOC probe counters, needed for DDR bandwidth measurement. NOC driver is specific to Intel Keem Bay SOC. NOC hardware counters are used for DDR statistics profiling, it is not related to timers. Interface details are provided in
include/uapi/linux/noc_uapi.h
trivial notes:
drivers/interconnect/intel/Kconfig
Quoted 1 line
drivers/interconnect/intel/Kconfig +0
-0
[]
drivers/interconnect/intel/Kconfig line 17
Quoted 10 lines
drivers/interconnect/intel/Kconfig +10
-0+config INTERCONNECT_INTEL_KEEMBAY + tristate "Intel Keem Bay Enable DDR profiling using NOC" + depends on INTERCONNECT_INTEL || ARCH_KEEMBAY || COMPILE_TEST + help + Enable this option for DDR bandwidth measurements using NOC + + Add support for Network-on-chip (NOC) in DDR Subsystem(DSS). + DSS NOC has capabilities to enable and get statistics profiling. + NOC driver enables features to configure and capture NOC probe + counters, needed for DSS bandwidth measurement.
Inconsistent tab/space indentation on this line
drivers/interconnect/intel/keembay-bwmon.c
Quoted 1 line
drivers/interconnect/intel/keembay-bwmon.c +0
-0
[]
Quoted 7 lines
+/**
+ *flex_noc_setup()- Setup two counters for the NOC probe
+ * @noc: NOC type to setup counters
+ * @counter: Counter number to set up counter n and n+1
+ * @trace_port: trace port number to setup counters
+ *
+ * This function will setup the counters for the trace port given.
This seems to be unnecessary kernel-doc and if it is useful, the return value isn't described.
Quoted 6 lines
drivers/interconnect/intel/keembay-bwmon.c +6
-0+int flex_noc_setup (enum noc_ss_type noc,enum noc_counter counter,int trace_port) +{ +int offset; + +if (noc >=NOC_TYPE_MAX || counter >=NOC_COUNTER_MAX ) +return -EINVAL ;
drivers/interconnect/intel/keembay-bwmon.c line 65
Quoted 10 lines
drivers/interconnect/intel/keembay-bwmon.c +10
-0+ + offset = f_offset[counter /2 ]; + +/* Stop ongoing stats */ +noc_writel (MAINCTL ,0 ); +noc_writel (CFGCTL ,0 ); + +/* Setup trace port and counters port select */ +noc_writel (TRACEPORTSEL , trace_port); +noc_writel ((c_offset[counter] +C_PORTSEL ), trace_port);
Lots of unnecessary parentheses
drivers/interconnect/intel/keembay-bwmon.c line 73
Quoted 8 lines
drivers/interconnect/intel/keembay-bwmon.c +8
-0+noc_writel ((c_offset[counter +1 ] +C_PORTSEL ), trace_port); + +/* Setup counter sources & triggers, Alarm mode - OFF */ +noc_writel ((c_offset[counter] +C_SRC ),COUNTERS_0_SRC_VAL ); +noc_writel ((c_offset[counter] +C_ALARMMODE ),COUNTERS_ALARMMODE_VAL ); +noc_writel ((c_offset[counter +1 ] +C_SRC ),COUNTERS_1_SRC_VAL ); +noc_writel ((c_offset[counter +1 ] +C_ALARMMODE ), +COUNTERS_ALARMMODE_VAL );
[]
Quoted 4 lines
drivers/interconnect/intel/keembay-bwmon.c +4
-0+enum noc_status flexnoc_counter_capture (enum noc_ss_type noc, +enum noc_counter counter,u32 *value) +{ +unsigned long j0, j1, delay;
drivers/interconnect/intel/keembay-bwmon.c line 146
Quoted 10 lines
drivers/interconnect/intel/keembay-bwmon.c +10
-0+u32 c0_0, c0_1; + +if (noc >=NOC_TYPE_MAX || + counter >=NOC_COUNTER_MAX || + !value) +return NOC_PROBE_ERR_INVALID_ARGS ; + + delay =msecs_to_jiffies (NOC_CAPTURE_TIMEOUT_MSEC ); + j0 = jiffies; + j1 = j0 + delay;
j0 and j1 seem unnecessary
timeout = jiffies + msecs_to_jiffies(NOC_CAPTURE_TIMEOUT_MSEC);
drivers/interconnect/intel/keembay-bwmon.c line 149
Quoted 3 lines
drivers/interconnect/intel/keembay-bwmon.c +3
-0+ do { + c0_0 = noc_readl((c_offset[counter] + C_VAL)); + usleep_range(10000, 11000);
Seems a long time.
Quoted 4 lines
drivers/interconnect/intel/keembay-bwmon.c +4
-0+ c0_1 =noc_readl ((c_offset[counter] +C_VAL )); +/* If mainctrl is zero , return error */ +if (noc_readl (MAINCTL ) ==0 ) +return NOC_PROBE_ERR_IN_PROGRESS ;
drivers/interconnect/intel/keembay-bwmon.c line 163
Quoted 10 lines
drivers/interconnect/intel/keembay-bwmon.c +10
-0+/* If counters are zero, keep reading */ +if (0 == c0_0 &&0 == c0_1) { +break ; + }else if (c0_0 != c0_1) { +continue ; + }else { +/* counters look good break the while */ +break ; + } + }while (time_before (jiffies, j1));
} while (time_before(jiffies, timeout);
[]
drivers/interconnect/intel/keembay-bwmon.c line 184
Quoted 10 lines
drivers/interconnect/intel/keembay-bwmon.c +10
-0+static long noc_ioctl (struct file *file,unsigned int cmd,unsigned long arg) +{ +struct flexnoc_countercapture capture_data; +void __user *argp = (void __user *)arg; +struct flexnoc_probestart probe_data; +struct flexnoc_setup setup_data; +int rc; + +if (!arg) { +pr_err ("NOC: Null pointer from user\n" );
Perhaps useful to add #define pr_fmt(fmt) KBUILD_MODNAME ": " fmt near the top of the file.
Hi,
Quoted 29 lines
-----Original Message-----
From: Joe Perches <[email protected]>
Sent: Tuesday, August 31, 2021 8:56 PM
To: N, Pandith <[email protected]>; [email protected]; linux- [email protected]; [email protected]
Cc: [email protected]; Zhou, Furong <[email protected]>; Sangannavar, Mallikarjunappa <[email protected]>; Raja Subramanian, Lakshmi Bai <[email protected]>
Subject: Re: [PATCH V6 1/1] interconnect: intel: Add Keem Bay noc driverQuoted 6 lines · On Tue, 2021-08-31 at 12:06 +0530, [email protected] wrote:
From: Pandith N <[email protected]>
Add support for Network on Chip(NOC) counters. Enable features to configure and capture NOC probe counters, needed for DDR bandwidth measurement. NOC driver is specific to Intel Keem Bay SOC. NOC hardware counters are used for DDR statistics profiling, it is not related to
timers.
Quoted 1 line
Interface details are provided in
include/uapi/linux/noc_uapi.htrivial notes:
drivers/interconnect/intel/Kconfig
Quoted 2 lines
drivers/interconnect/intel/Kconfig +0
-0[]
drivers/interconnect/intel/Kconfig
Quoted 3 lines
drivers/interconnect/intel/Kconfig +3
-0+config INTERCONNECT_INTEL_KEEMBAY + tristate "Intel Keem Bay Enable DDR profiling using NOC" + depends on INTERCONNECT_INTEL || ARCH_KEEMBAY ||
COMPILE_TEST
Quoted 10 lines
drivers/interconnect/intel/Kconfig line 17
Quoted 7 lines
drivers/interconnect/intel/Kconfig +7
-0+ help + Enable this option for DDR bandwidth measurements using NOC + + Add support for Network-on-chip (NOC) in DDR Subsystem(DSS). + DSS NOC has capabilities to enable and get statistics profiling. + NOC driver enables features to configure and capture NOC probe + counters, needed for DSS bandwidth measurement.Inconsistent tab/space indentation on this line
It will corrected with tab + 2 spaces
Quoted 11 lines
drivers/interconnect/intel/keembay-bwmon.c
Quoted 2 lines
drivers/interconnect/intel/keembay-bwmon.c +0
-0[]
Quoted 7 lines
+/**
+ *flex_noc_setup()- Setup two counters for the NOC probe
+ * @noc: NOC type to setup counters
+ * @counter: Counter number to set up counter n and n+1
+ * @trace_port: trace port number to setup counters
+ *
+ * This function will setup the counters for the trace port given.
Quoted 3 lines
This seems to be unnecessary kernel-doc and if it is useful, the return value isn't described.
Will add description for return value
Quoted 10 lines
Quoted 10 lines
drivers/interconnect/intel/keembay-bwmon.c +10
-0+int flex_noc_setup (enum noc_ss_type noc,enum noc_counter counter, +int trace_port) { +int offset; + +if (noc >=NOC_TYPE_MAX || counter >=NOC_COUNTER_MAX ) +return -EINVAL ; + + offset = f_offset[counter /2 ]; + +/* Stop ongoing stats */
Quoted 9 lines
drivers/interconnect/intel/keembay-bwmon.c line 65
Quoted 6 lines
drivers/interconnect/intel/keembay-bwmon.c +6
-0+noc_writel (MAINCTL ,0 ); +noc_writel (CFGCTL ,0 ); + +/* Setup trace port and counters port select */ +noc_writel (TRACEPORTSEL , trace_port); +noc_writel ((c_offset[counter] +C_PORTSEL ), trace_port);Lots of unnecessary parentheses
Will change as below :noc_writel(c_offset[counter] + C_PORTSEL, trace_port);
Quoted 11 lines
drivers/interconnect/intel/keembay-bwmon.c
Quoted 5 lines
drivers/interconnect/intel/keembay-bwmon.c +5
-0+noc_writel ((c_offset[counter +1 ] +C_PORTSEL ), trace_port); + +/* Setup counter sources & triggers, Alarm mode - OFF */ +noc_writel ((c_offset[counter] +C_SRC ),COUNTERS_0_SRC_VAL ); + noc_writel((c_offset[counter] + C_ALARMMODE),
COUNTERS_ALARMMODE_VAL);drivers/interconnect/intel/keembay-bwmon.c line 73
Quoted 3 lines
drivers/interconnect/intel/keembay-bwmon.c +3
-0+noc_writel ((c_offset[counter +1 ] +C_SRC ),COUNTERS_1_SRC_VAL ); +noc_writel ((c_offset[counter +1 ] +C_ALARMMODE ), +COUNTERS_ALARMMODE_VAL );[]
Will correct unnecessary parenthesis in above 4 statements
Quoted 4 lines
drivers/interconnect/intel/keembay-bwmon.c
Quoted 3 lines
drivers/interconnect/intel/keembay-bwmon.c +2
-0+enum noc_status flexnoc_counter_capture (enum noc_ss_type noc, + enum noc_counter counter, u32*value) {
Quoted 14 lines
drivers/interconnect/intel/keembay-bwmon.c line 146
Quoted 11 lines
drivers/interconnect/intel/keembay-bwmon.c +11
-0+unsigned long j0, j1, delay; +u32 c0_0, c0_1; + +if (noc >=NOC_TYPE_MAX || + counter >=NOC_COUNTER_MAX || + !value) +return NOC_PROBE_ERR_INVALID_ARGS ; + + delay =msecs_to_jiffies (NOC_CAPTURE_TIMEOUT_MSEC ); + j0 = jiffies; + j1 = j0 + delay;j0 and j1 seem unnecessary
Yes, will change it to single timeout variable.
Quoted 8 lines
timeout = jiffies + msecs_to_jiffies(NOC_CAPTURE_TIMEOUT_MSEC);drivers/interconnect/intel/keembay-bwmon.c line 149
Quoted 3 lines
drivers/interconnect/intel/keembay-bwmon.c +3
-0+ do { + c0_0 = noc_readl((c_offset[counter] + C_VAL)); + usleep_range(10000, 11000);Seems a long time.
Will review if this time can be optimized.
Quoted 8 lines
Quoted 8 lines
drivers/interconnect/intel/keembay-bwmon.c +8
-0+ c0_1 =noc_readl ((c_offset[counter] +C_VAL )); +/* If mainctrl is zero , return error */ +if (noc_readl (MAINCTL ) ==0 ) +return NOC_PROBE_ERR_IN_PROGRESS ; +/* If counters are zero, keep reading */ +if (0 == c0_0 &&0 == c0_1) { +break ; + }else if (c0_0 != c0_1) {
Quoted 9 lines
drivers/interconnect/intel/keembay-bwmon.c line 163
Quoted 6 lines
drivers/interconnect/intel/keembay-bwmon.c +6
-0+continue ; + }else { +/* counters look good break the while */ +break ; + } + }while (time_before (jiffies, j1));} while (time_before(jiffies, timeout);
Variable will be renamed as timeout.
Quoted 16 lines
[]
drivers/interconnect/intel/keembay-bwmon.c line 184
Quoted 10 lines
drivers/interconnect/intel/keembay-bwmon.c +10
-0+static long noc_ioctl (struct file *file,unsigned int cmd,unsigned +long arg) { +struct flexnoc_countercapture capture_data; +void __user *argp = (void __user *)arg; +struct flexnoc_probestart probe_data; +struct flexnoc_setup setup_data; +int rc; + +if (!arg) { +pr_err ("NOC: Null pointer from user\n" );Perhaps useful to add #define
pr_fmt(fmt)KBUILD_MODNAME": " fmt near the top of the file.
will add this line to the top of the file
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
Regards,
Pandith
Hi--
Quoted 8 lines · On 8/30/21 11:36 PM, [email protected] wrote:
drivers/interconnect/intel/Kconfig new file +2
-0@@ -0,0 +1,19 @@ +# Copyright (C) 2019 Intel Corporation +# SPDX-License-Identifier: GPL-2.0-only
drivers/interconnect/intel/Kconfig line 12
Quoted 10 lines
drivers/interconnect/intel/Kconfig new file +10
-0+config INTERCONNECT_INTEL + bool "Intel Network-on-chip interconnect drivers" + help + Support for Intel's Network on chip interconnect hardware. + +config INTERCONNECT_INTEL_KEEMBAY + tristate "Intel Keem Bay Enable DDR profiling using NOC" + depends on INTERCONNECT_INTEL || ARCH_KEEMBAY || COMPILE_TEST + help + Enable this option for DDR bandwidth measurements using NOC
NOC.
drivers/interconnect/intel/Kconfig line 14
Quoted 2 lines
drivers/interconnect/intel/Kconfig new file +2
-0+ + Add support for Network-on-chip (NOC) in DDR Subsystem(DSS).
Subsystem (DSS).
drivers/interconnect/intel/Kconfig line 17
Quoted 3 lines
drivers/interconnect/intel/Kconfig new file +3
-0+ DSS NOC has capabilities to enable and get statistics profiling. + NOC driver enables features to configure and capture NOC probe + counters, needed for DSS bandwidth measurement.
Indent above with tab + 2 spaces instead of many spaces.
Quoted 2 lines
drivers/interconnect/intel/Kconfig new file +2
-0+ Say Y if using a processor that includes the Intel VPU such as + Keem Bay. If unsure, say N.
Signature · 1 line
~Randy
Hi
Quoted 30 lines
-----Original Message-----
From: Randy Dunlap <[email protected]>
Sent: Tuesday, August 31, 2021 9:46 PM
To: N, Pandith <[email protected]>; [email protected]; linux- [email protected]; [email protected]
Cc: [email protected]; Zhou, Furong <[email protected]>; Sangannavar, Mallikarjunappa <[email protected]>; Raja Subramanian, Lakshmi Bai <[email protected]>
Subject: Re: [PATCH V6 1/1] interconnect: intel: Add Keem Bay noc driverHi--
Quoted 7 lines · On 8/30/21 11:36 PM, [email protected] wrote:
drivers/interconnect/intel/Kconfig new file +0
-0drivers/interconnect/intel/Kconfig line 9
Quoted 9 lines
drivers/interconnect/intel/Kconfig new file +9
-0@@ -0,0 +1,19 @@ +# Copyright (C) 2019 Intel Corporation # SPDX-License-Identifier: +GPL-2.0-only config INTERCONNECT_INTEL + bool "Intel Network-on-chip interconnect drivers" + help + Support for Intel's Network on chip interconnect hardware. + +config INTERCONNECT_INTEL_KEEMBAY + tristate "Intel Keem Bay Enable DDR profiling using NOC" + depends on INTERCONNECT_INTEL || ARCH_KEEMBAY ||
COMPILE_TEST
Quoted 5 lines
drivers/interconnect/intel/Kconfig line 11
Quoted 2 lines
drivers/interconnect/intel/Kconfig new file +2
-0+ help + Enable this option for DDR bandwidth measurements using NOCNOC.
will change it to NOC.
Quoted 11 lines
drivers/interconnect/intel/Kconfig line 13
Quoted 2 lines
drivers/interconnect/intel/Kconfig new file +2
-0+ + Add support for Network-on-chip (NOC) in DDR Subsystem(DSS).Subsystem (DSS).
drivers/interconnect/intel/Kconfig line 17
Quoted 3 lines
drivers/interconnect/intel/Kconfig new file +3
-0+ DSS NOC has capabilities to enable and get statistics profiling. + NOC driver enables features to configure and capture NOC probe + counters, needed for DSS bandwidth measurement.Indent above with tab + 2 spaces instead of many spaces.
Accepted, tab +2 space in last line
Quoted 6 lines
drivers/interconnect/intel/Kconfig line 19
Quoted 2 lines
drivers/interconnect/intel/Kconfig new file +2
-0+ Say Y if using a processor that includes the Intel VPU such as + Keem Bay. If unsure, say N.--
~Randy
Regards,
Pandith