[PATCH v1 0/5] Add cpuidle support for Tegra194

STALE1967d

15 messages, 5 authors, 2021-03-16 · open the first message on its own page

[PATCH v1 0/5] Add cpuidle support for Tegra194

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-04 06:10:17

This series adds cpuidle support for Tegra194 carmel CPUs.

MCE firmware is responsible for deciding on CPU idle power state
based on state information and MCE firmware background work.

Tegra MCE ARI driver is the interface driver to communicate with
MCE firmware from the kernel.

CPU idle driver passes idle state information to MCE through Tegra
MCE driver and requests idle state transition to MCE happens through
PSCI CPU suspend.

This series includes below patches
- Add CPUIDLE section to MAINTAINERS
- Add Tegra MCE ARI driver to communicate with MCE firmware from kernel
- Add dt-bindings for Tegra194 cpu idle states
- Add cpuidle driver to support Tegra194 CPUs idle state management
- Update Tegra194 device tree with cpuidle support to Tegra194 CPUs.


Sowjanya Komatineni (5):
  MAINTAINERS: Add Tegra CPUIDLE driver section
  firmware: tegra: Add Tegra194 MCE ARI driver
  dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes
  cpuidle: Add Tegra194 cpuidle driver
  arm64: dts: tegra194: Add CPU idle states

 .../bindings/arm/nvidia,tegra194-ccplex.yaml       |  53 ++++
 MAINTAINERS                                        |  12 +
 arch/arm64/boot/dts/nvidia/tegra194.dtsi           |  28 ++
 drivers/cpuidle/Kconfig.arm                        |  10 +
 drivers/cpuidle/Makefile                           |   1 +
 drivers/cpuidle/cpuidle-tegra194.c                 | 319 +++++++++++++++++++++
 drivers/firmware/tegra/Kconfig                     |  11 +
 drivers/firmware/tegra/Makefile                    |   4 +
 drivers/firmware/tegra/mce-tegra194.c              | 155 ++++++++++
 drivers/firmware/tegra/mce.c                       |  88 ++++++
 include/soc/tegra/mce.h                            |  32 +++
 include/soc/tegra/t194_nvg.h                       |  56 ++++
 12 files changed, 769 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-tegra194.c
 create mode 100644 drivers/firmware/tegra/mce-tegra194.c
 create mode 100644 drivers/firmware/tegra/mce.c
 create mode 100644 include/soc/tegra/mce.h
 create mode 100644 include/soc/tegra/t194_nvg.h

-- 
2.7.4

[PATCH v1 2/5] firmware: tegra: Add Tegra194 MCE ARI driver

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-04 06:10:17

Tegra MCE Abstract Request Interface (ARI) driver manages all NVG
requests to MCE firmware running in the background.

This patch adds Tegra194 MCE interface driver for communicating with
MCE firmware on CPU state configurations and state transition requests
from the CPU idle driver.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/firmware/tegra/Kconfig        |  11 +++
 drivers/firmware/tegra/Makefile       |   4 +
 drivers/firmware/tegra/mce-tegra194.c | 155 ++++++++++++++++++++++++++++++++++
 drivers/firmware/tegra/mce.c          |  88 +++++++++++++++++++
 include/soc/tegra/mce.h               |  32 +++++++
 include/soc/tegra/t194_nvg.h          |  56 ++++++++++++
 6 files changed, 346 insertions(+)
 create mode 100644 drivers/firmware/tegra/mce-tegra194.c
 create mode 100644 drivers/firmware/tegra/mce.c
 create mode 100644 include/soc/tegra/mce.h
 create mode 100644 include/soc/tegra/t194_nvg.h
diff --git a/drivers/firmware/tegra/Kconfig b/drivers/firmware/tegra/Kconfig
index 1c8ba1f..a14ef1c 100644
--- a/drivers/firmware/tegra/Kconfig
+++ b/drivers/firmware/tegra/Kconfig
@@ -23,4 +23,15 @@ config TEGRA_BPMP
 	  This driver manages the IPC interface between host CPU and the
 	  firmware running on BPMP.
 
+config TEGRA_MCE
+	bool "Tegra MCE driver"
+	depends on ARCH_TEGRA_194_SOC
+	help
+	  MCE (Micro Codec Engine) firmware is in charge of CPUs power state
+	  transitions.
+
+	  Tegra MCE driver is an interface driver to communicate with MCE
+	  firmware for all the CPU power state change requests from CPU idle
+	  driver.
+
 endmenu
diff --git a/drivers/firmware/tegra/Makefile b/drivers/firmware/tegra/Makefile
index 49c87e0..2c0417e 100644
--- a/drivers/firmware/tegra/Makefile
+++ b/drivers/firmware/tegra/Makefile
@@ -6,3 +6,7 @@ tegra-bpmp-$(CONFIG_ARCH_TEGRA_194_SOC)	+= bpmp-tegra186.o
 tegra-bpmp-$(CONFIG_DEBUG_FS)	+= bpmp-debugfs.o
 obj-$(CONFIG_TEGRA_BPMP)	+= tegra-bpmp.o
 obj-$(CONFIG_TEGRA_IVC)		+= ivc.o
+
+tegra-mce-y			= mce.o
+tegra-mce-$(CONFIG_ARCH_TEGRA_194_SOC)	+= mce-tegra194.o
+obj-$(CONFIG_TEGRA_MCE)		+= tegra-mce.o
diff --git a/drivers/firmware/tegra/mce-tegra194.c b/drivers/firmware/tegra/mce-tegra194.c
new file mode 100644
index 0000000..5cec761d
--- /dev/null
+++ b/drivers/firmware/tegra/mce-tegra194.c
@@ -0,0 +1,155 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION.
+ */
+
+#include <linux/init.h>
+#include <linux/module.h>
+#include <soc/tegra/t194_nvg.h>
+#include <soc/tegra/mce.h>
+
+#include <asm/smp_plat.h>
+
+/* Issue a NVG request with data */
+static noinline notrace void nvg_send_req_data(u64 req, u64 data)
+{
+	asm volatile ("msr s3_0_c15_c1_2, %0\n"
+		      "msr s3_0_c15_c1_3, %1\n"
+		      :: "r" (req), "r" (data));
+}
+
+/* Issue a NVG request with no data */
+static noinline notrace void nvg_send_req(u64 req)
+{
+	asm volatile ("msr s3_0_c15_c1_2, %0\n" :: "r" (req));
+}
+
+/* Issue a NVG request to read the command response */
+static noinline notrace u64 nvg_get_response(void)
+{
+	u64 ret;
+
+	asm volatile ("mrs %0, s3_0_c15_c1_3" : "=r" (ret));
+
+	return ret;
+}
+
+static int tegra194_mce_update_cstate_info(u32 cluster, u32 ccplex, u32 system,
+					   u8 force, u32 wake_mask, bool valid)
+{
+	nvg_cstate_info_channel_t cstate_info = { 0 };
+
+	/* disable preemption */
+	preempt_disable();
+
+	/* update CLUSTER_CSTATE? */
+	if (cluster) {
+		cstate_info.bits.cluster_state = cluster;
+		cstate_info.bits.update_cluster = 1;
+	}
+
+	/* update CCPLEX_CSTATE? */
+	if (ccplex) {
+		cstate_info.bits.cg_cstate = ccplex;
+		cstate_info.bits.update_cg = 1;
+	}
+
+	/* update SYSTEM_CSTATE? */
+	if (system) {
+		cstate_info.bits.system_cstate = system;
+		cstate_info.bits.update_system = 1;
+	}
+
+	/* update wake mask value? */
+	if (valid)
+		cstate_info.bits.update_wake_mask = 1;
+
+	/* set the wake mask */
+	cstate_info.bits.wake_mask = wake_mask;
+
+	/* set the updated cstate info */
+	nvg_send_req_data(TEGRA_NVG_CHANNEL_CSTATE_INFO, cstate_info.flat);
+
+	/* enable preemption */
+	preempt_enable();
+
+	return 0;
+}
+
+static int tegra194_mce_update_crossover_time(u32 type, u32 time)
+{
+	if (type != TEGRA_NVG_CHANNEL_CROSSOVER_C6_LOWER_BOUND &&
+	    type != TEGRA_NVG_CHANNEL_CROSSOVER_CC6_LOWER_BOUND &&
+	    type != TEGRA_NVG_CHANNEL_CROSSOVER_CG7_LOWER_BOUND) {
+		pr_err("%s: unknown crossover type (%d)\n", __func__, type);
+		return -EINVAL;
+	}
+
+	/* disable pre-emption*/
+	preempt_disable();
+
+	nvg_send_req_data(type, (u64)time);
+
+	/* enable pre-emption */
+	preempt_enable();
+
+	return 0;
+}
+
+static int tegra194_mce_read_cstate_stats(u32 state, u64 *stats)
+{
+	if (!stats)
+		return -EINVAL;
+
+	/* disable preemption */
+	preempt_disable();
+
+	nvg_send_req_data(TEGRA_NVG_CHANNEL_CSTATE_STAT_QUERY_REQUEST, (u64)state);
+	nvg_send_req(TEGRA_NVG_CHANNEL_CSTATE_STAT_QUERY_VALUE);
+	*stats = nvg_get_response();
+
+	/* enable preemption */
+	preempt_enable();
+
+	return 0;
+}
+
+static int tegra194_mce_read_versions(u32 *major, u32 *minor)
+{
+	u64 version;
+
+	if (!major || !minor)
+		return -EINVAL;
+
+	/* disable preemption */
+	preempt_disable();
+
+	nvg_send_req(TEGRA_NVG_CHANNEL_VERSION);
+	version = nvg_get_response();
+	*minor = (u32)version;
+	*major = (u32)(version >> 32);
+
+	/* enable preemption */
+	preempt_enable();
+
+	return 0;
+}
+
+static struct tegra_mce_ops t194_mce_ops = {
+	.update_cstate_info = tegra194_mce_update_cstate_info,
+	.update_crossover_time = tegra194_mce_update_crossover_time,
+	.read_cstate_stats = tegra194_mce_read_cstate_stats,
+	.read_versions = tegra194_mce_read_versions,
+};
+
+static int __init tegra194_mce_early_init(void)
+{
+	tegra_mce_set_ops(&t194_mce_ops);
+
+	return 0;
+}
+early_initcall(tegra194_mce_early_init);
+
+MODULE_DESCRIPTION("NVIDIA Tegra194 MCE driver");
+MODULE_AUTHOR("NVIDIA Corporation");
+MODULE_LICENSE("GPL v2");
diff --git a/drivers/firmware/tegra/mce.c b/drivers/firmware/tegra/mce.c
new file mode 100644
index 0000000..6d949ff
--- /dev/null
+++ b/drivers/firmware/tegra/mce.c
@@ -0,0 +1,88 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Copyright (c) 2021, NVIDIA CORPORATION.
+ */
+
+#include <linux/module.h>
+#include <soc/tegra/mce.h>
+
+static struct tegra_mce_ops *mce_ops;
+
+void tegra_mce_set_ops(struct tegra_mce_ops *tegra_plat_mce_ops)
+{
+	mce_ops = tegra_plat_mce_ops;
+}
+
+/**
+ * Specify deepest cluster/ccplex/system states allowed.
+ *
+ * @cluster:	deepest cluster-wide state
+ * @ccplex:	deepest ccplex-wide state
+ * @system:	deepest system-wide state
+ * @force:	forced system state
+ * @wake_mask:	wake mask to be updated
+ * @valid:	is wake_mask applicable?
+ *
+ * Returns 0 if success.
+ */
+int tegra_mce_update_cstate_info(u32 cluster, u32 ccplex, u32 system,
+				 u8 force, u32 wake_mask, bool valid)
+{
+	if (mce_ops && mce_ops->update_cstate_info)
+		return mce_ops->update_cstate_info(cluster, ccplex, system, force,
+						   wake_mask, valid);
+	else
+		return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(tegra_mce_update_cstate_info);
+
+/**
+ * Update threshold for one specific c-state crossover
+ *
+ * @type: type of state crossover.
+ * @time: idle time threshold.
+ *
+ * Returns 0 if success.
+ */
+int tegra_mce_update_crossover_time(u32 type, u32 time)
+{
+	if (mce_ops && mce_ops->update_crossover_time)
+		return mce_ops->update_crossover_time(type, time);
+	else
+		return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(tegra_mce_update_crossover_time);
+
+/**
+ * Query the runtime stats of a specific cstate
+ *
+ * @state: c-state of the stats.
+ * @stats: output integer to hold the stats.
+ *
+ * Returns 0 if success.
+ */
+int tegra_mce_read_cstate_stats(u32 state, u64 *stats)
+{
+	if (mce_ops && mce_ops->read_cstate_stats)
+		return mce_ops->read_cstate_stats(state, stats);
+	else
+		return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(tegra_mce_read_cstate_stats);
+
+/**
+ * Read out MCE API major/minor versions
+ *
+ * @major: output for major number.
+ * @minor: output for minor number.
+ *
+ * Returns 0 if success.
+ */
+int tegra_mce_read_versions(u32 *major, u32 *minor)
+{
+	if (mce_ops && mce_ops->read_versions)
+		return mce_ops->read_versions(major, minor);
+	else
+		return -EOPNOTSUPP;
+}
+EXPORT_SYMBOL_GPL(tegra_mce_read_versions);
diff --git a/include/soc/tegra/mce.h b/include/soc/tegra/mce.h
new file mode 100644
index 0000000..d4be89f
--- /dev/null
+++ b/include/soc/tegra/mce.h
@@ -0,0 +1,32 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 NVIDIA Corporation
+ */
+
+#ifndef __SOC_TEGRA_MCE_H__
+#define __SOC_TEGRA_MCE_H__
+
+/*
+ * For correct version validation, below two defines need to be
+ * updated whenever there is a new ARI implementation.
+ */
+#define CUR_ARI_VER_MAJOR	1
+#define CUR_ARI_VER_MINOR	2
+
+int tegra_mce_update_cstate_info(u32 cluster, u32 ccplex, u32 system,
+				 u8 force, u32 wake_mask, bool valid);
+int tegra_mce_update_crossover_time(u32 type, u32 time);
+int tegra_mce_read_cstate_stats(u32 state, u64 *stats);
+int tegra_mce_read_versions(u32 *major, u32 *minor);
+
+struct tegra_mce_ops {
+	int (*update_cstate_info)(u32 cluster, u32 ccplex, u32 system,
+				  u8 force, u32 wake_mask, bool valid);
+	int (*update_crossover_time)(u32 type, u32 time);
+	int (*read_cstate_stats)(u32 state, u64 *stats);
+	int (*read_versions)(u32 *major, u32 *minor);
+};
+
+void tegra_mce_set_ops(struct tegra_mce_ops *mce_ops);
+
+#endif /* __SOC_TEGRA_MCE_H__ */
diff --git a/include/soc/tegra/t194_nvg.h b/include/soc/tegra/t194_nvg.h
new file mode 100644
index 0000000..3a5d558
--- /dev/null
+++ b/include/soc/tegra/t194_nvg.h
@@ -0,0 +1,56 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*
+ * Copyright (C) 2021 NVIDIA Corporation
+ */
+
+#ifndef __T194_NVG_H__
+#define __T194_NVG_H__
+
+/* Header for the NVIDIA Generic interface (NVG) */
+
+/*
+ * Major version increments may break backwards compatibility and binary
+ * compatibility. Minor version increments occur when there is only new
+ * functionality.
+ */
+enum {
+	TEGRA_NVG_VERSION_MAJOR = 6,
+	TEGRA_NVG_VERSION_MINOR = 6,
+};
+
+enum {
+	TEGRA_NVG_CHANNEL_VERSION				= 0,
+	TEGRA_NVG_CHANNEL_CSTATE_INFO				= 4,
+	TEGRA_NVG_CHANNEL_CROSSOVER_C6_LOWER_BOUND		= 5,
+	TEGRA_NVG_CHANNEL_CROSSOVER_CC6_LOWER_BOUND		= 6,
+	TEGRA_NVG_CHANNEL_CROSSOVER_CG7_LOWER_BOUND		= 8,
+	TEGRA_NVG_CHANNEL_CSTATE_STAT_QUERY_REQUEST		= 10,
+	TEGRA_NVG_CHANNEL_CSTATE_STAT_QUERY_VALUE		= 11,
+};
+
+enum {
+	TEGRA_NVG_CORE_C0 = 0,
+	TEGRA_NVG_CORE_C1 = 1,
+	TEGRA_NVG_CORE_C6 = 6,
+};
+
+/* NVG Data subformats */
+typedef union {
+	u64 flat;
+	struct nvg_cstate_info_channel_t {
+		uint32_t cluster_state		: 3;
+		uint32_t reserved_6_3		: 4;
+		uint32_t update_cluster		: 1;
+		uint32_t cg_cstate		: 3;
+		uint32_t reserved_14_11		: 4;
+		uint32_t update_cg		: 1;
+		uint32_t system_cstate		: 4;
+		uint32_t reserved_22_20		: 3;
+		uint32_t update_system		: 1;
+		uint32_t reserved_30_24		: 7;
+		uint32_t update_wake_mask	: 1;
+		uint32_t wake_mask		: 32;
+	} bits;
+} nvg_cstate_info_channel_t;
+
+#endif /* __T194_NVG_H__ */
-- 
2.7.4

[PATCH v1 1/5] MAINTAINERS: Add Tegra CPUIDLE driver section

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-04 06:10:17

Add Tegra CPUIDLE driver section with maintainers and mailing list
entries.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 MAINTAINERS | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index cac8429..277fcfd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4679,6 +4679,18 @@ S:	Supported
 F:	drivers/cpuidle/cpuidle-psci.h
 F:	drivers/cpuidle/cpuidle-psci-domain.c
 
+CPUIDLE DRIVER - TEGRA194
+M:	Thierry Reding <thierry.reding@gmail.com>
+M:	Jonathan Hunter <jonathanh@nvidia.com>
+M:	Krishna Sitaraman <ksitaraman@nvidia.com>
+M:	Sanjay Chandrashekara <sanjayc@nvidia.com>
+M:	Sowjanya Komatineni <skomatineni@nvidia.com>
+L:	linux-pm@vger.kernel.org
+L:	linux-tegra@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml
+F:	drivers/cpuidle/cpuidle-tegra194.c
+
 CRAMFS FILESYSTEM
 M:	Nicolas Pitre <nico@fluxnic.net>
 S:	Maintained
-- 
2.7.4

[PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-04 06:10:17

This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 .../bindings/arm/nvidia,tegra194-ccplex.yaml       | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
diff --git a/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml
index c9675c4..e1a5005 100644
--- a/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml
+++ b/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml
@@ -30,6 +30,36 @@ properties:
       Specifies the bpmp node that needs to be queried to get
       operating point data for all CPUs.
 
+  cluster-deepest-power-state:
+    $ref: /schemas/types.yaml#/definitions/uint32
+    description: CPU cluster deepest power state ID.
+
+patternProperties:
+  "^[a-z0-9]+$":
+    type: object
+    description: |
+      CPU core idle state nodes.
+      Refer to Documentation/devicetree/bindings/arm/idle-states.yaml
+
+    properties:
+      compatible:
+        enum:
+          - nvidia,tegra194-cpuidle-core
+
+  cpu_crossover_thresholds:
+    type: object
+    description: CPU idle states crossover threshold time in uSec.
+
+    patternProperties:
+      "^[a-z0-9]+$":
+        type: object
+
+        properties:
+          crossover_c1_c6:
+            $ref: /schemas/types.yaml#/definitions/uint32
+          crossover_cc1_cc6:
+            $ref: /schemas/types.yaml#/definitions/uint32
+
 additionalProperties: true
 
 examples:
@@ -39,12 +69,14 @@ examples:
       nvidia,bpmp = <&bpmp>;
       #address-cells = <1>;
       #size-cells = <0>;
+      cluster-deepest-power-state = <0x6>;
 
       cpu0_0: cpu@0 {
         compatible = "nvidia,tegra194-carmel";
         device_type = "cpu";
         reg = <0x0>;
         enable-method = "psci";
+        cpu-idle-states = <&C6>;
       };
 
       cpu0_1: cpu@1 {
@@ -52,6 +84,7 @@ examples:
         device_type = "cpu";
         reg = <0x001>;
         enable-method = "psci";
+        cpu-idle-states = <&C6>;
       };
 
       cpu1_0: cpu@100 {
@@ -59,6 +92,7 @@ examples:
         device_type = "cpu";
         reg = <0x100>;
         enable-method = "psci";
+        cpu-idle-states = <&C6>;
       };
 
       cpu1_1: cpu@101 {
@@ -66,6 +100,25 @@ examples:
         device_type = "cpu";
         reg = <0x101>;
         enable-method = "psci";
+        cpu-idle-states = <&C6>;
+      };
+
+      cpu_core_power_states {
+       C6: c6 {
+         compatible = "nvidia,tegra194-cpuidle-core";
+         idle-state-name = "CPU powergated, state retained";
+         wakeup-latency-us = <2000>;
+         min-residency-us = <30000>;
+         arm,psci-suspend-param = <0x6>;
+         status = "okay";
+       };
+     };
+
+     cpu_crossover_thresholds {
+      thresholds {
+        crossover_c1_c6         = <30000>;
+        crossover_cc1_cc6       = <80000>;
       };
+     };
     };
 ...
-- 
2.7.4

[PATCH v1 4/5] cpuidle: Add Tegra194 cpuidle driver

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-04 06:10:49

This patch adds cpuidle driver for Tegra194.

Tegra194 Carmel CPU supports two core idle state C1 (clock gated)
and C6 (power gated with state restoration) and one cluster idle
state CC6 (power gated).

MCE firmware makes decision on core/cluster power state transition
based on its background tasks and states information provided by
CPU idle driver.

CPU idle driver provides deepest cluster power state, core power
state transition request, estimated time of next wake-up and states
crossover thresholds to MCE firmware through Tegra mce driver.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 drivers/cpuidle/Kconfig.arm        |  10 ++
 drivers/cpuidle/Makefile           |   1 +
 drivers/cpuidle/cpuidle-tegra194.c | 319 +++++++++++++++++++++++++++++++++++++
 3 files changed, 330 insertions(+)
 create mode 100644 drivers/cpuidle/cpuidle-tegra194.c
diff --git a/drivers/cpuidle/Kconfig.arm b/drivers/cpuidle/Kconfig.arm
index 0844fad..e9adad1 100644
--- a/drivers/cpuidle/Kconfig.arm
+++ b/drivers/cpuidle/Kconfig.arm
@@ -105,6 +105,16 @@ config ARM_TEGRA_CPUIDLE
 	help
 	  Select this to enable cpuidle for NVIDIA Tegra20/30/114/124 SoCs.
 
+config ARM_TEGRA194_CPUIDLE
+	tristate "CPU Idle Driver for NVIDIA Tegra194 SoC"
+	depends on ARCH_TEGRA_194_SOC
+	select ARM_CPU_SUSPEND
+	select DT_IDLE_STATES
+	select TEGRA_MCE
+	default y
+	help
+	  Select this to enable cpuidle for NVIDIA Tegra194 SoC.
+
 config ARM_QCOM_SPM_CPUIDLE
 	bool "CPU Idle Driver for Qualcomm Subsystem Power Manager (SPM)"
 	depends on (ARCH_QCOM || COMPILE_TEST) && !ARM64
diff --git a/drivers/cpuidle/Makefile b/drivers/cpuidle/Makefile
index 26bbc5e..4d89578 100644
--- a/drivers/cpuidle/Makefile
+++ b/drivers/cpuidle/Makefile
@@ -24,6 +24,7 @@ obj-$(CONFIG_ARM_CPUIDLE)		+= cpuidle-arm.o
 obj-$(CONFIG_ARM_PSCI_CPUIDLE)		+= cpuidle-psci.o
 obj-$(CONFIG_ARM_PSCI_CPUIDLE_DOMAIN)	+= cpuidle-psci-domain.o
 obj-$(CONFIG_ARM_TEGRA_CPUIDLE)		+= cpuidle-tegra.o
+obj-$(CONFIG_ARM_TEGRA194_CPUIDLE)      += cpuidle-tegra194.o
 obj-$(CONFIG_ARM_QCOM_SPM_CPUIDLE)	+= cpuidle-qcom-spm.o
 
 ###############################################################################
diff --git a/drivers/cpuidle/cpuidle-tegra194.c b/drivers/cpuidle/cpuidle-tegra194.c
new file mode 100644
index 0000000..4ae67ce
--- /dev/null
+++ b/drivers/cpuidle/cpuidle-tegra194.c
@@ -0,0 +1,319 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * CPU idle driver for Tegra194 CPUs
+ *
+ * Copyright (c) 2020-2021, NVIDIA Corporation.
+ */
+
+#include <asm/arch_timer.h>
+#include <asm/cpu.h>
+#include <asm/cpuidle.h>
+#include <asm/cputype.h>
+#include <asm/suspend.h>
+
+#include <linux/cpuhotplug.h>
+#include <linux/cpuidle.h>
+#include <linux/cpu_pm.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/module.h>
+#include <linux/of_platform.h>
+#include <linux/psci.h>
+#include <linux/suspend.h>
+
+#include <soc/tegra/mce.h>
+#include <soc/tegra/t194_nvg.h>
+
+#include "cpuidle-psci.h"
+#include "dt_idle_states.h"
+
+#define T194_NVG_CROSSOVER_C6		TEGRA_NVG_CHANNEL_CROSSOVER_C6_LOWER_BOUND
+#define T194_NVG_CROSSOVER_CC6		TEGRA_NVG_CHANNEL_CROSSOVER_CC6_LOWER_BOUND
+
+#define PSCI_PSTATE_ID_MASK		0xf
+#define PSCI_PSTATE_WKTIM_MASK		0x0ffffff0
+#define PSCI_PSTATE_WKTIM_SHIFT		4
+
+/*
+ * BG_TIME is margin added to target_residency so that actual HW has better
+ * chance entering deep idle state instead of getting back to shallower one.
+ * Units in us.
+ */
+#define BG_TIME				2000
+
+static DEFINE_PER_CPU_READ_MOSTLY(u32 *, psci_power_state);
+
+static struct cpuidle_driver t194_cpu_idle_driver;
+static enum cpuhp_state hp_state;
+static u32 deepest_cc_state;
+static u32 tsc_per_usec;
+
+static bool check_mce_version(void)
+{
+	u32 mce_version_major, mce_version_minor;
+	int err;
+
+	err = tegra_mce_read_versions(&mce_version_major, &mce_version_minor);
+	if (!err && mce_version_major >= TEGRA_NVG_VERSION_MAJOR)
+		return true;
+	else
+		return false;
+}
+
+static int
+t194_cpu_enter_state(struct cpuidle_device *dev, struct cpuidle_driver *drv, int index)
+{
+	u32 *state = __this_cpu_read(psci_power_state);
+	u32 power_state = state[index];
+	u32 wake_time;
+	int ret;
+
+	/*
+	 * MCE firmware does the state transition based on requested idle state,
+	 * state crossover thresholds and target residency time along with its
+	 * background work.
+	 * Pass the state target_residency time along with state ID to MCE
+	 * firmware through PSCI power-state value.
+	 *
+	 * LSB 8 bits of wake time is lost and only 24 MSB bits of wake time can fit
+	 * into additional bits of state value.
+	 */
+	wake_time = (drv->states[index].target_residency + BG_TIME) * tsc_per_usec;
+	power_state |= ((wake_time >> PSCI_PSTATE_WKTIM_SHIFT) & PSCI_PSTATE_WKTIM_MASK);
+
+	if ((power_state & PSCI_PSTATE_ID_MASK) == TEGRA_NVG_CORE_C6)
+		ret = CPU_PM_CPU_IDLE_ENTER_RETENTION_PARAM(psci_cpu_suspend_enter, index,
+							    power_state);
+	else
+		ret = CPU_PM_CPU_IDLE_ENTER_PARAM(psci_cpu_suspend_enter, index, power_state);
+
+	return ret;
+}
+
+static struct cpuidle_driver t194_cpu_idle_driver = {
+	.name = "tegra194_cpuidle_driver",
+	.owner = THIS_MODULE,
+	.states[0] = {
+		.enter			= t194_cpu_enter_state,
+		.exit_latency		= 1,
+		.target_residency	= 1,
+		.power_usage		= UINT_MAX,
+		.flags			= 0,
+		.name			= "c1",
+		.desc			= "CPU clock gated",
+	}
+};
+
+static const struct of_device_id t194_cpuidle_of_match[] = {
+	{ .compatible = "nvidia,tegra194-cpuidle-core", .data = t194_cpu_enter_state },
+	{ },
+};
+
+struct xover_table {
+	char *name;
+	int index;
+};
+
+static void send_crossover(void *data)
+{
+	struct device_node *of_states = (struct device_node *)data;
+	struct device_node *child;
+	u32 value;
+	int i;
+
+	struct xover_table table1[] = {
+		{"crossover_c1_c6", T194_NVG_CROSSOVER_C6},
+		{"crossover_cc1_cc6", T194_NVG_CROSSOVER_CC6},
+	};
+
+	/* pass the state crossover thresholds to MCE firmware */
+	for_each_child_of_node(of_states, child)
+		for (i = 0; i < ARRAY_SIZE(table1); i++) {
+			if (of_property_read_u32(child, table1[i].name, &value) == 0)
+				tegra_mce_update_crossover_time(table1[i].index,
+								value * tsc_per_usec);
+	}
+}
+
+static int crossover_init(struct device_node *cpunode)
+{
+	struct device_node *cpu_xover;
+
+	cpu_xover = of_get_child_by_name(cpunode, "cpu_crossover_thresholds");
+	if (!cpu_xover)
+		pr_warn("cpuidle: %s: missing crossover thresholds in dt\n", __func__);
+	else
+		on_each_cpu_mask(cpu_online_mask, send_crossover, cpu_xover, 1);
+
+	return 0;
+}
+
+static void program_cc_state(void *data)
+{
+	u32 *cc_state = (u32 *)data;
+
+	tegra_mce_update_cstate_info(*cc_state, 0, 0, 0, 0, 0);
+}
+
+static int
+tegra_suspend_notify_callback(struct notifier_block *nb, unsigned long action, void *pcpu)
+{
+	switch (action) {
+	case PM_POST_SUSPEND:
+	/*
+	 * Re-program deepest allowed cluster and cluster group power state
+	 * after system resumes from SC7
+	 */
+		on_each_cpu_mask(cpu_online_mask, program_cc_state, &deepest_cc_state, 1);
+		break;
+	}
+
+	return NOTIFY_OK;
+}
+
+static struct notifier_block suspend_notifier = {
+	.notifier_call = tegra_suspend_notify_callback,
+};
+
+static int tegra_cpu_online(unsigned int cpu)
+{
+	/*
+	 * Program deepest allowed cluster and cluster group power state
+	 * after a core in that cluster is onlined.
+	 */
+	smp_call_function_single(cpu, program_cc_state, &deepest_cc_state, 1);
+
+	return 0;
+}
+
+static int tegra_cpuidle_psci_dt_init(struct device *dev, struct cpuidle_driver *drv)
+{
+	unsigned int state_count = drv->state_count;
+	struct device_node *cpu_node, *state_node;
+	int i, cpu, state_idx = 1, ret = 0;
+	u32 *psci_states;
+
+	psci_states = devm_kcalloc(dev, state_count, sizeof(*psci_states), GFP_KERNEL);
+	if (!psci_states)
+		return -ENOMEM;
+
+	cpu_node = of_cpu_device_node_get(cpumask_first(drv->cpumask));
+	for (i = 1; ; i++) {
+		state_node = of_get_cpu_state_node(cpu_node, i - 1);
+		if (!state_node)
+			break;
+
+		if (!of_device_is_available(state_node)) {
+			of_node_put(state_node);
+			continue;
+		}
+
+		ret = psci_dt_parse_state_node(state_node, &psci_states[state_idx++]);
+		of_node_put(state_node);
+		if (ret)
+			goto put_cpunode;
+	}
+
+	of_node_put(cpu_node);
+
+	for_each_online_cpu(cpu)
+		per_cpu(psci_power_state, cpu) = psci_states;
+
+	return 0;
+
+put_cpunode:
+	of_node_put(cpu_node);
+	return ret;
+}
+
+static int __init tegra194_cpuidle_probe(struct platform_device *pdev)
+{
+	struct cpumask *cpumask;
+	int cpu, ret;
+
+	if (!check_mce_version()) {
+		pr_err("cpuidle: incompatible MCE version, cannot register driver\n");
+		return -ENODEV;
+	}
+
+	tsc_per_usec = arch_timer_get_cntfrq() / 1000000;
+
+	cpumask = devm_kzalloc(&pdev->dev, cpumask_size(), GFP_KERNEL);
+	for_each_online_cpu(cpu)
+		cpumask_set_cpu(cpu, cpumask);
+	t194_cpu_idle_driver.cpumask = cpumask;
+
+	/*
+	 * CCPLEX MCE firmware does core/cluster state transitions based on idle
+	 * thresholds along with requested state and target residency time.
+	 * So, state idle time crossover thresholds should be provided to MCE
+	 * firmware.
+	 */
+	crossover_init(pdev->dev.of_node);
+
+	ret = of_property_read_u32(pdev->dev.of_node,
+				   "cluster-deepest-power-state", &deepest_cc_state);
+	on_each_cpu_mask(cpu_online_mask, program_cc_state, &deepest_cc_state, 1);
+
+	ret = dt_init_idle_driver(&t194_cpu_idle_driver, t194_cpuidle_of_match, 1);
+	if (ret <= 0) {
+		pr_err("cpuidle: failed to init idle driver states\n");
+		ret = -ENODEV;
+		goto probe_exit;
+	}
+
+	/* initialize cpuidle states psci power-state from DT */
+	ret = tegra_cpuidle_psci_dt_init(&pdev->dev, &t194_cpu_idle_driver);
+	if (ret)
+		goto probe_exit;
+
+	ret = cpuidle_register(&t194_cpu_idle_driver, NULL);
+	if (ret) {
+		pr_err("cpuidle: failed to register cpuidle driver\n");
+		goto probe_exit;
+	}
+
+	ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "tegra_cpu:online", tegra_cpu_online, NULL);
+	if (ret < 0) {
+		pr_err("cpuidle: failed to setup cpu hotplug state callbacks\n");
+		goto cpuhp_error;
+	}
+
+	hp_state = ret;
+
+	register_pm_notifier(&suspend_notifier);
+
+	return 0;
+
+cpuhp_error:
+	cpuidle_unregister(&t194_cpu_idle_driver);
+probe_exit:
+	return ret;
+}
+
+static int tegra194_cpuidle_remove(struct platform_device *pdev)
+{
+	unregister_pm_notifier(&suspend_notifier);
+	cpuhp_remove_state(hp_state);
+	cpuidle_unregister(&t194_cpu_idle_driver);
+	kfree(t194_cpu_idle_driver.cpumask);
+
+	return 0;
+}
+
+static const struct of_device_id tegra194_cpuidle_of_match[] = {
+	{ .compatible = "nvidia,tegra194-ccplex" },
+	{ /* sentinel */ }
+};
+
+static struct platform_driver tegra194_cpuidle_driver __refdata = {
+	.driver = {
+		.owner = THIS_MODULE,
+		.name = "cpuidle-tegra194",
+		.of_match_table = of_match_ptr(tegra194_cpuidle_of_match),
+	},
+	.probe	= tegra194_cpuidle_probe,
+	.remove	= tegra194_cpuidle_remove,
+};
+
+module_platform_driver(tegra194_cpuidle_driver);
-- 
2.7.4

[PATCH v1 5/5] arm64: dts: tegra194: Add CPU idle states

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-04 06:10:49

This patch adds CPU core and cluster idle states to Tegra194
device tree

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 arch/arm64/boot/dts/nvidia/tegra194.dtsi | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)
diff --git a/arch/arm64/boot/dts/nvidia/tegra194.dtsi b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
index 9449156..f9c2731 100644
--- a/arch/arm64/boot/dts/nvidia/tegra194.dtsi
+++ b/arch/arm64/boot/dts/nvidia/tegra194.dtsi
@@ -2155,12 +2155,14 @@
 		nvidia,bpmp = <&bpmp>;
 		#address-cells = <1>;
 		#size-cells = <0>;
+		cluster-deepest-power-state = <0x6>;
 
 		cpu0_0: cpu@0 {
 			compatible = "nvidia,tegra194-carmel";
 			device_type = "cpu";
 			reg = <0x000>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2175,6 +2177,7 @@
 			device_type = "cpu";
 			reg = <0x001>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2189,6 +2192,7 @@
 			device_type = "cpu";
 			reg = <0x100>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2203,6 +2207,7 @@
 			device_type = "cpu";
 			reg = <0x101>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2217,6 +2222,7 @@
 			device_type = "cpu";
 			reg = <0x200>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2231,6 +2237,7 @@
 			device_type = "cpu";
 			reg = <0x201>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2245,6 +2252,7 @@
 			device_type = "cpu";
 			reg = <0x300>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2259,6 +2267,7 @@
 			device_type = "cpu";
 			reg = <0x301>;
 			enable-method = "psci";
+			cpu-idle-states = <&C6>;
 			i-cache-size = <131072>;
 			i-cache-line-size = <64>;
 			i-cache-sets = <512>;
@@ -2343,12 +2352,31 @@
 			cache-line-size = <64>;
 			cache-sets = <4096>;
 		};
+
+		cpu_core_power_states {
+			C6: c6 {
+				compatible = "nvidia,tegra194-cpuidle-core";
+				idle-state-name = "CPU powergated, state retained";
+				wakeup-latency-us = <2000>;
+				min-residency-us = <30000>;
+				arm,psci-suspend-param = <0x6>;
+				status = "okay";
+			};
+		};
+
+		cpu_crossover_thresholds {
+			thresholds {
+				crossover_c1_c6 = <30000>;
+				crossover_cc1_cc6 = <80000>;
+			};
+		};
 	};
 
 	psci {
 		compatible = "arm,psci-1.0";
 		status = "okay";
 		method = "smc";
+		cpu_suspend = <0xC4000001>;
 	};
 
 	sound {
-- 
2.7.4

Re: [PATCH v1 1/5] MAINTAINERS: Add Tegra CPUIDLE driver section

From: Daniel Lezcano <hidden>
Date: 2021-03-04 08:02:53

On 04/03/2021 07:08, Sowjanya Komatineni wrote:
quoted hunk
Add Tegra CPUIDLE driver section with maintainers and mailing list
entries.

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 MAINTAINERS | 12 ++++++++++++
 1 file changed, 12 insertions(+)
diff --git a/MAINTAINERS b/MAINTAINERS
index cac8429..277fcfd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4679,6 +4679,18 @@ S:	Supported
 F:	drivers/cpuidle/cpuidle-psci.h
 F:	drivers/cpuidle/cpuidle-psci-domain.c
 
+CPUIDLE DRIVER - TEGRA194
+M:	Thierry Reding <thierry.reding@gmail.com>
+M:	Jonathan Hunter <jonathanh@nvidia.com>
+M:	Krishna Sitaraman <ksitaraman@nvidia.com>
+M:	Sanjay Chandrashekara <sanjayc@nvidia.com>
+M:	Sowjanya Komatineni <skomatineni@nvidia.com>
It does not make sense to have so many maintainers for a single file.

+L:	linux-pm@vger.kernel.org
+L:	linux-tegra@vger.kernel.org
+S:	Maintained
+F:	Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml
+F:	drivers/cpuidle/cpuidle-tegra194.c
+
 CRAMFS FILESYSTEM
 M:	Nicolas Pitre [off-list ref]
 S:	Maintained

-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Rob Herring <robh@kernel.org>
Date: 2021-03-04 20:49:12

On Wed, 03 Mar 2021 22:08:10 -0800, Sowjanya Komatineni wrote:
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document

Signed-off-by: Sowjanya Komatineni <skomatineni@nvidia.com>
---
 .../bindings/arm/nvidia,tegra194-ccplex.yaml       | 53 ++++++++++++++++++++++
 1 file changed, 53 insertions(+)
My bot found errors running 'make dt_binding_check' on your patch:

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.example.dt.yaml: cpus: compatible: ['nvidia,tegra194-ccplex'] is not of type 'object'
	From schema: /builds/robherring/linux-dt-review/Documentation/devicetree/bindings/arm/nvidia,tegra194-ccplex.yaml

See https://patchwork.ozlabs.org/patch/1447108

This check can fail if there are any dependencies. The base for a patch
series is generally the most recent rc1.

If you already ran 'make dt_binding_check' and didn't see the above
error(s), then make sure 'yamllint' is installed and dt-schema is up to
date:

pip3 install dtschema --upgrade

Please check and re-submit.

Re: [PATCH v1 4/5] cpuidle: Add Tegra194 cpuidle driver

From: Dmitry Osipenko <digetx@gmail.com>
Date: 2021-03-05 13:50:53

04.03.2021 09:08, Sowjanya Komatineni пишет:
...
+static int __init tegra194_cpuidle_probe(struct platform_device *pdev)
+{
+	struct cpumask *cpumask;
+	int cpu, ret;
+
+	if (!check_mce_version()) {
+		pr_err("cpuidle: incompatible MCE version, cannot register driver\n");
Should be dev_err() everywhere.
+		return -ENODEV;
+	}
+
+	tsc_per_usec = arch_timer_get_cntfrq() / 1000000;
+
+	cpumask = devm_kzalloc(&pdev->dev, cpumask_size(), GFP_KERNEL);
+	for_each_online_cpu(cpu)
+		cpumask_set_cpu(cpu, cpumask);
cpumask_copy(..)?
+	t194_cpu_idle_driver.cpumask = cpumask;
Depending on 'online' mask instead of the 'present' mask looks odd. Is
this really intended to be so?

...
+static int tegra194_cpuidle_remove(struct platform_device *pdev)
+{
+	unregister_pm_notifier(&suspend_notifier);
+	cpuhp_remove_state(hp_state);
+	cpuidle_unregister(&t194_cpu_idle_driver);
+	kfree(t194_cpu_idle_driver.cpumask);
kfree() of a managed resource.

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sudeep Holla <hidden>
Date: 2021-03-08 04:39:04

On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.

-- 
Regards,
Sudeep

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-08 18:33:14

On 3/7/21 8:37 PM, Sudeep Holla wrote:
On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
quoted
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.
MCE firmware is in charge of state transition for Tegra194 carmel CPUs.

For run-time state transitions, need to provide state request along with 
its residency time to MCE firmware which is running in the background.

State min residency is updated into power_state value along with state 
id that is passed to psci_cpu_suspend_enter

Also states cross-over idle times need to be provided to MCE firmware.

MCE firmware decides on state transition based on these inputs along 
with its background work load.

So, Tegra specific CPU idle driver is required mainly to provide 
cross-over thresholds from DT and run time idle state information to MCE 
firmware through Tegra MCE communication APIs.

Allowing cross-over threshold through DT allows users to vary idle time 
thresholds for state transitions based on different use-cases.

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-10 23:20:45

On 3/8/21 10:32 AM, Sowjanya Komatineni wrote:
On 3/7/21 8:37 PM, Sudeep Holla wrote:
quoted
On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
quoted
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.
MCE firmware is in charge of state transition for Tegra194 carmel CPUs.

For run-time state transitions, need to provide state request along 
with its residency time to MCE firmware which is running in the 
background.

State min residency is updated into power_state value along with state 
id that is passed to psci_cpu_suspend_enter

Also states cross-over idle times need to be provided to MCE firmware.

MCE firmware decides on state transition based on these inputs along 
with its background work load.

So, Tegra specific CPU idle driver is required mainly to provide 
cross-over thresholds from DT and run time idle state information to 
MCE firmware through Tegra MCE communication APIs.

Allowing cross-over threshold through DT allows users to vary idle 
time thresholds for state transitions based on different use-cases.
Hi Sudeep,

Can you please let me know if you have any more concerns for having this 
Tegra specific cpuidle driver?

Thanks

Sowjanya

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sudeep Holla <hidden>
Date: 2021-03-11 02:53:26

On Mon, Mar 08, 2021 at 10:32:17AM -0800, Sowjanya Komatineni wrote:
On 3/7/21 8:37 PM, Sudeep Holla wrote:
quoted
On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
quoted
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.
MCE firmware is in charge of state transition for Tegra194 carmel CPUs.
Sure, but I assume only TF-A talks to MCE and not any OSPM/Linux kernel.
For run-time state transitions, need to provide state request along with its
residency time to MCE firmware which is running in the background.
Sounds similar to x86 mwait, perhaps we need to extend PSCI if we need
to make this firmware PSCI compliant or just say it is not and implement
completely independent implementation. I am not saying that is acceptable
ATM but I prefer not to mix some implementation to make it look like
PSCI compliant.
State min residency is updated into power_state value along with state id
that is passed to psci_cpu_suspend_enter
Sounds like a hack/workaround. I would prefer to standardise that. IIUC
the power_state is more static and derived from DT. I don't like to
overload that TBH. Need to check with authors of that binding.
Also states cross-over idle times need to be provided to MCE firmware.
New requirements if this has to be PSCI compliant.
MCE firmware decides on state transition based on these inputs along with
its background work load.

So, Tegra specific CPU idle driver is required mainly to provide cross-over
thresholds from DT and run time idle state information to MCE firmware
through Tegra MCE communication APIs.
I am worried if different vendors will come up with different custom
solution for this. We need to either standardise this is Linux/DT or
in PSCI.
Allowing cross-over threshold through DT allows users to vary idle time
thresholds for state transitions based on different use-cases.
Sounds like policy and not platform specific to be in DT, but I will leave
that to DT maintainers.

--
Regards,
Sudeep

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sowjanya Komatineni <skomatineni@nvidia.com>
Date: 2021-03-11 21:13:09

On 3/10/21 6:52 PM, Sudeep Holla wrote:
On Mon, Mar 08, 2021 at 10:32:17AM -0800, Sowjanya Komatineni wrote:
quoted
On 3/7/21 8:37 PM, Sudeep Holla wrote:
quoted
On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
quoted
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.
MCE firmware is in charge of state transition for Tegra194 carmel CPUs.
Sure, but I assume only TF-A talks to MCE and not any OSPM/Linux kernel.
No. Tegra194 CPU idle driver works with MCE firmware running in 
background so cpuidle kernel driver also talks to MCE firmware directly 
on state information.
quoted
For run-time state transitions, need to provide state request along with its
residency time to MCE firmware which is running in the background.
Sounds similar to x86 mwait, perhaps we need to extend PSCI if we need
to make this firmware PSCI compliant or just say it is not and implement
completely independent implementation. I am not saying that is acceptable
ATM but I prefer not to mix some implementation to make it look like
PSCI compliant.
quoted
State min residency is updated into power_state value along with state id
that is passed to psci_cpu_suspend_enter
Sounds like a hack/workaround. I would prefer to standardise that. IIUC
the power_state is more static and derived from DT. I don't like to
overload that TBH. Need to check with authors of that binding.
Passing state idle time to ATF along with state to enter is Tegra 
specific as ATF firmware updates idle time to Tegra MCE firmware which 
will be used for deciding on state transition along with other 
information and background load.

Not sure if this need to be standardized but will try to find alternate 
way to update idle time without misusing power-state value.

Will discuss on this internally and get back.
quoted
Also states cross-over idle times need to be provided to MCE firmware.
New requirements if this has to be PSCI compliant.
Updating cross-over idle times from DT to MCE firmware directly from 
cpuidle kernel driver with corresponding MCE ARI commands is again Tegra 
specific.
quoted
MCE firmware decides on state transition based on these inputs along with
its background work load.

So, Tegra specific CPU idle driver is required mainly to provide cross-over
thresholds from DT and run time idle state information to MCE firmware
through Tegra MCE communication APIs.
I am worried if different vendors will come up with different custom
solution for this. We need to either standardise this is Linux/DT or
in PSCI.
quoted
Allowing cross-over threshold through DT allows users to vary idle time
thresholds for state transitions based on different use-cases.
Sounds like policy and not platform specific to be in DT, but I will leave
that to DT maintainers.
cross-over idle times are based on supported CPU core and cluster states 
and updating these from DT to Tegra MCE firmware running in the 
background is Tegra specific.
--
Regards,
Sudeep

Re: [PATCH v1 3/5] dt-bindings: arm: Add cpu-idle-states to Tegra194 CPU nodes

From: Sudeep Holla <hidden>
Date: 2021-03-16 05:39:50

+Lorenzo

Hi Sowjanya,

Sorry for the delayed response. I am still in vacation 😉

On Thu, Mar 11, 2021 at 01:11:37PM -0800, Sowjanya Komatineni wrote:
On 3/10/21 6:52 PM, Sudeep Holla wrote:
quoted
On Mon, Mar 08, 2021 at 10:32:17AM -0800, Sowjanya Komatineni wrote:
quoted
On 3/7/21 8:37 PM, Sudeep Holla wrote:
quoted
On Wed, Mar 03, 2021 at 10:08:10PM -0800, Sowjanya Komatineni wrote:
quoted
This patch adds cpu-idle-states and corresponding state nodes to
Tegra194 CPU in dt-binding document
I see that this platform has PSCI support. Can you care to explain why
you need additional DT bindings and driver for PSCI based CPU suspend.
Until the reasons are convincing, consider NACK from my side for this
driver and DT bindings. You should be really using those bindings and
the driver may be with minor changes there.
MCE firmware is in charge of state transition for Tegra194 carmel CPUs.
Sure, but I assume only TF-A talks to MCE and not any OSPM/Linux kernel.
No. Tegra194 CPU idle driver works with MCE firmware running in background
so cpuidle kernel driver also talks to MCE firmware directly on state
information.
If that is the case I wouldn't term this as PSCI compliant firmware and
wouldn't attempt to use PSCI CPU idle driver. Now if we would what to allow
non-PSCI idle driver for Arm64 is entirely different question that deserves
a separate discussion IMO.
quoted
quoted
For run-time state transitions, need to provide state request along with its
residency time to MCE firmware which is running in the background.
Sounds similar to x86 mwait, perhaps we need to extend PSCI if we need
to make this firmware PSCI compliant or just say it is not and implement
completely independent implementation. I am not saying that is acceptable
ATM but I prefer not to mix some implementation to make it look like
PSCI compliant.
quoted
State min residency is updated into power_state value along with state id
that is passed to psci_cpu_suspend_enter
Sounds like a hack/workaround. I would prefer to standardise that. IIUC
the power_state is more static and derived from DT. I don't like to
overload that TBH. Need to check with authors of that binding.
Passing state idle time to ATF along with state to enter is Tegra specific
as ATF firmware updates idle time to Tegra MCE firmware which will be used
for deciding on state transition along with other information and background
load.
So far we don't have any platform specific PSCI in OSPM and I prefer to keep
it that way.
Not sure if this need to be standardized but will try to find alternate way
to update idle time without misusing power-state value.
Sure, we can always review and see if any alternatives are acceptable, but
I am bit nervous to tie this as PSCI if it is not strictly spec compliant.
Will discuss on this internally and get back.
Thanks.
quoted
quoted
Also states cross-over idle times need to be provided to MCE firmware.
New requirements if this has to be PSCI compliant.
Updating cross-over idle times from DT to MCE firmware directly from cpuidle
kernel driver with corresponding MCE ARI commands is again Tegra specific.
So all there are platform specific but static information you need from DT ?
If so, what can't it be made part of TF-A and OSPM can avoid interfering
with that info completely. My understanding was that OSPM provides runtime
hints like x86 mwait. If that's not the case, I am failing to understand
the need for OSPM to pass such static information from DT to the firmware.
Why can't that be just part of the firmware to begin with ?
quoted
quoted
MCE firmware decides on state transition based on these inputs along with
its background work load.
What do you mean by this *"background work load"* ?

--
Regards,
Sudeep
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help