2024-03-26 14:02:25

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 0/5] clk: qcom: gpucc-sc8280xp: fix GX external supply lookup

The SA8540P platform is closely related to SC8280XP but differs in that
it uses an external supply for the GX power domain.

This series adds a new SA8540P GPU clock controller compatible which
can be used to determine whether to look up the external supply.

This specifically avoids warnings such as:

gpu_cc-sc8280xp 3d90000.clock-controller: supply vdd-gfx not found, using dummy regulator

on SC8280XP, which were introduced in 6.9-rc1.

Note that this also avoids triggering a potential deadlock on SC8280XP
even if the underlying issue still remains for the derivative platforms
like SA8540P and SA8295P that actually use the supply. [1]

Also note that this is a better alternative to simply making the
external supply optional as that would suppress any warnings about
missing supplies on platforms that actually require it. This series
therefore supersedes [2].

Johan


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


Johan Hovold (5):
dt-bindings: clock: qcom: add SA8540P gpucc
arm64: dts: qcom: sa8540p: use sa8540p gpucc compatible
clk: qcom: gpucc-sc8280xp: make cc descriptor const
clk: qcom: gpucc-sc8280xp: fix GX external supply lookup
arm64: dts: qcom: sa8540p: drop fallback gpucc compatible

.../devicetree/bindings/clock/qcom,gpucc.yaml | 1 +
arch/arm64/boot/dts/qcom/sa8540p.dtsi | 2 +
drivers/clk/qcom/gpucc-sc8280xp.c | 42 ++++++++++++++++---
3 files changed, 39 insertions(+), 6 deletions(-)

--
2.43.0



2024-03-26 14:02:25

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 2/5] arm64: dts: qcom: sa8540p: use sa8540p gpucc compatible

The SA8540P platform is closely related to SC8280XP but differs in that
it uses an external supply for the GX power domain.

Use the new SA8540P compatible string for the GPU clock controller so
that the OS can determine which resources to look for.

Note that a fallback SC8280XP compatible is added temporarily to avoid
any temporary regressions for sa8295p-adp.

Fixes: fd5821a1a83c ("arm64: dts: qcom: sa8540p: Drop gfx.lvl as power-domain for gpucc")
Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8540p.dtsi | 2 ++
1 file changed, 2 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
index 23888029cc11..3b31a9ea3492 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
@@ -168,6 +168,8 @@ opp-2592000000 {
};

&gpucc {
+ compatible = "qcom,sa8540p-gpucc", "qcom,sc8280xp-gpucc";
+
/* SA8295P and SA8540P doesn't provide gfx.lvl */
/delete-property/ power-domains;

--
2.43.0


2024-03-26 14:02:53

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 5/5] arm64: dts: qcom: sa8540p: drop fallback gpucc compatible

Drop the fallback SC8280XP GPU clock controller compatible that was
added temporarily to handle the transition to the new SA8540P
compatible.

Signed-off-by: Johan Hovold <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8540p.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
index 3b31a9ea3492..9c2e99d30d08 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
@@ -168,7 +168,7 @@ opp-2592000000 {
};

&gpucc {
- compatible = "qcom,sa8540p-gpucc", "qcom,sc8280xp-gpucc";
+ compatible = "qcom,sa8540p-gpucc";

/* SA8295P and SA8540P doesn't provide gfx.lvl */
/delete-property/ power-domains;
--
2.43.0


2024-03-26 16:05:20

by Krzysztof Kozlowski

[permalink] [raw]
Subject: Re: [PATCH 2/5] arm64: dts: qcom: sa8540p: use sa8540p gpucc compatible

On 26/03/2024 15:01, Johan Hovold wrote:
> The SA8540P platform is closely related to SC8280XP but differs in that
> it uses an external supply for the GX power domain.
>
> Use the new SA8540P compatible string for the GPU clock controller so
> that the OS can determine which resources to look for.
>
> Note that a fallback SC8280XP compatible is added temporarily to avoid
> any temporary regressions for sa8295p-adp.
>
> Fixes: fd5821a1a83c ("arm64: dts: qcom: sa8540p: Drop gfx.lvl as power-domain for gpucc")
> Signed-off-by: Johan Hovold <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sa8540p.dtsi | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> index 23888029cc11..3b31a9ea3492 100644
> --- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> @@ -168,6 +168,8 @@ opp-2592000000 {
> };
>
> &gpucc {
> + compatible = "qcom,sa8540p-gpucc", "qcom,sc8280xp-gpucc";

This introduces new dtbs_check failures. Please fix the binding and drop
the last patch in the series.

Best regards,
Krzysztof


2024-03-26 16:51:51

by Johan Hovold

[permalink] [raw]
Subject: Re: [PATCH 2/5] arm64: dts: qcom: sa8540p: use sa8540p gpucc compatible

On Tue, Mar 26, 2024 at 05:02:43PM +0100, Krzysztof Kozlowski wrote:
> On 26/03/2024 15:01, Johan Hovold wrote:
> > The SA8540P platform is closely related to SC8280XP but differs in that
> > it uses an external supply for the GX power domain.
> >
> > Use the new SA8540P compatible string for the GPU clock controller so
> > that the OS can determine which resources to look for.
> >
> > Note that a fallback SC8280XP compatible is added temporarily to avoid
> > any temporary regressions for sa8295p-adp.
> >
> > Fixes: fd5821a1a83c ("arm64: dts: qcom: sa8540p: Drop gfx.lvl as power-domain for gpucc")
> > Signed-off-by: Johan Hovold <[email protected]>
> > ---
> > arch/arm64/boot/dts/qcom/sa8540p.dtsi | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> > index 23888029cc11..3b31a9ea3492 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> > +++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> > @@ -168,6 +168,8 @@ opp-2592000000 {
> > };
> >
> > &gpucc {
> > + compatible = "qcom,sa8540p-gpucc", "qcom,sc8280xp-gpucc";
>
> This introduces new dtbs_check failures. Please fix the binding and drop
> the last patch in the series.

I know, and this is done on purpose.

I doubt anyone cares if the sa8295p GPU breaks for one commit in case
this series goes in through the same tree or even for a couple of RCs in
case they go in through separate trees.

But we recently had a similar discussion about a bluetooth fix and
whatever course of action I would have chosen here, someone is bound to
whine.

In this case I figured it was worth doing the extra work. But this is
just a temporary workaround as "qcom,sa8540p-gpucc" is not truly
compatible with "qcom,sc8280xp-gpucc" as only the former depends on the
external supply.

Heck, I even spelled it out in the commit message...

Johan