2022-01-28 20:44:35

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 00/15] rk3399: Clean up and enable DDR DVFS

This series covers 2 primary tasks:

1) Resubmit prior work:

[RESEND PATCH v5 3/4] arm64: dts: rockchip: Enable dmc and dfi nodes on gru.
https://lore.kernel.org/lkml/[email protected]/
[RESEND PATCH v5 2/4] arm64: dts: rk3399: Add dfi and dmc nodes.
https://lore.kernel.org/lkml/[email protected]/

This series was partially merged a while back, but the remaining 2
patches were blocked mostly on stylistic grounds (alpha/numerical
ordering).

2) Integrate many updates, bugfixes, and clarifications that were done
by Rockchip and Google engineers when first launching this platform.
Many of these were not integrated in the earlier series (e.g., the OPPs
changed before production; earlier patchsets used pre-production
numbers).

Along the way, it seemed worthwhile to convert the binding docs to a
schema. Among other reasons, it actually helped catch several errors and
omissions in translation between downstream device trees and the version
that actually landed upstream.

See the patches for further details.

Regards,
Brian

Changes in v2:
- Fix yamllint issues
- Adapt to various review comments (use of *-hz, hyphens, node naming)
- Add a few new bugfixes
- Add some new properties (ported from downstream kernels) required for
stability
- Convert more properties from "cycles" to "nanoseconds"

Brian Norris (13):
dt-bindings: devfreq: rk3399_dmc: Convert to YAML
dt-bindings: devfreq: rk3399_dmc: Deprecate unused/redundant
properties
dt-bindings: devfreq: rk3399_dmc: Fix Hz units
dt-bindings: devfreq: rk3399_dmc: Specify idle params in nanoseconds
dt-bindings: devfreq: rk3399_dmc: Add more disable-freq properties
PM / devfreq: rk3399_dmc: Drop undocumented ondemand DT props
PM / devfreq: rk3399_dmc: Drop excess timing properties
PM / devfreq: rk3399_dmc: Use bitfield macro definitions for ODT_PD
PM / devfreq: rk3399_dmc: Support new disable-freq properties
PM / devfreq: rk3399_dmc: Support new *-ns properties
PM / devfreq: rk3399_dmc: Disable edev on remove()
PM / devfreq: rk3399_dmc: Use devm_pm_opp_of_add_table()
PM / devfreq: rk3399_dmc: Avoid static (reused) profile

Lin Huang (2):
arm64: dts: rk3399: Add dfi and dmc nodes
arm64: dts: rockchip: Enable dmc and dfi nodes on gru

.../bindings/devfreq/rk3399_dmc.txt | 212 ----------
.../bindings/devfreq/rk3399_dmc.yaml | 370 ++++++++++++++++++
.../dts/rockchip/rk3399-gru-chromebook.dtsi | 7 +
.../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 12 +
arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 28 ++
.../boot/dts/rockchip/rk3399-op1-opp.dtsi | 25 ++
arch/arm64/boot/dts/rockchip/rk3399.dtsi | 19 +
drivers/devfreq/rk3399_dmc.c | 299 +++++++-------
8 files changed, 595 insertions(+), 377 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
create mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml

--
2.35.0.rc0.227.g00780c9af4-goog


2022-01-28 20:44:35

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 01/15] dt-bindings: devfreq: rk3399_dmc: Convert to YAML

I want to add, deprecate, and bugfix some properties, as well as add the
first users. This is easier with a proper schema.

The transformation is mostly straightforward, plus a few notable tweaks:

* Renamed rockchip,dram_speed_bin to rockchip,ddr3_speed_bin. The
driver code and the example matched, but the description was
different. I went with the implementation.

* Drop upthreshold and downdifferential properties from the example.
These were undocumented (so, wouldn't pass validation), but were
representing software properties (governor tweaks). I drop them from
the driver in subsequent patches.

* Rename clock from pclk_ddr_mon to dmc_clk. The driver, DT example,
and all downstream users matched -- the binding definition was the
exception. Anyway, "dmc_clk" is a more appropriately generic name.

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
* rename to 'memory-controller' in example
* place 'required' after properties
* drop superluous free-form references and repetitions of other
bindings
* fix for yamllint

.../bindings/devfreq/rk3399_dmc.txt | 212 -------------
.../bindings/devfreq/rk3399_dmc.yaml | 293 ++++++++++++++++++
2 files changed, 293 insertions(+), 212 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
create mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
deleted file mode 100644
index 58fc8a6cebc7..000000000000
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
+++ /dev/null
@@ -1,212 +0,0 @@
-* Rockchip rk3399 DMC (Dynamic Memory Controller) device
-
-Required properties:
-- compatible: Must be "rockchip,rk3399-dmc".
-- devfreq-events: Node to get DDR loading, Refer to
- Documentation/devicetree/bindings/devfreq/event/
- rockchip-dfi.txt
-- clocks: Phandles for clock specified in "clock-names" property
-- clock-names : The name of clock used by the DFI, must be
- "pclk_ddr_mon";
-- operating-points-v2: Refer to Documentation/devicetree/bindings/opp/opp-v2.yaml
- for details.
-- center-supply: DMC supply node.
-- status: Marks the node enabled/disabled.
-- rockchip,pmu: Phandle to the syscon managing the "PMU general register
- files".
-
-Optional properties:
-- interrupts: The CPU interrupt number. The interrupt specifier
- format depends on the interrupt controller.
- It should be a DCF interrupt. When DDR DVFS finishes
- a DCF interrupt is triggered.
-- rockchip,pmu: Phandle to the syscon managing the "PMU general register
- files".
-
-Following properties relate to DDR timing:
-
-- rockchip,dram_speed_bin : Value reference include/dt-bindings/clock/rk3399-ddr.h,
- it selects the DDR3 cl-trp-trcd type. It must be
- set according to "Speed Bin" in DDR3 datasheet,
- DO NOT use a smaller "Speed Bin" than specified
- for the DDR3 being used.
-
-- rockchip,pd_idle : Configure the PD_IDLE value. Defines the
- power-down idle period in which memories are
- placed into power-down mode if bus is idle
- for PD_IDLE DFI clock cycles.
-
-- rockchip,sr_idle : Configure the SR_IDLE value. Defines the
- self-refresh idle period in which memories are
- placed into self-refresh mode if bus is idle
- for SR_IDLE * 1024 DFI clock cycles (DFI
- clocks freq is half of DRAM clock), default
- value is "0".
-
-- rockchip,sr_mc_gate_idle : Defines the memory self-refresh and controller
- clock gating idle period. Memories are placed
- into self-refresh mode and memory controller
- clock arg gating started if bus is idle for
- sr_mc_gate_idle*1024 DFI clock cycles.
-
-- rockchip,srpd_lite_idle : Defines the self-refresh power down idle
- period in which memories are placed into
- self-refresh power down mode if bus is idle
- for srpd_lite_idle * 1024 DFI clock cycles.
- This parameter is for LPDDR4 only.
-
-- rockchip,standby_idle : Defines the standby idle period in which
- memories are placed into self-refresh mode.
- The controller, pi, PHY and DRAM clock will
- be gated if bus is idle for standby_idle * DFI
- clock cycles.
-
-- rockchip,dram_dll_dis_freq : Defines the DDR3 DLL bypass frequency in MHz.
- When DDR frequency is less than DRAM_DLL_DISB_FREQ,
- DDR3 DLL will be bypassed. Note: if DLL was bypassed,
- the odt will also stop working.
-
-- rockchip,phy_dll_dis_freq : Defines the PHY dll bypass frequency in
- MHz (Mega Hz). When DDR frequency is less than
- DRAM_DLL_DISB_FREQ, PHY DLL will be bypassed.
- Note: PHY DLL and PHY ODT are independent.
-
-- rockchip,ddr3_odt_dis_freq : When the DRAM type is DDR3, this parameter defines
- the ODT disable frequency in MHz (Mega Hz).
- when the DDR frequency is less then ddr3_odt_dis_freq,
- the ODT on the DRAM side and controller side are
- both disabled.
-
-- rockchip,ddr3_drv : When the DRAM type is DDR3, this parameter defines
- the DRAM side driver strength in ohms. Default
- value is 40.
-
-- rockchip,ddr3_odt : When the DRAM type is DDR3, this parameter defines
- the DRAM side ODT strength in ohms. Default value
- is 120.
-
-- rockchip,phy_ddr3_ca_drv : When the DRAM type is DDR3, this parameter defines
- the phy side CA line (incluing command line,
- address line and clock line) driver strength.
- Default value is 40.
-
-- rockchip,phy_ddr3_dq_drv : When the DRAM type is DDR3, this parameter defines
- the PHY side DQ line (including DQS/DQ/DM line)
- driver strength. Default value is 40.
-
-- rockchip,phy_ddr3_odt : When the DRAM type is DDR3, this parameter defines
- the PHY side ODT strength. Default value is 240.
-
-- rockchip,lpddr3_odt_dis_freq : When the DRAM type is LPDDR3, this parameter defines
- then ODT disable frequency in MHz (Mega Hz).
- When DDR frequency is less then ddr3_odt_dis_freq,
- the ODT on the DRAM side and controller side are
- both disabled.
-
-- rockchip,lpddr3_drv : When the DRAM type is LPDDR3, this parameter defines
- the DRAM side driver strength in ohms. Default
- value is 34.
-
-- rockchip,lpddr3_odt : When the DRAM type is LPDDR3, this parameter defines
- the DRAM side ODT strength in ohms. Default value
- is 240.
-
-- rockchip,phy_lpddr3_ca_drv : When the DRAM type is LPDDR3, this parameter defines
- the PHY side CA line (including command line,
- address line and clock line) driver strength.
- Default value is 40.
-
-- rockchip,phy_lpddr3_dq_drv : When the DRAM type is LPDDR3, this parameter defines
- the PHY side DQ line (including DQS/DQ/DM line)
- driver strength. Default value is 40.
-
-- rockchip,phy_lpddr3_odt : When dram type is LPDDR3, this parameter define
- the phy side odt strength, default value is 240.
-
-- rockchip,lpddr4_odt_dis_freq : When the DRAM type is LPDDR4, this parameter
- defines the ODT disable frequency in
- MHz (Mega Hz). When the DDR frequency is less then
- ddr3_odt_dis_freq, the ODT on the DRAM side and
- controller side are both disabled.
-
-- rockchip,lpddr4_drv : When the DRAM type is LPDDR4, this parameter defines
- the DRAM side driver strength in ohms. Default
- value is 60.
-
-- rockchip,lpddr4_dq_odt : When the DRAM type is LPDDR4, this parameter defines
- the DRAM side ODT on DQS/DQ line strength in ohms.
- Default value is 40.
-
-- rockchip,lpddr4_ca_odt : When the DRAM type is LPDDR4, this parameter defines
- the DRAM side ODT on CA line strength in ohms.
- Default value is 40.
-
-- rockchip,phy_lpddr4_ca_drv : When the DRAM type is LPDDR4, this parameter defines
- the PHY side CA line (including command address
- line) driver strength. Default value is 40.
-
-- rockchip,phy_lpddr4_ck_cs_drv : When the DRAM type is LPDDR4, this parameter defines
- the PHY side clock line and CS line driver
- strength. Default value is 80.
-
-- rockchip,phy_lpddr4_dq_drv : When the DRAM type is LPDDR4, this parameter defines
- the PHY side DQ line (including DQS/DQ/DM line)
- driver strength. Default value is 80.
-
-- rockchip,phy_lpddr4_odt : When the DRAM type is LPDDR4, this parameter defines
- the PHY side ODT strength. Default value is 60.
-
-Example:
- dmc_opp_table: dmc_opp_table {
- compatible = "operating-points-v2";
-
- opp00 {
- opp-hz = /bits/ 64 <300000000>;
- opp-microvolt = <900000>;
- };
- opp01 {
- opp-hz = /bits/ 64 <666000000>;
- opp-microvolt = <900000>;
- };
- };
-
- dmc: dmc {
- compatible = "rockchip,rk3399-dmc";
- devfreq-events = <&dfi>;
- interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
- clocks = <&cru SCLK_DDRC>;
- clock-names = "dmc_clk";
- operating-points-v2 = <&dmc_opp_table>;
- center-supply = <&ppvar_centerlogic>;
- upthreshold = <15>;
- downdifferential = <10>;
- rockchip,ddr3_speed_bin = <21>;
- rockchip,pd_idle = <0x40>;
- rockchip,sr_idle = <0x2>;
- rockchip,sr_mc_gate_idle = <0x3>;
- rockchip,srpd_lite_idle = <0x4>;
- rockchip,standby_idle = <0x2000>;
- rockchip,dram_dll_dis_freq = <300>;
- rockchip,phy_dll_dis_freq = <125>;
- rockchip,auto_pd_dis_freq = <666>;
- rockchip,ddr3_odt_dis_freq = <333>;
- rockchip,ddr3_drv = <40>;
- rockchip,ddr3_odt = <120>;
- rockchip,phy_ddr3_ca_drv = <40>;
- rockchip,phy_ddr3_dq_drv = <40>;
- rockchip,phy_ddr3_odt = <240>;
- rockchip,lpddr3_odt_dis_freq = <333>;
- rockchip,lpddr3_drv = <34>;
- rockchip,lpddr3_odt = <240>;
- rockchip,phy_lpddr3_ca_drv = <40>;
- rockchip,phy_lpddr3_dq_drv = <40>;
- rockchip,phy_lpddr3_odt = <240>;
- rockchip,lpddr4_odt_dis_freq = <333>;
- rockchip,lpddr4_drv = <60>;
- rockchip,lpddr4_dq_odt = <40>;
- rockchip,lpddr4_ca_odt = <40>;
- rockchip,phy_lpddr4_ca_drv = <40>;
- rockchip,phy_lpddr4_ck_cs_drv = <80>;
- rockchip,phy_lpddr4_dq_drv = <80>;
- rockchip,phy_lpddr4_odt = <60>;
- };
diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
new file mode 100644
index 000000000000..467a7b5b374b
--- /dev/null
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
@@ -0,0 +1,293 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+# %YAML 1.2
+---
+$id: http://devicetree.org/schemas/devfreq/rk3399_dmc.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Rockchip rk3399 DMC (Dynamic Memory Controller) device
+
+maintainers:
+ - Brian Norris <[email protected]>
+
+properties:
+ compatible:
+ enum:
+ - rockchip,rk3399-dmc
+
+ devfreq-events:
+ $ref: /schemas/types.yaml#/definitions/phandle-array
+ minItems: 1
+ description:
+ Node to get DDR loading. Refer to
+ Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt.
+
+ clocks:
+ maxItems: 1
+
+ clock-names:
+ items:
+ - const: dmc_clk
+
+ operating-points-v2: true
+
+ center-supply:
+ description:
+ DMC regulator supply.
+
+ rockchip,pmu:
+ $ref: /schemas/types.yaml#/definitions/phandle
+ description:
+ Phandle to the syscon managing the "PMU general register files".
+
+ interrupts:
+ maxItems: 1
+ description:
+ The CPU interrupt number. It should be a DCF interrupt. When DDR DVFS
+ finishes, a DCF interrupt is triggered.
+
+ rockchip,ddr3_speed_bin:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ For values, reference include/dt-bindings/clock/rk3399-ddr.h. Selects the
+ DDR3 cl-trp-trcd type. It must be set according to "Speed Bin" in DDR3
+ datasheet; DO NOT use a smaller "Speed Bin" than specified for the DDR3
+ being used.
+
+ rockchip,pd_idle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Configure the PD_IDLE value. Defines the power-down idle period in which
+ memories are placed into power-down mode if bus is idle for PD_IDLE DFI
+ clock cycles.
+
+ rockchip,sr_idle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Configure the SR_IDLE value. Defines the self-refresh idle period in
+ which memories are placed into self-refresh mode if bus is idle for
+ SR_IDLE * 1024 DFI clock cycles (DFI clocks freq is half of DRAM clock).
+ Default value is "0".
+
+ rockchip,sr_mc_gate_idle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Defines the memory self-refresh and controller clock gating idle period.
+ Memories are placed into self-refresh mode and memory controller clock
+ arg gating started if bus is idle for sr_mc_gate_idle*1024 DFI clock
+ cycles.
+
+ rockchip,srpd_lite_idle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Defines the self-refresh power down idle period in which memories are
+ placed into self-refresh power down mode if bus is idle for
+ srpd_lite_idle * 1024 DFI clock cycles. This parameter is for LPDDR4
+ only.
+
+ rockchip,standby_idle:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Defines the standby idle period in which memories are placed into
+ self-refresh mode. The controller, pi, PHY and DRAM clock will be gated
+ if bus is idle for standby_idle * DFI clock cycles.
+
+ rockchip,dram_dll_dis_freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Defines the DDR3 DLL bypass frequency in MHz. When DDR frequency is less
+ than DRAM_DLL_DISB_FREQ, DDR3 DLL will be bypassed.
+ Note: if DLL was bypassed, the odt will also stop working.
+
+ rockchip,phy_dll_dis_freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description: |
+ Defines the PHY dll bypass frequency in MHz (Mega Hz). When DDR frequency
+ is less than DRAM_DLL_DISB_FREQ, PHY DLL will be bypassed.
+ Note: PHY DLL and PHY ODT are independent.
+
+ rockchip,auto_pd_dis_freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ Defines the auto PD disable frequency in MHz.
+
+ rockchip,ddr3_odt_dis_freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is DDR3, this parameter defines the ODT disable
+ frequency in MHz (Mega Hz). When the DDR frequency is less then
+ ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
+ disabled.
+
+ rockchip,ddr3_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is DDR3, this parameter defines the DRAM side drive
+ strength in ohms. Default value is 40.
+
+ rockchip,ddr3_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is DDR3, this parameter defines the DRAM side ODT
+ strength in ohms. Default value is 120.
+
+ rockchip,phy_ddr3_ca_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is DDR3, this parameter defines the phy side CA line
+ (incluing command line, address line and clock line) drive strength.
+ Default value is 40.
+
+ rockchip,phy_ddr3_dq_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is DDR3, this parameter defines the PHY side DQ line
+ (including DQS/DQ/DM line) drive strength. Default value is 40.
+
+ rockchip,phy_ddr3_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is DDR3, this parameter defines the PHY side ODT
+ strength. Default value is 240.
+
+ rockchip,lpddr3_odt_dis_freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR3, this parameter defines then ODT disable
+ frequency in MHz (Mega Hz). When DDR frequency is less then
+ ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
+ disabled.
+
+ rockchip,lpddr3_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR3, this parameter defines the DRAM side drive
+ strength in ohms. Default value is 34.
+
+ rockchip,lpddr3_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR3, this parameter defines the DRAM side ODT
+ strength in ohms. Default value is 240.
+
+ rockchip,phy_lpddr3_ca_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR3, this parameter defines the PHY side CA line
+ (including command line, address line and clock line) drive strength.
+ Default value is 40.
+
+ rockchip,phy_lpddr3_dq_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR3, this parameter defines the PHY side DQ line
+ (including DQS/DQ/DM line) drive strength. Default value is 40.
+
+ rockchip,phy_lpddr3_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When dram type is LPDDR3, this parameter define the phy side odt
+ strength, default value is 240.
+
+ rockchip,lpddr4_odt_dis_freq:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the ODT disable
+ frequency in MHz (Mega Hz). When the DDR frequency is less then
+ ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
+ disabled.
+
+ rockchip,lpddr4_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the DRAM side drive
+ strength in ohms. Default value is 60.
+
+ rockchip,lpddr4_dq_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the DRAM side ODT on
+ DQS/DQ line strength in ohms. Default value is 40.
+
+ rockchip,lpddr4_ca_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the DRAM side ODT on
+ CA line strength in ohms. Default value is 40.
+
+ rockchip,phy_lpddr4_ca_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the PHY side CA line
+ (including command address line) drive strength. Default value is 40.
+
+ rockchip,phy_lpddr4_ck_cs_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the PHY side clock
+ line and CS line drive strength. Default value is 80.
+
+ rockchip,phy_lpddr4_dq_drv:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the PHY side DQ line
+ (including DQS/DQ/DM line) drive strength. Default value is 80.
+
+ rockchip,phy_lpddr4_odt:
+ $ref: /schemas/types.yaml#/definitions/uint32
+ description:
+ When the DRAM type is LPDDR4, this parameter defines the PHY side ODT
+ strength. Default value is 60.
+
+required:
+ - compatible
+ - devfreq-events
+ - clocks
+ - clock-names
+ - operating-points-v2
+ - center-supply
+
+additionalProperties: false
+
+examples:
+ - |
+ #include <dt-bindings/clock/rk3399-cru.h>
+ #include <dt-bindings/interrupt-controller/arm-gic.h>
+ memory-controller {
+ compatible = "rockchip,rk3399-dmc";
+ devfreq-events = <&dfi>;
+ rockchip,pmu = <&pmu>;
+ interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
+ clocks = <&cru SCLK_DDRC>;
+ clock-names = "dmc_clk";
+ operating-points-v2 = <&dmc_opp_table>;
+ center-supply = <&ppvar_centerlogic>;
+ rockchip,ddr3_speed_bin = <21>;
+ rockchip,pd_idle = <0x40>;
+ rockchip,sr_idle = <0x2>;
+ rockchip,sr_mc_gate_idle = <0x3>;
+ rockchip,srpd_lite_idle = <0x4>;
+ rockchip,standby_idle = <0x2000>;
+ rockchip,dram_dll_dis_freq = <300>;
+ rockchip,phy_dll_dis_freq = <125>;
+ rockchip,auto_pd_dis_freq = <666>;
+ rockchip,ddr3_odt_dis_freq = <333>;
+ rockchip,ddr3_drv = <40>;
+ rockchip,ddr3_odt = <120>;
+ rockchip,phy_ddr3_ca_drv = <40>;
+ rockchip,phy_ddr3_dq_drv = <40>;
+ rockchip,phy_ddr3_odt = <240>;
+ rockchip,lpddr3_odt_dis_freq = <333>;
+ rockchip,lpddr3_drv = <34>;
+ rockchip,lpddr3_odt = <240>;
+ rockchip,phy_lpddr3_ca_drv = <40>;
+ rockchip,phy_lpddr3_dq_drv = <40>;
+ rockchip,phy_lpddr3_odt = <240>;
+ rockchip,lpddr4_odt_dis_freq = <333>;
+ rockchip,lpddr4_drv = <60>;
+ rockchip,lpddr4_dq_odt = <40>;
+ rockchip,lpddr4_ca_odt = <40>;
+ rockchip,phy_lpddr4_ca_drv = <40>;
+ rockchip,phy_lpddr4_ck_cs_drv = <80>;
+ rockchip,phy_lpddr4_dq_drv = <80>;
+ rockchip,phy_lpddr4_odt = <60>;
+ };
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 20:45:04

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 02/15] dt-bindings: devfreq: rk3399_dmc: Deprecate unused/redundant properties

These DRAM configuration properties are all handled in ARM Trusted
Firmware (and have been since the early days of this SoC), and there are
no in-tree users of the DMC binding yet. It's better to just defer to
firmware instead of maintaining this large list of properties.

There's also some confusion about units: many of these are specified in
MHz, but the downstream users and driver code are treating them as Hz, I
believe. Rather than straighten all that out, I just drop them.

Signed-off-by: Brian Norris <[email protected]>
---

(no changes since v1)

.../bindings/devfreq/rk3399_dmc.yaml | 42 +++++++++----------
1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
index 467a7b5b374b..fd62a8cd62d5 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
@@ -46,6 +46,7 @@ properties:
finishes, a DCF interrupt is triggered.

rockchip,ddr3_speed_bin:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
For values, reference include/dt-bindings/clock/rk3399-ddr.h. Selects the
@@ -92,6 +93,7 @@ properties:
if bus is idle for standby_idle * DFI clock cycles.

rockchip,dram_dll_dis_freq:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description: |
Defines the DDR3 DLL bypass frequency in MHz. When DDR frequency is less
@@ -99,6 +101,7 @@ properties:
Note: if DLL was bypassed, the odt will also stop working.

rockchip,phy_dll_dis_freq:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description: |
Defines the PHY dll bypass frequency in MHz (Mega Hz). When DDR frequency
@@ -106,6 +109,7 @@ properties:
Note: PHY DLL and PHY ODT are independent.

rockchip,auto_pd_dis_freq:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
Defines the auto PD disable frequency in MHz.
@@ -119,18 +123,21 @@ properties:
disabled.

rockchip,ddr3_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is DDR3, this parameter defines the DRAM side drive
strength in ohms. Default value is 40.

rockchip,ddr3_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is DDR3, this parameter defines the DRAM side ODT
strength in ohms. Default value is 120.

rockchip,phy_ddr3_ca_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is DDR3, this parameter defines the phy side CA line
@@ -138,12 +145,14 @@ properties:
Default value is 40.

rockchip,phy_ddr3_dq_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is DDR3, this parameter defines the PHY side DQ line
(including DQS/DQ/DM line) drive strength. Default value is 40.

rockchip,phy_ddr3_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is DDR3, this parameter defines the PHY side ODT
@@ -158,18 +167,21 @@ properties:
disabled.

rockchip,lpddr3_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR3, this parameter defines the DRAM side drive
strength in ohms. Default value is 34.

rockchip,lpddr3_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR3, this parameter defines the DRAM side ODT
strength in ohms. Default value is 240.

rockchip,phy_lpddr3_ca_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR3, this parameter defines the PHY side CA line
@@ -177,12 +189,14 @@ properties:
Default value is 40.

rockchip,phy_lpddr3_dq_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR3, this parameter defines the PHY side DQ line
(including DQS/DQ/DM line) drive strength. Default value is 40.

rockchip,phy_lpddr3_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When dram type is LPDDR3, this parameter define the phy side odt
@@ -197,42 +211,49 @@ properties:
disabled.

rockchip,lpddr4_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the DRAM side drive
strength in ohms. Default value is 60.

rockchip,lpddr4_dq_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the DRAM side ODT on
DQS/DQ line strength in ohms. Default value is 40.

rockchip,lpddr4_ca_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the DRAM side ODT on
CA line strength in ohms. Default value is 40.

rockchip,phy_lpddr4_ca_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the PHY side CA line
(including command address line) drive strength. Default value is 40.

rockchip,phy_lpddr4_ck_cs_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the PHY side clock
line and CS line drive strength. Default value is 80.

rockchip,phy_lpddr4_dq_drv:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the PHY side DQ line
(including DQS/DQ/DM line) drive strength. Default value is 80.

rockchip,phy_lpddr4_odt:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
When the DRAM type is LPDDR4, this parameter defines the PHY side ODT
@@ -261,33 +282,12 @@ examples:
clock-names = "dmc_clk";
operating-points-v2 = <&dmc_opp_table>;
center-supply = <&ppvar_centerlogic>;
- rockchip,ddr3_speed_bin = <21>;
rockchip,pd_idle = <0x40>;
rockchip,sr_idle = <0x2>;
rockchip,sr_mc_gate_idle = <0x3>;
rockchip,srpd_lite_idle = <0x4>;
rockchip,standby_idle = <0x2000>;
- rockchip,dram_dll_dis_freq = <300>;
- rockchip,phy_dll_dis_freq = <125>;
- rockchip,auto_pd_dis_freq = <666>;
rockchip,ddr3_odt_dis_freq = <333>;
- rockchip,ddr3_drv = <40>;
- rockchip,ddr3_odt = <120>;
- rockchip,phy_ddr3_ca_drv = <40>;
- rockchip,phy_ddr3_dq_drv = <40>;
- rockchip,phy_ddr3_odt = <240>;
rockchip,lpddr3_odt_dis_freq = <333>;
- rockchip,lpddr3_drv = <34>;
- rockchip,lpddr3_odt = <240>;
- rockchip,phy_lpddr3_ca_drv = <40>;
- rockchip,phy_lpddr3_dq_drv = <40>;
- rockchip,phy_lpddr3_odt = <240>;
rockchip,lpddr4_odt_dis_freq = <333>;
- rockchip,lpddr4_drv = <60>;
- rockchip,lpddr4_dq_odt = <40>;
- rockchip,lpddr4_ca_odt = <40>;
- rockchip,phy_lpddr4_ca_drv = <40>;
- rockchip,phy_lpddr4_ck_cs_drv = <80>;
- rockchip,phy_lpddr4_dq_drv = <80>;
- rockchip,phy_lpddr4_odt = <60>;
};
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 20:46:28

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 04/15] dt-bindings: devfreq: rk3399_dmc: Specify idle params in nanoseconds

It's inefficient to use the same number of cycles for all OPPs, since
lower frequencies make for longer idle times. Let's specify the idle
time instead, so software can pick the optimal number of cycles on its
own.

NB: these bindings aren't used anywhere yet.

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
- New patch

.../bindings/devfreq/rk3399_dmc.yaml | 50 +++++++++++++++++--
1 file changed, 45 insertions(+), 5 deletions(-)

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
index 8bb778df92ae..8786b7fa9b28 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
@@ -55,42 +55,52 @@ properties:
being used.

rockchip,pd_idle:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
Configure the PD_IDLE value. Defines the power-down idle period in which
memories are placed into power-down mode if bus is idle for PD_IDLE DFI
clock cycles.
+ See also rockchip,pd-idle-ns.

rockchip,sr_idle:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
Configure the SR_IDLE value. Defines the self-refresh idle period in
which memories are placed into self-refresh mode if bus is idle for
SR_IDLE * 1024 DFI clock cycles (DFI clocks freq is half of DRAM clock).
Default value is "0".
+ See also rockchip,sr-idle-ns.

rockchip,sr_mc_gate_idle:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
Defines the memory self-refresh and controller clock gating idle period.
Memories are placed into self-refresh mode and memory controller clock
arg gating started if bus is idle for sr_mc_gate_idle*1024 DFI clock
cycles.
+ See also rockchip,sr-mc-gate-idle-ns.

rockchip,srpd_lite_idle:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
Defines the self-refresh power down idle period in which memories are
placed into self-refresh power down mode if bus is idle for
srpd_lite_idle * 1024 DFI clock cycles. This parameter is for LPDDR4
only.
+ See also rockchip,srpd-lite-idle-ns.

rockchip,standby_idle:
+ deprecated: true
$ref: /schemas/types.yaml#/definitions/uint32
description:
Defines the standby idle period in which memories are placed into
self-refresh mode. The controller, pi, PHY and DRAM clock will be gated
if bus is idle for standby_idle * DFI clock cycles.
+ See also rockchip,standby-idle-ns.

rockchip,dram_dll_dis_freq:
deprecated: true
@@ -259,6 +269,36 @@ properties:
When the DRAM type is LPDDR4, this parameter defines the PHY side ODT
strength. Default value is 60.

+ rockchip,pd-idle-ns:
+ description:
+ Configure the PD_IDLE value in nanoseconds. Defines the power-down idle
+ period in which memories are placed into power-down mode if bus is idle
+ for PD_IDLE nanoseconds.
+
+ rockchip,sr-idle-ns:
+ description:
+ Configure the SR_IDLE value in nanoseconds. Defines the self-refresh idle
+ period in which memories are placed into self-refresh mode if bus is idle
+ for SR_IDLE nanoseconds. Default value is "0".
+
+ rockchip,sr-mc-gate-idle-ns:
+ description:
+ Defines the memory self-refresh and controller clock gating idle period in nanoseconds.
+ Memories are placed into self-refresh mode and memory controller clock
+ arg gating started if bus is idle for sr_mc_gate_idle nanoseconds.
+
+ rockchip,srpd-lite-idle-ns:
+ description:
+ Defines the self-refresh power down idle period in which memories are
+ placed into self-refresh power down mode if bus is idle for
+ srpd_lite_idle nanoseonds. This parameter is for LPDDR4 only.
+
+ rockchip,standby-idle-ns:
+ description:
+ Defines the standby idle period in which memories are placed into
+ self-refresh mode. The controller, pi, PHY and DRAM clock will be gated
+ if bus is idle for standby_idle nanoseconds.
+
required:
- compatible
- devfreq-events
@@ -282,11 +322,11 @@ examples:
clock-names = "dmc_clk";
operating-points-v2 = <&dmc_opp_table>;
center-supply = <&ppvar_centerlogic>;
- rockchip,pd_idle = <0x40>;
- rockchip,sr_idle = <0x2>;
- rockchip,sr_mc_gate_idle = <0x3>;
- rockchip,srpd_lite_idle = <0x4>;
- rockchip,standby_idle = <0x2000>;
+ rockchip,pd-idle-ns = <160>;
+ rockchip,sr-idle-ns = <10240>;
+ rockchip,sr-mc-gate-idle-ns = <40960>;
+ rockchip,srpd-lite-idle-ns = <61440>;
+ rockchip,standby-idle-ns = <81920>;
rockchip,ddr3_odt_dis_freq = <333000000>;
rockchip,lpddr3_odt_dis_freq = <333000000>;
rockchip,lpddr4_odt_dis_freq = <333000000>;
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 20:46:29

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 03/15] dt-bindings: devfreq: rk3399_dmc: Fix Hz units

The driver and all downstream device trees [1] are using Hz units, but
the document claims MHz. DRAM frequency for these systems can't possibly
exceed 2^32-1 Hz, so the choice of unit doesn't really matter than much.

Rather than add unnecessary risk in getting the units wrong, let's just
go with the unofficial convention and make the docs match reality.

A sub-1MHz frequency is extremely unlikely, so include a minimum in the
schema, to help catch anybody who might have believed this was MHz.

[1] And notably, also those trying to upstream them:
https://lore.kernel.org/lkml/[email protected]/

Signed-off-by: Brian Norris <[email protected]>
---

(no changes since v1)

.../bindings/devfreq/rk3399_dmc.yaml | 24 +++++++++----------
1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
index fd62a8cd62d5..8bb778df92ae 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
@@ -116,11 +116,11 @@ properties:

rockchip,ddr3_odt_dis_freq:
$ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1000000 # In case anyone thought this was MHz.
description:
When the DRAM type is DDR3, this parameter defines the ODT disable
- frequency in MHz (Mega Hz). When the DDR frequency is less then
- ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
- disabled.
+ frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
+ the ODT on the DRAM side and controller side are both disabled.

rockchip,ddr3_drv:
deprecated: true
@@ -160,11 +160,11 @@ properties:

rockchip,lpddr3_odt_dis_freq:
$ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1000000 # In case anyone thought this was MHz.
description:
When the DRAM type is LPDDR3, this parameter defines then ODT disable
- frequency in MHz (Mega Hz). When DDR frequency is less then
- ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
- disabled.
+ frequency in Hz. When DDR frequency is less then ddr3_odt_dis_freq, the
+ ODT on the DRAM side and controller side are both disabled.

rockchip,lpddr3_drv:
deprecated: true
@@ -204,11 +204,11 @@ properties:

rockchip,lpddr4_odt_dis_freq:
$ref: /schemas/types.yaml#/definitions/uint32
+ minimum: 1000000 # In case anyone thought this was MHz.
description:
When the DRAM type is LPDDR4, this parameter defines the ODT disable
- frequency in MHz (Mega Hz). When the DDR frequency is less then
- ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
- disabled.
+ frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
+ the ODT on the DRAM side and controller side are both disabled.

rockchip,lpddr4_drv:
deprecated: true
@@ -287,7 +287,7 @@ examples:
rockchip,sr_mc_gate_idle = <0x3>;
rockchip,srpd_lite_idle = <0x4>;
rockchip,standby_idle = <0x2000>;
- rockchip,ddr3_odt_dis_freq = <333>;
- rockchip,lpddr3_odt_dis_freq = <333>;
- rockchip,lpddr4_odt_dis_freq = <333>;
+ rockchip,ddr3_odt_dis_freq = <333000000>;
+ rockchip,lpddr3_odt_dis_freq = <333000000>;
+ rockchip,lpddr4_odt_dis_freq = <333000000>;
};
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:33:58

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 06/15] PM / devfreq: rk3399_dmc: Drop undocumented ondemand DT props

These properties are:

* undocumented
* directly representing software properties, not hardware properties
* unused (no in-tree users, yet; this IP block has so far only been used
in downstream kernels)

Let's just stick the values that downstream users have been using
directly in the driver and call it a day.

Signed-off-by: Brian Norris <[email protected]>
---

(no changes since v1)

drivers/devfreq/rk3399_dmc.c | 6 ++----
1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 293857ebfd75..e982862f6ac2 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -430,10 +430,8 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
goto err_edev;
}

- of_property_read_u32(np, "upthreshold",
- &data->ondemand_data.upthreshold);
- of_property_read_u32(np, "downdifferential",
- &data->ondemand_data.downdifferential);
+ data->ondemand_data.upthreshold = 25;
+ data->ondemand_data.downdifferential = 15;

data->rate = clk_get_rate(data->dmc_clk);

--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:33:58

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 05/15] dt-bindings: devfreq: rk3399_dmc: Add more disable-freq properties

DDR DVFS tuning has found that several power-saving features don't have
good tradeoffs at higher frequencies -- at higher frequencies, we'll see
glitches or other errors. Provide tuning controls so these can be
disabled at higher OPPs, and left active only at the lower ones.

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
- hyphens, not underscores
- *-hz units, and drop the types definition

.../bindings/devfreq/rk3399_dmc.yaml | 37 +++++++++++++++++++
1 file changed, 37 insertions(+)

diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
index 8786b7fa9b28..afa058c52c0b 100644
--- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
+++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
@@ -299,6 +299,38 @@ properties:
self-refresh mode. The controller, pi, PHY and DRAM clock will be gated
if bus is idle for standby_idle nanoseconds.

+ rockchip,pd-idle-dis-freq-hz:
+ description:
+ Defines the power-down idle disable frequency in Hz. When the DDR
+ frequency is greater than pd-idle-dis-freq, power-down idle is disabled.
+ See also rockchip,pd-idle-ns.
+
+ rockchip,sr-idle-dis-freq-hz:
+ description:
+ Defines the self-refresh idle disable frequency in Hz. When the DDR
+ frequency is greater than sr-idle-dis-freq, self-refresh idle is
+ disabled. See also rockchip,sr-idle-ns.
+
+ rockchip,sr-mc-gate-idle-dis-freq-hz:
+ description:
+ Defines the self-refresh and memory-controller clock gating disable
+ frequency in Hz. When the DDR frequency is greater than
+ sr-mc-gate-idle-dis-freq, the clock will not be gated when idle. See also
+ rockchip,sr-mc-gate-idle-ns.
+
+ rockchip,srpd-lite-idle-dis-freq-hz:
+ description:
+ Defines the self-refresh power down idle disable frequency in Hz. When
+ the DDR frequency is greater than srpd-lite-idle-dis-freq, memory will
+ not be placed into self-refresh power down mode when idle. See also
+ rockchip,srpd-lite-idle-ns.
+
+ rockchip,standby-idle-dis-freq-hz:
+ description:
+ Defines the standby idle disable frequency in Hz. When the DDR frequency
+ is greater than standby-idle-dis-freq, standby idle is disabled. See also
+ rockchip,standby-idle-ns.
+
required:
- compatible
- devfreq-events
@@ -330,4 +362,9 @@ examples:
rockchip,ddr3_odt_dis_freq = <333000000>;
rockchip,lpddr3_odt_dis_freq = <333000000>;
rockchip,lpddr4_odt_dis_freq = <333000000>;
+ rockchip,pd-idle-dis-freq-hz = <1000000000>;
+ rockchip,sr-idle-dis-freq-hz = <1000000000>;
+ rockchip,sr-mc-gate-idle-dis-freq-hz = <1000000000>;
+ rockchip,srpd-lite-idle-dis-freq-hz = <0>;
+ rockchip,standby-idle-dis-freq-hz = <928000000>;
};
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:33:58

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 07/15] PM / devfreq: rk3399_dmc: Drop excess timing properties

All of these properties are initialized by ARM Trusted Firmware, and
have been since the early days of this chip. It's redundant (and
possibly wrong) to do this here now. What's more, there seems to be some
confusion about the units and some of the definitions of this timing
struct: the DT docs say MHz for many of these, but downstream users were
in Hz (and therefore, the ATF interface was Hz). Also, the in-driver
usage for some of these (e.g., for comparing to target frequency) were
in Hz too. So doubly wrong.

We can avoid thinking about who got the right units by dropping the
unnecessary code and properties. They are marked deprecated in the
binding schema.

Signed-off-by: Brian Norris <[email protected]>
---

(no changes since v1)

drivers/devfreq/rk3399_dmc.c | 144 +++++++----------------------------
1 file changed, 29 insertions(+), 115 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index e982862f6ac2..8f447217303f 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -23,38 +23,6 @@
#include <soc/rockchip/rk3399_grf.h>
#include <soc/rockchip/rockchip_sip.h>

-struct dram_timing {
- unsigned int ddr3_speed_bin;
- unsigned int pd_idle;
- unsigned int sr_idle;
- unsigned int sr_mc_gate_idle;
- unsigned int srpd_lite_idle;
- unsigned int standby_idle;
- unsigned int auto_pd_dis_freq;
- unsigned int dram_dll_dis_freq;
- unsigned int phy_dll_dis_freq;
- unsigned int ddr3_odt_dis_freq;
- unsigned int ddr3_drv;
- unsigned int ddr3_odt;
- unsigned int phy_ddr3_ca_drv;
- unsigned int phy_ddr3_dq_drv;
- unsigned int phy_ddr3_odt;
- unsigned int lpddr3_odt_dis_freq;
- unsigned int lpddr3_drv;
- unsigned int lpddr3_odt;
- unsigned int phy_lpddr3_ca_drv;
- unsigned int phy_lpddr3_dq_drv;
- unsigned int phy_lpddr3_odt;
- unsigned int lpddr4_odt_dis_freq;
- unsigned int lpddr4_drv;
- unsigned int lpddr4_dq_odt;
- unsigned int lpddr4_ca_odt;
- unsigned int phy_lpddr4_ca_drv;
- unsigned int phy_lpddr4_ck_cs_drv;
- unsigned int phy_lpddr4_dq_drv;
- unsigned int phy_lpddr4_odt;
-};
-
struct rk3399_dmcfreq {
struct device *dev;
struct devfreq *devfreq;
@@ -62,13 +30,21 @@ struct rk3399_dmcfreq {
struct clk *dmc_clk;
struct devfreq_event_dev *edev;
struct mutex lock;
- struct dram_timing timing;
struct regulator *vdd_center;
struct regmap *regmap_pmu;
unsigned long rate, target_rate;
unsigned long volt, target_volt;
unsigned int odt_dis_freq;
int odt_pd_arg0, odt_pd_arg1;
+
+ unsigned int pd_idle;
+ unsigned int sr_idle;
+ unsigned int sr_mc_gate_idle;
+ unsigned int srpd_lite_idle;
+ unsigned int standby_idle;
+ unsigned int ddr3_odt_dis_freq;
+ unsigned int lpddr3_odt_dis_freq;
+ unsigned int lpddr4_odt_dis_freq;
};

static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
@@ -238,69 +214,27 @@ static __maybe_unused int rk3399_dmcfreq_resume(struct device *dev)
static SIMPLE_DEV_PM_OPS(rk3399_dmcfreq_pm, rk3399_dmcfreq_suspend,
rk3399_dmcfreq_resume);

-static int of_get_ddr_timings(struct dram_timing *timing,
- struct device_node *np)
+static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data,
+ struct device_node *np)
{
int ret = 0;

- ret = of_property_read_u32(np, "rockchip,ddr3_speed_bin",
- &timing->ddr3_speed_bin);
ret |= of_property_read_u32(np, "rockchip,pd_idle",
- &timing->pd_idle);
+ &data->pd_idle);
ret |= of_property_read_u32(np, "rockchip,sr_idle",
- &timing->sr_idle);
+ &data->sr_idle);
ret |= of_property_read_u32(np, "rockchip,sr_mc_gate_idle",
- &timing->sr_mc_gate_idle);
+ &data->sr_mc_gate_idle);
ret |= of_property_read_u32(np, "rockchip,srpd_lite_idle",
- &timing->srpd_lite_idle);
+ &data->srpd_lite_idle);
ret |= of_property_read_u32(np, "rockchip,standby_idle",
- &timing->standby_idle);
- ret |= of_property_read_u32(np, "rockchip,auto_pd_dis_freq",
- &timing->auto_pd_dis_freq);
- ret |= of_property_read_u32(np, "rockchip,dram_dll_dis_freq",
- &timing->dram_dll_dis_freq);
- ret |= of_property_read_u32(np, "rockchip,phy_dll_dis_freq",
- &timing->phy_dll_dis_freq);
+ &data->standby_idle);
ret |= of_property_read_u32(np, "rockchip,ddr3_odt_dis_freq",
- &timing->ddr3_odt_dis_freq);
- ret |= of_property_read_u32(np, "rockchip,ddr3_drv",
- &timing->ddr3_drv);
- ret |= of_property_read_u32(np, "rockchip,ddr3_odt",
- &timing->ddr3_odt);
- ret |= of_property_read_u32(np, "rockchip,phy_ddr3_ca_drv",
- &timing->phy_ddr3_ca_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_ddr3_dq_drv",
- &timing->phy_ddr3_dq_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_ddr3_odt",
- &timing->phy_ddr3_odt);
+ &data->ddr3_odt_dis_freq);
ret |= of_property_read_u32(np, "rockchip,lpddr3_odt_dis_freq",
- &timing->lpddr3_odt_dis_freq);
- ret |= of_property_read_u32(np, "rockchip,lpddr3_drv",
- &timing->lpddr3_drv);
- ret |= of_property_read_u32(np, "rockchip,lpddr3_odt",
- &timing->lpddr3_odt);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr3_ca_drv",
- &timing->phy_lpddr3_ca_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr3_dq_drv",
- &timing->phy_lpddr3_dq_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr3_odt",
- &timing->phy_lpddr3_odt);
+ &data->lpddr3_odt_dis_freq);
ret |= of_property_read_u32(np, "rockchip,lpddr4_odt_dis_freq",
- &timing->lpddr4_odt_dis_freq);
- ret |= of_property_read_u32(np, "rockchip,lpddr4_drv",
- &timing->lpddr4_drv);
- ret |= of_property_read_u32(np, "rockchip,lpddr4_dq_odt",
- &timing->lpddr4_dq_odt);
- ret |= of_property_read_u32(np, "rockchip,lpddr4_ca_odt",
- &timing->lpddr4_ca_odt);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_ca_drv",
- &timing->phy_lpddr4_ca_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_ck_cs_drv",
- &timing->phy_lpddr4_ck_cs_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_dq_drv",
- &timing->phy_lpddr4_dq_drv);
- ret |= of_property_read_u32(np, "rockchip,phy_lpddr4_odt",
- &timing->phy_lpddr4_odt);
+ &data->lpddr4_odt_dis_freq);

return ret;
}
@@ -311,8 +245,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
struct device *dev = &pdev->dev;
struct device_node *np = pdev->dev.of_node, *node;
struct rk3399_dmcfreq *data;
- int ret, index, size;
- uint32_t *timing;
+ int ret;
struct dev_pm_opp *opp;
u32 ddr_type;
u32 val;
@@ -343,26 +276,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
return ret;
}

- /*
- * Get dram timing and pass it to arm trust firmware,
- * the dram driver in arm trust firmware will get these
- * timing and to do dram initial.
- */
- if (!of_get_ddr_timings(&data->timing, np)) {
- timing = &data->timing.ddr3_speed_bin;
- size = sizeof(struct dram_timing) / 4;
- for (index = 0; index < size; index++) {
- arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, *timing++, index,
- ROCKCHIP_SIP_CONFIG_DRAM_SET_PARAM,
- 0, 0, 0, 0, &res);
- if (res.a0) {
- dev_err(dev, "Failed to set dram param: %ld\n",
- res.a0);
- ret = -EINVAL;
- goto err_edev;
- }
- }
- }
+ rk3399_dmcfreq_of_props(data, np);

node = of_parse_phandle(np, "rockchip,pmu", 0);
if (!node)
@@ -381,13 +295,13 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)

switch (ddr_type) {
case RK3399_PMUGRF_DDRTYPE_DDR3:
- data->odt_dis_freq = data->timing.ddr3_odt_dis_freq;
+ data->odt_dis_freq = data->ddr3_odt_dis_freq;
break;
case RK3399_PMUGRF_DDRTYPE_LPDDR3:
- data->odt_dis_freq = data->timing.lpddr3_odt_dis_freq;
+ data->odt_dis_freq = data->lpddr3_odt_dis_freq;
break;
case RK3399_PMUGRF_DDRTYPE_LPDDR4:
- data->odt_dis_freq = data->timing.lpddr4_odt_dis_freq;
+ data->odt_dis_freq = data->lpddr4_odt_dis_freq;
break;
default:
ret = -EINVAL;
@@ -414,11 +328,11 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
* arg2:
* bit[0] : odt enable
*/
- data->odt_pd_arg0 = (data->timing.sr_idle & 0xff) |
- ((data->timing.sr_mc_gate_idle & 0xff) << 8) |
- ((data->timing.standby_idle & 0xffff) << 16);
- data->odt_pd_arg1 = (data->timing.pd_idle & 0xfff) |
- ((data->timing.srpd_lite_idle & 0xfff) << 16);
+ data->odt_pd_arg0 = (data->sr_idle & 0xff) |
+ ((data->sr_mc_gate_idle & 0xff) << 8) |
+ ((data->standby_idle & 0xffff) << 16);
+ data->odt_pd_arg1 = (data->pd_idle & 0xfff) |
+ ((data->srpd_lite_idle & 0xfff) << 16);

/*
* We add a devfreq driver to our parent since it has a device tree node
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:43:44

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 08/15] PM / devfreq: rk3399_dmc: Use bitfield macro definitions for ODT_PD

We're going to add new usages, and it's cleaner to work with macros
instead of comments and magic numbers.

Signed-off-by: Brian Norris <[email protected]>
---

(no changes since v1)

drivers/devfreq/rk3399_dmc.c | 43 ++++++++++++++++++++----------------
1 file changed, 24 insertions(+), 19 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 8f447217303f..c4efbc15cbb1 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -5,6 +5,7 @@
*/

#include <linux/arm-smccc.h>
+#include <linux/bitfield.h>
#include <linux/clk.h>
#include <linux/delay.h>
#include <linux/devfreq.h>
@@ -23,6 +24,15 @@
#include <soc/rockchip/rk3399_grf.h>
#include <soc/rockchip/rockchip_sip.h>

+#define RK3399_SET_ODT_PD_0_SR_IDLE GENMASK(7, 0)
+#define RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE GENMASK(15, 8)
+#define RK3399_SET_ODT_PD_0_STANDBY_IDLE GENMASK(31, 16)
+
+#define RK3399_SET_ODT_PD_1_PD_IDLE GENMASK(11, 0)
+#define RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE GENMASK(27, 16)
+
+#define RK3399_SET_ODT_PD_2_ODT_ENABLE BIT(0)
+
struct rk3399_dmcfreq {
struct device *dev;
struct devfreq *devfreq;
@@ -55,7 +65,6 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
unsigned long old_clk_rate = dmcfreq->rate;
unsigned long target_volt, target_rate;
struct arm_smccc_res res;
- bool odt_enable = false;
int err;

opp = devfreq_recommended_opp(dev, freq, flags);
@@ -72,8 +81,10 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
mutex_lock(&dmcfreq->lock);

if (dmcfreq->regmap_pmu) {
+ unsigned int odt_pd_arg2 = 0;
+
if (target_rate >= dmcfreq->odt_dis_freq)
- odt_enable = true;
+ odt_pd_arg2 |= RK3399_SET_ODT_PD_2_ODT_ENABLE;

/*
* This makes a SMC call to the TF-A to set the DDR PD
@@ -83,7 +94,7 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
dmcfreq->odt_pd_arg1,
ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
- odt_enable, 0, 0, 0, &res);
+ odt_pd_arg2, 0, 0, 0, &res);
}

/*
@@ -316,23 +327,17 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
/*
* In TF-A there is a platform SIP call to set the PD (power-down)
* timings and to enable or disable the ODT (on-die termination).
- * This call needs three arguments as follows:
- *
- * arg0:
- * bit[0-7] : sr_idle
- * bit[8-15] : sr_mc_gate_idle
- * bit[16-31] : standby idle
- * arg1:
- * bit[0-11] : pd_idle
- * bit[16-27] : srpd_lite_idle
- * arg2:
- * bit[0] : odt enable
*/
- data->odt_pd_arg0 = (data->sr_idle & 0xff) |
- ((data->sr_mc_gate_idle & 0xff) << 8) |
- ((data->standby_idle & 0xffff) << 16);
- data->odt_pd_arg1 = (data->pd_idle & 0xfff) |
- ((data->srpd_lite_idle & 0xfff) << 16);
+ data->odt_pd_arg0 =
+ FIELD_PREP(RK3399_SET_ODT_PD_0_SR_IDLE, data->sr_idle) |
+ FIELD_PREP(RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE,
+ data->sr_mc_gate_idle) |
+ FIELD_PREP(RK3399_SET_ODT_PD_0_STANDBY_IDLE,
+ data->standby_idle);
+ data->odt_pd_arg1 =
+ FIELD_PREP(RK3399_SET_ODT_PD_1_PD_IDLE, data->pd_idle) |
+ FIELD_PREP(RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE,
+ data->srpd_lite_idle);

/*
* We add a devfreq driver to our parent since it has a device tree node
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:43:49

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 10/15] PM / devfreq: rk3399_dmc: Support new *-ns properties

We want to keep the idle time fixed, so compute based on the current DDR
frequency.

The old properties were deprecated and never used, so we can safely drop
them from the driver.

This is a rewritten version of work by Lin Huang <[email protected]>.

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
- New patch

drivers/devfreq/rk3399_dmc.c | 85 +++++++++++++++++++++---------------
1 file changed, 50 insertions(+), 35 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index fc740c1f6747..f778564cab49 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -24,6 +24,8 @@
#include <soc/rockchip/rk3399_grf.h>
#include <soc/rockchip/rockchip_sip.h>

+#define NS_TO_CYCLE(NS, MHz) (((NS) * (MHz)) / NSEC_PER_USEC)
+
#define RK3399_SET_ODT_PD_0_SR_IDLE GENMASK(7, 0)
#define RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE GENMASK(15, 8)
#define RK3399_SET_ODT_PD_0_STANDBY_IDLE GENMASK(31, 16)
@@ -45,13 +47,12 @@ struct rk3399_dmcfreq {
unsigned long rate, target_rate;
unsigned long volt, target_volt;
unsigned int odt_dis_freq;
- int odt_pd_arg0, odt_pd_arg1;

- unsigned int pd_idle;
- unsigned int sr_idle;
- unsigned int sr_mc_gate_idle;
- unsigned int srpd_lite_idle;
- unsigned int standby_idle;
+ unsigned int pd_idle_ns;
+ unsigned int sr_idle_ns;
+ unsigned int sr_mc_gate_idle_ns;
+ unsigned int srpd_lite_idle_ns;
+ unsigned int standby_idle_ns;
unsigned int ddr3_odt_dis_freq;
unsigned int lpddr3_odt_dis_freq;
unsigned int lpddr4_odt_dis_freq;
@@ -70,9 +71,14 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
struct dev_pm_opp *opp;
unsigned long old_clk_rate = dmcfreq->rate;
unsigned long target_volt, target_rate;
+ unsigned int ddrcon_mhz;
struct arm_smccc_res res;
int err;

+ u32 odt_pd_arg0 = 0;
+ u32 odt_pd_arg1 = 0;
+ u32 odt_pd_arg2 = 0;
+
opp = devfreq_recommended_opp(dev, freq, flags);
if (IS_ERR(opp))
return PTR_ERR(opp);
@@ -86,11 +92,35 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,

mutex_lock(&dmcfreq->lock);

- if (dmcfreq->regmap_pmu) {
- unsigned int odt_pd_arg0 = dmcfreq->odt_pd_arg0;
- unsigned int odt_pd_arg1 = dmcfreq->odt_pd_arg1;
- unsigned int odt_pd_arg2 = 0;
+ /*
+ * Some idle parameters may be based on the DDR controller clock, which
+ * is half of the DDR frequency.
+ * pd_idle and standby_idle are based on the controller clock cycle.
+ * sr_idle_cycle, sr_mc_gate_idle_cycle, and srpd_lite_idle_cycle
+ * are based on the 1024 controller clock cycle
+ */
+ ddrcon_mhz = target_rate / USEC_PER_SEC / 2;
+
+ u32p_replace_bits(&odt_pd_arg1,
+ NS_TO_CYCLE(dmcfreq->pd_idle_ns, ddrcon_mhz),
+ RK3399_SET_ODT_PD_1_PD_IDLE);
+ u32p_replace_bits(&odt_pd_arg0,
+ NS_TO_CYCLE(dmcfreq->standby_idle_ns, ddrcon_mhz),
+ RK3399_SET_ODT_PD_0_STANDBY_IDLE);
+ u32p_replace_bits(&odt_pd_arg0,
+ DIV_ROUND_UP(NS_TO_CYCLE(dmcfreq->sr_idle_ns,
+ ddrcon_mhz), 1024),
+ RK3399_SET_ODT_PD_0_SR_IDLE);
+ u32p_replace_bits(&odt_pd_arg0,
+ DIV_ROUND_UP(NS_TO_CYCLE(dmcfreq->sr_mc_gate_idle_ns,
+ ddrcon_mhz), 1024),
+ RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE);
+ u32p_replace_bits(&odt_pd_arg1,
+ DIV_ROUND_UP(NS_TO_CYCLE(dmcfreq->srpd_lite_idle_ns,
+ ddrcon_mhz), 1024),
+ RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE);

+ if (dmcfreq->regmap_pmu) {
if (target_rate >= dmcfreq->sr_idle_dis_freq)
odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_SR_IDLE;

@@ -262,16 +292,16 @@ static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data,
data->srpd_lite_idle_dis_freq =
data->standby_idle_dis_freq = UINT_MAX;

- ret |= of_property_read_u32(np, "rockchip,pd_idle",
- &data->pd_idle);
- ret |= of_property_read_u32(np, "rockchip,sr_idle",
- &data->sr_idle);
- ret |= of_property_read_u32(np, "rockchip,sr_mc_gate_idle",
- &data->sr_mc_gate_idle);
- ret |= of_property_read_u32(np, "rockchip,srpd_lite_idle",
- &data->srpd_lite_idle);
- ret |= of_property_read_u32(np, "rockchip,standby_idle",
- &data->standby_idle);
+ ret |= of_property_read_u32(np, "rockchip,pd-idle-ns",
+ &data->pd_idle_ns);
+ ret |= of_property_read_u32(np, "rockchip,sr-idle-ns",
+ &data->sr_idle_ns);
+ ret |= of_property_read_u32(np, "rockchip,sr-mc-gate-idle-ns",
+ &data->sr_mc_gate_idle_ns);
+ ret |= of_property_read_u32(np, "rockchip,srpd-lite-idle-ns",
+ &data->srpd_lite_idle_ns);
+ ret |= of_property_read_u32(np, "rockchip,standby-idle-ns",
+ &data->standby_idle_ns);
ret |= of_property_read_u32(np, "rockchip,ddr3_odt_dis_freq",
&data->ddr3_odt_dis_freq);
ret |= of_property_read_u32(np, "rockchip,lpddr3_odt_dis_freq",
@@ -367,21 +397,6 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
ROCKCHIP_SIP_CONFIG_DRAM_INIT,
0, 0, 0, 0, &res);

- /*
- * In TF-A there is a platform SIP call to set the PD (power-down)
- * timings and to enable or disable the ODT (on-die termination).
- */
- data->odt_pd_arg0 =
- FIELD_PREP(RK3399_SET_ODT_PD_0_SR_IDLE, data->sr_idle) |
- FIELD_PREP(RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE,
- data->sr_mc_gate_idle) |
- FIELD_PREP(RK3399_SET_ODT_PD_0_STANDBY_IDLE,
- data->standby_idle);
- data->odt_pd_arg1 =
- FIELD_PREP(RK3399_SET_ODT_PD_1_PD_IDLE, data->pd_idle) |
- FIELD_PREP(RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE,
- data->srpd_lite_idle);
-
/*
* We add a devfreq driver to our parent since it has a device tree node
* with operating points.
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:43:57

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 11/15] arm64: dts: rk3399: Add dfi and dmc nodes

From: Lin Huang <[email protected]>

These are required to support DDR DVFS on RK3399 platforms.

Signed-off-by: Lin Huang <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
Signed-off-by: GaĆ«l PORTAY <[email protected]>
Signed-off-by: Daniel Lezcano <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
Change since Daniel's posting: reordered by unit address, per existing
style

---

Changes in v2:
- rename dmc to memory-controller

Changes in v1:
This is based on a v5 posting from various authors:
https://lore.kernel.org/lkml/[email protected]/
Much of that series was already merged, so I start over with the
numbering.

arch/arm64/boot/dts/rockchip/rk3399.dtsi | 19 +++++++++++++++++++
1 file changed, 19 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399.dtsi b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
index d3cdf6f42a30..4096ef6f7b72 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399.dtsi
@@ -1295,6 +1295,25 @@ pwm3: pwm@ff420030 {
status = "disabled";
};

+ dfi: dfi@ff630000 {
+ reg = <0x00 0xff630000 0x00 0x4000>;
+ compatible = "rockchip,rk3399-dfi";
+ rockchip,pmu = <&pmugrf>;
+ interrupts = <GIC_SPI 131 IRQ_TYPE_LEVEL_HIGH 0>;
+ clocks = <&cru PCLK_DDR_MON>;
+ clock-names = "pclk_ddr_mon";
+ status = "disabled";
+ };
+
+ dmc: memory-controller {
+ compatible = "rockchip,rk3399-dmc";
+ rockchip,pmu = <&pmugrf>;
+ devfreq-events = <&dfi>;
+ clocks = <&cru SCLK_DDRC>;
+ clock-names = "dmc_clk";
+ status = "disabled";
+ };
+
vpu: video-codec@ff650000 {
compatible = "rockchip,rk3399-vpu";
reg = <0x0 0xff650000 0x0 0x800>;
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:48:23

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 13/15] PM / devfreq: rk3399_dmc: Disable edev on remove()

Otherwise we hit an unablanced enable-count when unbinding the DFI
device:

[ 1279.659119] ------------[ cut here ]------------
[ 1279.659179] WARNING: CPU: 2 PID: 5638 at drivers/devfreq/devfreq-event.c:360 devfreq_event_remove_edev+0x84/0x8c
...
[ 1279.659352] Hardware name: Google Kevin (DT)
[ 1279.659363] pstate: 80400005 (Nzcv daif +PAN -UAO -TCO BTYPE=--)
[ 1279.659371] pc : devfreq_event_remove_edev+0x84/0x8c
[ 1279.659380] lr : devm_devfreq_event_release+0x1c/0x28
...
[ 1279.659571] Call trace:
[ 1279.659582] devfreq_event_remove_edev+0x84/0x8c
[ 1279.659590] devm_devfreq_event_release+0x1c/0x28
[ 1279.659602] release_nodes+0x1cc/0x244
[ 1279.659611] devres_release_all+0x44/0x60
[ 1279.659621] device_release_driver_internal+0x11c/0x1ac
[ 1279.659629] device_driver_detach+0x20/0x2c
[ 1279.659641] unbind_store+0x7c/0xb0
[ 1279.659650] drv_attr_store+0x2c/0x40
[ 1279.659663] sysfs_kf_write+0x44/0x58
[ 1279.659672] kernfs_fop_write_iter+0xf4/0x190
[ 1279.659684] vfs_write+0x2b0/0x2e4
[ 1279.659693] ksys_write+0x80/0xec
[ 1279.659701] __arm64_sys_write+0x24/0x30
[ 1279.659714] el0_svc_common+0xf0/0x1d8
[ 1279.659724] do_el0_svc_compat+0x28/0x3c
[ 1279.659738] el0_svc_compat+0x10/0x1c
[ 1279.659746] el0_sync_compat_handler+0xa8/0xcc
[ 1279.659758] el0_sync_compat+0x188/0x1c0
[ 1279.659768] ---[ end trace cec200e5094155b4 ]---

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
- New patch

drivers/devfreq/rk3399_dmc.c | 2 ++
1 file changed, 2 insertions(+)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index f778564cab49..fca9fcbd4249 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -452,6 +452,8 @@ static int rk3399_dmcfreq_remove(struct platform_device *pdev)
{
struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(&pdev->dev);

+ devfreq_event_disable_edev(dmcfreq->edev);
+
/*
* Before remove the opp table we need to unregister the opp notifier.
*/
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:48:26

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 14/15] PM / devfreq: rk3399_dmc: Use devm_pm_opp_of_add_table()

This simplifies error-cleanup and remove().

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
- New patch

drivers/devfreq/rk3399_dmc.c | 14 +++-----------
1 file changed, 3 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index fca9fcbd4249..9615658d04ae 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -401,7 +401,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
* We add a devfreq driver to our parent since it has a device tree node
* with operating points.
*/
- if (dev_pm_opp_of_add_table(dev)) {
+ if (devm_pm_opp_of_add_table(dev)) {
dev_err(dev, "Invalid operating-points in device tree.\n");
ret = -EINVAL;
goto err_edev;
@@ -415,7 +415,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
opp = devfreq_recommended_opp(dev, &data->rate, 0);
if (IS_ERR(opp)) {
ret = PTR_ERR(opp);
- goto err_free_opp;
+ goto err_edev;
}

data->rate = dev_pm_opp_get_freq(opp);
@@ -430,7 +430,7 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
&data->ondemand_data);
if (IS_ERR(data->devfreq)) {
ret = PTR_ERR(data->devfreq);
- goto err_free_opp;
+ goto err_edev;
}

devm_devfreq_register_opp_notifier(dev, data->devfreq);
@@ -440,8 +440,6 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)

return 0;

-err_free_opp:
- dev_pm_opp_of_remove_table(&pdev->dev);
err_edev:
devfreq_event_disable_edev(data->edev);

@@ -454,12 +452,6 @@ static int rk3399_dmcfreq_remove(struct platform_device *pdev)

devfreq_event_disable_edev(dmcfreq->edev);

- /*
- * Before remove the opp table we need to unregister the opp notifier.
- */
- devm_devfreq_unregister_opp_notifier(dmcfreq->dev, dmcfreq->devfreq);
- dev_pm_opp_of_remove_table(dmcfreq->dev);
-
return 0;
}

--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:48:38

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 09/15] PM / devfreq: rk3399_dmc: Support new disable-freq properties

Implement the newly-defined properties to allow disabling certain
power-saving-at-idle features at higher frequencies.

This is a rewritten version of work by Lin Huang <[email protected]>.

Signed-off-by: Brian Norris <[email protected]>
---

(no changes since v1)

drivers/devfreq/rk3399_dmc.c | 51 +++++++++++++++++++++++++++++++++---
1 file changed, 47 insertions(+), 4 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index c4efbc15cbb1..fc740c1f6747 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -55,6 +55,12 @@ struct rk3399_dmcfreq {
unsigned int ddr3_odt_dis_freq;
unsigned int lpddr3_odt_dis_freq;
unsigned int lpddr4_odt_dis_freq;
+
+ unsigned int pd_idle_dis_freq;
+ unsigned int sr_idle_dis_freq;
+ unsigned int sr_mc_gate_idle_dis_freq;
+ unsigned int srpd_lite_idle_dis_freq;
+ unsigned int standby_idle_dis_freq;
};

static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
@@ -81,8 +87,25 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
mutex_lock(&dmcfreq->lock);

if (dmcfreq->regmap_pmu) {
+ unsigned int odt_pd_arg0 = dmcfreq->odt_pd_arg0;
+ unsigned int odt_pd_arg1 = dmcfreq->odt_pd_arg1;
unsigned int odt_pd_arg2 = 0;

+ if (target_rate >= dmcfreq->sr_idle_dis_freq)
+ odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_SR_IDLE;
+
+ if (target_rate >= dmcfreq->sr_mc_gate_idle_dis_freq)
+ odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_SR_MC_GATE_IDLE;
+
+ if (target_rate >= dmcfreq->standby_idle_dis_freq)
+ odt_pd_arg0 &= ~RK3399_SET_ODT_PD_0_STANDBY_IDLE;
+
+ if (target_rate >= dmcfreq->pd_idle_dis_freq)
+ odt_pd_arg1 &= ~RK3399_SET_ODT_PD_1_PD_IDLE;
+
+ if (target_rate >= dmcfreq->srpd_lite_idle_dis_freq)
+ odt_pd_arg1 &= ~RK3399_SET_ODT_PD_1_SRPD_LITE_IDLE;
+
if (target_rate >= dmcfreq->odt_dis_freq)
odt_pd_arg2 |= RK3399_SET_ODT_PD_2_ODT_ENABLE;

@@ -91,10 +114,9 @@ static int rk3399_dmcfreq_target(struct device *dev, unsigned long *freq,
* (power-down) timings and to enable or disable the
* ODT (on-die termination) resistors.
*/
- arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, dmcfreq->odt_pd_arg0,
- dmcfreq->odt_pd_arg1,
- ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD,
- odt_pd_arg2, 0, 0, 0, &res);
+ arm_smccc_smc(ROCKCHIP_SIP_DRAM_FREQ, odt_pd_arg0, odt_pd_arg1,
+ ROCKCHIP_SIP_CONFIG_DRAM_SET_ODT_PD, odt_pd_arg2,
+ 0, 0, 0, &res);
}

/*
@@ -230,6 +252,16 @@ static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data,
{
int ret = 0;

+ /*
+ * These are all optional, and serve as minimum bounds. Give them large
+ * (i.e., never "disabled") values if the DT doesn't specify one.
+ */
+ data->pd_idle_dis_freq =
+ data->sr_idle_dis_freq =
+ data->sr_mc_gate_idle_dis_freq =
+ data->srpd_lite_idle_dis_freq =
+ data->standby_idle_dis_freq = UINT_MAX;
+
ret |= of_property_read_u32(np, "rockchip,pd_idle",
&data->pd_idle);
ret |= of_property_read_u32(np, "rockchip,sr_idle",
@@ -247,6 +279,17 @@ static int rk3399_dmcfreq_of_props(struct rk3399_dmcfreq *data,
ret |= of_property_read_u32(np, "rockchip,lpddr4_odt_dis_freq",
&data->lpddr4_odt_dis_freq);

+ ret |= of_property_read_u32(np, "rockchip,pd-idle-dis-freq-hz",
+ &data->pd_idle_dis_freq);
+ ret |= of_property_read_u32(np, "rockchip,sr-idle-dis-freq-hz",
+ &data->sr_idle_dis_freq);
+ ret |= of_property_read_u32(np, "rockchip,sr-mc-gate-idle-dis-freq-hz",
+ &data->sr_mc_gate_idle_dis_freq);
+ ret |= of_property_read_u32(np, "rockchip,srpd-lite-idle-dis-freq-hz",
+ &data->srpd_lite_idle_dis_freq);
+ ret |= of_property_read_u32(np, "rockchip,standby-idle-dis-freq-hz",
+ &data->standby_idle_dis_freq);
+
return ret;
}

--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:48:41

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 15/15] PM / devfreq: rk3399_dmc: Avoid static (reused) profile

This static struct can get reused if the device gets removed/reprobed,
and that causes use-after-free in its ->freq_table.

Let's just move the struct to our dynamic allocation.

Signed-off-by: Brian Norris <[email protected]>
---

Changes in v2:
- New patch

drivers/devfreq/rk3399_dmc.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/drivers/devfreq/rk3399_dmc.c b/drivers/devfreq/rk3399_dmc.c
index 9615658d04ae..e494d1497d60 100644
--- a/drivers/devfreq/rk3399_dmc.c
+++ b/drivers/devfreq/rk3399_dmc.c
@@ -38,6 +38,7 @@
struct rk3399_dmcfreq {
struct device *dev;
struct devfreq *devfreq;
+ struct devfreq_dev_profile profile;
struct devfreq_simple_ondemand_data ondemand_data;
struct clk *dmc_clk;
struct devfreq_event_dev *edev;
@@ -228,13 +229,6 @@ static int rk3399_dmcfreq_get_cur_freq(struct device *dev, unsigned long *freq)
return 0;
}

-static struct devfreq_dev_profile rk3399_devfreq_dmc_profile = {
- .polling_ms = 200,
- .target = rk3399_dmcfreq_target,
- .get_dev_status = rk3399_dmcfreq_get_dev_status,
- .get_cur_freq = rk3399_dmcfreq_get_cur_freq,
-};
-
static __maybe_unused int rk3399_dmcfreq_suspend(struct device *dev)
{
struct rk3399_dmcfreq *dmcfreq = dev_get_drvdata(dev);
@@ -422,10 +416,16 @@ static int rk3399_dmcfreq_probe(struct platform_device *pdev)
data->volt = dev_pm_opp_get_voltage(opp);
dev_pm_opp_put(opp);

- rk3399_devfreq_dmc_profile.initial_freq = data->rate;
+ data->profile = (struct devfreq_dev_profile) {
+ .polling_ms = 200,
+ .target = rk3399_dmcfreq_target,
+ .get_dev_status = rk3399_dmcfreq_get_dev_status,
+ .get_cur_freq = rk3399_dmcfreq_get_cur_freq,
+ .initial_freq = data->rate,
+ };

data->devfreq = devm_devfreq_add_device(dev,
- &rk3399_devfreq_dmc_profile,
+ &data->profile,
DEVFREQ_GOV_SIMPLE_ONDEMAND,
&data->ondemand_data);
if (IS_ERR(data->devfreq)) {
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 21:48:48

by Brian Norris

[permalink] [raw]
Subject: [PATCH v2 12/15] arm64: dts: rockchip: Enable dmc and dfi nodes on gru

From: Lin Huang <[email protected]>

Enable the DMC (Dynamic Memory Controller) and the DFI (DDR PHY
Interface) nodes on gru boards so we can support DDR DVFS.

Signed-off-by: Lin Huang <[email protected]>
Signed-off-by: Enric Balletbo i Serra <[email protected]>
Signed-off-by: GaĆ«l PORTAY <[email protected]>
Signed-off-by: Daniel Lezcano <[email protected]>
Signed-off-by: Brian Norris <[email protected]>
Updates since the old series:

- reordered alphabetically by phandle name, per style
- drop a ton of deprecated/unused properties
- add required center-supply for scarlet
- add new *_idle_dis_freq properties
- drop the lowest (200 MHz) OPP; this was never stabilized for
production
- bump the voltage (0.9V -> 0.925V) for the highest OPP on Chromebook
models; later (tablet) models were more stable, with a fixed DDR
regulator
- bump odt_dis_freq to 666 MHz; early versions used 333 MHz, but
stabilization efforts landed on 666 MHz for production

---

Changes in v2:
- Adapt to new properties

Changes in v1:
This was part of a previous series, at:
https://lore.kernel.org/r/[email protected]
I've picked up a bunch of changes and fixes, so I've restarted the patch
series numbering.

.../dts/rockchip/rk3399-gru-chromebook.dtsi | 7 +++++
.../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 12 ++++++++
arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 28 +++++++++++++++++++
.../boot/dts/rockchip/rk3399-op1-opp.dtsi | 25 +++++++++++++++++
4 files changed, 72 insertions(+)

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
index 9b2c679f5eca..cc8950046d94 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
@@ -234,6 +234,13 @@ &cdn_dp {
extcon = <&usbc_extcon0>, <&usbc_extcon1>;
};

+&dmc {
+ center-supply = <&ppvar_centerlogic>;
+ rockchip,pd-idle-dis-freq-hz = <800000000>;
+ rockchip,sr-idle-dis-freq-hz = <800000000>;
+ rockchip,sr-mc-gate-idle-dis-freq-hz = <800000000>;
+};
+
&edp {
status = "okay";

diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
index a9817b3d7edc..913d845eb51a 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
@@ -391,6 +391,18 @@ &cru {
<400000000>;
};

+/* The center supply is fixed to .9V on scarlet */
+&dmc {
+ center-supply = <&pp900_s0>;
+};
+
+/* We don't need .925 V for 928 MHz on scarlet */
+&dmc_opp_table {
+ opp03 {
+ opp-microvolt = <900000>;
+ };
+};
+
&gpio0 {
gpio-line-names = /* GPIO0 A 0-7 */
"CLK_32K_AP",
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
index 162f08bca0d4..23bfba86daab 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
@@ -373,6 +373,34 @@ &cru {
<200000000>;
};

+&dfi {
+ status = "okay";
+};
+
+&dmc {
+ status = "okay";
+
+ rockchip,pd-idle-ns = <160>;
+ rockchip,sr-idle-ns = <10240>;
+ rockchip,sr-mc-gate-idle-ns = <40960>;
+ rockchip,srpd-lite-idle-ns = <61440>;
+ rockchip,standby-idle-ns = <81920>;
+
+ rockchip,ddr3_odt_dis_freq = <666000000>;
+ rockchip,lpddr3_odt_dis_freq = <666000000>;
+ rockchip,lpddr4_odt_dis_freq = <666000000>;
+
+ rockchip,sr-mc-gate-idle-dis-freq-hz = <1000000000>;
+ rockchip,srpd-lite-idle-dis-freq-hz = <0>;
+ rockchip,standby-idle-dis-freq-hz = <928000000>;
+};
+
+&dmc_opp_table {
+ opp03 {
+ opp-suspend;
+ };
+};
+
&emmc_phy {
status = "okay";
};
diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
index 2180e0f75003..6e29e74f6fc6 100644
--- a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
+++ b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
@@ -110,6 +110,27 @@ opp05 {
opp-microvolt = <1075000>;
};
};
+
+ dmc_opp_table: dmc_opp_table {
+ compatible = "operating-points-v2";
+
+ opp00 {
+ opp-hz = /bits/ 64 <400000000>;
+ opp-microvolt = <900000>;
+ };
+ opp01 {
+ opp-hz = /bits/ 64 <666000000>;
+ opp-microvolt = <900000>;
+ };
+ opp02 {
+ opp-hz = /bits/ 64 <800000000>;
+ opp-microvolt = <900000>;
+ };
+ opp03 {
+ opp-hz = /bits/ 64 <928000000>;
+ opp-microvolt = <925000>;
+ };
+ };
};

&cpu_l0 {
@@ -136,6 +157,10 @@ &cpu_b1 {
operating-points-v2 = <&cluster1_opp>;
};

+&dmc {
+ operating-points-v2 = <&dmc_opp_table>;
+};
+
&gpu {
operating-points-v2 = <&gpu_opp_table>;
};
--
2.35.0.rc0.227.g00780c9af4-goog

2022-01-28 22:22:14

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 02/15] dt-bindings: devfreq: rk3399_dmc: Deprecate unused/redundant properties

On Thu, Jan 27, 2022 at 3:08 PM Brian Norris <[email protected]> wrote:
>
> These DRAM configuration properties are all handled in ARM Trusted
> Firmware (and have been since the early days of this SoC), and there are
> no in-tree users of the DMC binding yet. It's better to just defer to
> firmware instead of maintaining this large list of properties.
>
> There's also some confusion about units: many of these are specified in
> MHz, but the downstream users and driver code are treating them as Hz, I
> believe. Rather than straighten all that out, I just drop them.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> (no changes since v1)

Apologies, I didn't include Rob's Reviewed-by tag on patch 2 and 3. If
this goes for version 3, I'll include them.

Brian

2022-02-04 09:27:12

by Chanwoo Choi

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] dt-bindings: devfreq: rk3399_dmc: Fix Hz units

On 1/28/22 8:07 AM, Brian Norris wrote:
> The driver and all downstream device trees [1] are using Hz units, but
> the document claims MHz. DRAM frequency for these systems can't possibly
> exceed 2^32-1 Hz, so the choice of unit doesn't really matter than much.
>
> Rather than add unnecessary risk in getting the units wrong, let's just
> go with the unofficial convention and make the docs match reality.
>
> A sub-1MHz frequency is extremely unlikely, so include a minimum in the
> schema, to help catch anybody who might have believed this was MHz.
>
> [1] And notably, also those trying to upstream them:
> https://protect2.fireeye.com/v1/url?k=0a7de78e-55e6dec8-0a7c6cc1-0cc47a3003e8-4f0969a9fa7b496e&q=1&e=6129c5df-8bd2-4072-86ef-79b79b36ec89&u=https%3A%2F%2Flore.kernel.org%2Flkml%2F20210308233858.24741-3-daniel.lezcano%40linaro.org%2F
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> (no changes since v1)
>
> .../bindings/devfreq/rk3399_dmc.yaml | 24 +++++++++----------
> 1 file changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> index fd62a8cd62d5..8bb778df92ae 100644
> --- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> @@ -116,11 +116,11 @@ properties:
>
> rockchip,ddr3_odt_dis_freq:
> $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1000000 # In case anyone thought this was MHz.
> description:
> When the DRAM type is DDR3, this parameter defines the ODT disable
> - frequency in MHz (Mega Hz). When the DDR frequency is less then
> - ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> - disabled.
> + frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
> + the ODT on the DRAM side and controller side are both disabled.
>
> rockchip,ddr3_drv:
> deprecated: true
> @@ -160,11 +160,11 @@ properties:
>
> rockchip,lpddr3_odt_dis_freq:
> $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1000000 # In case anyone thought this was MHz.
> description:
> When the DRAM type is LPDDR3, this parameter defines then ODT disable
> - frequency in MHz (Mega Hz). When DDR frequency is less then
> - ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> - disabled.
> + frequency in Hz. When DDR frequency is less then ddr3_odt_dis_freq, the
> + ODT on the DRAM side and controller side are both disabled.
>
> rockchip,lpddr3_drv:
> deprecated: true
> @@ -204,11 +204,11 @@ properties:
>
> rockchip,lpddr4_odt_dis_freq:
> $ref: /schemas/types.yaml#/definitions/uint32
> + minimum: 1000000 # In case anyone thought this was MHz.
> description:
> When the DRAM type is LPDDR4, this parameter defines the ODT disable
> - frequency in MHz (Mega Hz). When the DDR frequency is less then
> - ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> - disabled.
> + frequency in Hz. When the DDR frequency is less then ddr3_odt_dis_freq,
> + the ODT on the DRAM side and controller side are both disabled.
>
> rockchip,lpddr4_drv:
> deprecated: true
> @@ -287,7 +287,7 @@ examples:
> rockchip,sr_mc_gate_idle = <0x3>;
> rockchip,srpd_lite_idle = <0x4>;
> rockchip,standby_idle = <0x2000>;
> - rockchip,ddr3_odt_dis_freq = <333>;
> - rockchip,lpddr3_odt_dis_freq = <333>;
> - rockchip,lpddr4_odt_dis_freq = <333>;
> + rockchip,ddr3_odt_dis_freq = <333000000>;
> + rockchip,lpddr3_odt_dis_freq = <333000000>;
> + rockchip,lpddr4_odt_dis_freq = <333000000>;
> };
>

Acked-by: Chanwoo Choi <[email protected]>

--
Best Regards,
Chanwoo Choi
Samsung Electronics

2022-02-09 23:25:50

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] dt-bindings: devfreq: rk3399_dmc: Convert to YAML

On Thu, Jan 27, 2022 at 03:07:12PM -0800, Brian Norris wrote:
> I want to add, deprecate, and bugfix some properties, as well as add the
> first users. This is easier with a proper schema.
>
> The transformation is mostly straightforward, plus a few notable tweaks:
>
> * Renamed rockchip,dram_speed_bin to rockchip,ddr3_speed_bin. The
> driver code and the example matched, but the description was
> different. I went with the implementation.
>
> * Drop upthreshold and downdifferential properties from the example.
> These were undocumented (so, wouldn't pass validation), but were
> representing software properties (governor tweaks). I drop them from
> the driver in subsequent patches.
>
> * Rename clock from pclk_ddr_mon to dmc_clk. The driver, DT example,
> and all downstream users matched -- the binding definition was the
> exception. Anyway, "dmc_clk" is a more appropriately generic name.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> Changes in v2:
> * rename to 'memory-controller' in example
> * place 'required' after properties
> * drop superluous free-form references and repetitions of other
> bindings
> * fix for yamllint
>
> .../bindings/devfreq/rk3399_dmc.txt | 212 -------------
> .../bindings/devfreq/rk3399_dmc.yaml | 293 ++++++++++++++++++
> 2 files changed, 293 insertions(+), 212 deletions(-)
> delete mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
> create mode 100644 Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
>
> diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
> deleted file mode 100644
> index 58fc8a6cebc7..000000000000
> --- a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.txt
> +++ /dev/null
> @@ -1,212 +0,0 @@
> -* Rockchip rk3399 DMC (Dynamic Memory Controller) device
> -
> -Required properties:
> -- compatible: Must be "rockchip,rk3399-dmc".
> -- devfreq-events: Node to get DDR loading, Refer to
> - Documentation/devicetree/bindings/devfreq/event/
> - rockchip-dfi.txt
> -- clocks: Phandles for clock specified in "clock-names" property
> -- clock-names : The name of clock used by the DFI, must be
> - "pclk_ddr_mon";
> -- operating-points-v2: Refer to Documentation/devicetree/bindings/opp/opp-v2.yaml
> - for details.
> -- center-supply: DMC supply node.
> -- status: Marks the node enabled/disabled.
> -- rockchip,pmu: Phandle to the syscon managing the "PMU general register
> - files".
> -
> -Optional properties:
> -- interrupts: The CPU interrupt number. The interrupt specifier
> - format depends on the interrupt controller.
> - It should be a DCF interrupt. When DDR DVFS finishes
> - a DCF interrupt is triggered.
> -- rockchip,pmu: Phandle to the syscon managing the "PMU general register
> - files".
> -
> -Following properties relate to DDR timing:
> -
> -- rockchip,dram_speed_bin : Value reference include/dt-bindings/clock/rk3399-ddr.h,
> - it selects the DDR3 cl-trp-trcd type. It must be
> - set according to "Speed Bin" in DDR3 datasheet,
> - DO NOT use a smaller "Speed Bin" than specified
> - for the DDR3 being used.
> -
> -- rockchip,pd_idle : Configure the PD_IDLE value. Defines the
> - power-down idle period in which memories are
> - placed into power-down mode if bus is idle
> - for PD_IDLE DFI clock cycles.
> -
> -- rockchip,sr_idle : Configure the SR_IDLE value. Defines the
> - self-refresh idle period in which memories are
> - placed into self-refresh mode if bus is idle
> - for SR_IDLE * 1024 DFI clock cycles (DFI
> - clocks freq is half of DRAM clock), default
> - value is "0".
> -
> -- rockchip,sr_mc_gate_idle : Defines the memory self-refresh and controller
> - clock gating idle period. Memories are placed
> - into self-refresh mode and memory controller
> - clock arg gating started if bus is idle for
> - sr_mc_gate_idle*1024 DFI clock cycles.
> -
> -- rockchip,srpd_lite_idle : Defines the self-refresh power down idle
> - period in which memories are placed into
> - self-refresh power down mode if bus is idle
> - for srpd_lite_idle * 1024 DFI clock cycles.
> - This parameter is for LPDDR4 only.
> -
> -- rockchip,standby_idle : Defines the standby idle period in which
> - memories are placed into self-refresh mode.
> - The controller, pi, PHY and DRAM clock will
> - be gated if bus is idle for standby_idle * DFI
> - clock cycles.
> -
> -- rockchip,dram_dll_dis_freq : Defines the DDR3 DLL bypass frequency in MHz.
> - When DDR frequency is less than DRAM_DLL_DISB_FREQ,
> - DDR3 DLL will be bypassed. Note: if DLL was bypassed,
> - the odt will also stop working.
> -
> -- rockchip,phy_dll_dis_freq : Defines the PHY dll bypass frequency in
> - MHz (Mega Hz). When DDR frequency is less than
> - DRAM_DLL_DISB_FREQ, PHY DLL will be bypassed.
> - Note: PHY DLL and PHY ODT are independent.
> -
> -- rockchip,ddr3_odt_dis_freq : When the DRAM type is DDR3, this parameter defines
> - the ODT disable frequency in MHz (Mega Hz).
> - when the DDR frequency is less then ddr3_odt_dis_freq,
> - the ODT on the DRAM side and controller side are
> - both disabled.
> -
> -- rockchip,ddr3_drv : When the DRAM type is DDR3, this parameter defines
> - the DRAM side driver strength in ohms. Default
> - value is 40.
> -
> -- rockchip,ddr3_odt : When the DRAM type is DDR3, this parameter defines
> - the DRAM side ODT strength in ohms. Default value
> - is 120.
> -
> -- rockchip,phy_ddr3_ca_drv : When the DRAM type is DDR3, this parameter defines
> - the phy side CA line (incluing command line,
> - address line and clock line) driver strength.
> - Default value is 40.
> -
> -- rockchip,phy_ddr3_dq_drv : When the DRAM type is DDR3, this parameter defines
> - the PHY side DQ line (including DQS/DQ/DM line)
> - driver strength. Default value is 40.
> -
> -- rockchip,phy_ddr3_odt : When the DRAM type is DDR3, this parameter defines
> - the PHY side ODT strength. Default value is 240.
> -
> -- rockchip,lpddr3_odt_dis_freq : When the DRAM type is LPDDR3, this parameter defines
> - then ODT disable frequency in MHz (Mega Hz).
> - When DDR frequency is less then ddr3_odt_dis_freq,
> - the ODT on the DRAM side and controller side are
> - both disabled.
> -
> -- rockchip,lpddr3_drv : When the DRAM type is LPDDR3, this parameter defines
> - the DRAM side driver strength in ohms. Default
> - value is 34.
> -
> -- rockchip,lpddr3_odt : When the DRAM type is LPDDR3, this parameter defines
> - the DRAM side ODT strength in ohms. Default value
> - is 240.
> -
> -- rockchip,phy_lpddr3_ca_drv : When the DRAM type is LPDDR3, this parameter defines
> - the PHY side CA line (including command line,
> - address line and clock line) driver strength.
> - Default value is 40.
> -
> -- rockchip,phy_lpddr3_dq_drv : When the DRAM type is LPDDR3, this parameter defines
> - the PHY side DQ line (including DQS/DQ/DM line)
> - driver strength. Default value is 40.
> -
> -- rockchip,phy_lpddr3_odt : When dram type is LPDDR3, this parameter define
> - the phy side odt strength, default value is 240.
> -
> -- rockchip,lpddr4_odt_dis_freq : When the DRAM type is LPDDR4, this parameter
> - defines the ODT disable frequency in
> - MHz (Mega Hz). When the DDR frequency is less then
> - ddr3_odt_dis_freq, the ODT on the DRAM side and
> - controller side are both disabled.
> -
> -- rockchip,lpddr4_drv : When the DRAM type is LPDDR4, this parameter defines
> - the DRAM side driver strength in ohms. Default
> - value is 60.
> -
> -- rockchip,lpddr4_dq_odt : When the DRAM type is LPDDR4, this parameter defines
> - the DRAM side ODT on DQS/DQ line strength in ohms.
> - Default value is 40.
> -
> -- rockchip,lpddr4_ca_odt : When the DRAM type is LPDDR4, this parameter defines
> - the DRAM side ODT on CA line strength in ohms.
> - Default value is 40.
> -
> -- rockchip,phy_lpddr4_ca_drv : When the DRAM type is LPDDR4, this parameter defines
> - the PHY side CA line (including command address
> - line) driver strength. Default value is 40.
> -
> -- rockchip,phy_lpddr4_ck_cs_drv : When the DRAM type is LPDDR4, this parameter defines
> - the PHY side clock line and CS line driver
> - strength. Default value is 80.
> -
> -- rockchip,phy_lpddr4_dq_drv : When the DRAM type is LPDDR4, this parameter defines
> - the PHY side DQ line (including DQS/DQ/DM line)
> - driver strength. Default value is 80.
> -
> -- rockchip,phy_lpddr4_odt : When the DRAM type is LPDDR4, this parameter defines
> - the PHY side ODT strength. Default value is 60.
> -
> -Example:
> - dmc_opp_table: dmc_opp_table {
> - compatible = "operating-points-v2";
> -
> - opp00 {
> - opp-hz = /bits/ 64 <300000000>;
> - opp-microvolt = <900000>;
> - };
> - opp01 {
> - opp-hz = /bits/ 64 <666000000>;
> - opp-microvolt = <900000>;
> - };
> - };
> -
> - dmc: dmc {
> - compatible = "rockchip,rk3399-dmc";
> - devfreq-events = <&dfi>;
> - interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> - clocks = <&cru SCLK_DDRC>;
> - clock-names = "dmc_clk";
> - operating-points-v2 = <&dmc_opp_table>;
> - center-supply = <&ppvar_centerlogic>;
> - upthreshold = <15>;
> - downdifferential = <10>;
> - rockchip,ddr3_speed_bin = <21>;
> - rockchip,pd_idle = <0x40>;
> - rockchip,sr_idle = <0x2>;
> - rockchip,sr_mc_gate_idle = <0x3>;
> - rockchip,srpd_lite_idle = <0x4>;
> - rockchip,standby_idle = <0x2000>;
> - rockchip,dram_dll_dis_freq = <300>;
> - rockchip,phy_dll_dis_freq = <125>;
> - rockchip,auto_pd_dis_freq = <666>;
> - rockchip,ddr3_odt_dis_freq = <333>;
> - rockchip,ddr3_drv = <40>;
> - rockchip,ddr3_odt = <120>;
> - rockchip,phy_ddr3_ca_drv = <40>;
> - rockchip,phy_ddr3_dq_drv = <40>;
> - rockchip,phy_ddr3_odt = <240>;
> - rockchip,lpddr3_odt_dis_freq = <333>;
> - rockchip,lpddr3_drv = <34>;
> - rockchip,lpddr3_odt = <240>;
> - rockchip,phy_lpddr3_ca_drv = <40>;
> - rockchip,phy_lpddr3_dq_drv = <40>;
> - rockchip,phy_lpddr3_odt = <240>;
> - rockchip,lpddr4_odt_dis_freq = <333>;
> - rockchip,lpddr4_drv = <60>;
> - rockchip,lpddr4_dq_odt = <40>;
> - rockchip,lpddr4_ca_odt = <40>;
> - rockchip,phy_lpddr4_ca_drv = <40>;
> - rockchip,phy_lpddr4_ck_cs_drv = <80>;
> - rockchip,phy_lpddr4_dq_drv = <80>;
> - rockchip,phy_lpddr4_odt = <60>;
> - };
> diff --git a/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> new file mode 100644
> index 000000000000..467a7b5b374b
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> @@ -0,0 +1,293 @@
> +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> +# %YAML 1.2
> +---
> +$id: http://devicetree.org/schemas/devfreq/rk3399_dmc.yaml#
> +$schema: http://devicetree.org/meta-schemas/core.yaml#
> +
> +title: Rockchip rk3399 DMC (Dynamic Memory Controller) device
> +
> +maintainers:
> + - Brian Norris <[email protected]>
> +
> +properties:
> + compatible:
> + enum:
> + - rockchip,rk3399-dmc
> +
> + devfreq-events:
> + $ref: /schemas/types.yaml#/definitions/phandle-array
> + minItems: 1

What's the max?

If this is just phandles (no arg cells), then you need:

items:
maxItems: 1

IOW, fully describe the number of entries and cells for each entry.

> + description:
> + Node to get DDR loading. Refer to
> + Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt.
> +
> + clocks:
> + maxItems: 1
> +
> + clock-names:
> + items:
> + - const: dmc_clk
> +
> + operating-points-v2: true
> +
> + center-supply:
> + description:
> + DMC regulator supply.
> +
> + rockchip,pmu:
> + $ref: /schemas/types.yaml#/definitions/phandle
> + description:
> + Phandle to the syscon managing the "PMU general register files".
> +
> + interrupts:
> + maxItems: 1
> + description:
> + The CPU interrupt number. It should be a DCF interrupt. When DDR DVFS
> + finishes, a DCF interrupt is triggered.
> +
> + rockchip,ddr3_speed_bin:

Since you are changing this, s/_/-/

> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + For values, reference include/dt-bindings/clock/rk3399-ddr.h. Selects the
> + DDR3 cl-trp-trcd type. It must be set according to "Speed Bin" in DDR3
> + datasheet; DO NOT use a smaller "Speed Bin" than specified for the DDR3
> + being used.
> +
> + rockchip,pd_idle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Configure the PD_IDLE value. Defines the power-down idle period in which
> + memories are placed into power-down mode if bus is idle for PD_IDLE DFI
> + clock cycles.
> +
> + rockchip,sr_idle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Configure the SR_IDLE value. Defines the self-refresh idle period in
> + which memories are placed into self-refresh mode if bus is idle for
> + SR_IDLE * 1024 DFI clock cycles (DFI clocks freq is half of DRAM clock).
> + Default value is "0".
> +
> + rockchip,sr_mc_gate_idle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Defines the memory self-refresh and controller clock gating idle period.
> + Memories are placed into self-refresh mode and memory controller clock
> + arg gating started if bus is idle for sr_mc_gate_idle*1024 DFI clock
> + cycles.
> +
> + rockchip,srpd_lite_idle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Defines the self-refresh power down idle period in which memories are
> + placed into self-refresh power down mode if bus is idle for
> + srpd_lite_idle * 1024 DFI clock cycles. This parameter is for LPDDR4
> + only.
> +
> + rockchip,standby_idle:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Defines the standby idle period in which memories are placed into
> + self-refresh mode. The controller, pi, PHY and DRAM clock will be gated
> + if bus is idle for standby_idle * DFI clock cycles.
> +
> + rockchip,dram_dll_dis_freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Defines the DDR3 DLL bypass frequency in MHz. When DDR frequency is less
> + than DRAM_DLL_DISB_FREQ, DDR3 DLL will be bypassed.
> + Note: if DLL was bypassed, the odt will also stop working.
> +
> + rockchip,phy_dll_dis_freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description: |
> + Defines the PHY dll bypass frequency in MHz (Mega Hz). When DDR frequency
> + is less than DRAM_DLL_DISB_FREQ, PHY DLL will be bypassed.
> + Note: PHY DLL and PHY ODT are independent.
> +
> + rockchip,auto_pd_dis_freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + Defines the auto PD disable frequency in MHz.
> +
> + rockchip,ddr3_odt_dis_freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is DDR3, this parameter defines the ODT disable
> + frequency in MHz (Mega Hz). When the DDR frequency is less then
> + ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> + disabled.
> +
> + rockchip,ddr3_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is DDR3, this parameter defines the DRAM side drive
> + strength in ohms. Default value is 40.
> +
> + rockchip,ddr3_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is DDR3, this parameter defines the DRAM side ODT
> + strength in ohms. Default value is 120.
> +
> + rockchip,phy_ddr3_ca_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is DDR3, this parameter defines the phy side CA line
> + (incluing command line, address line and clock line) drive strength.
> + Default value is 40.
> +
> + rockchip,phy_ddr3_dq_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is DDR3, this parameter defines the PHY side DQ line
> + (including DQS/DQ/DM line) drive strength. Default value is 40.
> +
> + rockchip,phy_ddr3_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is DDR3, this parameter defines the PHY side ODT
> + strength. Default value is 240.
> +
> + rockchip,lpddr3_odt_dis_freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR3, this parameter defines then ODT disable
> + frequency in MHz (Mega Hz). When DDR frequency is less then
> + ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> + disabled.
> +
> + rockchip,lpddr3_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR3, this parameter defines the DRAM side drive
> + strength in ohms. Default value is 34.
> +
> + rockchip,lpddr3_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR3, this parameter defines the DRAM side ODT
> + strength in ohms. Default value is 240.
> +
> + rockchip,phy_lpddr3_ca_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR3, this parameter defines the PHY side CA line
> + (including command line, address line and clock line) drive strength.
> + Default value is 40.
> +
> + rockchip,phy_lpddr3_dq_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR3, this parameter defines the PHY side DQ line
> + (including DQS/DQ/DM line) drive strength. Default value is 40.
> +
> + rockchip,phy_lpddr3_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When dram type is LPDDR3, this parameter define the phy side odt
> + strength, default value is 240.
> +
> + rockchip,lpddr4_odt_dis_freq:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the ODT disable
> + frequency in MHz (Mega Hz). When the DDR frequency is less then
> + ddr3_odt_dis_freq, the ODT on the DRAM side and controller side are both
> + disabled.
> +
> + rockchip,lpddr4_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the DRAM side drive
> + strength in ohms. Default value is 60.
> +
> + rockchip,lpddr4_dq_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the DRAM side ODT on
> + DQS/DQ line strength in ohms. Default value is 40.
> +
> + rockchip,lpddr4_ca_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the DRAM side ODT on
> + CA line strength in ohms. Default value is 40.
> +
> + rockchip,phy_lpddr4_ca_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the PHY side CA line
> + (including command address line) drive strength. Default value is 40.
> +
> + rockchip,phy_lpddr4_ck_cs_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the PHY side clock
> + line and CS line drive strength. Default value is 80.
> +
> + rockchip,phy_lpddr4_dq_drv:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the PHY side DQ line
> + (including DQS/DQ/DM line) drive strength. Default value is 80.
> +
> + rockchip,phy_lpddr4_odt:
> + $ref: /schemas/types.yaml#/definitions/uint32
> + description:
> + When the DRAM type is LPDDR4, this parameter defines the PHY side ODT
> + strength. Default value is 60.
> +
> +required:
> + - compatible
> + - devfreq-events
> + - clocks
> + - clock-names
> + - operating-points-v2
> + - center-supply
> +
> +additionalProperties: false
> +
> +examples:
> + - |
> + #include <dt-bindings/clock/rk3399-cru.h>
> + #include <dt-bindings/interrupt-controller/arm-gic.h>
> + memory-controller {
> + compatible = "rockchip,rk3399-dmc";
> + devfreq-events = <&dfi>;
> + rockchip,pmu = <&pmu>;
> + interrupts = <GIC_SPI 1 IRQ_TYPE_LEVEL_HIGH>;
> + clocks = <&cru SCLK_DDRC>;
> + clock-names = "dmc_clk";
> + operating-points-v2 = <&dmc_opp_table>;
> + center-supply = <&ppvar_centerlogic>;
> + rockchip,ddr3_speed_bin = <21>;
> + rockchip,pd_idle = <0x40>;
> + rockchip,sr_idle = <0x2>;
> + rockchip,sr_mc_gate_idle = <0x3>;
> + rockchip,srpd_lite_idle = <0x4>;
> + rockchip,standby_idle = <0x2000>;
> + rockchip,dram_dll_dis_freq = <300>;
> + rockchip,phy_dll_dis_freq = <125>;
> + rockchip,auto_pd_dis_freq = <666>;
> + rockchip,ddr3_odt_dis_freq = <333>;
> + rockchip,ddr3_drv = <40>;
> + rockchip,ddr3_odt = <120>;
> + rockchip,phy_ddr3_ca_drv = <40>;
> + rockchip,phy_ddr3_dq_drv = <40>;
> + rockchip,phy_ddr3_odt = <240>;
> + rockchip,lpddr3_odt_dis_freq = <333>;
> + rockchip,lpddr3_drv = <34>;
> + rockchip,lpddr3_odt = <240>;
> + rockchip,phy_lpddr3_ca_drv = <40>;
> + rockchip,phy_lpddr3_dq_drv = <40>;
> + rockchip,phy_lpddr3_odt = <240>;
> + rockchip,lpddr4_odt_dis_freq = <333>;
> + rockchip,lpddr4_drv = <60>;
> + rockchip,lpddr4_dq_odt = <40>;
> + rockchip,lpddr4_ca_odt = <40>;
> + rockchip,phy_lpddr4_ca_drv = <40>;
> + rockchip,phy_lpddr4_ck_cs_drv = <80>;
> + rockchip,phy_lpddr4_dq_drv = <80>;
> + rockchip,phy_lpddr4_odt = <60>;
> + };
> --
> 2.35.0.rc0.227.g00780c9af4-goog
>
>

2022-02-09 23:33:38

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 05/15] dt-bindings: devfreq: rk3399_dmc: Add more disable-freq properties

On Thu, 27 Jan 2022 15:07:16 -0800, Brian Norris wrote:
> DDR DVFS tuning has found that several power-saving features don't have
> good tradeoffs at higher frequencies -- at higher frequencies, we'll see
> glitches or other errors. Provide tuning controls so these can be
> disabled at higher OPPs, and left active only at the lower ones.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> Changes in v2:
> - hyphens, not underscores
> - *-hz units, and drop the types definition
>
> .../bindings/devfreq/rk3399_dmc.yaml | 37 +++++++++++++++++++
> 1 file changed, 37 insertions(+)
>

Reviewed-by: Rob Herring <[email protected]>

2022-02-09 23:46:01

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 03/15] dt-bindings: devfreq: rk3399_dmc: Fix Hz units

On Thu, Jan 27, 2022 at 03:07:14PM -0800, Brian Norris wrote:
> The driver and all downstream device trees [1] are using Hz units, but
> the document claims MHz. DRAM frequency for these systems can't possibly
> exceed 2^32-1 Hz, so the choice of unit doesn't really matter than much.
>
> Rather than add unnecessary risk in getting the units wrong, let's just
> go with the unofficial convention and make the docs match reality.
>
> A sub-1MHz frequency is extremely unlikely, so include a minimum in the
> schema, to help catch anybody who might have believed this was MHz.
>
> [1] And notably, also those trying to upstream them:
> https://lore.kernel.org/lkml/[email protected]/
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> (no changes since v1)
>
> .../bindings/devfreq/rk3399_dmc.yaml | 24 +++++++++----------
> 1 file changed, 12 insertions(+), 12 deletions(-)

Reviewed-by: Rob Herring <[email protected]>

2022-02-09 23:52:43

by Rob Herring (Arm)

[permalink] [raw]
Subject: Re: [PATCH v2 04/15] dt-bindings: devfreq: rk3399_dmc: Specify idle params in nanoseconds

On Thu, 27 Jan 2022 15:07:15 -0800, Brian Norris wrote:
> It's inefficient to use the same number of cycles for all OPPs, since
> lower frequencies make for longer idle times. Let's specify the idle
> time instead, so software can pick the optimal number of cycles on its
> own.
>
> NB: these bindings aren't used anywhere yet.
>
> Signed-off-by: Brian Norris <[email protected]>
> ---
>
> Changes in v2:
> - New patch
>
> .../bindings/devfreq/rk3399_dmc.yaml | 50 +++++++++++++++++--
> 1 file changed, 45 insertions(+), 5 deletions(-)
>

Reviewed-by: Rob Herring <[email protected]>

2022-03-04 17:36:08

by Peter Geis

[permalink] [raw]
Subject: Re: [PATCH v2 12/15] arm64: dts: rockchip: Enable dmc and dfi nodes on gru

On Thu, Jan 27, 2022 at 6:17 PM Brian Norris <[email protected]> wrote:
>
> From: Lin Huang <[email protected]>
>
> Enable the DMC (Dynamic Memory Controller) and the DFI (DDR PHY
> Interface) nodes on gru boards so we can support DDR DVFS.
>
> Signed-off-by: Lin Huang <[email protected]>
> Signed-off-by: Enric Balletbo i Serra <[email protected]>
> Signed-off-by: GaĆ«l PORTAY <[email protected]>
> Signed-off-by: Daniel Lezcano <[email protected]>
> Signed-off-by: Brian Norris <[email protected]>
> Updates since the old series:
>
> - reordered alphabetically by phandle name, per style
> - drop a ton of deprecated/unused properties
> - add required center-supply for scarlet
> - add new *_idle_dis_freq properties
> - drop the lowest (200 MHz) OPP; this was never stabilized for
> production
> - bump the voltage (0.9V -> 0.925V) for the highest OPP on Chromebook
> models; later (tablet) models were more stable, with a fixed DDR
> regulator
> - bump odt_dis_freq to 666 MHz; early versions used 333 MHz, but
> stabilization efforts landed on 666 MHz for production
>
> ---
>
> Changes in v2:
> - Adapt to new properties
>
> Changes in v1:
> This was part of a previous series, at:
> https://lore.kernel.org/r/[email protected]
> I've picked up a bunch of changes and fixes, so I've restarted the patch
> series numbering.

Good Morning,

I'm trying to bring this series over to rockpro64 (and eventually the
pinephone-pro) and am running into some snags.
Essentially, anytime a transition happens, the board locks up.
I've disabled the extra power save disable flags and adjusted the OPPs
for rockpro64's power.
Transitions anywhere from the default 800mhz cause a lock.

I'm digging deeper, but I'm hoping you can answer some questions in
the meantime:
1. Does this require something from firmware that isn't available on
Mainline ATF? (AKA special firmware to the Chromebook line)
2. If not, do you have any recommendations off the top of your head?

Thanks,
Peter Geis

>
> .../dts/rockchip/rk3399-gru-chromebook.dtsi | 7 +++++
> .../boot/dts/rockchip/rk3399-gru-scarlet.dtsi | 12 ++++++++
> arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi | 28 +++++++++++++++++++
> .../boot/dts/rockchip/rk3399-op1-opp.dtsi | 25 +++++++++++++++++
> 4 files changed, 72 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> index 9b2c679f5eca..cc8950046d94 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-chromebook.dtsi
> @@ -234,6 +234,13 @@ &cdn_dp {
> extcon = <&usbc_extcon0>, <&usbc_extcon1>;
> };
>
> +&dmc {
> + center-supply = <&ppvar_centerlogic>;
> + rockchip,pd-idle-dis-freq-hz = <800000000>;
> + rockchip,sr-idle-dis-freq-hz = <800000000>;
> + rockchip,sr-mc-gate-idle-dis-freq-hz = <800000000>;
> +};
> +
> &edp {
> status = "okay";
>
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> index a9817b3d7edc..913d845eb51a 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru-scarlet.dtsi
> @@ -391,6 +391,18 @@ &cru {
> <400000000>;
> };
>
> +/* The center supply is fixed to .9V on scarlet */
> +&dmc {
> + center-supply = <&pp900_s0>;
> +};
> +
> +/* We don't need .925 V for 928 MHz on scarlet */
> +&dmc_opp_table {
> + opp03 {
> + opp-microvolt = <900000>;
> + };
> +};
> +
> &gpio0 {
> gpio-line-names = /* GPIO0 A 0-7 */
> "CLK_32K_AP",
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> index 162f08bca0d4..23bfba86daab 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-gru.dtsi
> @@ -373,6 +373,34 @@ &cru {
> <200000000>;
> };
>
> +&dfi {
> + status = "okay";
> +};
> +
> +&dmc {
> + status = "okay";
> +
> + rockchip,pd-idle-ns = <160>;
> + rockchip,sr-idle-ns = <10240>;
> + rockchip,sr-mc-gate-idle-ns = <40960>;
> + rockchip,srpd-lite-idle-ns = <61440>;
> + rockchip,standby-idle-ns = <81920>;
> +
> + rockchip,ddr3_odt_dis_freq = <666000000>;
> + rockchip,lpddr3_odt_dis_freq = <666000000>;
> + rockchip,lpddr4_odt_dis_freq = <666000000>;
> +
> + rockchip,sr-mc-gate-idle-dis-freq-hz = <1000000000>;
> + rockchip,srpd-lite-idle-dis-freq-hz = <0>;
> + rockchip,standby-idle-dis-freq-hz = <928000000>;
> +};
> +
> +&dmc_opp_table {
> + opp03 {
> + opp-suspend;
> + };
> +};
> +
> &emmc_phy {
> status = "okay";
> };
> diff --git a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> index 2180e0f75003..6e29e74f6fc6 100644
> --- a/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> +++ b/arch/arm64/boot/dts/rockchip/rk3399-op1-opp.dtsi
> @@ -110,6 +110,27 @@ opp05 {
> opp-microvolt = <1075000>;
> };
> };
> +
> + dmc_opp_table: dmc_opp_table {
> + compatible = "operating-points-v2";
> +
> + opp00 {
> + opp-hz = /bits/ 64 <400000000>;
> + opp-microvolt = <900000>;
> + };
> + opp01 {
> + opp-hz = /bits/ 64 <666000000>;
> + opp-microvolt = <900000>;
> + };
> + opp02 {
> + opp-hz = /bits/ 64 <800000000>;
> + opp-microvolt = <900000>;
> + };
> + opp03 {
> + opp-hz = /bits/ 64 <928000000>;
> + opp-microvolt = <925000>;
> + };
> + };
> };
>
> &cpu_l0 {
> @@ -136,6 +157,10 @@ &cpu_b1 {
> operating-points-v2 = <&cluster1_opp>;
> };
>
> +&dmc {
> + operating-points-v2 = <&dmc_opp_table>;
> +};
> +
> &gpu {
> operating-points-v2 = <&gpu_opp_table>;
> };
> --
> 2.35.0.rc0.227.g00780c9af4-goog
>
>
> _______________________________________________
> Linux-rockchip mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-rockchip

2022-03-04 21:25:31

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 12/15] arm64: dts: rockchip: Enable dmc and dfi nodes on gru

Hi Peter,

On Fri, Mar 4, 2022 at 6:47 AM Peter Geis <[email protected]> wrote:
> I'm trying to bring this series over to rockpro64 (and eventually the
> pinephone-pro) and am running into some snags.

Dumb question: is DDR DVFS supported on these systems in the
"production" (vendor kernel? I'm not really familiar with these
devices) software? I partly ask, because while I didn't do some of
this first-hand, I'm aware that it was a real ordeal to get things
stabilized (e.g., getting all the timings right; communicating the
right details from bootloader to ATF; etc.), so if no one did these
things in the first place, it's probably difficult to get working now
just by guessing.

But if it works on a customized kernel, then that's a different story.

> Essentially, anytime a transition happens, the board locks up.
> I've disabled the extra power save disable flags and adjusted the OPPs
> for rockpro64's power.
> Transitions anywhere from the default 800mhz cause a lock.
>
> I'm digging deeper, but I'm hoping you can answer some questions in
> the meantime:
> 1. Does this require something from firmware that isn't available on
> Mainline ATF? (AKA special firmware to the Chromebook line)

I don't know precisely. Chromebooks track mainline ATF, but somewhere
before initial product launch, each platform gets its own firmware
branch. On that firmware branch, we still try to stay in sync with
mainline to some extent (e.g., submit to mainline and cherry-pick to
branch), but it's not guaranteed.

Anyway, you can inspect our code here:
https://chromium.googlesource.com/chromiumos/third_party/arm-trusted-firmware/+log/refs/heads/firmware-gru-8785.B
https://chromium.googlesource.com/chromiumos/third_party/coreboot/+log/refs/heads/firmware-gru-8785.B

Brian

2022-03-04 21:46:11

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 01/15] dt-bindings: devfreq: rk3399_dmc: Convert to YAML

On Wed, Feb 09, 2022 at 02:17:33PM -0600, Rob Herring wrote:
> On Thu, Jan 27, 2022 at 03:07:12PM -0800, Brian Norris wrote:
> > I want to add, deprecate, and bugfix some properties, as well as add the
> > first users. This is easier with a proper schema.
> >
> > The transformation is mostly straightforward, plus a few notable tweaks:
> >
> > * Renamed rockchip,dram_speed_bin to rockchip,ddr3_speed_bin. The
> > driver code and the example matched, but the description was
> > different. I went with the implementation.

...

> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/devfreq/rk3399_dmc.yaml
> > @@ -0,0 +1,293 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
> > +# %YAML 1.2
> > +---
> > +$id: http://devicetree.org/schemas/devfreq/rk3399_dmc.yaml#
> > +$schema: http://devicetree.org/meta-schemas/core.yaml#
> > +
> > +title: Rockchip rk3399 DMC (Dynamic Memory Controller) device
> > +
> > +maintainers:
> > + - Brian Norris <[email protected]>
> > +
> > +properties:
> > + compatible:
> > + enum:
> > + - rockchip,rk3399-dmc
> > +
> > + devfreq-events:
> > + $ref: /schemas/types.yaml#/definitions/phandle-array
> > + minItems: 1
>
> What's the max?
>
> If this is just phandles (no arg cells), then you need:
>
> items:
> maxItems: 1
>
> IOW, fully describe the number of entries and cells for each entry.

We only need 1, with no args. Will add |maxItems|.

> > + description:
> > + Node to get DDR loading. Refer to
> > + Documentation/devicetree/bindings/devfreq/event/rockchip-dfi.txt.
> > +
> > + clocks:
> > + maxItems: 1
> > +
> > + clock-names:
> > + items:
> > + - const: dmc_clk
> > +
> > + operating-points-v2: true
> > +
> > + center-supply:
> > + description:
> > + DMC regulator supply.
> > +
> > + rockchip,pmu:
> > + $ref: /schemas/types.yaml#/definitions/phandle
> > + description:
> > + Phandle to the syscon managing the "PMU general register files".
> > +
> > + interrupts:
> > + maxItems: 1
> > + description:
> > + The CPU interrupt number. It should be a DCF interrupt. When DDR DVFS
> > + finishes, a DCF interrupt is triggered.
> > +
> > + rockchip,ddr3_speed_bin:
>
> Since you are changing this, s/_/-/

I'm only including this because the driver already supports the
rockchip,ddr3_speed_bin spelling. But I'm also deprecating it (because
it's not really needed) and removing it later in the series. I'd rather
not change the spelling again in the middle, when it doesn't really have
any net effect.

I can add some clarifying notes in the commit message, about impending
deprecations, so this makes a little more sense as a standalone commit.

Or if it's somehow better, I can just drop the to-be-deprecated
properties right now in the .yaml conversion? As it happens, I've seen
at least one (probably more) other YAML conversion that made breaking
changes at the same time...

> > + $ref: /schemas/types.yaml#/definitions/uint32
> > + description:
> > + For values, reference include/dt-bindings/clock/rk3399-ddr.h. Selects the
> > + DDR3 cl-trp-trcd type. It must be set according to "Speed Bin" in DDR3
> > + datasheet; DO NOT use a smaller "Speed Bin" than specified for the DDR3
> > + being used.

Brian

2022-04-06 14:00:41

by Brian Norris

[permalink] [raw]
Subject: Re: [PATCH v2 12/15] arm64: dts: rockchip: Enable dmc and dfi nodes on gru

Hello again Peter,

On Fri, Mar 4, 2022 at 6:47 AM Peter Geis <[email protected]> wrote:
> Transitions anywhere from the default 800mhz cause a lock.
>
> I'm digging deeper, but I'm hoping you can answer some questions in
> the meantime:
> 1. Does this require something from firmware that isn't available on
> Mainline ATF? (AKA special firmware to the Chromebook line)
> 2. If not, do you have any recommendations off the top of your head?

I may have a better answer for you now. In the intervening time
period, I've discovered a potentially-relevant bug, involving
interactions between the kernel power-domain driver and ATF. See this
series for my current fixes:

https://lore.kernel.org/linux-rockchip/[email protected]/
[RFC PATCH 0/2] rockchip / devfreq: Coordinate DRAM controller
resources between ATF and kernel

If that happens to help you (it may help, for instance, if your system
was toggling NPLL off/on like mine was; it also may help if you're
hitting a race on PMU_BUS_IDLE_REQ like noticed in patch 1), I'd love
your feedback there.

It's still possible your problems are completely unrelated though.

Regards,
Brian

2022-04-07 03:26:40

by Peter Geis

[permalink] [raw]
Subject: Re: [PATCH v2 12/15] arm64: dts: rockchip: Enable dmc and dfi nodes on gru

On Tue, Apr 5, 2022 at 10:05 PM Brian Norris <[email protected]> wrote:
>
> Hello again Peter,
>
> On Fri, Mar 4, 2022 at 6:47 AM Peter Geis <[email protected]> wrote:
> > Transitions anywhere from the default 800mhz cause a lock.
> >
> > I'm digging deeper, but I'm hoping you can answer some questions in
> > the meantime:
> > 1. Does this require something from firmware that isn't available on
> > Mainline ATF? (AKA special firmware to the Chromebook line)
> > 2. If not, do you have any recommendations off the top of your head?
>
> I may have a better answer for you now. In the intervening time
> period, I've discovered a potentially-relevant bug, involving
> interactions between the kernel power-domain driver and ATF. See this
> series for my current fixes:
>
> https://lore.kernel.org/linux-rockchip/[email protected]/
> [RFC PATCH 0/2] rockchip / devfreq: Coordinate DRAM controller
> resources between ATF and kernel
>
> If that happens to help you (it may help, for instance, if your system
> was toggling NPLL off/on like mine was; it also may help if you're
> hitting a race on PMU_BUS_IDLE_REQ like noticed in patch 1), I'd love
> your feedback there.
>
> It's still possible your problems are completely unrelated though.

Thank you, that is certainly possible to be the problem here as well.
I won't have time to test them till this weekend, but you can count on
my feedback as soon as I get the time to do so.

>
> Regards,
> Brian