[PATCH 0/3] Apple PMGR fixup: Implement support for min-ps

[PATCH 0/3] Apple PMGR fixup: Implement support for min-ps

[Hector Martin]

Hi folks,

Just a brief add-on series to the PMGR submission, which adds support
for configuring the minimum power state for auto-PM mode. This is
apparently required to avoid breaking DCP (which isn't upstream yet).

This applies on top of the previous PMGR submission [1]

[1] https://lore.kernel.org/linux-arm-kernel/20211124073419.181799-1-marcan@marcan.st/

Hector Martin (3):
  dt-bindings: power: apple,pmgr-pwrstate: Add apple,min-ps prop
  soc: apple: apple-pmgr-pwrstate: Add auto-PM min level support
  arm64: dts: apple: t8103: Add apple,min-ps to DCP PMGR nodes

 .../devicetree/bindings/power/apple,pmgr-pwrstate.yaml    | 8 ++++++++
 arch/arm64/boot/dts/apple/t8103-pmgr.dtsi                 | 2 ++
 drivers/soc/apple/apple-pmgr-pwrstate.c                   | 8 ++++++++
 3 files changed, 18 insertions(+)

-- 
2.33.0

[PATCH 1/3] dt-bindings: power: apple,pmgr-pwrstate: Add apple,min-ps prop

[Hector Martin]

A few devices (DCP/DCPEXT) need to have the minimum power state for
auto-PM configured. Add a property that allows the DT to specify this
value.

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 .../devicetree/bindings/power/apple,pmgr-pwrstate.yaml    | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml b/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml
index 5056d08b8261..19a194980142 100644
--- a/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml
+++ b/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml
@@ -61,6 +61,14 @@ properties:
       Forces this power domain to always be powered up.
     type: boolean
 
+  apple,min-state:
+    description:
+      Specifies the minimum power state for auto-PM.
+      0 = power gated, 4 = clock gated, 15 = on.
+    $ref: /schemas/types.yaml#/definitions/uint32
+    minimum: 0
+    maximum: 15
+
 required:
   - compatible
   - reg
-- 
2.33.0

Re: [PATCH 1/3] dt-bindings: power: apple,pmgr-pwrstate: Add apple,min-ps prop

[Hector Martin]

On 09/12/2021 13.44, Hector Martin wrote:
> A few devices (DCP/DCPEXT) need to have the minimum power state for
> auto-PM configured. Add a property that allows the DT to specify this
> value.

Argh, just realized right after sending this that there is a mismatch 
between the commit description and the actual prop name (same with the 
third patch).

If everything else is good I'll be happy to fix the commit description 
before merging it.

> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>   .../devicetree/bindings/power/apple,pmgr-pwrstate.yaml    | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml b/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml
> index 5056d08b8261..19a194980142 100644
> --- a/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml
> +++ b/Documentation/devicetree/bindings/power/apple,pmgr-pwrstate.yaml
> @@ -61,6 +61,14 @@ properties:
>         Forces this power domain to always be powered up.
>       type: boolean
>   
> +  apple,min-state:
> +    description:
> +      Specifies the minimum power state for auto-PM.
> +      0 = power gated, 4 = clock gated, 15 = on.
> +    $ref: /schemas/types.yaml#/definitions/uint32
> +    minimum: 0
> +    maximum: 15
> +
>   required:
>     - compatible
>     - reg
> 


-- 
Hector Martin (marcan@marcan.st)
Public Key: https://mrcn.st/pub

Re: [PATCH 1/3] dt-bindings: power: apple,pmgr-pwrstate: Add apple,min-ps prop

[Rob Herring]

On Thu, 09 Dec 2021 13:44:59 +0900, Hector Martin wrote:
> A few devices (DCP/DCPEXT) need to have the minimum power state for
> auto-PM configured. Add a property that allows the DT to specify this
> value.
> 
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  .../devicetree/bindings/power/apple,pmgr-pwrstate.yaml    | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Reviewed-by: Rob Herring <robh@kernel.org>

[PATCH 2/3] soc: apple: apple-pmgr-pwrstate: Add auto-PM min level support

[Hector Martin]

This is seemingly required for DCP/DCPEXT, without which they refuse to
boot properly. They need to be set to minimum state 4 (clock gated).

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 drivers/soc/apple/apple-pmgr-pwrstate.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/soc/apple/apple-pmgr-pwrstate.c b/drivers/soc/apple/apple-pmgr-pwrstate.c
index 8ad9d5fdefbf..590bb7439372 100644
--- a/drivers/soc/apple/apple-pmgr-pwrstate.c
+++ b/drivers/soc/apple/apple-pmgr-pwrstate.c
@@ -20,6 +20,7 @@
 #define APPLE_PMGR_RESET        BIT(31)
 #define APPLE_PMGR_AUTO_ENABLE  BIT(28)
 #define APPLE_PMGR_PS_AUTO      GENMASK(27, 24)
+#define APPLE_PMGR_PS_MIN       GENMASK(19, 16)
 #define APPLE_PMGR_PARENT_OFF   BIT(11)
 #define APPLE_PMGR_DEV_DISABLE  BIT(10)
 #define APPLE_PMGR_WAS_CLKGATED BIT(9)
@@ -42,6 +43,7 @@ struct apple_pmgr_ps {
 	struct reset_controller_dev rcdev;
 	struct regmap *regmap;
 	u32 offset;
+	u32 min_state;
 };
 
 #define genpd_to_apple_pmgr_ps(_genpd) container_of(_genpd, struct apple_pmgr_ps, genpd)
@@ -224,6 +226,12 @@ static int apple_pmgr_ps_probe(struct platform_device *pdev)
 	ps->genpd.power_on = apple_pmgr_ps_power_on;
 	ps->genpd.power_off = apple_pmgr_ps_power_off;
 
+	ret = of_property_read_u32(node, "apple,min-state", &ps->min_state);
+	if (ret >= 0) {
+		regmap_update_bits(regmap, ps->offset, APPLE_PMGR_FLAGS | APPLE_PMGR_PS_MIN,
+				   FIELD_PREP(APPLE_PMGR_PS_MIN, ps->min_state));
+	}
+
 	active = apple_pmgr_ps_is_active(ps);
 	if (of_property_read_bool(node, "apple,always-on")) {
 		ps->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
-- 
2.33.0

Re: [PATCH 2/3] soc: apple: apple-pmgr-pwrstate: Add auto-PM min level support

[Sven Peter]

On Thu, Dec 9, 2021, at 05:45, Hector Martin wrote:
> This is seemingly required for DCP/DCPEXT, without which they refuse to
> boot properly. They need to be set to minimum state 4 (clock gated).
>
> Signed-off-by: Hector Martin <marcan@marcan.st>
> ---
>  drivers/soc/apple/apple-pmgr-pwrstate.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/drivers/soc/apple/apple-pmgr-pwrstate.c 
> b/drivers/soc/apple/apple-pmgr-pwrstate.c
> index 8ad9d5fdefbf..590bb7439372 100644
> --- a/drivers/soc/apple/apple-pmgr-pwrstate.c
> +++ b/drivers/soc/apple/apple-pmgr-pwrstate.c
> @@ -20,6 +20,7 @@
>  #define APPLE_PMGR_RESET        BIT(31)
>  #define APPLE_PMGR_AUTO_ENABLE  BIT(28)
>  #define APPLE_PMGR_PS_AUTO      GENMASK(27, 24)
> +#define APPLE_PMGR_PS_MIN       GENMASK(19, 16)
>  #define APPLE_PMGR_PARENT_OFF   BIT(11)
>  #define APPLE_PMGR_DEV_DISABLE  BIT(10)
>  #define APPLE_PMGR_WAS_CLKGATED BIT(9)
> @@ -42,6 +43,7 @@ struct apple_pmgr_ps {
>  	struct reset_controller_dev rcdev;
>  	struct regmap *regmap;
>  	u32 offset;
> +	u32 min_state;
>  };
> 
>  #define genpd_to_apple_pmgr_ps(_genpd) container_of(_genpd, struct 
> apple_pmgr_ps, genpd)
> @@ -224,6 +226,12 @@ static int apple_pmgr_ps_probe(struct 
> platform_device *pdev)
>  	ps->genpd.power_on = apple_pmgr_ps_power_on;
>  	ps->genpd.power_off = apple_pmgr_ps_power_off;
> 
> +	ret = of_property_read_u32(node, "apple,min-state", &ps->min_state);
> +	if (ret >= 0) {


I think of_property_read_u32 never returns anything > 0 such that
"if (ret == 0)" should be enough. You could also add a check that min_state
must be <= 0b1111 here if you're feeling paranoid.

Either way,

Reviewed-by: Sven Peter <sven@svenpeter.dev>



> +		regmap_update_bits(regmap, ps->offset, APPLE_PMGR_FLAGS | APPLE_PMGR_PS_MIN,
> +				   FIELD_PREP(APPLE_PMGR_PS_MIN, ps->min_state));
> +	}
> +
>  	active = apple_pmgr_ps_is_active(ps);
>  	if (of_property_read_bool(node, "apple,always-on")) {
>  		ps->genpd.flags |= GENPD_FLAG_ALWAYS_ON;
> -- 
> 2.33.0

[PATCH 3/3] arm64: dts: apple: t8103: Add apple,min-ps to DCP PMGR nodes

[Hector Martin]

This is required for DCP to boot successfully; it seems if power gating
is allowed, they do not wake up properly.

Signed-off-by: Hector Martin <marcan@marcan.st>
---
 arch/arm64/boot/dts/apple/t8103-pmgr.dtsi | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi b/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi
index 1310be74df1d..fc51bc872468 100644
--- a/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi
+++ b/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi
@@ -665,6 +665,7 @@ ps_dispext_cpu0: power-controller@378 {
 		#reset-cells = <0>;
 		label = "dispext_cpu0";
 		power-domains = <&ps_dispext_fe>;
+		apple,min-state = <4>;
 	};
 
 	ps_jpg: power-controller@3c0 {
@@ -1005,6 +1006,7 @@ ps_disp0_cpu0: power-controller@10018 {
 		label = "disp0_cpu0";
 		power-domains = <&ps_disp0_fe>;
 		apple,always-on; /* TODO: figure out if we can enable PM here */
+		apple,min-state = <4>;
 	};
 };
 
-- 
2.33.0

Re: [PATCH 3/3] arm64: dts: apple: t8103: Add apple,min-ps to DCP PMGR nodes

[Sven Peter]

On Thu, Dec 9, 2021, at 05:45, Hector Martin wrote:
> This is required for DCP to boot successfully; it seems if power gating
> is allowed, they do not wake up properly.
>
> Signed-off-by: Hector Martin <marcan@marcan.st>

Reviewed-by: Sven Peter <sven@svenpeter.dev>

> ---
>  arch/arm64/boot/dts/apple/t8103-pmgr.dtsi | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi 
> b/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi
> index 1310be74df1d..fc51bc872468 100644
> --- a/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi
> +++ b/arch/arm64/boot/dts/apple/t8103-pmgr.dtsi
> @@ -665,6 +665,7 @@ ps_dispext_cpu0: power-controller@378 {
>  		#reset-cells = <0>;
>  		label = "dispext_cpu0";
>  		power-domains = <&ps_dispext_fe>;
> +		apple,min-state = <4>;
>  	};
> 
>  	ps_jpg: power-controller@3c0 {
> @@ -1005,6 +1006,7 @@ ps_disp0_cpu0: power-controller@10018 {
>  		label = "disp0_cpu0";
>  		power-domains = <&ps_disp0_fe>;
>  		apple,always-on; /* TODO: figure out if we can enable PM here */
> +		apple,min-state = <4>;
>  	};
>  };
> 
> -- 
> 2.33.0