Skip to contentSkip to conversation

soc: mediatek: SVS: introduce MTK SVS engine

PATCH Superseded Roger Lu 3 participants 23 messages Jan 7, 2022 – Jan 26, 2022

Subsystems (2)

v21 of 25 Superseded Latest is v25 →

lore
Share & tools
lore

Roger Lu

author
#0 lore raw

v21 cover soc: mediatek: SVS: introduce MTK SVS engine

The Smart Voltage Scaling(SVS) engine is a piece of hardware which calculates suitable SVS bank voltages to OPP voltage table. Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck when receiving OPP_EVENT_ADJUST_VOLTAGE.

1. SVS driver uses OPP adjust event in [1] to update OPP table voltage part.
2. SVS driver gets thermal/GPU device by node [2][3] and CPU device by get_cpu_device().
After retrieving subsys device, SVS driver calls device_link_add() to make sure probe/suspend callback priority.

[1] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=25cb20a212a1f989385dfe23230817e69c62bee5
[2] https://git.kernel.org/pub/scm/linux/kernel/git/vireshk/pm.git/commit/?h=opp/linux-next&id=b325ce39785b1408040d90365a6ab1aa36e94f87
[3] https://git.kernel.org/pub/scm/linux/kernel/git/matthias.bgg/linux.git/commit/?h=v5.16-next/dts64&id=a8168cebf1bca1b5269e8a7eb2626fb76814d6e2

change since v20:
- Rebase to kernel 5.16-rc1
- Separate mt8183 svs as two patches for minimun version support. patch1 (init01 ~ init02) and patch2 (mon mode).
- Remove unused flags in each patch.
- Move @SVSB_PHASE_ERROR before @SVSB_PHASE_INIT01 for order correctness.
- Reorder the struct member comment and coding for the consistenty. Also revise the ambiguous comments.
- Use [writel,readl]_relaxed instead of [writel,readl].
- Fix ->efuse_parsing() not return "false" mistake.
- Move more commen settings out of svs bank structure for shrinking data structure size.
- Split up the "get platform data" and "platform probe" behavior.
- Change the naming of "svs_is_supported()" to "svs_is_efuse_data_correct()" for better understanding.
- SVSB_DC_SIGNED_BIT is configured as BIT(15) instead of 0x8000.
- Add the missing variables comments back in the structure.
- Add define "MAX_OPP_ENTRIES" for representing opp total count.
- Add more kernel log description when error happens.
- Add missing error handler when using nvmem_cell_read().

Roger Lu (8):
[v21,1/8] dt-bindings: soc: mediatek: add mtk svs dt-bindings [v21,2/8] arm64: dts: mt8183: add svs device information [v21,3/8] soc: mediatek: SVS: introduce MTK SVS engine [v21,4/8] soc: mediatek: SVS: add monitor mode [v21,5/8] soc: mediatek: SVS: add debug commands [v21,6/8] dt-bindings: soc: mediatek: add mt8192 svs dt-bindings [v21,7/8] arm64: dts: mt8192: add svs device information [v21,8/8] soc: mediatek: SVS: add mt8192 SVS GPU driver

 .../bindings/soc/mediatek/mtk-svs.yaml        |   92 +
 arch/arm64/boot/dts/mediatek/mt8183.dtsi      |   16 +
 arch/arm64/boot/dts/mediatek/mt8192.dtsi      |   39 +
 drivers/soc/mediatek/Kconfig                  |   10 +
 drivers/soc/mediatek/Makefile                 |    1 +
 drivers/soc/mediatek/mtk-svs.c                | 2461 +++++++++++++++++
 6 files changed, 2619 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml
 create mode 100644 drivers/soc/mediatek/mtk-svs.c
author
#1 lore raw

PATCH v21 1/8 dt-bindings: soc: mediatek: add mtk svs dt-bindings

Document the binding for enabling mtk svs on MediaTek SoC.

---
 .../bindings/soc/mediatek/mtk-svs.yaml        | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml
Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml new file +84 -0
new file mode 100644
index 000000000000..a855ced410f8
@@ -0,0 +1,84 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/soc/mediatek/mtk-svs.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Mediatek Smart Voltage Scaling (SVS) Device Tree Bindings
+
+maintainers:
+  - Roger Lu <[email protected]>
+  - Matthias Brugger <[email protected]>
+  - Kevin Hilman <[email protected]>
+
+description: |+
+  The SVS engine is a piece of hardware which has several
+  controllers(banks) for calculating suitable voltage to
+  different power domains(CPU/GPU/CCI) according to
+  chip process corner, temperatures and other factors. Then DVFS
+  driver could apply SVS bank voltage to PMIC/Buck.
+
+properties:
+  compatible:
+    enum:
+      - mediatek,mt8183-svs
+
+  reg:
+    maxItems: 1
+    description: Address range of the MTK SVS controller.
+
+  interrupts:
+    maxItems: 1
+
+  clocks:
+    maxItems: 1
+    description: Main clock for MTK SVS controller to work.
+
+  clock-names:
+    const: main
+
+  nvmem-cells:
+    minItems: 1
+    maxItems: 2
+    description:
+      Phandle to the calibration data provided by a nvmem device.
+    items:
+      - description: SVS efuse for SVS controller
+      - description: Thermal efuse for SVS controller
+
+  nvmem-cell-names:
+    items:
+      - const: svs-calibration-data
+      - const: t-calibration-data
+
+required:
+  - compatible
+  - reg
+  - interrupts
+  - clocks
+  - clock-names
+  - nvmem-cells
+  - nvmem-cell-names
+
+additionalProperties: false
+
+examples:
+  - |
+    #include <dt-bindings/clock/mt8183-clk.h>
+    #include <dt-bindings/interrupt-controller/arm-gic.h>
+    #include <dt-bindings/interrupt-controller/irq.h>
+
+    soc {
+        #address-cells = <2>;
+        #size-cells = <2>;
+
+        svs@1100b000 {
+            compatible = "mediatek,mt8183-svs";
+            reg = <0 0x1100b000 0 0x1000>;
+            interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
+            clocks = <&infracfg CLK_INFRA_THERM>;
+            clock-names = "main";
+            nvmem-cells = <&svs_calibration>, <&thermal_calibration>;
+            nvmem-cell-names = "svs-calibration-data", "t-calibration-data";
+        };
+    };
Signature · 1 line
2.18.0
maintainer
#2 lore raw

Re: [PATCH v21 1/8] dt-bindings: soc: mediatek: add mtk svs dt-bindings

Quoted 9 lines · Roger Lu

Document the binding for enabling mtk svs on MediaTek SoC.

---
 .../bindings/soc/mediatek/mtk-svs.yaml        | 84 +++++++++++++++++++
 1 file changed, 84 insertions(+)
 create mode 100644 Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml

My bot found errors running 'make DT_CHECKER_FLAGS=-m dt_binding_check' on your patch (DT_CHECKER_FLAGS is new in v5.13):

yamllint warnings/errors:

dtschema/dtc warnings/errors:
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml: properties:nvmem-cells: {'minItems': 1, 'maxItems': 2, 'description': 'Phandle to the calibration data provided by a nvmem device.', 'items': [{'description': 'SVS efuse for SVS controller'}, {'description': 'Thermal efuse for SVS controller'}]} should not be valid under {'required': ['maxItems']}
hint: "maxItems" is not needed with an "items" list
from schema $id: http://devicetree.org/meta-schemas/items.yaml#
/builds/robherring/linux-dt-review/Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml: ignoring, error in schema: properties: nvmem-cells
Documentation/devicetree/bindings/soc/mediatek/mtk-svs.example.dt.yaml:0:0: /example-0/soc/svs@1100b000: failed to match any schema with compatible: ['mediatek,mt8183-svs']

doc reference errors (make refcheckdocs):

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

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.

author
#3 lore raw

PATCH v21 2/8 arm64: dts: mt8183: add svs device information

In reply to Roger Lu

Add compitable/reg/irq/clock/efuse setting in svs node.

---
 arch/arm64/boot/dts/mediatek/mt8183.dtsi | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)
arch/arm64/boot/dts/mediatek/mt8183.dtsi +16 -0
index ba4584faca5a..9de6aa59d9a7 100644
@@ -743,6 +743,18 @@
 			status = "disabled";
 		};
 
+		svs: svs@1100b000 {
+			compatible = "mediatek,mt8183-svs";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <GIC_SPI 127 IRQ_TYPE_LEVEL_LOW>;
+			clocks = <&infracfg CLK_INFRA_THERM>;
+			clock-names = "main";
+			nvmem-cells = <&svs_calibration>,
+				      <&thermal_calibration>;
+			nvmem-cell-names = "svs-calibration-data",
+					   "t-calibration-data";
+		};
+
 		thermal: thermal@1100b000 {
 			#thermal-sensor-cells = <1>;
 			compatible = "mediatek,mt8183-thermal";
@@ -1261,6 +1273,10 @@
 			mipi_tx_calibration: calib@190 {
 				reg = <0x190 0xc>;
 			};
+
+			svs_calibration: calib@580 {
+				reg = <0x580 0x64>;
+			};
 		};
 
 		u3phy: t-phy@11f40000 {
Signature · 1 line
2.18.0
author
#5 lore raw

PATCH v21 3/8 soc: mediatek: SVS: introduce MTK SVS engine

In reply to Roger Lu

The Smart Voltage Scaling(SVS) engine is a piece of hardware which calculates suitable SVS bank voltages to OPP voltage table. Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck when receiving OPP_EVENT_ADJUST_VOLTAGE.

---
 drivers/soc/mediatek/Kconfig   |   10 +
 drivers/soc/mediatek/Makefile  |    1 +
 drivers/soc/mediatek/mtk-svs.c | 1446 ++++++++++++++++++++++++++++++++
 3 files changed, 1457 insertions(+)
 create mode 100644 drivers/soc/mediatek/mtk-svs.c
drivers/soc/mediatek/Kconfig +10 -0
index fdd8bc08569e..3c3eedea35f7 100644
@@ -73,4 +73,14 @@ config MTK_MMSYS
 	  Say yes here to add support for the MediaTek Multimedia
 	  Subsystem (MMSYS).
 
+config MTK_SVS
+	tristate "MediaTek Smart Voltage Scaling(SVS)"
+	depends on MTK_EFUSE && NVMEM
+	help
+	  The Smart Voltage Scaling(SVS) engine is a piece of hardware
+	  which has several controllers(banks) for calculating suitable
+	  voltage to different power domains(CPU/GPU/CCI) according to
+	  chip process corner, temperatures and other factors. Then DVFS
+	  driver could apply SVS bank voltage to PMIC/Buck.
+
 endmenu
drivers/soc/mediatek/Makefile +1 -0
index 90270f8114ed..0e9e703c931a 100644
@@ -7,3 +7,4 @@ obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
 obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
 obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
 obj-$(CONFIG_MTK_MMSYS) += mtk-mutex.o
+obj-$(CONFIG_MTK_SVS) += mtk-svs.o
drivers/soc/mediatek/mtk-svs.c new file +1446 -0

1,446 lines not on this page · raw message

Signature · 1 line
2.18.0
maintainer Reviewed-by on v21 3/8
#6 lore raw

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 109 lines

The Smart Voltage Scaling(SVS) engine is a piece of hardware which calculates suitable SVS bank voltages to OPP voltage table. Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck when receiving OPP_EVENT_ADJUST_VOLTAGE.

---
  drivers/soc/mediatek/Kconfig   |   10 +
  drivers/soc/mediatek/Makefile  |    1 +
  drivers/soc/mediatek/mtk-svs.c | 1446 ++++++++++++++++++++++++++++++++
  3 files changed, 1457 insertions(+)

create mode 100644 drivers/soc/mediatek/mtk-svs.c

drivers/soc/mediatek/Kconfig +10 -0
index fdd8bc08569e..3c3eedea35f7 100644
@@ -73,4 +73,14 @@ config MTK_MMSYS
  	  Say yes here to add support for the MediaTek Multimedia
  	  Subsystem (MMSYS).
 
+config MTK_SVS
+	tristate "MediaTek Smart Voltage Scaling(SVS)"
+	depends on MTK_EFUSE && NVMEM
+	help
+	  The Smart Voltage Scaling(SVS) engine is a piece of hardware
+	  which has several controllers(banks) for calculating suitable
+	  voltage to different power domains(CPU/GPU/CCI) according to
+	  chip process corner, temperatures and other factors. Then DVFS
+	  driver could apply SVS bank voltage to PMIC/Buck.
+
  endmenu
drivers/soc/mediatek/Makefile +1 -0
index 90270f8114ed..0e9e703c931a 100644
@@ -7,3 +7,4 @@ obj-$(CONFIG_MTK_SCPSYS) += mtk-scpsys.o
  obj-$(CONFIG_MTK_SCPSYS_PM_DOMAINS) += mtk-pm-domains.o
  obj-$(CONFIG_MTK_MMSYS) += mtk-mmsys.o
  obj-$(CONFIG_MTK_MMSYS) += mtk-mutex.o
+obj-$(CONFIG_MTK_SVS) += mtk-svs.o
drivers/soc/mediatek/mtk-svs.c new file +62 -0
new file mode 100644
index 000000000000..fc7e2ee44a92
@@ -0,0 +1,1446 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * Copyright (C) 2020 MediaTek Inc.
+ */
+
+#include <linux/bits.h>
+#include <linux/clk.h>
+#include <linux/completion.h>
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/interrupt.h>
+#include <linux/kernel.h>
+#include <linux/kthread.h>
+#include <linux/module.h>
+#include <linux/mutex.h>
+#include <linux/nvmem-consumer.h>
+#include <linux/of_address.h>
+#include <linux/of_irq.h>
+#include <linux/of_platform.h>
+#include <linux/platform_device.h>
+#include <linux/pm_domain.h>
+#include <linux/pm_opp.h>
+#include <linux/pm_qos.h>
+#include <linux/pm_runtime.h>
+#include <linux/regulator/consumer.h>
+#include <linux/slab.h>
+#include <linux/spinlock.h>
+
+/* svs bank 1-line sw id */
+#define SVSB_CPU_LITTLE			BIT(0)
+#define SVSB_CPU_BIG			BIT(1)
+#define SVSB_CCI			BIT(2)
+#define SVSB_GPU			BIT(3)
+
+/* svs bank mode support */
+#define SVSB_MODE_ALL_DISABLE		0
+#define SVSB_MODE_INIT01		BIT(1)
+#define SVSB_MODE_INIT02		BIT(2)
+
+/* svs bank volt flags */
+#define SVSB_INIT01_PD_REQ		BIT(0)
+#define SVSB_INIT01_VOLT_IGNORE		BIT(1)
+#define SVSB_INIT01_VOLT_INC_ONLY	BIT(2)
+
+/* svs bank common setting */
+#define MAX_OPP_ENTRIES			16
+#define SVSB_DC_SIGNED_BIT		BIT(15)
+#define SVSB_DET_CLK_EN			BIT(31)
+#define SVSB_DET_MAX			0xffff
+#define SVSB_DET_WINDOW			0xa28
+#define SVSB_DTHI			0x1
+#define SVSB_DTLO			0xfe
+#define SVSB_EN_INIT01			0x1
+#define SVSB_EN_INIT02			0x5
+#define SVSB_EN_MASK			0x7
+#define SVSB_EN_OFF			0x0
+#define SVSB_INTEN_INIT0x		0x00005f01
+#define SVSB_INTSTS_CLEAN		0x00ffffff
+#define SVSB_INTSTS_COMPLETE		0x1
+#define SVSB_RUNCONFIG_DEFAULT		0x80000000
+
+static DEFINE_SPINLOCK(svs_lock);

snip....

drivers/soc/mediatek/mtk-svs.c line 1308

Quoted 11 lines
drivers/soc/mediatek/mtk-svs.c new file +11 -0
+
+struct svs_platform_data {
+	char *name;
+	struct svs_bank *banks;
+	bool (*efuse_parsing)(struct svs_platform *svsp);
+	unsigned long irqflags;
+	const u32 *regs;
+	u32 bank_max;
+	int (*probe)(struct svs_platform *svsp);
+};
+

Please move the definition of struct svs_platform_data at the beginning of the file for increased readability.

With that done,

author
#7 lore raw

Hi AngeloGioacchino,

Sorry for the late reply and thanks for the advice.

Quoted 13 lines · AngeloGioacchino Del Regno

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 12 lines

The Smart Voltage Scaling(SVS) engine is a piece of hardware which calculates suitable SVS bank voltages to OPP voltage table. Then, DVFS driver could apply those SVS bank voltages to PMIC/Buck when receiving OPP_EVENT_ADJUST_VOLTAGE.

---
  drivers/soc/mediatek/Kconfig   |   10 +
  drivers/soc/mediatek/Makefile  |    1 +
  drivers/soc/mediatek/mtk-svs.c | 1446 ++++++++++++++++++++++++++++++++
  3 files changed, 1457 insertions(+)

create mode 100644 drivers/soc/mediatek/mtk-svs.c

[snip]

Quoted 6 lines
Quoted 6 lines
drivers/soc/mediatek/mtk-svs.c new file +6 -0
+
+struct svs_platform_data {
+	char *name;
+	struct svs_bank *banks;
+	bool (*efuse_parsing)(struct svs_platform *svsp);
+	unsigned long irqflags;
Quoted 8 lines

drivers/soc/mediatek/mtk-svs.c line 1308

Quoted 5 lines
drivers/soc/mediatek/mtk-svs.c new file +5 -0
+	const u32 *regs;
+	u32 bank_max;
+	int (*probe)(struct svs_platform *svsp);
+};
+

Please move the definition of struct svs_platform_data at the beginning of the file for increased readability.

Okay. I'll move them at the beginning of the file in the next patch.

Quoted 5 lines

With that done,

Reviewed-by: AngeloGioacchino Del Regno <
[email protected]>

author
#8 lore raw

PATCH v21 4/8 soc: mediatek: SVS: add monitor mode

In reply to Roger Lu

SVS monitor mode is based on different thermal temperature to provide suitable SVS bank voltages.

---
 drivers/soc/mediatek/mtk-svs.c | 253 ++++++++++++++++++++++++++++++++-
 1 file changed, 247 insertions(+), 6 deletions(-)
drivers/soc/mediatek/mtk-svs.c +247 -6
index fc7e2ee44a92..042c6e8e9069 100644
@@ -25,6 +25,7 @@
 #include <linux/regulator/consumer.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
+#include <linux/thermal.h>
 
 /* svs bank 1-line sw id */
 #define SVSB_CPU_LITTLE			BIT(0)
@@ -36,6 +37,7 @@
 #define SVSB_MODE_ALL_DISABLE		0
 #define SVSB_MODE_INIT01		BIT(1)
 #define SVSB_MODE_INIT02		BIT(2)
+#define SVSB_MODE_MON			BIT(3)
 
 /* svs bank volt flags */
 #define SVSB_INIT01_PD_REQ		BIT(0)
@@ -53,11 +55,16 @@
 #define SVSB_EN_INIT01			0x1
 #define SVSB_EN_INIT02			0x5
 #define SVSB_EN_MASK			0x7
+#define SVSB_EN_MON			0x2
 #define SVSB_EN_OFF			0x0
 #define SVSB_INTEN_INIT0x		0x00005f01
+#define SVSB_INTEN_MONVOPEN		0x00ff0000
 #define SVSB_INTSTS_CLEAN		0x00ffffff
 #define SVSB_INTSTS_COMPLETE		0x1
+#define SVSB_INTSTS_MONVOP		0x00ff0000
 #define SVSB_RUNCONFIG_DEFAULT		0x80000000
+#define SVSB_TEMP_LOWER_BOUND		0xb2
+#define SVSB_TEMP_UPPER_BOUND		0x64
 
 static DEFINE_SPINLOCK(svs_lock);
 
@@ -66,6 +73,7 @@ static DEFINE_SPINLOCK(svs_lock);
  * @SVSB_PHASE_ERROR: svs bank encounters unexpected condition
  * @SVSB_PHASE_INIT01: svs bank basic init for data calibration
  * @SVSB_PHASE_INIT02: svs bank can provide voltages to opp table
+ * @SVSB_PHASE_MON: svs bank can provide voltages with thermal effect
  * @SVSB_PHASE_MAX: total number of svs bank phase (debug purpose)
  *
  * Each svs bank has its own independent phase and we enable each svs bank by
@@ -73,12 +81,13 @@ static DEFINE_SPINLOCK(svs_lock);
  * condition, it will fire an irq (PHASE_ERROR) to inform svs software.
  *
  * svs bank general phase-enabled order:
- * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02
+ * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02 -> SVSB_PHASE_MON
  */
 enum svsb_phase {
 	SVSB_PHASE_ERROR = 0,
 	SVSB_PHASE_INIT01,
 	SVSB_PHASE_INIT02,
+	SVSB_PHASE_MON,
 	SVSB_PHASE_MAX,
 };
 
@@ -209,9 +218,11 @@ static const u32 svs_regs_v2[] = {
  * @probe: svs platform probe function pointer
  * @irqflags: svs platform irq settings flags
  * @efuse_max: total number of svs efuse
+ * @tefuse_max: total number of thermal efuse
  * @regs: svs platform registers map
  * @bank_max: total number of svs banks
  * @efuse: svs efuse data received from NVMEM framework
+ * @tefuse: thermal efuse data received from NVMEM framework
  */
 struct svs_platform {
 	char *name;
@@ -224,9 +235,11 @@ struct svs_platform {
 	int (*probe)(struct svs_platform *svsp);
 	unsigned long irqflags;
 	size_t efuse_max;
+	size_t tefuse_max;
 	const u32 *regs;
 	u32 bank_max;
 	u32 *efuse;
+	u32 *tefuse;
 };
 
 /**
@@ -241,6 +254,7 @@ struct svs_platform {
  * @get_volts: function pointer to get bank voltages
  * @name: bank name
  * @buck_name: regulator name
+ * @tzone_name: thermal zone name
  * @suspended: suspend flag of this bank
  * @phase: bank current phase
  * @volt_od: bank voltage overdrive
@@ -270,6 +284,13 @@ struct svs_platform {
  * @sw_id: bank software identification
  * @cpu_id: cpu core id for SVS CPU bank use only
  * @ctl0: TS-x selection
+ * @temp: bank temperature
+ * @tzone_htemp: thermal zone high temperature threshold
+ * @tzone_htemp_voffset: thermal zone high temperature voltage offset
+ * @tzone_ltemp: thermal zone low temperature threshold
+ * @tzone_ltemp_voffset: thermal zone low temperature voltage offset
+ * @bts: svs efuse data
+ * @mts: svs efuse data
  * @bdes: svs efuse data
  * @mdes: svs efuse data
  * @mtdes: svs efuse data
@@ -292,6 +313,7 @@ struct svs_bank {
 	void (*get_volts)(struct svs_platform *svsp);
 	char *name;
 	char *buck_name;
+	char *tzone_name;
 	bool suspended;
 	enum svsb_phase phase;
 	s32 volt_od;
@@ -321,6 +343,13 @@ struct svs_bank {
 	u32 sw_id;
 	u32 cpu_id;
 	u32 ctl0;
+	u32 temp;
+	u32 tzone_htemp;
+	u32 tzone_htemp_voffset;
+	u32 tzone_ltemp;
+	u32 tzone_ltemp_voffset;
+	u32 bts;
+	u32 mts;
 	u32 bdes;
 	u32 mdes;
 	u32 mtdes;
@@ -361,10 +390,21 @@ static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step,
 	return (svsb_volt * svsb_volt_step) + svsb_volt_base;
 }
 
+static int svs_get_zone_temperature(const char *tzone_name, int *tzone_temp)
+{
+	struct thermal_zone_device *tzd;
+
+	tzd = thermal_zone_get_zone_by_name(tzone_name);
+	if (IS_ERR(tzd))
+		return PTR_ERR(tzd);
+
+	return thermal_zone_get_temp(tzd, tzone_temp);
+}
+
 static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 {
-	int ret = -EPERM;
-	u32 i, svsb_volt, opp_volt;
+	int ret = -EPERM, tzone_temp = 0;
+	u32 i, svsb_volt, opp_volt, temp_voffset = 0;
 
 	mutex_lock(&svsb->lock);
 
@@ -378,6 +418,22 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 		goto unlock_mutex;
 	}
 
+	/* Get thermal effect */
+	if (svsb->phase == SVSB_PHASE_MON) {
+		ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
+		if (ret || (svsb->temp > SVSB_TEMP_UPPER_BOUND &&
+			    svsb->temp < SVSB_TEMP_LOWER_BOUND)) {
+			dev_err(svsb->dev, "%s: %d (0x%x), run default volts\n",
+				svsb->tzone_name, ret, svsb->temp);
+			svsb->phase = SVSB_PHASE_ERROR;
+		}
+
+		if (tzone_temp >= svsb->tzone_htemp)
+			temp_voffset += svsb->tzone_htemp_voffset;
+		else if (tzone_temp <= svsb->tzone_ltemp)
+			temp_voffset += svsb->tzone_ltemp_voffset;
+	}
+
 	/* vmin <= svsb_volt (opp_volt) <= default opp voltage */
 	for (i = 0; i < svsb->opp_count; i++) {
 		switch (svsb->phase) {
@@ -393,6 +449,13 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 							     svsb->volt_step,
 							     svsb->volt_base);
 			break;
+		case SVSB_PHASE_MON:
+			svsb_volt = max(svsb->volt[i] + temp_voffset,
+					svsb->vmin);
+			opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
+							     svsb->volt_step,
+							     svsb->volt_base);
+			break;
 		default:
 			dev_err(svsb->dev, "unknown phase: %u\n", svsb->phase);
 			ret = -EINVAL;
@@ -488,7 +551,7 @@ static void svs_set_bank_phase(struct svs_platform *svsp,
 {
 	struct svs_bank *svsb = svsp->pbank;
 	u32 des_char, temp_char, det_char, limit_vals;
-	u32 init2vals;
+	u32 init2vals, ts_calcs;
 
 	svs_switch_bank(svsp);
 
@@ -529,6 +592,12 @@ static void svs_set_bank_phase(struct svs_platform *svsp,
 		svs_writel_relaxed(svsp, init2vals, INIT2VALS);
 		svs_writel_relaxed(svsp, SVSB_EN_INIT02, SVSEN);
 		break;
+	case SVSB_PHASE_MON:
+		ts_calcs = (svsb->bts << 12) | svsb->mts;
+		svs_writel_relaxed(svsp, ts_calcs, TSCALCS);
+		svs_writel_relaxed(svsp, SVSB_INTEN_MONVOPEN, INTEN);
+		svs_writel_relaxed(svsp, SVSB_EN_MON, SVSEN);
+		break;
 	default:
 		dev_err(svsb->dev, "requested unknown target phase: %u\n",
 			target_phase);
@@ -548,6 +617,7 @@ static inline void svs_error_isr_handler(struct svs_platform *svsp)
 	dev_err(svsb->dev, "SMSTATE0 = 0x%08x, SMSTATE1 = 0x%08x\n",
 		svs_readl_relaxed(svsp, SMSTATE0),
 		svs_readl_relaxed(svsp, SMSTATE1));
+	dev_err(svsb->dev, "TEMP = 0x%08x\n", svs_readl_relaxed(svsp, TEMP));
 
 	svsb->mode_support = SVSB_MODE_ALL_DISABLE;
 	svsb->phase = SVSB_PHASE_ERROR;
@@ -599,6 +669,17 @@ static inline void svs_init02_isr_handler(struct svs_platform *svsp)
 	svs_writel_relaxed(svsp, SVSB_INTSTS_COMPLETE, INTSTS);
 }
 
+static inline void svs_mon_mode_isr_handler(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+
+	svsb->phase = SVSB_PHASE_MON;
+	svsb->temp = svs_readl_relaxed(svsp, TEMP) & GENMASK(7, 0);
+	svsb->get_volts(svsp);
+
+	svs_writel_relaxed(svsp, SVSB_INTSTS_MONVOP, INTSTS);
+}
+
 static irqreturn_t svs_isr(int irq, void *data)
 {
 	struct svs_platform *svsp = data;
@@ -629,6 +710,8 @@ static irqreturn_t svs_isr(int irq, void *data)
 		else if (int_sts == SVSB_INTSTS_COMPLETE &&
 			 svs_en == SVSB_EN_INIT02)
 			svs_init02_isr_handler(svsp);
+		else if (int_sts & SVSB_INTSTS_MONVOP)
+			svs_mon_mode_isr_handler(svsp);
 		else
 			svs_error_isr_handler(svsp);
 
@@ -858,6 +941,25 @@ static int svs_init02(struct svs_platform *svsp)
 	return 0;
 }
 
+static void svs_mon_mode(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb;
+	unsigned long flags;
+	u32 idx;
+
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+
+		if (!(svsb->mode_support & SVSB_MODE_MON))
+			continue;
+
+		spin_lock_irqsave(&svs_lock, flags);
+		svsp->pbank = svsb;
+		svs_set_bank_phase(svsp, SVSB_PHASE_MON);
+		spin_unlock_irqrestore(&svs_lock, flags);
+	}
+}
+
 static int svs_start(struct svs_platform *svsp)
 {
 	int ret;
@@ -870,6 +972,8 @@ static int svs_start(struct svs_platform *svsp)
 	if (ret)
 		return ret;
 
+	svs_mon_mode(svsp);
+
 	return 0;
 }
 
@@ -923,6 +1027,8 @@ static int svs_resume(struct device *dev)
 	if (ret)
 		return ret;
 
+	svs_mon_mode(svsp);
+
 	return 0;
 }
 
@@ -1007,7 +1113,11 @@ static int svs_bank_resource_setup(struct svs_platform *svsp)
 static bool svs_mt8183_efuse_parsing(struct svs_platform *svsp)
 {
 	struct svs_bank *svsb;
-	u32 idx, i, ft_pgm;
+	struct nvmem_cell *cell;
+	int format[6], x_roomt[6], o_vtsmcu[5], o_vtsabb, tb_roomt = 0;
+	int adc_ge_t, adc_oe_t, ge, oe, gain, degc_cali, adc_cali_en_t;
+	int o_slope, o_slope_sign, ts_id;
+	u32 idx, i, ft_pgm, mts, temp0, temp1, temp2;
 
 	for (i = 0; i < svsp->efuse_max; i++)
 		if (svsp->efuse[i])
@@ -1083,6 +1193,126 @@ static bool svs_mt8183_efuse_parsing(struct svs_platform *svsp)
 		}
 	}
 
+	/* Get thermal efuse by nvmem */
+	cell = nvmem_cell_get(svsp->dev, "t-calibration-data");
+	if (IS_ERR(cell)) {
+		dev_err(svsp->dev, "no \"t-calibration-data\"? %ld\n",
+			PTR_ERR(cell));
+		goto remove_mt8183_svsb_mon_mode;
+	}
+
+	svsp->tefuse = nvmem_cell_read(cell, &svsp->tefuse_max);
+	if (IS_ERR(svsp->tefuse)) {
+		dev_err(svsp->dev, "cannot read thermal efuse: %ld\n",
+			PTR_ERR(svsp->tefuse));
+		goto remove_mt8183_svsb_mon_mode;
+	}
+
+	svsp->tefuse_max /= sizeof(u32);
+	nvmem_cell_put(cell);
+
+	/* Thermal efuse parsing */
+	adc_ge_t = (svsp->tefuse[1] >> 22) & GENMASK(9, 0);
+	adc_oe_t = (svsp->tefuse[1] >> 12) & GENMASK(9, 0);
+
+	o_vtsmcu[0] = (svsp->tefuse[0] >> 17) & GENMASK(8, 0);
+	o_vtsmcu[1] = (svsp->tefuse[0] >> 8) & GENMASK(8, 0);
+	o_vtsmcu[2] = svsp->tefuse[1] & GENMASK(8, 0);
+	o_vtsmcu[3] = (svsp->tefuse[2] >> 23) & GENMASK(8, 0);
+	o_vtsmcu[4] = (svsp->tefuse[2] >> 5) & GENMASK(8, 0);
+	o_vtsabb = (svsp->tefuse[2] >> 14) & GENMASK(8, 0);
+
+	degc_cali = (svsp->tefuse[0] >> 1) & GENMASK(5, 0);
+	adc_cali_en_t = svsp->tefuse[0] & BIT(0);
+	o_slope_sign = (svsp->tefuse[0] >> 7) & BIT(0);
+
+	ts_id = (svsp->tefuse[1] >> 9) & BIT(0);
+	o_slope = (svsp->tefuse[0] >> 26) & GENMASK(5, 0);
+
+	if (adc_cali_en_t == 1) {
+		if (!ts_id)
+			o_slope = 0;
+
+		if (adc_ge_t < 265 || adc_ge_t > 758 ||
+		    adc_oe_t < 265 || adc_oe_t > 758 ||
+		    o_vtsmcu[0] < -8 || o_vtsmcu[0] > 484 ||
+		    o_vtsmcu[1] < -8 || o_vtsmcu[1] > 484 ||
+		    o_vtsmcu[2] < -8 || o_vtsmcu[2] > 484 ||
+		    o_vtsmcu[3] < -8 || o_vtsmcu[3] > 484 ||
+		    o_vtsmcu[4] < -8 || o_vtsmcu[4] > 484 ||
+		    o_vtsabb < -8 || o_vtsabb > 484 ||
+		    degc_cali < 1 || degc_cali > 63) {
+			dev_err(svsp->dev, "bad thermal efuse, no mon mode\n");
+			goto remove_mt8183_svsb_mon_mode;
+		}
+	} else {
+		dev_err(svsp->dev, "no thermal efuse, no mon mode\n");
+		goto remove_mt8183_svsb_mon_mode;
+	}
+
+	ge = ((adc_ge_t - 512) * 10000) / 4096;
+	oe = (adc_oe_t - 512);
+	gain = (10000 + ge);
+
+	format[0] = (o_vtsmcu[0] + 3350 - oe);
+	format[1] = (o_vtsmcu[1] + 3350 - oe);
+	format[2] = (o_vtsmcu[2] + 3350 - oe);
+	format[3] = (o_vtsmcu[3] + 3350 - oe);
+	format[4] = (o_vtsmcu[4] + 3350 - oe);
+	format[5] = (o_vtsabb + 3350 - oe);
+
+	for (i = 0; i < 6; i++)
+		x_roomt[i] = (((format[i] * 10000) / 4096) * 10000) / gain;
+
+	temp0 = (10000 * 100000 / gain) * 15 / 18;
+
+	if (!o_slope_sign)
+		mts = (temp0 * 10) / (1534 + o_slope * 10);
+	else
+		mts = (temp0 * 10) / (1534 - o_slope * 10);
+
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+		svsb->mts = mts;
+
+		switch (svsb->sw_id) {
+		case SVSB_CPU_LITTLE:
+			tb_roomt = x_roomt[3];
+			break;
+		case SVSB_CPU_BIG:
+			tb_roomt = x_roomt[4];
+			break;
+		case SVSB_CCI:
+			tb_roomt = x_roomt[3];
+			break;
+		case SVSB_GPU:
+			tb_roomt = x_roomt[1];
+			break;
+		default:
+			dev_err(svsb->dev, "unknown sw_id: %u\n", svsb->sw_id);
+			goto remove_mt8183_svsb_mon_mode;
+		}
+
+		temp0 = (degc_cali * 10 / 2);
+		temp1 = ((10000 * 100000 / 4096 / gain) *
+			 oe + tb_roomt * 10) * 15 / 18;
+
+		if (!o_slope_sign)
+			temp2 = temp1 * 100 / (1534 + o_slope * 10);
+		else
+			temp2 = temp1 * 100 / (1534 - o_slope * 10);
+
+		svsb->bts = (temp0 + temp2 - 250) * 4 / 10;
+	}
+
+	return true;
+
+remove_mt8183_svsb_mon_mode:
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+		svsb->mode_support &= ~SVSB_MODE_MON;
+	}
+
 	return true;
 }
 
@@ -1165,9 +1395,14 @@ static struct device *svs_add_device_link(struct svs_platform *svsp,
 
 static int svs_mt8183_platform_probe(struct svs_platform *svsp)
 {
+	struct device *dev;
 	struct svs_bank *svsb;
 	u32 idx;
 
+	dev = svs_add_device_link(svsp, "thermal");
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
 	for (idx = 0; idx < svsp->bank_max; idx++) {
 		svsb = &svsp->banks[idx];
 
@@ -1275,9 +1510,11 @@ static struct svs_bank svs_mt8183_banks[] = {
 		.set_freq_pct		= svs_set_bank_freq_pct_v2,
 		.get_volts		= svs_get_bank_volts_v2,
 		.buck_name		= "mali",
+		.tzone_name		= "tzts2",
 		.volt_flags		= SVSB_INIT01_PD_REQ |
 					  SVSB_INIT01_VOLT_INC_ONLY,
-		.mode_support		= SVSB_MODE_INIT01 | SVSB_MODE_INIT02,
+		.mode_support		= SVSB_MODE_INIT01 | SVSB_MODE_INIT02 |
+					  SVSB_MODE_MON,
 		.opp_count		= MAX_OPP_ENTRIES,
 		.freq_base		= 900000000,
 		.vboot			= 0x30,
@@ -1293,6 +1530,10 @@ static struct svs_bank svs_mt8183_banks[] = {
 		.core_sel		= 0x8fff0003,
 		.int_st			= BIT(3),
 		.ctl0			= 0x00050001,
+		.tzone_htemp		= 85000,
+		.tzone_htemp_voffset	= 0,
+		.tzone_ltemp		= 25000,
+		.tzone_ltemp_voffset	= 3,
 	},
 };
 
Signature · 1 line
2.18.0
#9 lore raw

Re: [PATCH v21 4/8] soc: mediatek: SVS: add monitor mode

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 137 lines

SVS monitor mode is based on different thermal temperature to provide suitable SVS bank voltages.

---
  drivers/soc/mediatek/mtk-svs.c | 253 ++++++++++++++++++++++++++++++++-
  1 file changed, 247 insertions(+), 6 deletions(-)
drivers/soc/mediatek/mtk-svs.c +30 -1
index fc7e2ee44a92..042c6e8e9069 100644
@@ -25,6 +25,7 @@
  #include <linux/regulator/consumer.h>
  #include <linux/slab.h>
  #include <linux/spinlock.h>
+#include <linux/thermal.h>
 
  /* svs bank 1-line sw id */
  #define SVSB_CPU_LITTLE			BIT(0)
@@ -36,6 +37,7 @@
  #define SVSB_MODE_ALL_DISABLE		0
  #define SVSB_MODE_INIT01		BIT(1)
  #define SVSB_MODE_INIT02		BIT(2)
+#define SVSB_MODE_MON			BIT(3)
 
  /* svs bank volt flags */
  #define SVSB_INIT01_PD_REQ		BIT(0)
@@ -53,11 +55,16 @@
  #define SVSB_EN_INIT01			0x1
  #define SVSB_EN_INIT02			0x5
  #define SVSB_EN_MASK			0x7
+#define SVSB_EN_MON			0x2
  #define SVSB_EN_OFF			0x0
  #define SVSB_INTEN_INIT0x		0x00005f01
+#define SVSB_INTEN_MONVOPEN		0x00ff0000
  #define SVSB_INTSTS_CLEAN		0x00ffffff
  #define SVSB_INTSTS_COMPLETE		0x1
+#define SVSB_INTSTS_MONVOP		0x00ff0000
  #define SVSB_RUNCONFIG_DEFAULT		0x80000000
+#define SVSB_TEMP_LOWER_BOUND		0xb2
+#define SVSB_TEMP_UPPER_BOUND		0x64
 
  static DEFINE_SPINLOCK(svs_lock);
 
@@ -66,6 +73,7 @@ static DEFINE_SPINLOCK(svs_lock);
   * @SVSB_PHASE_ERROR: svs bank encounters unexpected condition
   * @SVSB_PHASE_INIT01: svs bank basic init for data calibration
   * @SVSB_PHASE_INIT02: svs bank can provide voltages to opp table
+ * @SVSB_PHASE_MON: svs bank can provide voltages with thermal effect
   * @SVSB_PHASE_MAX: total number of svs bank phase (debug purpose)
   *
   * Each svs bank has its own independent phase and we enable each svs bank by
@@ -73,12 +81,13 @@ static DEFINE_SPINLOCK(svs_lock);
   * condition, it will fire an irq (PHASE_ERROR) to inform svs software.
   *
   * svs bank general phase-enabled order:
- * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02
+ * SVSB_PHASE_INIT01 -> SVSB_PHASE_INIT02 -> SVSB_PHASE_MON
   */
  enum svsb_phase {
  	SVSB_PHASE_ERROR = 0,
  	SVSB_PHASE_INIT01,
  	SVSB_PHASE_INIT02,
+	SVSB_PHASE_MON,
  	SVSB_PHASE_MAX,
  };
 
@@ -209,9 +218,11 @@ static const u32 svs_regs_v2[] = {
   * @probe: svs platform probe function pointer
   * @irqflags: svs platform irq settings flags
   * @efuse_max: total number of svs efuse
+ * @tefuse_max: total number of thermal efuse
   * @regs: svs platform registers map
   * @bank_max: total number of svs banks
   * @efuse: svs efuse data received from NVMEM framework
+ * @tefuse: thermal efuse data received from NVMEM framework
   */
  struct svs_platform {
  	char *name;
@@ -224,9 +235,11 @@ struct svs_platform {
  	int (*probe)(struct svs_platform *svsp);
  	unsigned long irqflags;
  	size_t efuse_max;
+	size_t tefuse_max;
  	const u32 *regs;
  	u32 bank_max;
  	u32 *efuse;
+	u32 *tefuse;
  };
 
  /**
@@ -241,6 +254,7 @@ struct svs_platform {
   * @get_volts: function pointer to get bank voltages
   * @name: bank name
   * @buck_name: regulator name
+ * @tzone_name: thermal zone name
   * @suspended: suspend flag of this bank
   * @phase: bank current phase
   * @volt_od: bank voltage overdrive
@@ -270,6 +284,13 @@ struct svs_platform {
   * @sw_id: bank software identification
   * @cpu_id: cpu core id for SVS CPU bank use only
   * @ctl0: TS-x selection
+ * @temp: bank temperature
+ * @tzone_htemp: thermal zone high temperature threshold
+ * @tzone_htemp_voffset: thermal zone high temperature voltage offset
+ * @tzone_ltemp: thermal zone low temperature threshold
+ * @tzone_ltemp_voffset: thermal zone low temperature voltage offset
+ * @bts: svs efuse data
+ * @mts: svs efuse data
   * @bdes: svs efuse data
   * @mdes: svs efuse data
   * @mtdes: svs efuse data
@@ -292,6 +313,7 @@ struct svs_bank {
  	void (*get_volts)(struct svs_platform *svsp);
  	char *name;
  	char *buck_name;
+	char *tzone_name;
  	bool suspended;
  	enum svsb_phase phase;
  	s32 volt_od;
@@ -321,6 +343,13 @@ struct svs_bank {
  	u32 sw_id;
  	u32 cpu_id;
  	u32 ctl0;
+	u32 temp;
+	u32 tzone_htemp;
+	u32 tzone_htemp_voffset;
+	u32 tzone_ltemp;
+	u32 tzone_ltemp_voffset;
+	u32 bts;
+	u32 mts;
  	u32 bdes;
  	u32 mdes;
  	u32 mtdes;

drivers/soc/mediatek/mtk-svs.c static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step, · line 392

Quoted 3 lines
drivers/soc/mediatek/mtk-svs.c +0 -0
@@ -361,10 +390,21 @@ static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step,
  	return (svsb_volt * svsb_volt_step) + svsb_volt_base;
  }
 

I'm sorry for the double review, but this went unnoticed in the previous one.

drivers/soc/mediatek/mtk-svs.c static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step, · line 397

Quoted 5 lines
drivers/soc/mediatek/mtk-svs.c +5 -0
+static int svs_get_zone_temperature(const char *tzone_name, int *tzone_temp)
+{
+	struct thermal_zone_device *tzd;
+
+	tzd = thermal_zone_get_zone_by_name(tzone_name);

This call is expensive, as it's iterating through the (possibly) entire thermal_tz_list (drivers/thermal/thermal_core.c) so, for performance purposes, noting that you're using this in svs_adjust_pm_opp_volts(), it's not a good idea to call it at every ISR.

I would instead propose to get a pointer to the thermal_zone at driver probe time and cache that in struct svs_bank: this function would also be removed as the only thing that you'd need to do then would be just one call...

[read forward...]

Quoted 16 lines
drivers/soc/mediatek/mtk-svs.c +8 -2
+	if (IS_ERR(tzd))
+		return PTR_ERR(tzd);
+
+	return thermal_zone_get_temp(tzd, tzone_temp);
+}
+
  static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
  {
-	int ret = -EPERM;
-	u32 i, svsb_volt, opp_volt;
+	int ret = -EPERM, tzone_temp = 0;
+	u32 i, svsb_volt, opp_volt, temp_voffset = 0;
 
  	mutex_lock(&svsb->lock);
 

drivers/soc/mediatek/mtk-svs.c static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update) · line 423

Quoted 6 lines
drivers/soc/mediatek/mtk-svs.c +3 -0
@@ -378,6 +418,22 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
  		goto unlock_mutex;
  	}
 
+	/* Get thermal effect */
+	if (svsb->phase == SVSB_PHASE_MON) {
+		ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);

... so you can simply call ...

	ret = thermal_zone_get_temp(svsb->tzd, tzone_temp);

...without any need for any helper.

Quoted 6 lines
drivers/soc/mediatek/mtk-svs.c +6 -0
+		if (ret || (svsb->temp > SVSB_TEMP_UPPER_BOUND &&
+			    svsb->temp < SVSB_TEMP_LOWER_BOUND)) {
+			dev_err(svsb->dev, "%s: %d (0x%x), run default volts\n",
+				svsb->tzone_name, ret, svsb->temp);
+			svsb->phase = SVSB_PHASE_ERROR;
+		}

drivers/soc/mediatek/mtk-svs.c static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update) · line 439

Quoted 10 lines
drivers/soc/mediatek/mtk-svs.c +7 -0
+
+		if (tzone_temp >= svsb->tzone_htemp)
+			temp_voffset += svsb->tzone_htemp_voffset;
+		else if (tzone_temp <= svsb->tzone_ltemp)
+			temp_voffset += svsb->tzone_ltemp_voffset;
+	}
+
  	/* vmin <= svsb_volt (opp_volt) <= default opp voltage */
  	for (i = 0; i < svsb->opp_count; i++) {
  		switch (svsb->phase) {

Apart from that, the commit looks good. Looking forward to review the new version!

Regards,
- Angelo

author
#10 lore raw

Re: [PATCH v21 4/8] soc: mediatek: SVS: add monitor mode

Hi AngeloGioacchino,

Sorry for the late reply and thanks for the advice.

Quoted 26 lines · AngeloGioacchino Del Regno

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 25 lines

SVS monitor mode is based on different thermal temperature to provide suitable SVS bank voltages.

---
  drivers/soc/mediatek/mtk-svs.c | 253 ++++++++++++++++++++++++++++++++-
  1 file changed, 247 insertions(+), 6 deletions(-)
drivers/soc/mediatek/mtk-svs.c +2 -0
index fc7e2ee44a92..042c6e8e9069 100644
@@ -25,6 +25,7 @@
  #include <linux/regulator/consumer.h>
  #include <linux/slab.h>
  #include <linux/spinlock.h>
+#include <linux/thermal.h>
 
  /* svs bank 1-line sw id */
  #define SVSB_CPU_LITTLE			BIT(0)
@@ -36,6 +37,7 @@
  #define SVSB_MODE_ALL_DISABLE		0
  #define SVSB_MODE_INIT01		BIT(1)
  #define SVSB_MODE_INIT02		BIT(2)
+#define SVSB_MODE_MON			BIT(3)

[snip]

Quoted 66 lines
Quoted 46 lines

/**

drivers/soc/mediatek/mtk-svs.c +16 -0
@@ -241,6 +254,7 @@ struct svs_platform {
   * @get_volts: function pointer to get bank voltages
   * @name: bank name
   * @buck_name: regulator name
+ * @tzone_name: thermal zone name
   * @suspended: suspend flag of this bank
   * @phase: bank current phase
   * @volt_od: bank voltage overdrive
@@ -270,6 +284,13 @@ struct svs_platform {
   * @sw_id: bank software identification
   * @cpu_id: cpu core id for SVS CPU bank use only
   * @ctl0: TS-x selection
+ * @temp: bank temperature
+ * @tzone_htemp: thermal zone high temperature threshold
+ * @tzone_htemp_voffset: thermal zone high temperature voltage offset
+ * @tzone_ltemp: thermal zone low temperature threshold
+ * @tzone_ltemp_voffset: thermal zone low temperature voltage offset
+ * @bts: svs efuse data
+ * @mts: svs efuse data
   * @bdes: svs efuse data
   * @mdes: svs efuse data
   * @mtdes: svs efuse data
@@ -292,6 +313,7 @@ struct svs_bank {
  	void (*get_volts)(struct svs_platform *svsp);
  	char *name;
  	char *buck_name;
+	char *tzone_name;
  	bool suspended;
  	enum svsb_phase phase;
  	s32 volt_od;
@@ -321,6 +343,13 @@ struct svs_bank {
  	u32 sw_id;
  	u32 cpu_id;
  	u32 ctl0;
+	u32 temp;
+	u32 tzone_htemp;
+	u32 tzone_htemp_voffset;
+	u32 tzone_ltemp;
+	u32 tzone_ltemp_voffset;
+	u32 bts;
+	u32 mts;
  	u32 bdes;
  	u32 mdes;
  	u32 mtdes;

drivers/soc/mediatek/mtk-svs.c static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, · line 393

Quoted 4 lines
drivers/soc/mediatek/mtk-svs.c +0 -0
@@ -361,10 +390,21 @@ static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt,
 u32 svsb_volt_step,
  	return (svsb_volt * svsb_volt_step) + svsb_volt_base;
  }
 

I'm sorry for the double review, but this went unnoticed in the previous one.

drivers/soc/mediatek/mtk-svs.c static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step, · line 397

Quoted 6 lines
drivers/soc/mediatek/mtk-svs.c +5 -0
+static int svs_get_zone_temperature(const char *tzone_name, int
 *tzone_temp)
+{
+	struct thermal_zone_device *tzd;
+
+	tzd = thermal_zone_get_zone_by_name(tzone_name);

This call is expensive, as it's iterating through the (possibly) entire thermal_tz_list (drivers/thermal/thermal_core.c) so, for performance purposes, noting that you're using this in svs_adjust_pm_opp_volts(), it's not a good idea
to call it at every ISR.

Quoted 5 lines

I would instead propose to get a pointer to the thermal_zone at driver probe time and cache that in struct svs_bank: this function would also be removed as the only thing that you'd need to do then would be just one call...

[read forward...]

No problem. I'll cache thermal_zone at driver probe time and remove this API in the next patch. Thanks.

Quoted 29 lines
Quoted 18 lines
drivers/soc/mediatek/mtk-svs.c +8 -2
+	if (IS_ERR(tzd))
+		return PTR_ERR(tzd);
+
+	return thermal_zone_get_temp(tzd, tzone_temp);
+}
+
  static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool
 force_update)
  {
-	int ret = -EPERM;
-	u32 i, svsb_volt, opp_volt;
+	int ret = -EPERM, tzone_temp = 0;
+	u32 i, svsb_volt, opp_volt, temp_voffset = 0;
  
  	mutex_lock(&svsb->lock);

drivers/soc/mediatek/mtk-svs.c static int svs_adjust_pm_opp_volts(struct svs_bank · line 424

Quoted 7 lines
drivers/soc/mediatek/mtk-svs.c +3 -0
@@ -378,6 +418,22 @@ static int svs_adjust_pm_opp_volts(struct svs_bank
 *svsb, bool force_update)
  		goto unlock_mutex;
  	}
 
+	/* Get thermal effect */
+	if (svsb->phase == SVSB_PHASE_MON) {
+		ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);

... so you can simply call ...

Quoted 4 lines
	ret = thermal_zone_get_temp(svsb->tzd, tzone_temp);

...without any need for any helper.

Sure, I'll call thermal_zone_get_temp() directly after applying this recommended change in the next patch. Thanks.

Quoted 23 lines
Quoted 16 lines
drivers/soc/mediatek/mtk-svs.c +13 -0
+		if (ret || (svsb->temp > SVSB_TEMP_UPPER_BOUND &&
+			    svsb->temp < SVSB_TEMP_LOWER_BOUND)) {
+			dev_err(svsb->dev, "%s: %d (0x%x), run default volts\n",
+				svsb->tzone_name, ret, svsb->temp);
+			svsb->phase = SVSB_PHASE_ERROR;
+		}
+
+		if (tzone_temp >= svsb->tzone_htemp)
+			temp_voffset += svsb->tzone_htemp_voffset;
+		else if (tzone_temp <= svsb->tzone_ltemp)
+			temp_voffset += svsb->tzone_ltemp_voffset;
+	}
+
  	/* vmin <= svsb_volt (opp_volt) <= default opp voltage */
  	for (i = 0; i < svsb->opp_count; i++) {
Quoted 1 line
  		switch (svsb->phase) {

Apart from that, the commit looks good. Looking forward to review the new version!

Regards,
- Angelo

author
#11 lore raw

PATCH v21 5/8 soc: mediatek: SVS: add debug commands

In reply to Roger Lu

The purpose of SVS is to help find the suitable voltages for DVFS. Therefore, if SVS bank voltages are concerned to be wrong, we can adjust SVS bank voltages by this patch.

---
 drivers/soc/mediatek/mtk-svs.c | 321 ++++++++++++++++++++++++++++++++-
 1 file changed, 318 insertions(+), 3 deletions(-)
drivers/soc/mediatek/mtk-svs.c +318 -3
index 042c6e8e9069..93cdaecadd6d 100644
@@ -6,6 +6,7 @@
 #include <linux/bits.h>
 #include <linux/clk.h>
 #include <linux/completion.h>
+#include <linux/debugfs.h>
 #include <linux/device.h>
 #include <linux/init.h>
 #include <linux/interrupt.h>
@@ -23,6 +24,7 @@
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
 #include <linux/thermal.h>
@@ -68,6 +70,39 @@
 
 static DEFINE_SPINLOCK(svs_lock);
 
+#define debug_fops_ro(name)						\
+	static int svs_##name##_debug_open(struct inode *inode,		\
+					   struct file *filp)		\
+	{								\
+		return single_open(filp, svs_##name##_debug_show,	\
+				   inode->i_private);			\
+	}								\
+	static const struct file_operations svs_##name##_debug_fops = {	\
+		.owner = THIS_MODULE,					\
+		.open = svs_##name##_debug_open,			\
+		.read = seq_read,					\
+		.llseek = seq_lseek,					\
+		.release = single_release,				\
+	}
+
+#define debug_fops_rw(name)						\
+	static int svs_##name##_debug_open(struct inode *inode,		\
+					   struct file *filp)		\
+	{								\
+		return single_open(filp, svs_##name##_debug_show,	\
+				   inode->i_private);			\
+	}								\
+	static const struct file_operations svs_##name##_debug_fops = {	\
+		.owner = THIS_MODULE,					\
+		.open = svs_##name##_debug_open,			\
+		.read = seq_read,					\
+		.write = svs_##name##_debug_write,			\
+		.llseek = seq_lseek,					\
+		.release = single_release,				\
+	}
+
+#define svs_dentry_data(name)	{__stringify(name), &svs_##name##_debug_fops}
+
 /**
  * enum svsb_phase - svs bank phase enumeration
  * @SVSB_PHASE_ERROR: svs bank encounters unexpected condition
@@ -258,6 +293,8 @@ struct svs_platform {
  * @suspended: suspend flag of this bank
  * @phase: bank current phase
  * @volt_od: bank voltage overdrive
+ * @voffset: bank voltage offset for debug purpose
+ * @reg_data: bank register data in different phase for debug purpose
  * @pm_runtime_enabled_count: bank pm runtime enabled count
  * @mode_support: bank mode support.
  * @freq_base: reference frequency for bank init
@@ -317,6 +354,8 @@ struct svs_bank {
 	bool suspended;
 	enum svsb_phase phase;
 	s32 volt_od;
+	s32 voffset;
+	u32 reg_data[SVSB_PHASE_MAX][SVS_REG_MAX];
 	u32 pm_runtime_enabled_count;
 	u32 mode_support;
 	u32 freq_base;
@@ -444,14 +483,15 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 			/* do nothing */
 			goto unlock_mutex;
 		case SVSB_PHASE_INIT02:
-			svsb_volt = max(svsb->volt[i], svsb->vmin);
+			svsb_volt = max(svsb->volt[i] + svsb->voffset,
+					svsb->vmin);
 			opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
 							     svsb->volt_step,
 							     svsb->volt_base);
 			break;
 		case SVSB_PHASE_MON:
-			svsb_volt = max(svsb->volt[i] + temp_voffset,
-					svsb->vmin);
+			svsb_volt = max(svsb->volt[i] + temp_voffset +
+					svsb->voffset, svsb->vmin);
 			opp_volt = svs_bank_volt_to_opp_volt(svsb_volt,
 							     svsb->volt_step,
 							     svsb->volt_base);
@@ -480,6 +520,257 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 	return ret;
 }
 
+static int svs_dump_debug_show(struct seq_file *m, void *p)
+{
+	struct svs_platform *svsp = (struct svs_platform *)m->private;
+	struct svs_bank *svsb;
+	unsigned long svs_reg_addr;
+	u32 idx, i, j, bank_id;
+
+	for (i = 0; i < svsp->efuse_max; i++)
+		if (svsp->efuse && svsp->efuse[i])
+			seq_printf(m, "M_HW_RES%d = 0x%08x\n",
+				   i, svsp->efuse[i]);
+
+	for (i = 0; i < svsp->tefuse_max; i++)
+		if (svsp->tefuse)
+			seq_printf(m, "THERMAL_EFUSE%d = 0x%08x\n",
+				   i, svsp->tefuse[i]);
+
+	for (bank_id = 0, idx = 0; idx < svsp->bank_max; idx++, bank_id++) {
+		svsb = &svsp->banks[idx];
+
+		for (i = SVSB_PHASE_INIT01; i <= SVSB_PHASE_MON; i++) {
+			seq_printf(m, "Bank_number = %u\n", bank_id);
+
+			if (i == SVSB_PHASE_INIT01 || i == SVSB_PHASE_INIT02)
+				seq_printf(m, "mode = init%d\n", i);
+			else if (i == SVSB_PHASE_MON)
+				seq_puts(m, "mode = mon\n");
+			else
+				seq_puts(m, "mode = error\n");
+
+			for (j = DESCHAR; j < SVS_REG_MAX; j++) {
+				svs_reg_addr = (unsigned long)(svsp->base +
+							       svsp->regs[j]);
+				seq_printf(m, "0x%08lx = 0x%08x\n",
+					   svs_reg_addr, svsb->reg_data[i][j]);
+			}
+		}
+	}
+
+	return 0;
+}
+
+debug_fops_ro(dump);
+
+static int svs_enable_debug_show(struct seq_file *m, void *v)
+{
+	struct svs_bank *svsb = (struct svs_bank *)m->private;
+
+	switch (svsb->phase) {
+	case SVSB_PHASE_ERROR:
+		seq_puts(m, "disabled\n");
+		break;
+	case SVSB_PHASE_INIT01:
+		seq_puts(m, "init1\n");
+		break;
+	case SVSB_PHASE_INIT02:
+		seq_puts(m, "init2\n");
+		break;
+	case SVSB_PHASE_MON:
+		seq_puts(m, "mon mode\n");
+		break;
+	default:
+		seq_puts(m, "unknown\n");
+		break;
+	}
+
+	return 0;
+}
+
+static ssize_t svs_enable_debug_write(struct file *filp,
+				      const char __user *buffer,
+				      size_t count, loff_t *pos)
+{
+	struct svs_bank *svsb = file_inode(filp)->i_private;
+	struct svs_platform *svsp = dev_get_drvdata(svsb->dev);
+	unsigned long flags;
+	int enabled, ret;
+	char *buf = NULL;
+
+	if (count >= PAGE_SIZE)
+		return -EINVAL;
+
+	buf = (char *)memdup_user_nul(buffer, count);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	ret = kstrtoint(buf, 10, &enabled);
+	if (ret)
+		return ret;
+
+	if (!enabled) {
+		spin_lock_irqsave(&svs_lock, flags);
+		svsp->pbank = svsb;
+		svsb->mode_support = SVSB_MODE_ALL_DISABLE;
+		svs_switch_bank(svsp);
+		svs_writel_relaxed(svsp, SVSB_EN_OFF, SVSEN);
+		svs_writel_relaxed(svsp, SVSB_INTSTS_CLEAN, INTSTS);
+		spin_unlock_irqrestore(&svs_lock, flags);
+
+		svsb->phase = SVSB_PHASE_ERROR;
+		svs_adjust_pm_opp_volts(svsb, true);
+	}
+
+	kfree(buf);
+
+	return count;
+}
+
+debug_fops_rw(enable);
+
+static int svs_status_debug_show(struct seq_file *m, void *v)
+{
+	struct svs_bank *svsb = (struct svs_bank *)m->private;
+	struct dev_pm_opp *opp;
+	int tzone_temp = 0, ret;
+	u32 i;
+
+	ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
+	if (ret)
+		seq_printf(m, "%s: temperature ignore\n", svsb->name);
+	else
+		seq_printf(m, "%s: temperature = %d\n", svsb->name, tzone_temp);
+
+	for (i = 0; i < svsb->opp_count; i++) {
+		opp = dev_pm_opp_find_freq_exact(svsb->opp_dev,
+						 svsb->opp_dfreq[i], true);
+		if (IS_ERR(opp)) {
+			seq_printf(m, "%s: cannot find freq = %u (%ld)\n",
+				   svsb->name, svsb->opp_dfreq[i],
+				   PTR_ERR(opp));
+			return PTR_ERR(opp);
+		}
+
+		seq_printf(m, "opp_freq[%02u]: %u, opp_volt[%02u]: %lu, ",
+			   i, svsb->opp_dfreq[i], i,
+			   dev_pm_opp_get_voltage(opp));
+		seq_printf(m, "svsb_volt[%02u]: 0x%x, freq_pct[%02u]: %u\n",
+			   i, svsb->volt[i], i, svsb->freq_pct[i]);
+		dev_pm_opp_put(opp);
+	}
+
+	return 0;
+}
+
+debug_fops_ro(status);
+
+static int svs_voffset_debug_show(struct seq_file *m, void *v)
+{
+	struct svs_bank *svsb = (struct svs_bank *)m->private;
+
+	seq_printf(m, "%d\n", svsb->voffset);
+
+	return 0;
+}
+
+static ssize_t svs_voffset_debug_write(struct file *filp,
+				       const char __user *buffer,
+				       size_t count, loff_t *pos)
+{
+	struct svs_bank *svsb = file_inode(filp)->i_private;
+	char *buf = NULL;
+	s32 voffset;
+
+	if (count >= PAGE_SIZE)
+		return -EINVAL;
+
+	buf = (char *)memdup_user_nul(buffer, count);
+	if (IS_ERR(buf))
+		return PTR_ERR(buf);
+
+	if (!kstrtoint(buf, 10, &voffset)) {
+		svsb->voffset = voffset;
+		svs_adjust_pm_opp_volts(svsb, true);
+	}
+
+	kfree(buf);
+
+	return count;
+}
+
+debug_fops_rw(voffset);
+
+static int svs_create_debug_cmds(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb;
+	struct dentry *svs_dir, *svsb_dir, *file_entry;
+	const char *d = "/sys/kernel/debug/svs";
+	u32 i, idx;
+
+	struct svs_dentry {
+		const char *name;
+		const struct file_operations *fops;
+	};
+
+	struct svs_dentry svs_entries[] = {
+		svs_dentry_data(dump),
+	};
+
+	struct svs_dentry svsb_entries[] = {
+		svs_dentry_data(enable),
+		svs_dentry_data(status),
+		svs_dentry_data(voffset),
+	};
+
+	svs_dir = debugfs_create_dir("svs", NULL);
+	if (IS_ERR(svs_dir)) {
+		dev_err(svsp->dev, "cannot create %s: %ld\n",
+			d, PTR_ERR(svs_dir));
+		return PTR_ERR(svs_dir);
+	}
+
+	for (i = 0; i < ARRAY_SIZE(svs_entries); i++) {
+		file_entry = debugfs_create_file(svs_entries[i].name, 0664,
+						 svs_dir, svsp,
+						 svs_entries[i].fops);
+		if (IS_ERR(file_entry)) {
+			dev_err(svsp->dev, "cannot create %s/%s: %ld\n",
+				d, svs_entries[i].name, PTR_ERR(file_entry));
+			return PTR_ERR(file_entry);
+		}
+	}
+
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+
+		if (svsb->mode_support == SVSB_MODE_ALL_DISABLE)
+			continue;
+
+		svsb_dir = debugfs_create_dir(svsb->name, svs_dir);
+		if (IS_ERR(svsb_dir)) {
+			dev_err(svsp->dev, "cannot create %s/%s: %ld\n",
+				d, svsb->name, PTR_ERR(svsb_dir));
+			return PTR_ERR(svsb_dir);
+		}
+
+		for (i = 0; i < ARRAY_SIZE(svsb_entries); i++) {
+			file_entry = debugfs_create_file(svsb_entries[i].name,
+							 0664, svsb_dir, svsb,
+							 svsb_entries[i].fops);
+			if (IS_ERR(file_entry)) {
+				dev_err(svsp->dev, "no %s/%s/%s?: %ld\n",
+					d, svsb->name, svsb_entries[i].name,
+					PTR_ERR(file_entry));
+				return PTR_ERR(file_entry);
+			}
+		}
+	}
+
+	return 0;
+}
+
 static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx)
 {
 	u32 vx;
@@ -605,6 +896,16 @@ static void svs_set_bank_phase(struct svs_platform *svsp,
 	}
 }
 
+static inline void svs_save_bank_register_data(struct svs_platform *svsp,
+					       enum svsb_phase phase)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	enum svs_reg_index rg_i;
+
+	for (rg_i = DESCHAR; rg_i < SVS_REG_MAX; rg_i++)
+		svsb->reg_data[phase][rg_i] = svs_readl_relaxed(svsp, rg_i);
+}
+
 static inline void svs_error_isr_handler(struct svs_platform *svsp)
 {
 	struct svs_bank *svsb = svsp->pbank;
@@ -619,6 +920,8 @@ static inline void svs_error_isr_handler(struct svs_platform *svsp)
 		svs_readl_relaxed(svsp, SMSTATE1));
 	dev_err(svsb->dev, "TEMP = 0x%08x\n", svs_readl_relaxed(svsp, TEMP));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_ERROR);
+
 	svsb->mode_support = SVSB_MODE_ALL_DISABLE;
 	svsb->phase = SVSB_PHASE_ERROR;
 
@@ -635,6 +938,8 @@ static inline void svs_init01_isr_handler(struct svs_platform *svsp)
 		 svs_readl_relaxed(svsp, VDESIGN30),
 		 svs_readl_relaxed(svsp, DCVALUES));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_INIT01);
+
 	svsb->phase = SVSB_PHASE_INIT01;
 	svsb->dc_voffset_in = ~(svs_readl_relaxed(svsp, DCVALUES) &
 				GENMASK(15, 0)) + 1;
@@ -662,6 +967,8 @@ static inline void svs_init02_isr_handler(struct svs_platform *svsp)
 		 svs_readl_relaxed(svsp, VOP30),
 		 svs_readl_relaxed(svsp, DCVALUES));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_INIT02);
+
 	svsb->phase = SVSB_PHASE_INIT02;
 	svsb->get_volts(svsp);
 
@@ -673,6 +980,8 @@ static inline void svs_mon_mode_isr_handler(struct svs_platform *svsp)
 {
 	struct svs_bank *svsb = svsp->pbank;
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_MON);
+
 	svsb->phase = SVSB_PHASE_MON;
 	svsb->temp = svs_readl_relaxed(svsp, TEMP) & GENMASK(7, 0);
 	svsb->get_volts(svsp);
@@ -1658,6 +1967,12 @@ static int svs_probe(struct platform_device *pdev)
 		goto svs_probe_iounmap;
 	}
 
+	ret = svs_create_debug_cmds(svsp);
+	if (ret) {
+		dev_err(svsp->dev, "svs create debug cmds fail: %d\n", ret);
+		goto svs_probe_iounmap;
+	}
+
 	return 0;
 
 svs_probe_iounmap:
Signature · 1 line
2.18.0
#12 lore raw

Re: [PATCH v21 5/8] soc: mediatek: SVS: add debug commands

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 13 lines

The purpose of SVS is to help find the suitable voltages for DVFS. Therefore, if SVS bank voltages are concerned to be wrong, we can adjust SVS bank voltages by this patch.

---
  drivers/soc/mediatek/mtk-svs.c | 321 ++++++++++++++++++++++++++++++++-
  1 file changed, 318 insertions(+), 3 deletions(-)
drivers/soc/mediatek/mtk-svs.c +0 -0
index 042c6e8e9069..93cdaecadd6d 100644

..snip..

drivers/soc/mediatek/mtk-svs.c static void svs_set_bank_phase(struct svs_platform *svsp, · line 904

Quoted 10 lines
drivers/soc/mediatek/mtk-svs.c +6 -0
@@ -605,6 +896,16 @@ static void svs_set_bank_phase(struct svs_platform *svsp,
  	}
  }
 
+static inline void svs_save_bank_register_data(struct svs_platform *svsp,
+					       enum svsb_phase phase)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	enum svs_reg_index rg_i;
+

I think that it'd be a good idea to add an enable parameter, so that we don't always do a register dump; after all, this is a debugging feature and it's going to be completely irrelevant to the user, so keeping this disabled by default would ensure to get no performance degradation (even if small) unless really wanted.

So, in this case, here we'd have

	if (!svsp->debug_enabled)
		return;

drivers/soc/mediatek/mtk-svs.c static void svs_set_bank_phase(struct svs_platform *svsp, · line 908

Quoted 4 lines
drivers/soc/mediatek/mtk-svs.c +4 -0
+	for (rg_i = DESCHAR; rg_i < SVS_REG_MAX; rg_i++)
+		svsb->reg_data[phase][rg_i] = svs_readl_relaxed(svsp, rg_i);
+}
+

Of course, this implies adding a new debugfs entry to enable/disable the debugging. Everything else looks good :)

Quoted 53 lines
drivers/soc/mediatek/mtk-svs.c +14 -0
  static inline void svs_error_isr_handler(struct svs_platform *svsp)
  {
  	struct svs_bank *svsb = svsp->pbank;
@@ -619,6 +920,8 @@ static inline void svs_error_isr_handler(struct svs_platform *svsp)
  		svs_readl_relaxed(svsp, SMSTATE1));
  	dev_err(svsb->dev, "TEMP = 0x%08x\n", svs_readl_relaxed(svsp, TEMP));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_ERROR);
+
  	svsb->mode_support = SVSB_MODE_ALL_DISABLE;
  	svsb->phase = SVSB_PHASE_ERROR;
 
@@ -635,6 +938,8 @@ static inline void svs_init01_isr_handler(struct svs_platform *svsp)
  		 svs_readl_relaxed(svsp, VDESIGN30),
  		 svs_readl_relaxed(svsp, DCVALUES));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_INIT01);
+
  	svsb->phase = SVSB_PHASE_INIT01;
  	svsb->dc_voffset_in = ~(svs_readl_relaxed(svsp, DCVALUES) &
  				GENMASK(15, 0)) + 1;
@@ -662,6 +967,8 @@ static inline void svs_init02_isr_handler(struct svs_platform *svsp)
  		 svs_readl_relaxed(svsp, VOP30),
  		 svs_readl_relaxed(svsp, DCVALUES));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_INIT02);
+
  	svsb->phase = SVSB_PHASE_INIT02;
  	svsb->get_volts(svsp);
 
@@ -673,6 +980,8 @@ static inline void svs_mon_mode_isr_handler(struct svs_platform *svsp)
  {
  	struct svs_bank *svsb = svsp->pbank;
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_MON);
+
  	svsb->phase = SVSB_PHASE_MON;
  	svsb->temp = svs_readl_relaxed(svsp, TEMP) & GENMASK(7, 0);
  	svsb->get_volts(svsp);
@@ -1658,6 +1967,12 @@ static int svs_probe(struct platform_device *pdev)
  		goto svs_probe_iounmap;
  	}
 
+	ret = svs_create_debug_cmds(svsp);
+	if (ret) {
+		dev_err(svsp->dev, "svs create debug cmds fail: %d\n", ret);
+		goto svs_probe_iounmap;
+	}
+
  	return 0;
 
  svs_probe_iounmap:
author
#13 lore raw

Re: [PATCH v21 5/8] soc: mediatek: SVS: add debug commands

Hi AngeloGioacchino,

Sorry for the late reply and thanks for the advice.

Quoted 35 lines · AngeloGioacchino Del Regno

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 13 lines

The purpose of SVS is to help find the suitable voltages for DVFS. Therefore, if SVS bank voltages are concerned to be wrong, we can adjust SVS bank voltages by this patch.

---
  drivers/soc/mediatek/mtk-svs.c | 321 ++++++++++++++++++++++++++++++++-
  1 file changed, 318 insertions(+), 3 deletions(-)
drivers/soc/mediatek/mtk-svs.c +0 -0
index 042c6e8e9069..93cdaecadd6d 100644

..snip..

drivers/soc/mediatek/mtk-svs.c static void svs_set_bank_phase(struct svs_platform · line 905

Quoted 11 lines
drivers/soc/mediatek/mtk-svs.c +6 -0
@@ -605,6 +896,16 @@ static void svs_set_bank_phase(struct svs_platform
 *svsp,
  	}
  }
 
+static inline void svs_save_bank_register_data(struct svs_platform *svsp,
+					       enum svsb_phase phase)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	enum svs_reg_index rg_i;
+

I think that it'd be a good idea to add an enable parameter, so that we don't always do a register dump; after all, this is a debugging feature and it's going to be completely irrelevant to the user, so keeping this disabled by default would ensure to get no performance degradation (even if small) unless really wanted.

Quoted 4 lines

So, in this case, here we'd have

	if (!svsp->debug_enabled)
		return;

Thanks for pointing out the concern. Excuse us, we really need this to be enabled by default. If we add a enable flag here, we'll face below problems and make debug more difficult.

1. If we enable it afterward by cmd, init0[1~2] registers' data cannot be recorded expectedly because the init flow has been finished already and won't be run anymore. So, it doesn't work by using cmd to enable the flag.
2. If we add a enable flag here, it means we have to re-build the kernel load in order to enable this flag. However, we cannot re-build the kernel load and needs to debug directly sometimes. It's a sad situation... :(

Quoted 9 lines

drivers/soc/mediatek/mtk-svs.c static void svs_set_bank_phase(struct svs_platform *svsp, · line 908

Quoted 5 lines
drivers/soc/mediatek/mtk-svs.c +4 -0
+	for (rg_i = DESCHAR; rg_i < SVS_REG_MAX; rg_i++)
+		svsb->reg_data[phase][rg_i] = svs_readl_relaxed(svsp, rg_i);
+}
+

Of course, this implies adding a new debugfs entry to enable/disable the debugging.
Everything else looks good :)

Oh, excuse us, we have to keep the old design for better instant support and thanks for the understanding.

Quoted 12 lines
Quoted 12 lines
drivers/soc/mediatek/mtk-svs.c +0 -0
  static inline void svs_error_isr_handler(struct svs_platform *svsp)
  {
  	struct svs_bank *svsb = svsp->pbank;
drivers/soc/mediatek/mtk-svs.c +2 -0
@@ -619,6 +920,8 @@ static inline void svs_error_isr_handler(struct
 svs_platform *svsp)
  		svs_readl_relaxed(svsp, SMSTATE1));
  	dev_err(svsb->dev, "TEMP = 0x%08x\n", svs_readl_relaxed(svsp, TEMP));
 
+	svs_save_bank_register_data(svsp, SVSB_PHASE_ERROR);
+
  	svsb->mode_support = SVSB_MODE_ALL_DISABLE;
  	svsb->phase = SVSB_PHASE_ERROR;

[snip]

author
#14 lore raw

PATCH v21 6/8 dt-bindings: soc: mediatek: add mt8192 svs dt-bindings

In reply to Roger Lu

Add mt8192 svs compatible/resets in dt-bindings.

---
 .../devicetree/bindings/soc/mediatek/mtk-svs.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)
Documentation/devicetree/bindings/soc/mediatek/mtk-svs.yaml +8 -0
index a855ced410f8..59342e627b67 100644
@@ -22,6 +22,7 @@ properties:
   compatible:
     enum:
       - mediatek,mt8183-svs
+      - mediatek,mt8192-svs
 
   reg:
     maxItems: 1
@@ -51,6 +52,13 @@ properties:
       - const: svs-calibration-data
       - const: t-calibration-data
 
+  resets:
+    maxItems: 1
+
+  reset-names:
+    items:
+      - const: svs_rst
+
 required:
   - compatible
   - reg
Signature · 1 line
2.18.0
maintainer Reviewed-by on v21 6/8
#15 lore raw

Re: [PATCH v21 6/8] dt-bindings: soc: mediatek: add mt8192 svs dt-bindings

Quoted 7 lines · Roger Lu

Add mt8192 svs compatible/resets in dt-bindings.

---
 .../devicetree/bindings/soc/mediatek/mtk-svs.yaml         | 8 ++++++++
 1 file changed, 8 insertions(+)
author
#16 lore raw

PATCH v21 7/8 arm64: dts: mt8192: add svs device information

In reply to Roger Lu

Add compitable/reg/irq/clock/efuse/reset setting in svs node.

---
 arch/arm64/boot/dts/mediatek/mt8192.dtsi | 39 ++++++++++++++++++++++++
 1 file changed, 39 insertions(+)
arch/arm64/boot/dts/mediatek/mt8192.dtsi +39 -0
index c7c7d4e017ae..c111e26489dd 100644
@@ -9,6 +9,8 @@
 #include <dt-bindings/interrupt-controller/arm-gic.h>
 #include <dt-bindings/interrupt-controller/irq.h>
 #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
+#include <dt-bindings/reset/ti-syscon.h>
+
 
 / {
 	compatible = "mediatek,mt8192";
@@ -268,6 +270,14 @@
 			compatible = "mediatek,mt8192-infracfg", "syscon";
 			reg = <0 0x10001000 0 0x1000>;
 			#clock-cells = <1>;
+
+			infracfg_rst: reset-controller {
+				compatible = "mediatek,infra-reset", "ti,syscon-reset";
+				#reset-cells = <1>;
+				ti,reset-bits = <
+					0x150 5 0x154 5 0 0     (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: svs */
+				>;
+			};
 		};
 
 		pericfg: syscon@10003000 {
@@ -362,6 +372,20 @@
 			status = "disabled";
 		};
 
+		svs: svs@1100b000 {
+			compatible = "mediatek,mt8192-svs";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH 0>;
+			clocks = <&infracfg CLK_INFRA_THERM>;
+			clock-names = "main";
+			nvmem-cells = <&svs_calibration>,
+				      <&lvts_e_data1>;
+			nvmem-cell-names = "svs-calibration-data",
+					   "t-calibration-data";
+			resets = <&infracfg_rst 0>;
+			reset-names = "svs_rst";
+		};
+
 		spi1: spi@11010000 {
 			compatible = "mediatek,mt8192-spi",
 				     "mediatek,mt6765-spi";
@@ -479,6 +503,21 @@
 			#clock-cells = <1>;
 		};
 
+		efuse: efuse@11c10000 {
+			compatible = "mediatek,efuse";
+			reg = <0 0x11c10000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			lvts_e_data1: data1@1c0 {
+				reg = <0x1c0 0x58>;
+			};
+
+			svs_calibration: calib@580 {
+				reg = <0x580 0x68>;
+			};
+		};
+
 		i2c3: i2c3@11cb0000 {
 			compatible = "mediatek,mt8192-i2c";
 			reg = <0 0x11cb0000 0 0x1000>,
Signature · 1 line
2.18.0
maintainer Reviewed-by on v21 7/8
#17 lore raw

Re: [PATCH v21 7/8] arm64: dts: mt8192: add svs device information

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 1 line

Add compitable/reg/irq/clock/efuse/reset setting in svs node.

Typo: compitable => compatible
.. also, you're not only adding the svs node, but also efuse: please add that information in the commit description.

Quoted 2 lines

This patch seems to not apply on top of the current linux-next, can you please rebase it? That would resolve issues with this series and would be picked sooner.

Apart from that...

Quoted 76 lines
---
  arch/arm64/boot/dts/mediatek/mt8192.dtsi | 39 ++++++++++++++++++++++++
  1 file changed, 39 insertions(+)
arch/arm64/boot/dts/mediatek/mt8192.dtsi +39 -0
index c7c7d4e017ae..c111e26489dd 100644
@@ -9,6 +9,8 @@
  #include <dt-bindings/interrupt-controller/arm-gic.h>
  #include <dt-bindings/interrupt-controller/irq.h>
  #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
+#include <dt-bindings/reset/ti-syscon.h>
+
 
  / {
  	compatible = "mediatek,mt8192";
@@ -268,6 +270,14 @@
  			compatible = "mediatek,mt8192-infracfg", "syscon";
  			reg = <0 0x10001000 0 0x1000>;
  			#clock-cells = <1>;
+
+			infracfg_rst: reset-controller {
+				compatible = "mediatek,infra-reset", "ti,syscon-reset";
+				#reset-cells = <1>;
+				ti,reset-bits = <
+					0x150 5 0x154 5 0 0     (ASSERT_SET | DEASSERT_SET | STATUS_NONE) /* 0: svs */
+				>;
+			};
  		};
 
  		pericfg: syscon@10003000 {
@@ -362,6 +372,20 @@
  			status = "disabled";
  		};
 
+		svs: svs@1100b000 {
+			compatible = "mediatek,mt8192-svs";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH 0>;
+			clocks = <&infracfg CLK_INFRA_THERM>;
+			clock-names = "main";
+			nvmem-cells = <&svs_calibration>,
+				      <&lvts_e_data1>;
+			nvmem-cell-names = "svs-calibration-data",
+					   "t-calibration-data";
+			resets = <&infracfg_rst 0>;
+			reset-names = "svs_rst";
+		};
+
  		spi1: spi@11010000 {
  			compatible = "mediatek,mt8192-spi",
  				     "mediatek,mt6765-spi";
@@ -479,6 +503,21 @@
  			#clock-cells = <1>;
  		};
 
+		efuse: efuse@11c10000 {
+			compatible = "mediatek,efuse";
+			reg = <0 0x11c10000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			lvts_e_data1: data1@1c0 {
+				reg = <0x1c0 0x58>;
+			};
+
+			svs_calibration: calib@580 {
+				reg = <0x580 0x68>;
+			};
+		};
+
  		i2c3: i2c3@11cb0000 {
  			compatible = "mediatek,mt8192-i2c";
  			reg = <0 0x11cb0000 0 0x1000>,
Signature · 6 lines
AngeloGioacchino Del Regno
Software Engineer

Collabora Ltd.
Platinum Building, St John's Innovation Park, Cambridge CB4 0DS, UK
Registered in England & Wales, no. 5513718
author
#18 lore raw

Re: [PATCH v21 7/8] arm64: dts: mt8192: add svs device information

Hi AngeloGioacchino,

Quoted 8 lines · AngeloGioacchino Del Regno

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 1 line

Add compitable/reg/irq/clock/efuse/reset setting in svs node.

Typo: compitable => compatible
.. also, you're not only adding the svs node, but also efuse: please add that information in the commit description.

Quoted 1 line
Quoted 7 lines
Quoted 1 line

This patch seems to not apply on top of the current linux-next, can you please rebase it? That would resolve issues with this series and would be picked sooner.

Apart from that...

Sorry to make you confuse. After having discussion internally, we'll submit another complete mt8192.dtsi patch including this svs node. Therefore, I'll drop this path from svs patchset in order not to make the reviewer confuse. Thanks for the comments a lot.

Quoted 85 lines

Reviewed-by: AngeloGioacchino Del Regno <
[email protected]>

Quoted 79 lines
---
  arch/arm64/boot/dts/mediatek/mt8192.dtsi | 39 ++++++++++++++++++++++++
  1 file changed, 39 insertions(+)
arch/arm64/boot/dts/mediatek/mt8192.dtsi +8 -0
index c7c7d4e017ae..c111e26489dd 100644
@@ -9,6 +9,8 @@
  #include <dt-bindings/interrupt-controller/arm-gic.h>
  #include <dt-bindings/interrupt-controller/irq.h>
  #include <dt-bindings/pinctrl/mt8192-pinfunc.h>
+#include <dt-bindings/reset/ti-syscon.h>
+
 
  / {
  	compatible = "mediatek,mt8192";
@@ -268,6 +270,14 @@
  			compatible = "mediatek,mt8192-infracfg", "syscon";
  			reg = <0 0x10001000 0 0x1000>;
  			#clock-cells = <1>;
+
+			infracfg_rst: reset-controller {
+				compatible = "mediatek,infra-reset", "ti,syscon-
 reset";
+				#reset-cells = <1>;
+				ti,reset-bits = <
+					0x150 5 0x154 5 0 0     (ASSERT_SET |

DEASSERT_SET | STATUS_NONE) /* 0: svs */

arch/arm64/boot/dts/mediatek/mt8192.dtsi +31 -0
+				>;
+			};
  		};
 
  		pericfg: syscon@10003000 {
@@ -362,6 +372,20 @@
  			status = "disabled";
  		};
 
+		svs: svs@1100b000 {
+			compatible = "mediatek,mt8192-svs";
+			reg = <0 0x1100b000 0 0x1000>;
+			interrupts = <GIC_SPI 167 IRQ_TYPE_LEVEL_HIGH 0>;
+			clocks = <&infracfg CLK_INFRA_THERM>;
+			clock-names = "main";
+			nvmem-cells = <&svs_calibration>,
+				      <&lvts_e_data1>;
+			nvmem-cell-names = "svs-calibration-data",
+					   "t-calibration-data";
+			resets = <&infracfg_rst 0>;
+			reset-names = "svs_rst";
+		};
+
  		spi1: spi@11010000 {
  			compatible = "mediatek,mt8192-spi",
  				     "mediatek,mt6765-spi";
@@ -479,6 +503,21 @@
  			#clock-cells = <1>;
  		};
 
+		efuse: efuse@11c10000 {
+			compatible = "mediatek,efuse";
+			reg = <0 0x11c10000 0 0x1000>;
+			#address-cells = <1>;
+			#size-cells = <1>;
+
+			lvts_e_data1: data1@1c0 {
+				reg = <0x1c0 0x58>;
+			};
+
+			svs_calibration: calib@580 {
+				reg = <0x580 0x68>;
+			};
+		};
+
  		i2c3: i2c3@11cb0000 {
  			compatible = "mediatek,mt8192-i2c";
  			reg = <0 0x11cb0000 0 0x1000>,
#19 lore raw

Re: [PATCH v21 7/8] arm64: dts: mt8192: add svs device information

Il 24/01/22 11:48, Roger Lu ha scritto:

Quoted 17 lines

Hi AngeloGioacchino,

Quoted 14 lines · AngeloGioacchino Del Regno

Il 07/01/22 10:51, Roger Lu ha scritto:

Quoted 1 line

Add compitable/reg/irq/clock/efuse/reset setting in svs node.

Typo: compitable => compatible
.. also, you're not only adding the svs node, but also efuse: please add that information in the commit description.

Quoted 2 lines

This patch seems to not apply on top of the current linux-next, can you please rebase it? That would resolve issues with this series and would be picked sooner.

Quoted 7 lines
Quoted 1 line

Apart from that...

Sorry to make you confuse. After having discussion internally, we'll submit another complete mt8192.dtsi patch including this svs node. Therefore, I'll drop this path from svs patchset in order not to make the reviewer confuse. Thanks for the comments a lot.

Yes that decision looks good to me, as this allows different maintainers to apply patches on a per-subsystem basis.
Thanks!

Regards,
Angelo

author
#20 lore raw

PATCH v21 8/8 soc: mediatek: SVS: add mt8192 SVS GPU driver

In reply to Roger Lu

mt8192 SVS GPU uses 2-line (high/low bank) HW architecture to provide bank voltages. High bank helps update higher frequency's voltage and low bank helps update lower frequency's voltage.

---
 drivers/soc/mediatek/mtk-svs.c | 469 ++++++++++++++++++++++++++++++++-
 1 file changed, 464 insertions(+), 5 deletions(-)
drivers/soc/mediatek/mtk-svs.c +464 -5
index 93cdaecadd6d..bec6524ab33a 100644
@@ -24,6 +24,7 @@
 #include <linux/pm_qos.h>
 #include <linux/pm_runtime.h>
 #include <linux/regulator/consumer.h>
+#include <linux/reset.h>
 #include <linux/seq_file.h>
 #include <linux/slab.h>
 #include <linux/spinlock.h>
@@ -35,6 +36,10 @@
 #define SVSB_CCI			BIT(2)
 #define SVSB_GPU			BIT(3)
 
+/* svs bank 2-line type */
+#define SVSB_LOW			BIT(8)
+#define SVSB_HIGH			BIT(9)
+
 /* svs bank mode support */
 #define SVSB_MODE_ALL_DISABLE		0
 #define SVSB_MODE_INIT01		BIT(1)
@@ -45,6 +50,8 @@
 #define SVSB_INIT01_PD_REQ		BIT(0)
 #define SVSB_INIT01_VOLT_IGNORE		BIT(1)
 #define SVSB_INIT01_VOLT_INC_ONLY	BIT(2)
+#define SVSB_MON_VOLT_IGNORE		BIT(16)
+#define SVSB_REMOVE_DVTFIXED_VOLT	BIT(24)
 
 /* svs bank common setting */
 #define MAX_OPP_ENTRIES			16
@@ -249,6 +256,7 @@ static const u32 svs_regs_v2[] = {
  * @main_clk: main clock for svs bank
  * @pbank: svs bank pointer protected by spin_lock section
  * @banks: svs banks that svs platform supports
+ * @rst: svs platform reset control
  * @efuse_parsing: svs platform efuse parsing function pointer
  * @probe: svs platform probe function pointer
  * @irqflags: svs platform irq settings flags
@@ -266,6 +274,7 @@ struct svs_platform {
 	struct clk *main_clk;
 	struct svs_bank *pbank;
 	struct svs_bank *banks;
+	struct reset_control *rst;
 	bool (*efuse_parsing)(struct svs_platform *svsp);
 	int (*probe)(struct svs_platform *svsp);
 	unsigned long irqflags;
@@ -298,6 +307,7 @@ struct svs_platform {
  * @pm_runtime_enabled_count: bank pm runtime enabled count
  * @mode_support: bank mode support.
  * @freq_base: reference frequency for bank init
+ * @turn_freq_base: refenrece frequency for turn point
  * @vboot: voltage request for bank init01 only
  * @opp_dfreq: default opp frequency table
  * @opp_dvolt: default opp voltage table
@@ -333,6 +343,8 @@ struct svs_platform {
  * @mtdes: svs efuse data
  * @dcbdet: svs efuse data
  * @dcmdet: svs efuse data
+ * @turn_pt: turn point informs which opp_volt calculated by high/low bank.
+ * @type: bank type to represent it is 2-line (high/low) bank or 1-line bank.
  *
  * Svs bank will generate suitalbe voltages by below general math equation
  * and provide these voltages to opp voltage table.
@@ -359,6 +371,7 @@ struct svs_bank {
 	u32 pm_runtime_enabled_count;
 	u32 mode_support;
 	u32 freq_base;
+	u32 turn_freq_base;
 	u32 vboot;
 	u32 opp_dfreq[MAX_OPP_ENTRIES];
 	u32 opp_dvolt[MAX_OPP_ENTRIES];
@@ -394,6 +407,8 @@ struct svs_bank {
 	u32 mtdes;
 	u32 dcbdet;
 	u32 dcmdet;
+	u32 turn_pt;
+	u32 type;
 };
 
 static u32 percent(u32 numerator, u32 denominator)
@@ -429,6 +444,37 @@ static u32 svs_bank_volt_to_opp_volt(u32 svsb_volt, u32 svsb_volt_step,
 	return (svsb_volt * svsb_volt_step) + svsb_volt_base;
 }
 
+static u32 svs_opp_volt_to_bank_volt(u32 opp_u_volt, u32 svsb_volt_step,
+				     u32 svsb_volt_base)
+{
+	return (opp_u_volt - svsb_volt_base) / svsb_volt_step;
+}
+
+static int svs_sync_bank_volts_from_opp(struct svs_bank *svsb)
+{
+	struct dev_pm_opp *opp;
+	u32 i, opp_u_volt;
+
+	for (i = 0; i < svsb->opp_count; i++) {
+		opp = dev_pm_opp_find_freq_exact(svsb->opp_dev,
+						 svsb->opp_dfreq[i],
+						 true);
+		if (IS_ERR(opp)) {
+			dev_err(svsb->dev, "cannot find freq = %u (%ld)\n",
+				svsb->opp_dfreq[i], PTR_ERR(opp));
+			return PTR_ERR(opp);
+		}
+
+		opp_u_volt = dev_pm_opp_get_voltage(opp);
+		svsb->volt[i] = svs_opp_volt_to_bank_volt(opp_u_volt,
+							  svsb->volt_step,
+							  svsb->volt_base);
+		dev_pm_opp_put(opp);
+	}
+
+	return 0;
+}
+
 static int svs_get_zone_temperature(const char *tzone_name, int *tzone_temp)
 {
 	struct thermal_zone_device *tzd;
@@ -443,7 +489,7 @@ static int svs_get_zone_temperature(const char *tzone_name, int *tzone_temp)
 static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 {
 	int ret = -EPERM, tzone_temp = 0;
-	u32 i, svsb_volt, opp_volt, temp_voffset = 0;
+	u32 i, svsb_volt, opp_volt, temp_voffset = 0, opp_start, opp_stop;
 
 	mutex_lock(&svsb->lock);
 
@@ -457,6 +503,21 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 		goto unlock_mutex;
 	}
 
+	/*
+	 * 2-line bank updates its corresponding opp volts.
+	 * 1-line bank updates all opp volts.
+	 */
+	if (svsb->type == SVSB_HIGH) {
+		opp_start = 0;
+		opp_stop = svsb->turn_pt;
+	} else if (svsb->type == SVSB_LOW) {
+		opp_start = svsb->turn_pt;
+		opp_stop = svsb->opp_count;
+	} else {
+		opp_start = 0;
+		opp_stop = svsb->opp_count;
+	}
+
 	/* Get thermal effect */
 	if (svsb->phase == SVSB_PHASE_MON) {
 		ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
@@ -471,10 +532,16 @@ static int svs_adjust_pm_opp_volts(struct svs_bank *svsb, bool force_update)
 			temp_voffset += svsb->tzone_htemp_voffset;
 		else if (tzone_temp <= svsb->tzone_ltemp)
 			temp_voffset += svsb->tzone_ltemp_voffset;
+
+		/* 2-line bank update all opp volts when running mode */
+		if (svsb->type == SVSB_HIGH || svsb->type == SVSB_LOW) {
+			opp_start = 0;
+			opp_stop = svsb->opp_count;
+		}
 	}
 
 	/* vmin <= svsb_volt (opp_volt) <= default opp voltage */
-	for (i = 0; i < svsb->opp_count; i++) {
+	for (i = opp_start; i < opp_stop; i++) {
 		switch (svsb->phase) {
 		case SVSB_PHASE_ERROR:
 			opp_volt = svsb->opp_dvolt[i];
@@ -639,9 +706,11 @@ static int svs_status_debug_show(struct seq_file *m, void *v)
 
 	ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
 	if (ret)
-		seq_printf(m, "%s: temperature ignore\n", svsb->name);
+		seq_printf(m, "%s: temperature ignore, turn_pt = %u\n",
+			   svsb->name, svsb->turn_pt);
 	else
-		seq_printf(m, "%s: temperature = %d\n", svsb->name, tzone_temp);
+		seq_printf(m, "%s: temperature = %d, turn_pt = %u\n",
+			   svsb->name, tzone_temp, svsb->turn_pt);
 
 	for (i = 0; i < svsb->opp_count; i++) {
 		opp = dev_pm_opp_find_freq_exact(svsb->opp_dev,
@@ -784,6 +853,181 @@ static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx)
 	return DIV_ROUND_UP(vx, 100);
 }
 
+static void svs_get_bank_volts_v3(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	u32 i, j, *vop, vop74, vop30, mask7_0 = GENMASK(7, 0);
+	u32 b_sft, bits8 = 8, shift_byte = 0, reg_bytes = 4;
+	u32 middle_index = (svsb->opp_count / 2);
+	u32 opp_start = 0, opp_stop = 0, turn_pt = svsb->turn_pt;
+
+	if (svsb->phase == SVSB_PHASE_MON &&
+	    svsb->volt_flags & SVSB_MON_VOLT_IGNORE)
+		return;
+
+	vop74 = svs_readl_relaxed(svsp, VOP74);
+	vop30 = svs_readl_relaxed(svsp, VOP30);
+
+	/* Target is to set svsb->volt[] by algorithm */
+	if (turn_pt < middle_index) {
+		if (svsb->type == SVSB_HIGH) {
+			/* volt[0] ~ volt[turn_pt - 1] */
+			for (i = 0; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/* volt[turn_pt] + volt[j] ~ volt[opp_count - 1] */
+			j = svsb->opp_count - 7;
+			svsb->volt[turn_pt] = vop30 & mask7_0;
+			shift_byte++;
+			for (i = j; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+
+			/* volt[turn_pt + 1] ~ volt[j - 1] by interpolate */
+			for (i = turn_pt + 1; i < j; i++)
+				svsb->volt[i] =
+					interpolate(svsb->freq_pct[turn_pt],
+						    svsb->freq_pct[j],
+						    svsb->volt[turn_pt],
+						    svsb->volt[j],
+						    svsb->freq_pct[i]);
+		}
+	} else {
+		if (svsb->type == SVSB_HIGH) {
+			/* volt[0] + volt[j] ~ volt[turn_pt - 1] */
+			j = turn_pt - 7;
+			svsb->volt[0] = vop30 & mask7_0;
+			shift_byte++;
+			for (i = j; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+
+			/* volt[1] ~ volt[j - 1] by interpolate */
+			for (i = 1; i < j; i++)
+				svsb->volt[i] =
+					interpolate(svsb->freq_pct[0],
+						    svsb->freq_pct[j],
+						    svsb->volt[0],
+						    svsb->volt[j],
+						    svsb->freq_pct[i]);
+		} else if (svsb->type == SVSB_LOW) {
+			/* volt[turn_pt] ~ volt[opp_count - 1] */
+			for (i = turn_pt; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+		}
+	}
+
+	if (svsb->type == SVSB_HIGH) {
+		opp_start = 0;
+		opp_stop = svsb->turn_pt;
+	} else if (svsb->type == SVSB_LOW) {
+		opp_start = svsb->turn_pt;
+		opp_stop = svsb->opp_count;
+	}
+
+	for (i = opp_start; i < opp_stop; i++)
+		if (svsb->volt_flags & SVSB_REMOVE_DVTFIXED_VOLT)
+			svsb->volt[i] -= svsb->dvt_fixed;
+}
+
+static void svs_set_bank_freq_pct_v3(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	u32 i, j, *freq_pct, freq_pct74 = 0, freq_pct30 = 0;
+	u32 b_sft, bits8 = 8, shift_byte = 0, reg_bytes = 4;
+	u32 middle_index = (svsb->opp_count / 2);
+	u32 turn_pt = middle_index;
+
+	for (i = 0; i < svsb->opp_count; i++) {
+		if (svsb->opp_dfreq[i] <= svsb->turn_freq_base) {
+			svsb->turn_pt = i;
+			break;
+		}
+	}
+
+	turn_pt = svsb->turn_pt;
+
+	/* Target is to fill out freq_pct74 / freq_pct30 by algorithm */
+	if (turn_pt < middle_index) {
+		if (svsb->type == SVSB_HIGH) {
+			/* Edge case for preventing freq_pct30 from being 0 */
+			if (turn_pt == 0)
+				freq_pct30 = svsb->freq_pct[0];
+
+			/* freq_pct[0] ~ freq_pct[turn_pt - 1] */
+			for (i = 0; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/*
+			 * freq_pct[turn_pt] +
+			 * freq_pct[opp_count - 7] ~ freq_pct[opp_count -1]
+			 */
+			freq_pct30 = svsb->freq_pct[turn_pt];
+			shift_byte++;
+			j = svsb->opp_count - 7;
+			for (i = j; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		}
+	} else {
+		if (svsb->type == SVSB_HIGH) {
+			/*
+			 * freq_pct[0] +
+			 * freq_pct[turn_pt - 7] ~ freq_pct[turn_pt - 1]
+			 */
+			freq_pct30 = svsb->freq_pct[0];
+			shift_byte++;
+			j = turn_pt - 7;
+			for (i = j; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/* freq_pct[turn_pt] ~ freq_pct[opp_count - 1] */
+			for (i = turn_pt; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		}
+	}
+
+	svs_writel_relaxed(svsp, freq_pct74, FREQPCT74);
+	svs_writel_relaxed(svsp, freq_pct30, FREQPCT30);
+}
+
 static void svs_get_bank_volts_v2(struct svs_platform *svsp)
 {
 	struct svs_bank *svsb = svsp->pbank;
@@ -1247,6 +1491,25 @@ static int svs_init02(struct svs_platform *svsp)
 		}
 	}
 
+	/*
+	 * 2-line high/low bank update its corresponding opp voltages only.
+	 * Therefore, we sync voltages from opp for high/low bank voltages
+	 * consistency.
+	 */
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+
+		if (!(svsb->mode_support & SVSB_MODE_INIT02))
+			continue;
+
+		if (svsb->type == SVSB_HIGH || svsb->type == SVSB_LOW) {
+			if (svs_sync_bank_volts_from_opp(svsb)) {
+				dev_err(svsb->dev, "sync volt fail\n");
+				return -EPERM;
+			}
+		}
+	}
+
 	return 0;
 }
 
@@ -1291,6 +1554,7 @@ static int svs_suspend(struct device *dev)
 	struct svs_platform *svsp = dev_get_drvdata(dev);
 	struct svs_bank *svsb;
 	unsigned long flags;
+	int ret;
 	u32 idx;
 
 	for (idx = 0; idx < svsp->bank_max; idx++) {
@@ -1309,6 +1573,12 @@ static int svs_suspend(struct device *dev)
 		svs_adjust_pm_opp_volts(svsb, true);
 	}
 
+	ret = reset_control_assert(svsp->rst);
+	if (ret) {
+		dev_err(svsp->dev, "cannot assert reset %d\n", ret);
+		return ret;
+	}
+
 	clk_disable_unprepare(svsp->main_clk);
 
 	return 0;
@@ -1327,6 +1597,12 @@ static int svs_resume(struct device *dev)
 		return ret;
 	}
 
+	ret = reset_control_deassert(svsp->rst);
+	if (ret) {
+		dev_err(svsp->dev, "cannot deassert reset %d\n", ret);
+		return ret;
+	}
+
 	for (idx = 0; idx < svsp->bank_max; idx++) {
 		svsb = &svsp->banks[idx];
 		svsb->suspended = false;
@@ -1365,7 +1641,12 @@ static int svs_bank_resource_setup(struct svs_platform *svsp)
 			svsb->name = "SVSB_CCI";
 			break;
 		case SVSB_GPU:
-			svsb->name = "SVSB_GPU";
+			if (svsb->type == SVSB_HIGH)
+				svsb->name = "SVSB_GPU_HIGH";
+			else if (svsb->type == SVSB_LOW)
+				svsb->name = "SVSB_GPU_LOW";
+			else
+				svsb->name = "SVSB_GPU";
 			break;
 		default:
 			dev_err(svsb->dev, "unknown sw_id: %u\n", svsb->sw_id);
@@ -1419,6 +1700,84 @@ static int svs_bank_resource_setup(struct svs_platform *svsp)
 	return 0;
 }
 
+static bool svs_mt8192_efuse_parsing(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb;
+	struct nvmem_cell *cell;
+	u32 idx, i, vmin, golden_temp;
+
+	for (i = 0; i < svsp->efuse_max; i++)
+		if (svsp->efuse[i])
+			dev_info(svsp->dev, "M_HW_RES%d: 0x%08x\n",
+				 i, svsp->efuse[i]);
+
+	if (!svsp->efuse[9]) {
+		dev_notice(svsp->dev, "svs_efuse[9] = 0x0?\n");
+		return false;
+	}
+
+	/* Svs efuse parsing */
+	vmin = (svsp->efuse[19] >> 4) & GENMASK(1, 0);
+
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+
+		if (vmin == 0x1)
+			svsb->vmin = 0x1e;
+
+		if (svsb->type == SVSB_LOW) {
+			svsb->mtdes = svsp->efuse[10] & GENMASK(7, 0);
+			svsb->bdes = (svsp->efuse[10] >> 16) & GENMASK(7, 0);
+			svsb->mdes = (svsp->efuse[10] >> 24) & GENMASK(7, 0);
+			svsb->dcbdet = (svsp->efuse[17]) & GENMASK(7, 0);
+			svsb->dcmdet = (svsp->efuse[17] >> 8) & GENMASK(7, 0);
+		} else if (svsb->type == SVSB_HIGH) {
+			svsb->mtdes = svsp->efuse[9] & GENMASK(7, 0);
+			svsb->bdes = (svsp->efuse[9] >> 16) & GENMASK(7, 0);
+			svsb->mdes = (svsp->efuse[9] >> 24) & GENMASK(7, 0);
+			svsb->dcbdet = (svsp->efuse[17] >> 16) & GENMASK(7, 0);
+			svsb->dcmdet = (svsp->efuse[17] >> 24) & GENMASK(7, 0);
+		}
+
+		svsb->vmax += svsb->dvt_fixed;
+	}
+
+	/* Thermal efuse parsing */
+	cell = nvmem_cell_get(svsp->dev, "t-calibration-data");
+	if (IS_ERR_OR_NULL(cell)) {
+		dev_err(svsp->dev, "no \"t-calibration-data\"? %ld\n",
+			PTR_ERR(cell));
+		return false;
+	}
+
+	svsp->tefuse = nvmem_cell_read(cell, &svsp->tefuse_max);
+	if (IS_ERR(svsp->tefuse)) {
+		dev_err(svsp->dev, "cannot read thermal efuse: %ld\n",
+			PTR_ERR(svsp->tefuse));
+		return false;
+	}
+
+	svsp->tefuse_max /= sizeof(u32);
+	nvmem_cell_put(cell);
+
+	for (i = 0; i < svsp->tefuse_max; i++)
+		if (svsp->tefuse[i] != 0)
+			break;
+
+	if (i == svsp->tefuse_max)
+		golden_temp = 50; /* All thermal efuse data are 0 */
+	else
+		golden_temp = (svsp->tefuse[0] >> 24) & GENMASK(7, 0);
+
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+		svsb->mts = 500;
+		svsb->bts = (((500 * golden_temp + 250460) / 1000) - 25) * 4;
+	}
+
+	return true;
+}
+
 static bool svs_mt8183_efuse_parsing(struct svs_platform *svsp)
 {
 	struct svs_bank *svsb;
@@ -1702,6 +2061,38 @@ static struct device *svs_add_device_link(struct svs_platform *svsp,
 	return dev;
 }
 
+static int svs_mt8192_platform_probe(struct svs_platform *svsp)
+{
+	struct device *dev;
+	struct svs_bank *svsb;
+	u32 idx;
+
+	svsp->rst = devm_reset_control_get_optional(svsp->dev, "svs_rst");
+	if (IS_ERR(svsp->rst)) {
+		dev_err_probe(svsp->dev, PTR_ERR(svsp->rst),
+			      "cannot get svs reset control\n");
+		return PTR_ERR(svsp->rst);
+	}
+
+	dev = svs_add_device_link(svsp, "lvts");
+	if (IS_ERR(dev))
+		return PTR_ERR(dev);
+
+	for (idx = 0; idx < svsp->bank_max; idx++) {
+		svsb = &svsp->banks[idx];
+
+		if (svsb->type == SVSB_HIGH)
+			svsb->opp_dev = svs_add_device_link(svsp, "mali");
+		else if (svsb->type == SVSB_LOW)
+			svsb->opp_dev = svs_get_subsys_device(svsp, "mali");
+
+		if (IS_ERR(svsb->opp_dev))
+			return PTR_ERR(svsb->opp_dev);
+	}
+
+	return 0;
+}
+
 static int svs_mt8183_platform_probe(struct svs_platform *svsp)
 {
 	struct device *dev;
@@ -1742,6 +2133,61 @@ static int svs_mt8183_platform_probe(struct svs_platform *svsp)
 	return 0;
 }
 
+static struct svs_bank svs_mt8192_banks[] = {
+	{
+		.sw_id			= SVSB_GPU,
+		.type			= SVSB_LOW,
+		.set_freq_pct		= svs_set_bank_freq_pct_v3,
+		.get_volts		= svs_get_bank_volts_v3,
+		.volt_flags		= SVSB_REMOVE_DVTFIXED_VOLT,
+		.mode_support		= SVSB_MODE_INIT02,
+		.opp_count		= MAX_OPP_ENTRIES,
+		.freq_base		= 688000000,
+		.turn_freq_base		= 688000000,
+		.volt_step		= 6250,
+		.volt_base		= 400000,
+		.vmax			= 0x60,
+		.vmin			= 0x1a,
+		.age_config		= 0x555555,
+		.dc_config		= 0x1,
+		.dvt_fixed		= 0x1,
+		.vco			= 0x18,
+		.chk_shift		= 0x87,
+		.core_sel		= 0x0fff0100,
+		.int_st			= BIT(0),
+		.ctl0			= 0x00540003,
+	},
+	{
+		.sw_id			= SVSB_GPU,
+		.type			= SVSB_HIGH,
+		.set_freq_pct		= svs_set_bank_freq_pct_v3,
+		.get_volts		= svs_get_bank_volts_v3,
+		.tzone_name		= "gpu1",
+		.volt_flags		= SVSB_REMOVE_DVTFIXED_VOLT |
+					  SVSB_MON_VOLT_IGNORE,
+		.mode_support		= SVSB_MODE_INIT02 | SVSB_MODE_MON,
+		.opp_count		= MAX_OPP_ENTRIES,
+		.freq_base		= 902000000,
+		.turn_freq_base		= 688000000,
+		.volt_step		= 6250,
+		.volt_base		= 400000,
+		.vmax			= 0x60,
+		.vmin			= 0x1a,
+		.age_config		= 0x555555,
+		.dc_config		= 0x1,
+		.dvt_fixed		= 0x6,
+		.vco			= 0x18,
+		.chk_shift		= 0x87,
+		.core_sel		= 0x0fff0101,
+		.int_st			= BIT(1),
+		.ctl0			= 0x00540003,
+		.tzone_htemp		= 85000,
+		.tzone_htemp_voffset	= 0,
+		.tzone_ltemp		= 25000,
+		.tzone_ltemp_voffset	= 7,
+	},
+};
+
 static struct svs_bank svs_mt8183_banks[] = {
 	{
 		.sw_id			= SVSB_CPU_LITTLE,
@@ -1856,6 +2302,16 @@ struct svs_platform_data {
 	int (*probe)(struct svs_platform *svsp);
 };
 
+static const struct svs_platform_data svs_mt8192_platform_data = {
+	.name = "mt8192-svs",
+	.banks = svs_mt8192_banks,
+	.efuse_parsing = svs_mt8192_efuse_parsing,
+	.irqflags = IRQF_TRIGGER_HIGH,
+	.regs = svs_regs_v2,
+	.bank_max = ARRAY_SIZE(svs_mt8192_banks),
+	.probe = svs_mt8192_platform_probe,
+};
+
 static const struct svs_platform_data svs_mt8183_platform_data = {
 	.name = "mt8183-svs",
 	.banks = svs_mt8183_banks,
@@ -1868,6 +2324,9 @@ static const struct svs_platform_data svs_mt8183_platform_data = {
 
 static const struct of_device_id svs_of_match[] = {
 	{
+		.compatible = "mediatek,mt8192-svs",
+		.data = &svs_mt8192_platform_data,
+	}, {
 		.compatible = "mediatek,mt8183-svs",
 		.data = &svs_mt8183_platform_data,
 	}, {
Signature · 1 line
2.18.0
#21 lore raw

Re: [PATCH v21 8/8] soc: mediatek: SVS: add mt8192 SVS GPU driver

Il 07/01/22 10:52, Roger Lu ha scritto:

Quoted 13 lines

mt8192 SVS GPU uses 2-line (high/low bank) HW architecture to provide bank voltages. High bank helps update higher frequency's voltage and low bank helps update lower frequency's voltage.

---
  drivers/soc/mediatek/mtk-svs.c | 469 ++++++++++++++++++++++++++++++++-
  1 file changed, 464 insertions(+), 5 deletions(-)
drivers/soc/mediatek/mtk-svs.c +0 -0
index 93cdaecadd6d..bec6524ab33a 100644

..snip..

Quoted 15 lines
drivers/soc/mediatek/mtk-svs.c +4 -2
@@ -639,9 +706,11 @@ static int svs_status_debug_show(struct seq_file *m, void *v)
 
  	ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
  	if (ret)
-		seq_printf(m, "%s: temperature ignore\n", svsb->name);
+		seq_printf(m, "%s: temperature ignore, turn_pt = %u\n",
+			   svsb->name, svsb->turn_pt);
  	else
-		seq_printf(m, "%s: temperature = %d\n", svsb->name, tzone_temp);
+		seq_printf(m, "%s: temperature = %d, turn_pt = %u\n",
+			   svsb->name, tzone_temp, svsb->turn_pt);
 
  	for (i = 0; i < svsb->opp_count; i++) {
  		opp = dev_pm_opp_find_freq_exact(svsb->opp_dev,

drivers/soc/mediatek/mtk-svs.c static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx) · line 860

Quoted 8 lines
drivers/soc/mediatek/mtk-svs.c +5 -0
@@ -784,6 +853,181 @@ static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, u32 fx)
  	return DIV_ROUND_UP(vx, 100);
  }
 
+static void svs_get_bank_volts_v3(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	u32 i, j, *vop, vop74, vop30, mask7_0 = GENMASK(7, 0);
+	u32 b_sft, bits8 = 8, shift_byte = 0, reg_bytes = 4;

mask7_0, bits8, reg_bytes are constants, and it's not right to declare constants as variables like you're doing here.

Please replace all usages of mask7_0 with either a definition or with the call to the GENMASK macro;
also, replace all usages of bits8 with a definition, or just 8; finally, either make reg_bytes a const u8 or a definition.

In my opinion, a #define would be preferred, since the exact same comments on the exact same values also apply to function svs_set_bank_freq_pct_v3().

After that fix, you'll get my R-b.

I feel like v22 will be golden :)

Regards,
- Angelo

Quoted 173 lines
drivers/soc/mediatek/mtk-svs.c +170 -0
+	u32 middle_index = (svsb->opp_count / 2);
+	u32 opp_start = 0, opp_stop = 0, turn_pt = svsb->turn_pt;
+
+	if (svsb->phase == SVSB_PHASE_MON &&
+	    svsb->volt_flags & SVSB_MON_VOLT_IGNORE)
+		return;
+
+	vop74 = svs_readl_relaxed(svsp, VOP74);
+	vop30 = svs_readl_relaxed(svsp, VOP30);
+
+	/* Target is to set svsb->volt[] by algorithm */
+	if (turn_pt < middle_index) {
+		if (svsb->type == SVSB_HIGH) {
+			/* volt[0] ~ volt[turn_pt - 1] */
+			for (i = 0; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/* volt[turn_pt] + volt[j] ~ volt[opp_count - 1] */
+			j = svsb->opp_count - 7;
+			svsb->volt[turn_pt] = vop30 & mask7_0;
+			shift_byte++;
+			for (i = j; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+
+			/* volt[turn_pt + 1] ~ volt[j - 1] by interpolate */
+			for (i = turn_pt + 1; i < j; i++)
+				svsb->volt[i] =
+					interpolate(svsb->freq_pct[turn_pt],
+						    svsb->freq_pct[j],
+						    svsb->volt[turn_pt],
+						    svsb->volt[j],
+						    svsb->freq_pct[i]);
+		}
+	} else {
+		if (svsb->type == SVSB_HIGH) {
+			/* volt[0] + volt[j] ~ volt[turn_pt - 1] */
+			j = turn_pt - 7;
+			svsb->volt[0] = vop30 & mask7_0;
+			shift_byte++;
+			for (i = j; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+
+			/* volt[1] ~ volt[j - 1] by interpolate */
+			for (i = 1; i < j; i++)
+				svsb->volt[i] =
+					interpolate(svsb->freq_pct[0],
+						    svsb->freq_pct[j],
+						    svsb->volt[0],
+						    svsb->volt[j],
+						    svsb->freq_pct[i]);
+		} else if (svsb->type == SVSB_LOW) {
+			/* volt[turn_pt] ~ volt[opp_count - 1] */
+			for (i = turn_pt; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+		}
+	}
+
+	if (svsb->type == SVSB_HIGH) {
+		opp_start = 0;
+		opp_stop = svsb->turn_pt;
+	} else if (svsb->type == SVSB_LOW) {
+		opp_start = svsb->turn_pt;
+		opp_stop = svsb->opp_count;
+	}
+
+	for (i = opp_start; i < opp_stop; i++)
+		if (svsb->volt_flags & SVSB_REMOVE_DVTFIXED_VOLT)
+			svsb->volt[i] -= svsb->dvt_fixed;
+}
+
+static void svs_set_bank_freq_pct_v3(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	u32 i, j, *freq_pct, freq_pct74 = 0, freq_pct30 = 0;
+	u32 b_sft, bits8 = 8, shift_byte = 0, reg_bytes = 4;
+	u32 middle_index = (svsb->opp_count / 2);
+	u32 turn_pt = middle_index;
+
+	for (i = 0; i < svsb->opp_count; i++) {
+		if (svsb->opp_dfreq[i] <= svsb->turn_freq_base) {
+			svsb->turn_pt = i;
+			break;
+		}
+	}
+
+	turn_pt = svsb->turn_pt;
+
+	/* Target is to fill out freq_pct74 / freq_pct30 by algorithm */
+	if (turn_pt < middle_index) {
+		if (svsb->type == SVSB_HIGH) {
+			/* Edge case for preventing freq_pct30 from being 0 */
+			if (turn_pt == 0)
+				freq_pct30 = svsb->freq_pct[0];
+
+			/* freq_pct[0] ~ freq_pct[turn_pt - 1] */
+			for (i = 0; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/*
+			 * freq_pct[turn_pt] +
+			 * freq_pct[opp_count - 7] ~ freq_pct[opp_count -1]
+			 */
+			freq_pct30 = svsb->freq_pct[turn_pt];
+			shift_byte++;
+			j = svsb->opp_count - 7;
+			for (i = j; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		}
+	} else {
+		if (svsb->type == SVSB_HIGH) {
+			/*
+			 * freq_pct[0] +
+			 * freq_pct[turn_pt - 7] ~ freq_pct[turn_pt - 1]
+			 */
+			freq_pct30 = svsb->freq_pct[0];
+			shift_byte++;
+			j = turn_pt - 7;
+			for (i = j; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/* freq_pct[turn_pt] ~ freq_pct[opp_count - 1] */
+			for (i = turn_pt; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		}
+	}
+
+	svs_writel_relaxed(svsp, freq_pct74, FREQPCT74);
+	svs_writel_relaxed(svsp, freq_pct30, FREQPCT30);
+}
+
  static void svs_get_bank_volts_v2(struct svs_platform *svsp)
  {
  	struct svs_bank *svsb = svsp->pbank;
author
#22 lore raw

Re: [PATCH v21 8/8] soc: mediatek: SVS: add mt8192 SVS GPU driver

Hi AngeloGioacchino,

Sorry for the late reply.

Quoted 54 lines · AngeloGioacchino Del Regno

Il 07/01/22 10:52, Roger Lu ha scritto:

Quoted 13 lines

mt8192 SVS GPU uses 2-line (high/low bank) HW architecture to provide bank voltages. High bank helps update higher frequency's voltage and low bank helps update lower frequency's voltage.

---
  drivers/soc/mediatek/mtk-svs.c | 469 ++++++++++++++++++++++++++++++++-
  1 file changed, 464 insertions(+), 5 deletions(-)
drivers/soc/mediatek/mtk-svs.c +0 -0
index 93cdaecadd6d..bec6524ab33a 100644

..snip..

Quoted 16 lines
drivers/soc/mediatek/mtk-svs.c +4 -2
@@ -639,9 +706,11 @@ static int svs_status_debug_show(struct seq_file *m,
 void *v)
 
  	ret = svs_get_zone_temperature(svsb->tzone_name, &tzone_temp);
  	if (ret)
-		seq_printf(m, "%s: temperature ignore\n", svsb->name);
+		seq_printf(m, "%s: temperature ignore, turn_pt = %u\n",
+			   svsb->name, svsb->turn_pt);
  	else
-		seq_printf(m, "%s: temperature = %d\n", svsb->name, tzone_temp);
+		seq_printf(m, "%s: temperature = %d, turn_pt = %u\n",
+			   svsb->name, tzone_temp, svsb->turn_pt);
 
  	for (i = 0; i < svsb->opp_count; i++) {

opp = dev_pm_opp_find_freq_exact(svsb->opp_dev,

drivers/soc/mediatek/mtk-svs.c static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1, · line 861

Quoted 9 lines
drivers/soc/mediatek/mtk-svs.c +5 -0
@@ -784,6 +853,181 @@ static u32 interpolate(u32 f0, u32 f1, u32 v0, u32 v1,
 u32 fx)
  	return DIV_ROUND_UP(vx, 100);
  }
 
+static void svs_get_bank_volts_v3(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	u32 i, j, *vop, vop74, vop30, mask7_0 = GENMASK(7, 0);
+	u32 b_sft, bits8 = 8, shift_byte = 0, reg_bytes = 4;

mask7_0, bits8, reg_bytes are constants, and it's not right to declare constants
as variables like you're doing here.

Please replace all usages of mask7_0 with either a definition or with the call
to the GENMASK macro;
also, replace all usages of bits8 with a definition, or just 8; finally, either make reg_bytes a const u8 or a definition.

Quoted 6 lines

In my opinion, a #define would be preferred, since the exact same comments on the exact same values also apply to function svs_set_bank_freq_pct_v3().

After that fix, you'll get my R-b.

I feel like v22 will be golden :)

No problem. I'll use GENMASK macro and replace bits8/reg_bytes with definition. Thanks for the tutorial a lot. :)

Quoted 175 lines

Regards,
- Angelo

Quoted 171 lines
drivers/soc/mediatek/mtk-svs.c +170 -0
+	u32 middle_index = (svsb->opp_count / 2);
+	u32 opp_start = 0, opp_stop = 0, turn_pt = svsb->turn_pt;
+
+	if (svsb->phase == SVSB_PHASE_MON &&
+	    svsb->volt_flags & SVSB_MON_VOLT_IGNORE)
+		return;
+
+	vop74 = svs_readl_relaxed(svsp, VOP74);
+	vop30 = svs_readl_relaxed(svsp, VOP30);
+
+	/* Target is to set svsb->volt[] by algorithm */
+	if (turn_pt < middle_index) {
+		if (svsb->type == SVSB_HIGH) {
+			/* volt[0] ~ volt[turn_pt - 1] */
+			for (i = 0; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/* volt[turn_pt] + volt[j] ~ volt[opp_count - 1] */
+			j = svsb->opp_count - 7;
+			svsb->volt[turn_pt] = vop30 & mask7_0;
+			shift_byte++;
+			for (i = j; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+
+			/* volt[turn_pt + 1] ~ volt[j - 1] by interpolate */
+			for (i = turn_pt + 1; i < j; i++)
+				svsb->volt[i] =
+					interpolate(svsb->freq_pct[turn_pt],
+						    svsb->freq_pct[j],
+						    svsb->volt[turn_pt],
+						    svsb->volt[j],
+						    svsb->freq_pct[i]);
+		}
+	} else {
+		if (svsb->type == SVSB_HIGH) {
+			/* volt[0] + volt[j] ~ volt[turn_pt - 1] */
+			j = turn_pt - 7;
+			svsb->volt[0] = vop30 & mask7_0;
+			shift_byte++;
+			for (i = j; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+
+			/* volt[1] ~ volt[j - 1] by interpolate */
+			for (i = 1; i < j; i++)
+				svsb->volt[i] =
+					interpolate(svsb->freq_pct[0],
+						    svsb->freq_pct[j],
+						    svsb->volt[0],
+						    svsb->volt[j],
+						    svsb->freq_pct[i]);
+		} else if (svsb->type == SVSB_LOW) {
+			/* volt[turn_pt] ~ volt[opp_count - 1] */
+			for (i = turn_pt; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				vop = (shift_byte < reg_bytes) ? &vop30 :
+								 &vop74;
+				svsb->volt[i] = (*vop >> b_sft) & mask7_0;
+				shift_byte++;
+			}
+		}
+	}
+
+	if (svsb->type == SVSB_HIGH) {
+		opp_start = 0;
+		opp_stop = svsb->turn_pt;
+	} else if (svsb->type == SVSB_LOW) {
+		opp_start = svsb->turn_pt;
+		opp_stop = svsb->opp_count;
+	}
+
+	for (i = opp_start; i < opp_stop; i++)
+		if (svsb->volt_flags & SVSB_REMOVE_DVTFIXED_VOLT)
+			svsb->volt[i] -= svsb->dvt_fixed;
+}
+
+static void svs_set_bank_freq_pct_v3(struct svs_platform *svsp)
+{
+	struct svs_bank *svsb = svsp->pbank;
+	u32 i, j, *freq_pct, freq_pct74 = 0, freq_pct30 = 0;
+	u32 b_sft, bits8 = 8, shift_byte = 0, reg_bytes = 4;
+	u32 middle_index = (svsb->opp_count / 2);
+	u32 turn_pt = middle_index;
+
+	for (i = 0; i < svsb->opp_count; i++) {
+		if (svsb->opp_dfreq[i] <= svsb->turn_freq_base) {
+			svsb->turn_pt = i;
+			break;
+		}
+	}
+
+	turn_pt = svsb->turn_pt;
+
+	/* Target is to fill out freq_pct74 / freq_pct30 by algorithm */
+	if (turn_pt < middle_index) {
+		if (svsb->type == SVSB_HIGH) {
+			/* Edge case for preventing freq_pct30 from being 0 */
+			if (turn_pt == 0)
+				freq_pct30 = svsb->freq_pct[0];
+
+			/* freq_pct[0] ~ freq_pct[turn_pt - 1] */
+			for (i = 0; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/*
+			 * freq_pct[turn_pt] +
+			 * freq_pct[opp_count - 7] ~ freq_pct[opp_count -1]
+			 */
+			freq_pct30 = svsb->freq_pct[turn_pt];
+			shift_byte++;
+			j = svsb->opp_count - 7;
+			for (i = j; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		}
+	} else {
+		if (svsb->type == SVSB_HIGH) {
+			/*
+			 * freq_pct[0] +
+			 * freq_pct[turn_pt - 7] ~ freq_pct[turn_pt - 1]
+			 */
+			freq_pct30 = svsb->freq_pct[0];
+			shift_byte++;
+			j = turn_pt - 7;
+			for (i = j; i < turn_pt; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		} else if (svsb->type == SVSB_LOW) {
+			/* freq_pct[turn_pt] ~ freq_pct[opp_count - 1] */
+			for (i = turn_pt; i < svsb->opp_count; i++) {
+				b_sft = bits8 * (shift_byte % reg_bytes);
+				freq_pct = (shift_byte < reg_bytes) ?
+					   &freq_pct30 : &freq_pct74;
+				*freq_pct |= (svsb->freq_pct[i] << b_sft);
+				shift_byte++;
+			}
+		}
+	}
+
+	svs_writel_relaxed(svsp, freq_pct74, FREQPCT74);
+	svs_writel_relaxed(svsp, freq_pct30, FREQPCT30);
+}
+
  static void svs_get_bank_volts_v2(struct svs_platform *svsp)
Quoted 9 lines
Quoted 2 lines
drivers/soc/mediatek/mtk-svs.c +0 -0
  {
  	struct svs_bank *svsb = svsp->pbank;

_______________________________________________
Linux-mediatek mailing list
[email protected]

https://urldefense.com/v3/__http://lists.infradead.org/mailman/listinfo/linux-mediatek__;!!CTRNKA9wMg0ARbw!2hHinI997WLSwDhGwMLxsnPzZMt8US0HmcDwqKvo2yvyiHuJ7AR4BSd7C_Z2haFK$

Quoted 1 line