[PATCH 0/8] arm64: dts: qcom: sa8295p: Enable GPU

[PATCH 0/8] arm64: dts: qcom: sa8295p: Enable GPU

[Bjorn Andersson]

Due to the different PMIC configuration found in the SA8295P platform,
compared to SC8280XP, the VDD_GFX pads are supplied by an dedicated
MAX20411 LDO.

Support for expressing the regulator supply is added to the binding, the
support for enabling the parent supply for GX is added, the missing
gfx.lvl power-domain is dropped, and the DeviceTree is wired up to
enable the GPU in this configuration.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
Bjorn Andersson (8):
      dt-bindings: clock: qcom: Allow VDD_GFX supply to GX
      clk: qcom: gdsc: Enable supply reglator in GPU GX handler
      clk: qcom: gpucc-sc8280xp: Add external supply for GX gdsc
      soc: qcom: rpmhpd: Drop SA8540P gfx.lvl
      arm64: dts: qcom: sa8540p: Drop gfx.lvl as power-domain for gpucc
      arm64: dts: qcom: sa8295p-adp: add max20411
      arm64: dts: qcom: sa8295p-adp: Enable GPU
      arm64: defconfig: Enable MAX20411 regulator driver

 .../devicetree/bindings/clock/qcom,gpucc.yaml      |  3 +
 arch/arm64/boot/dts/qcom/sa8295p-adp.dts           | 69 ++++++++++++++++++++++
 arch/arm64/boot/dts/qcom/sa8540p.dtsi              |  2 +
 arch/arm64/configs/defconfig                       |  1 +
 drivers/clk/qcom/gdsc.c                            |  8 ++-
 drivers/clk/qcom/gpucc-sc8280xp.c                  |  1 +
 drivers/pmdomain/qcom/rpmhpd.c                     |  1 -
 7 files changed, 83 insertions(+), 2 deletions(-)
---
base-commit: 20d857259d7d10cd0d5e8b60608455986167cfad
change-id: 20231220-sa8295p-gpu-51c5f343e3ec

Best regards,
-- 
Bjorn Andersson <quic_bjorande@quicinc.com>

[PATCH 1/8] dt-bindings: clock: qcom: Allow VDD_GFX supply to GX

[Bjorn Andersson]

In some designs the SoC's VDD_GFX pads are supplied by an external
regulator, rather than a power-domain. Allow this to be described in the
GPU clock controller binding.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
index f369fa34e00c..013ef78d2b31 100644
--- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
+++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
@@ -53,6 +53,9 @@ properties:
   power-domains:
     maxItems: 1
 
+  vdd-gfx-supply:
+    description: Regulator supply for the VDD_GFX pads
+
   '#clock-cells':
     const: 1
 

-- 
2.25.1

Re: [PATCH 1/8] dt-bindings: clock: qcom: Allow VDD_GFX supply to GX

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 05:51, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> In some designs the SoC's VDD_GFX pads are supplied by an external
> regulator, rather than a power-domain. Allow this to be described in the
> GPU clock controller binding.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 3 +++
>  1 file changed, 3 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> index f369fa34e00c..013ef78d2b31 100644
> --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> @@ -53,6 +53,9 @@ properties:
>    power-domains:
>      maxItems: 1
>
> +  vdd-gfx-supply:
> +    description: Regulator supply for the VDD_GFX pads
> +
>    '#clock-cells':
>      const: 1

I think it might be good to restrict this property to a particular
platform (via if:not:properties:compatible:contains
then:properties:vdd-gfx-supply:false).


-- 
With best wishes
Dmitry

Re: [PATCH 1/8] dt-bindings: clock: qcom: Allow VDD_GFX supply to GX

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 09:03, Dmitry Baryshkov
<dmitry.baryshkov@linaro.org> wrote:
>
> On Thu, 21 Dec 2023 at 05:51, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> >
> > In some designs the SoC's VDD_GFX pads are supplied by an external
> > regulator, rather than a power-domain. Allow this to be described in the
> > GPU clock controller binding.
> >
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > index f369fa34e00c..013ef78d2b31 100644
> > --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > @@ -53,6 +53,9 @@ properties:
> >    power-domains:
> >      maxItems: 1
> >
> > +  vdd-gfx-supply:
> > +    description: Regulator supply for the VDD_GFX pads
> > +
> >    '#clock-cells':
> >      const: 1
>
> I think it might be good to restrict this property to a particular
> platform (via if:not:properties:compatible:contains
> then:properties:vdd-gfx-supply:false).

After reading the last patches in the series, there is another
suggestion. Maybe we should explicitly say that there should be either
power-domains or vdd-gfx-supply, but not both.


-- 
With best wishes
Dmitry

Re: [PATCH 1/8] dt-bindings: clock: qcom: Allow VDD_GFX supply to GX

[Bjorn Andersson]

On Thu, Dec 21, 2023 at 09:10:28AM +0200, Dmitry Baryshkov wrote:
> On Thu, 21 Dec 2023 at 09:03, Dmitry Baryshkov
> <dmitry.baryshkov@linaro.org> wrote:
> >
> > On Thu, 21 Dec 2023 at 05:51, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
> > >
> > > In some designs the SoC's VDD_GFX pads are supplied by an external
> > > regulator, rather than a power-domain. Allow this to be described in the
> > > GPU clock controller binding.
> > >
> > > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > > ---
> > >  Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 3 +++
> > >  1 file changed, 3 insertions(+)
> > >
> > > diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > > index f369fa34e00c..013ef78d2b31 100644
> > > --- a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > > +++ b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > > @@ -53,6 +53,9 @@ properties:
> > >    power-domains:
> > >      maxItems: 1
> > >
> > > +  vdd-gfx-supply:
> > > +    description: Regulator supply for the VDD_GFX pads
> > > +
> > >    '#clock-cells':
> > >      const: 1
> >
> > I think it might be good to restrict this property to a particular
> > platform (via if:not:properties:compatible:contains
> > then:properties:vdd-gfx-supply:false).
> 
> After reading the last patches in the series, there is another
> suggestion. Maybe we should explicitly say that there should be either
> power-domains or vdd-gfx-supply, but not both.
> 

Even on this platform it's not a property of the SoC, but surrounding
design. So I like this proposal.

Thanks,
Bjorn

[PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Bjorn Andersson]

The GX GDSC is modelled to aid the GMU in powering down the GPU in the
event that the GPU crashes, so that it can be restarted again. But in
the event that the power-domain is supplied through a dedicated
regulator (in contrast to being a subdomin of another power-domain),
something needs to turn that regulator on, both to make sure things are
powered and to match the operation in gdsc_disable().

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/clk/qcom/gdsc.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 5358e28122ab..d1139c895503 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
  */
 int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
 {
+	struct gdsc *sc = domain_to_gdsc(domain);
+	int ret = 0;
+
 	/* Do nothing but give genpd the impression that we were successful */
-	return 0;
+	if (sc->rsupply)
+		ret = regulator_enable(sc->rsupply);
+
+	return ret;
 }
 EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);

-- 
2.25.1

Re: [PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Stephen Boyd]

Quoting Bjorn Andersson (2023-12-20 19:50:36)
> The GX GDSC is modelled to aid the GMU in powering down the GPU in the
> event that the GPU crashes, so that it can be restarted again. But in
> the event that the power-domain is supplied through a dedicated
> regulator (in contrast to being a subdomin of another power-domain),
> something needs to turn that regulator on, both to make sure things are
> powered and to match the operation in gdsc_disable().
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/clk/qcom/gdsc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 5358e28122ab..d1139c895503 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
>   */
>  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>  {
> +       struct gdsc *sc = domain_to_gdsc(domain);
> +       int ret = 0;
> +
>         /* Do nothing but give genpd the impression that we were successful */

Update this comment.

> -       return 0;
> +       if (sc->rsupply)
> +               ret = regulator_enable(sc->rsupply);
> +
> +       return ret;
>  }

Re: [PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Konrad Dybcio]

On 21.12.2023 04:50, Bjorn Andersson wrote:
> The GX GDSC is modelled to aid the GMU in powering down the GPU in the
> event that the GPU crashes, so that it can be restarted again. But in
> the event that the power-domain is supplied through a dedicated
> regulator (in contrast to being a subdomin of another power-domain),
> something needs to turn that regulator on, both to make sure things are
> powered and to match the operation in gdsc_disable().
> 
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/clk/qcom/gdsc.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> index 5358e28122ab..d1139c895503 100644
> --- a/drivers/clk/qcom/gdsc.c
> +++ b/drivers/clk/qcom/gdsc.c
> @@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
>   */
>  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
I suppose the name is confusing now..

But at the same time I can't come up with anything that's less than
like 6 words..

Konrad

Re: [PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 15:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 21.12.2023 04:50, Bjorn Andersson wrote:
> > The GX GDSC is modelled to aid the GMU in powering down the GPU in the
> > event that the GPU crashes, so that it can be restarted again. But in
> > the event that the power-domain is supplied through a dedicated
> > regulator (in contrast to being a subdomin of another power-domain),
> > something needs to turn that regulator on, both to make sure things are
> > powered and to match the operation in gdsc_disable().
> >
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  drivers/clk/qcom/gdsc.c | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> > index 5358e28122ab..d1139c895503 100644
> > --- a/drivers/clk/qcom/gdsc.c
> > +++ b/drivers/clk/qcom/gdsc.c
> > @@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
> >   */
> >  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
> I suppose the name is confusing now..
>
> But at the same time I can't come up with anything that's less than
> like 6 words..

gdsc_gx_enable() ;-)

-- 
With best wishes
Dmitry

Re: [PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Konrad Dybcio]

On 21.12.2023 14:04, Dmitry Baryshkov wrote:
> On Thu, 21 Dec 2023 at 15:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>
>> On 21.12.2023 04:50, Bjorn Andersson wrote:
>>> The GX GDSC is modelled to aid the GMU in powering down the GPU in the
>>> event that the GPU crashes, so that it can be restarted again. But in
>>> the event that the power-domain is supplied through a dedicated
>>> regulator (in contrast to being a subdomin of another power-domain),
>>> something needs to turn that regulator on, both to make sure things are
>>> powered and to match the operation in gdsc_disable().
>>>
>>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>>> ---
>>>  drivers/clk/qcom/gdsc.c | 8 +++++++-
>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>>> index 5358e28122ab..d1139c895503 100644
>>> --- a/drivers/clk/qcom/gdsc.c
>>> +++ b/drivers/clk/qcom/gdsc.c
>>> @@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
>>>   */
>>>  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>> I suppose the name is confusing now..
>>
>> But at the same time I can't come up with anything that's less than
>> like 6 words..
> 
> gdsc_gx_enable() ;-)
except not really only gx and not really enable :(

gdsc_shared_enable would probably be closer to our current
nomenclature..

Konrad

Re: [PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Konrad Dybcio]

On 21.12.2023 14:16, Konrad Dybcio wrote:
> On 21.12.2023 14:04, Dmitry Baryshkov wrote:
>> On Thu, 21 Dec 2023 at 15:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>>>
>>> On 21.12.2023 04:50, Bjorn Andersson wrote:
>>>> The GX GDSC is modelled to aid the GMU in powering down the GPU in the
>>>> event that the GPU crashes, so that it can be restarted again. But in
>>>> the event that the power-domain is supplied through a dedicated
>>>> regulator (in contrast to being a subdomin of another power-domain),
>>>> something needs to turn that regulator on, both to make sure things are
>>>> powered and to match the operation in gdsc_disable().
>>>>
>>>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
>>>> ---
>>>>  drivers/clk/qcom/gdsc.c | 8 +++++++-
>>>>  1 file changed, 7 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
>>>> index 5358e28122ab..d1139c895503 100644
>>>> --- a/drivers/clk/qcom/gdsc.c
>>>> +++ b/drivers/clk/qcom/gdsc.c
>>>> @@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
>>>>   */
>>>>  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
>>> I suppose the name is confusing now..
>>>
>>> But at the same time I can't come up with anything that's less than
>>> like 6 words..
>>
>> gdsc_gx_enable() ;-)
> except not really only gx and not really enable :(
> 
> gdsc_shared_enable would probably be closer to our current
> nomenclature..
but then VOTABLE also taps into the concept of "shared" :/

Konrad

Re: [PATCH 2/8] clk: qcom: gdsc: Enable supply reglator in GPU GX handler

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 15:16, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
>
> On 21.12.2023 14:04, Dmitry Baryshkov wrote:
> > On Thu, 21 Dec 2023 at 15:01, Konrad Dybcio <konrad.dybcio@linaro.org> wrote:
> >>
> >> On 21.12.2023 04:50, Bjorn Andersson wrote:
> >>> The GX GDSC is modelled to aid the GMU in powering down the GPU in the
> >>> event that the GPU crashes, so that it can be restarted again. But in
> >>> the event that the power-domain is supplied through a dedicated
> >>> regulator (in contrast to being a subdomin of another power-domain),
> >>> something needs to turn that regulator on, both to make sure things are
> >>> powered and to match the operation in gdsc_disable().
> >>>
> >>> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> >>> ---
> >>>  drivers/clk/qcom/gdsc.c | 8 +++++++-
> >>>  1 file changed, 7 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
> >>> index 5358e28122ab..d1139c895503 100644
> >>> --- a/drivers/clk/qcom/gdsc.c
> >>> +++ b/drivers/clk/qcom/gdsc.c
> >>> @@ -557,7 +557,13 @@ void gdsc_unregister(struct gdsc_desc *desc)
> >>>   */
> >>>  int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
> >> I suppose the name is confusing now..
> >>
> >> But at the same time I can't come up with anything that's less than
> >> like 6 words..
> >
> > gdsc_gx_enable() ;-)
> except not really only gx and not really enable :(
>
> gdsc_shared_enable would probably be closer to our current
> nomenclature..

gdsc_dummy_gx_enable*(

Or gdsc_dummy_gmu_gx_enable(). Still less than 6 words. I'm trying my best!


-- 
With best wishes
Dmitry

[PATCH 3/8] clk: qcom: gpucc-sc8280xp: Add external supply for GX gdsc

[Bjorn Andersson]

On SA8295P and SA8540P the GFX rail is powered by a dedicated external
regulator, instead of the rpmh-controlled "gfx.lvl".

Define the "vdd-gfx" as the supply regulator for the GDSC, to cause the
gdsc logic to look for, and control, this external power supply.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/clk/qcom/gpucc-sc8280xp.c b/drivers/clk/qcom/gpucc-sc8280xp.c
index 8e147ee294ee..e2b3bc000c71 100644
--- a/drivers/clk/qcom/gpucc-sc8280xp.c
+++ b/drivers/clk/qcom/gpucc-sc8280xp.c
@@ -399,6 +399,7 @@ static struct gdsc gx_gdsc = {
 	},
 	.pwrsts = PWRSTS_OFF_ON,
 	.flags = CLAMP_IO | RETAIN_FF_ENABLE,
+	.supply = "vdd-gfx",
 };
 
 static struct gdsc *gpu_cc_sc8280xp_gdscs[] = {

-- 
2.25.1

Re: [PATCH 3/8] clk: qcom: gpucc-sc8280xp: Add external supply for GX gdsc

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 05:51, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> On SA8295P and SA8540P the GFX rail is powered by a dedicated external
> regulator, instead of the rpmh-controlled "gfx.lvl".
>
> Define the "vdd-gfx" as the supply regulator for the GDSC, to cause the
> gdsc logic to look for, and control, this external power supply.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
>  1 file changed, 1 insertion(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH 4/8] soc: qcom: rpmhpd: Drop SA8540P gfx.lvl

[Bjorn Andersson]

On SA8295P and SA8540P gfx.lvl is not provdied by rpmh, but rather is
handled by an external regulator (max20411). Drop gfx.lvl from the list
of power-domains exposed on this platform.

Fixes: f68f1cb3437d ("soc: qcom: rpmhpd: add sc8280xp & sa8540p rpmh power-domains")
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 drivers/pmdomain/qcom/rpmhpd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/pmdomain/qcom/rpmhpd.c b/drivers/pmdomain/qcom/rpmhpd.c
index 3078896b1300..27a73ff72614 100644
--- a/drivers/pmdomain/qcom/rpmhpd.c
+++ b/drivers/pmdomain/qcom/rpmhpd.c
@@ -217,7 +217,6 @@ static struct rpmhpd *sa8540p_rpmhpds[] = {
 	[SC8280XP_CX] = &cx,
 	[SC8280XP_CX_AO] = &cx_ao,
 	[SC8280XP_EBI] = &ebi,
-	[SC8280XP_GFX] = &gfx,
 	[SC8280XP_LCX] = &lcx,
 	[SC8280XP_LMX] = &lmx,
 	[SC8280XP_MMCX] = &mmcx,

-- 
2.25.1

Re: [PATCH 4/8] soc: qcom: rpmhpd: Drop SA8540P gfx.lvl

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 05:51, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> On SA8295P and SA8540P gfx.lvl is not provdied by rpmh, but rather is
> handled by an external regulator (max20411). Drop gfx.lvl from the list
> of power-domains exposed on this platform.
>
> Fixes: f68f1cb3437d ("soc: qcom: rpmhpd: add sc8280xp & sa8540p rpmh power-domains")
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  drivers/pmdomain/qcom/rpmhpd.c | 1 -
>  1 file changed, 1 deletion(-)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

-- 
With best wishes
Dmitry

[PATCH 5/8] arm64: dts: qcom: sa8540p: Drop gfx.lvl as power-domain for gpucc

[Bjorn Andersson]

The SA8295P and SA8540P uses an external regulator (max20411), and
gfx.lvl is not provided by rpmh. Drop the power-domains property of the
gpucc node to reflect this.

Fixes: eec51ab2fd6f ("arm64: dts: qcom: sc8280xp: Add GPU related nodes")
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 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 96b2c59ad02b..b604404ebf64 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
@@ -168,6 +168,8 @@ opp-2592000000 {
 };
 
 &gpucc {
+	/delete-property/ power-domains;
+
 	status = "disabled";
 };
 

-- 
2.25.1

Re: [PATCH 5/8] arm64: dts: qcom: sa8540p: Drop gfx.lvl as power-domain for gpucc

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 05:51, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> The SA8295P and SA8540P uses an external regulator (max20411), and
> gfx.lvl is not provided by rpmh. Drop the power-domains property of the
> gpucc node to reflect this.
>
> Fixes: eec51ab2fd6f ("arm64: dts: qcom: sc8280xp: Add GPU related nodes")
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8540p.dtsi | 2 ++
>  1 file changed, 2 insertions(+)

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>

>
> diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> index 96b2c59ad02b..b604404ebf64 100644
> --- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> +++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
> @@ -168,6 +168,8 @@ opp-2592000000 {
>  };
>
>  &gpucc {
> +       /delete-property/ power-domains;

Nit: maybe this deserves a comment.

> +
>         status = "disabled";
>  };
>
>
> --
> 2.25.1
>
>


-- 
With best wishes
Dmitry

[PATCH 6/8] arm64: dts: qcom: sa8295p-adp: add max20411

[Bjorn Andersson]

From: Bjorn Andersson <andersson@kernel.org>

The SA8295P ADP has a MAX20411 LDO regulator on I2C 12, supplying the
VDD_GFX pads. Enable the bus and add the maxim,max20411 device on the
bus.

Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 40 ++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
index fd253942e5e5..e16406c9c19d 100644
--- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
@@ -266,6 +266,26 @@ &dispcc1 {
 	status = "okay";
 };
 
+&i2c12 {
+	pinctrl-names = "default";
+	pinctrl-0 = <&qup1_i2c4_state>;
+
+	status = "okay";
+
+	vdd_gfx: regulator@39 {
+		compatible = "maxim,max20411";
+		reg = <0x39>;
+
+		regulator-min-microvolt = <800000>;
+		regulator-max-microvolt = <968750>;
+
+		enable-gpios = <&pmm8540a_gpios 2 GPIO_ACTIVE_HIGH>;
+
+		pinctrl-names = "default";
+		pinctrl-0 = <&max20411_en>;
+	};
+};
+
 &mdss0 {
 	status = "okay";
 };
@@ -476,6 +496,10 @@ &pcie4_phy {
 	status = "okay";
 };
 
+&qup1 {
+	status = "okay";
+};
+
 &qup2 {
 	status = "okay";
 };
@@ -728,4 +752,20 @@ wake-n-pins {
 			bias-pull-up;
 		};
 	};
+
+	qup1_i2c4_state: qup1-i2c4-state {
+		pins = "gpio0", "gpio1";
+		function = "qup12";
+
+		drive-strength = <2>;
+		bias-pull-up;
+	};
+};
+
+&pmm8540a_gpios {
+	max20411_en: max20411-en-state {
+		pins = "gpio2";
+		function = "normal";
+		output-enable;
+	};
 };

-- 
2.25.1

Re: [PATCH 6/8] arm64: dts: qcom: sa8295p-adp: add max20411

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 05:52, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> From: Bjorn Andersson <andersson@kernel.org>
>
> The SA8295P ADP has a MAX20411 LDO regulator on I2C 12, supplying the
> VDD_GFX pads. Enable the bus and add the maxim,max20411 device on the
> bus.
>
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 40 ++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index fd253942e5e5..e16406c9c19d 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -266,6 +266,26 @@ &dispcc1 {
>         status = "okay";
>  };
>
> +&i2c12 {
> +       pinctrl-names = "default";
> +       pinctrl-0 = <&qup1_i2c4_state>;
> +
> +       status = "okay";
> +
> +       vdd_gfx: regulator@39 {
> +               compatible = "maxim,max20411";
> +               reg = <0x39>;
> +
> +               regulator-min-microvolt = <800000>;
> +               regulator-max-microvolt = <968750>;
> +
> +               enable-gpios = <&pmm8540a_gpios 2 GPIO_ACTIVE_HIGH>;
> +
> +               pinctrl-names = "default";
> +               pinctrl-0 = <&max20411_en>;
> +       };
> +};
> +
>  &mdss0 {
>         status = "okay";
>  };
> @@ -476,6 +496,10 @@ &pcie4_phy {
>         status = "okay";
>  };
>
> +&qup1 {
> +       status = "okay";
> +};
> +
>  &qup2 {
>         status = "okay";
>  };
> @@ -728,4 +752,20 @@ wake-n-pins {
>                         bias-pull-up;
>                 };
>         };
> +
> +       qup1_i2c4_state: qup1-i2c4-state {
> +               pins = "gpio0", "gpio1";
> +               function = "qup12";
> +
> +               drive-strength = <2>;
> +               bias-pull-up;
> +       };
> +};
> +
> +&pmm8540a_gpios {

I think pmm8540a_gpios comes before tlmm in the dictionary order.
Other than that LGTM

> +       max20411_en: max20411-en-state {
> +               pins = "gpio2";
> +               function = "normal";
> +               output-enable;
> +       };
>  };



-- 
With best wishes
Dmitry

Re: [PATCH 6/8] arm64: dts: qcom: sa8295p-adp: add max20411

[Konrad Dybcio]

On 21.12.2023 04:50, Bjorn Andersson wrote:
> From: Bjorn Andersson <andersson@kernel.org>
> 
> The SA8295P ADP has a MAX20411 LDO regulator on I2C 12, supplying the
> VDD_GFX pads. Enable the bus and add the maxim,max20411 device on the
> bus.
> 
> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> ---
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 40 ++++++++++++++++++++++++++++++++
>  1 file changed, 40 insertions(+)
> 
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index fd253942e5e5..e16406c9c19d 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -266,6 +266,26 @@ &dispcc1 {
>  	status = "okay";
>  };
>  
> +&i2c12 {
> +	pinctrl-names = "default";
> +	pinctrl-0 = <&qup1_i2c4_state>;
property-n
property-names

(same below)
> +
> +	status = "okay";
> +
> +	vdd_gfx: regulator@39 {
> +		compatible = "maxim,max20411";
> +		reg = <0x39>;
> +
> +		regulator-min-microvolt = <800000>;
> +		regulator-max-microvolt = <968750>;
Is this ever going to be scaled? I suppose you could add some OPP code to
drm/msm and use opp-microvolts.. Or lock this down at min=max

Konrad

Re: [PATCH 6/8] arm64: dts: qcom: sa8295p-adp: add max20411

[Bjorn Andersson]

On Thu, Dec 21, 2023 at 01:51:58PM +0100, Konrad Dybcio wrote:
> On 21.12.2023 04:50, Bjorn Andersson wrote:
> > From: Bjorn Andersson <andersson@kernel.org>
> > 
> > The SA8295P ADP has a MAX20411 LDO regulator on I2C 12, supplying the
> > VDD_GFX pads. Enable the bus and add the maxim,max20411 device on the
> > bus.
> > 
> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
> > ---
> >  arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 40 ++++++++++++++++++++++++++++++++
> >  1 file changed, 40 insertions(+)
> > 
> > diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > index fd253942e5e5..e16406c9c19d 100644
> > --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> > @@ -266,6 +266,26 @@ &dispcc1 {
> >  	status = "okay";
> >  };
> >  
> > +&i2c12 {
> > +	pinctrl-names = "default";
> > +	pinctrl-0 = <&qup1_i2c4_state>;
> property-n
> property-names
> 
> (same below)
> > +
> > +	status = "okay";
> > +
> > +	vdd_gfx: regulator@39 {
> > +		compatible = "maxim,max20411";
> > +		reg = <0x39>;
> > +
> > +		regulator-min-microvolt = <800000>;
> > +		regulator-max-microvolt = <968750>;
> Is this ever going to be scaled? I suppose you could add some OPP code to
> drm/msm and use opp-microvolts.. Or lock this down at min=max
> 

Downstream leave it at 0.8V, so let's lock it down for now.

Regards,
Bjorn

> Konrad

[PATCH 7/8] arm64: dts: qcom: sa8295p-adp: Enable GPU

[Bjorn Andersson]

With the necessary support in place for supplying VDD_GFX from the
MAX20411 regulator, enable the GPU clock controller, GMU, Adreno SMMU
and the GPU on the SA8295P ADP.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
index e16406c9c19d..7775c5f4d2e8 100644
--- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
+++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
@@ -108,6 +108,13 @@ edp3_connector_in: endpoint {
 			};
 		};
 	};
+
+	reserved-memory {
+		gpu_mem: gpu-mem@8bf00000 {
+			reg = <0 0x8bf00000 0 0x2000>;
+			no-map;
+		};
+	};
 };
 
 &apps_rsc {
@@ -286,6 +293,28 @@ vdd_gfx: regulator@39 {
 	};
 };
 
+&gpucc {
+	vdd-gfx-supply = <&vdd_gfx>;
+	status = "okay";
+};
+
+&gmu {
+	status = "okay";
+};
+
+&gpu {
+	status = "okay";
+
+	zap-shader {
+		memory-region = <&gpu_mem>;
+		firmware-name = "qcom/sa8295p/a690_zap.mdt";
+	};
+};
+
+&gpu_smmu {
+	status = "okay";
+};
+
 &mdss0 {
 	status = "okay";
 };

-- 
2.25.1

Re: [PATCH 7/8] arm64: dts: qcom: sa8295p-adp: Enable GPU

[Dmitry Baryshkov]

On Thu, 21 Dec 2023 at 05:52, Bjorn Andersson <quic_bjorande@quicinc.com> wrote:
>
> With the necessary support in place for supplying VDD_GFX from the
> MAX20411 regulator, enable the GPU clock controller, GMU, Adreno SMMU
> and the GPU on the SA8295P ADP.
>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 29 +++++++++++++++++++++++++++++
>  1 file changed, 29 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index e16406c9c19d..7775c5f4d2e8 100644
> --- a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> +++ b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> @@ -108,6 +108,13 @@ edp3_connector_in: endpoint {
>                         };
>                 };
>         };
> +
> +       reserved-memory {
> +               gpu_mem: gpu-mem@8bf00000 {
> +                       reg = <0 0x8bf00000 0 0x2000>;
> +                       no-map;
> +               };
> +       };
>  };
>
>  &apps_rsc {
> @@ -286,6 +293,28 @@ vdd_gfx: regulator@39 {
>         };
>  };
>
> +&gpucc {
> +       vdd-gfx-supply = <&vdd_gfx>;
> +       status = "okay";
> +};
> +
> +&gmu {
> +       status = "okay";
> +};
> +
> +&gpu {
> +       status = "okay";
> +
> +       zap-shader {
> +               memory-region = <&gpu_mem>;
> +               firmware-name = "qcom/sa8295p/a690_zap.mdt";

Can we please have .mbn here? Other than that, it looks good to me.

> +       };
> +};
> +
> +&gpu_smmu {
> +       status = "okay";
> +};
> +
>  &mdss0 {
>         status = "okay";
>  };
>
> --
> 2.25.1
>
>


-- 
With best wishes
Dmitry

[PATCH 8/8] arm64: defconfig: Enable MAX20411 regulator driver

[Bjorn Andersson]

The Qualcomm SA8295P ADP board uses a max20411 to power the GPU
subsystem.

Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 arch/arm64/configs/defconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/arm64/configs/defconfig b/arch/arm64/configs/defconfig
index ef1061089548..ec94a0c4fd03 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -751,6 +751,7 @@ CONFIG_REGULATOR_HI6421V530=y
 CONFIG_REGULATOR_HI655X=y
 CONFIG_REGULATOR_MAX77620=y
 CONFIG_REGULATOR_MAX8973=y
+CONFIG_REGULATOR_MAX20411=m
 CONFIG_REGULATOR_MP8859=y
 CONFIG_REGULATOR_MT6315=m
 CONFIG_REGULATOR_MT6357=y

-- 
2.25.1