2023-02-08 00:36:45

by Doug Anderson

[permalink] [raw]
Subject: [PATCH] arm64: dts: qcom: sc7280: Power herobrine's 3.3 eDP/TS rail more properly

This is the equivalent of commit f5b4811e8758 ("arm64: dts: qcom:
sc7180: Add trogdor eDP/touchscreen regulator off-on-time") and commit
23ff866987de ("arm64: dts: qcom: sc7180: Start the trogdor
eDP/touchscreen regulator on"), but for herobrine instead of trogdor.

The motivations for herobrine are the same as for trogdor.

NOTES:
* Currently for herobrine all boards are eDP, not MIPI. If/when we
have herobrine derivatives that are MIPI they we can evaluate
whether the same off-on-delay makes sense for them. For trogdor we
didn't add the delay to MIPI panels because the problem was found
late and nobody had complained about it. For herobrine defaulting to
assuming the same 500ms makes sense and if we find we need to
optimize later we can.
* Currently there are no oddball herobrine boards like homestar where
the panel really likes to be power cycled. If we have an oddball
board it will need to split the eDP and touchscreen rail anyway
(like homestar did) and we'll have to delete the "regulator-boot-on"
from that board.

Signed-off-by: Douglas Anderson <[email protected]>
---
This patch should be applied atop my recent series adjusting the
herobrine touchscreen rails [1]. If I need to send a v2 of that series
I will add this at the end of it as patch #8.

[1] https://lore.kernel.org/all/[email protected]/

arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 16 ++++++++++++++++
1 file changed, 16 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
index ded36b5d28c7..312cc0e1cbc7 100644
--- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
+++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
@@ -110,6 +110,22 @@ pp3300_left_in_mlb: pp3300-left-in-mlb-regulator {

regulator-enable-ramp-delay = <3000>;

+ /*
+ * eDP panel specs nearly always have a spec that says you
+ * shouldn't turn them off an on again without waiting 500ms.
+ * Add this as a board constraint since this rail is shared
+ * between the panel and touchscreen.
+ */
+ off-on-delay-us = <500000>;
+
+ /*
+ * Stat the regulator on. This has the advantage of starting
+ * the slow process of powering the panel on as soon as we
+ * probe the regulator. It also avoids tripping the
+ * off-on-delay immediately on every bootup.
+ */
+ regulator-boot-on;
+
vin-supply = <&pp3300_z1>;
};

--
2.39.1.519.gcb327c4b5f-goog



2023-02-08 08:17:36

by Konrad Dybcio

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sc7280: Power herobrine's 3.3 eDP/TS rail more properly



On 8.02.2023 01:36, Douglas Anderson wrote:
> This is the equivalent of commit f5b4811e8758 ("arm64: dts: qcom:
> sc7180: Add trogdor eDP/touchscreen regulator off-on-time") and commit
> 23ff866987de ("arm64: dts: qcom: sc7180: Start the trogdor
> eDP/touchscreen regulator on"), but for herobrine instead of trogdor.
>
> The motivations for herobrine are the same as for trogdor.
>
> NOTES:
> * Currently for herobrine all boards are eDP, not MIPI. If/when we
> have herobrine derivatives that are MIPI they we can evaluate
> whether the same off-on-delay makes sense for them. For trogdor we
> didn't add the delay to MIPI panels because the problem was found
> late and nobody had complained about it. For herobrine defaulting to
> assuming the same 500ms makes sense and if we find we need to
> optimize later we can.
> * Currently there are no oddball herobrine boards like homestar where
> the panel really likes to be power cycled. If we have an oddball
> board it will need to split the eDP and touchscreen rail anyway
> (like homestar did) and we'll have to delete the "regulator-boot-on"
> from that board.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>

Konrad
> This patch should be applied atop my recent series adjusting the
> herobrine touchscreen rails [1]. If I need to send a v2 of that series
> I will add this at the end of it as patch #8.
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index ded36b5d28c7..312cc0e1cbc7 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -110,6 +110,22 @@ pp3300_left_in_mlb: pp3300-left-in-mlb-regulator {
>
> regulator-enable-ramp-delay = <3000>;
>
> + /*
> + * eDP panel specs nearly always have a spec that says you
> + * shouldn't turn them off an on again without waiting 500ms.
> + * Add this as a board constraint since this rail is shared
> + * between the panel and touchscreen.
> + */
> + off-on-delay-us = <500000>;
> +
> + /*
> + * Stat the regulator on. This has the advantage of starting
> + * the slow process of powering the panel on as soon as we
> + * probe the regulator. It also avoids tripping the
> + * off-on-delay immediately on every bootup.
> + */
> + regulator-boot-on;
> +
> vin-supply = <&pp3300_z1>;
> };
>

2023-02-08 18:10:08

by Matthias Kaehlcke

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sc7280: Power herobrine's 3.3 eDP/TS rail more properly

On Tue, Feb 07, 2023 at 04:36:10PM -0800, Douglas Anderson wrote:
> This is the equivalent of commit f5b4811e8758 ("arm64: dts: qcom:
> sc7180: Add trogdor eDP/touchscreen regulator off-on-time") and commit
> 23ff866987de ("arm64: dts: qcom: sc7180: Start the trogdor
> eDP/touchscreen regulator on"), but for herobrine instead of trogdor.
>
> The motivations for herobrine are the same as for trogdor.
>
> NOTES:
> * Currently for herobrine all boards are eDP, not MIPI. If/when we
> have herobrine derivatives that are MIPI they we can evaluate
> whether the same off-on-delay makes sense for them. For trogdor we
> didn't add the delay to MIPI panels because the problem was found
> late and nobody had complained about it. For herobrine defaulting to
> assuming the same 500ms makes sense and if we find we need to
> optimize later we can.
> * Currently there are no oddball herobrine boards like homestar where
> the panel really likes to be power cycled. If we have an oddball
> board it will need to split the eDP and touchscreen rail anyway
> (like homestar did) and we'll have to delete the "regulator-boot-on"
> from that board.
>
> Signed-off-by: Douglas Anderson <[email protected]>
> ---
> This patch should be applied atop my recent series adjusting the
> herobrine touchscreen rails [1]. If I need to send a v2 of that series
> I will add this at the end of it as patch #8.
>
> [1] https://lore.kernel.org/all/[email protected]/
>
> arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> index ded36b5d28c7..312cc0e1cbc7 100644
> --- a/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sc7280-herobrine.dtsi
> @@ -110,6 +110,22 @@ pp3300_left_in_mlb: pp3300-left-in-mlb-regulator {
>
> regulator-enable-ramp-delay = <3000>;
>
> + /*
> + * eDP panel specs nearly always have a spec that says you

uber-nit: you could replace the second 'spec' with 'requirement' to avoid
the repetition.

> + * shouldn't turn them off an on again without waiting 500ms.
> + * Add this as a board constraint since this rail is shared
> + * between the panel and touchscreen.
> + */
> + off-on-delay-us = <500000>;
> +
> + /*
> + * Stat the regulator on. This has the advantage of starting

s/Stat/Start/ ?

> + * the slow process of powering the panel on as soon as we
> + * probe the regulator. It also avoids tripping the
> + * off-on-delay immediately on every bootup.
> + */
> + regulator-boot-on;
> +
> vin-supply = <&pp3300_z1>;
> };

Reviewed-by: Matthias Kaehlcke <[email protected]>

2023-02-09 04:34:22

by Bjorn Andersson

[permalink] [raw]
Subject: Re: [PATCH] arm64: dts: qcom: sc7280: Power herobrine's 3.3 eDP/TS rail more properly

On Tue, 7 Feb 2023 16:36:10 -0800, Douglas Anderson wrote:
> This is the equivalent of commit f5b4811e8758 ("arm64: dts: qcom:
> sc7180: Add trogdor eDP/touchscreen regulator off-on-time") and commit
> 23ff866987de ("arm64: dts: qcom: sc7180: Start the trogdor
> eDP/touchscreen regulator on"), but for herobrine instead of trogdor.
>
> The motivations for herobrine are the same as for trogdor.
>
> [...]

Applied, thanks!

[1/1] arm64: dts: qcom: sc7280: Power herobrine's 3.3 eDP/TS rail more properly
commit: f069ede81ef438d99e76112d8738c4dc3d1766f9

Best regards,
--
Bjorn Andersson <[email protected]>