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 <[email protected]>
---
Changes in v2:
- Made gpucc binding accept either power-domain or vdd-gfx-supply
- Updated comment in gdsc_gx_do_nothing_enable()
- Added a comment for the /delete-property/ power-domains
- Fixed node and property sort order in dts
- Switched zap firmware to use mbn file
- Link to v1: https://lore.kernel.org/r/[email protected]
---
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 | 16 +++++
arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 69 ++++++++++++++++++++++
arch/arm64/boot/dts/qcom/sa8540p.dtsi | 3 +
arch/arm64/configs/defconfig | 1 +
drivers/clk/qcom/gdsc.c | 12 +++-
drivers/clk/qcom/gpucc-sc8280xp.c | 1 +
drivers/pmdomain/qcom/rpmhpd.c | 1 -
7 files changed, 100 insertions(+), 3 deletions(-)
---
base-commit: 20d857259d7d10cd0d5e8b60608455986167cfad
change-id: 20231220-sa8295p-gpu-51c5f343e3ec
Best regards,
--
Bjorn Andersson <[email protected]>
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")
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
arch/arm64/boot/dts/qcom/sa8540p.dtsi | 3 +++
1 file changed, 3 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/sa8540p.dtsi b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
index 96b2c59ad02b..23888029cc11 100644
--- a/arch/arm64/boot/dts/qcom/sa8540p.dtsi
+++ b/arch/arm64/boot/dts/qcom/sa8540p.dtsi
@@ -168,6 +168,9 @@ opp-2592000000 {
};
&gpucc {
+ /* SA8295P and SA8540P doesn't provide gfx.lvl */
+ /delete-property/ power-domains;
+
status = "disabled";
};
--
2.25.1
The Qualcomm SA8295P ADP board uses a max20411 to power the GPU
subsystem.
Signed-off-by: Bjorn Andersson <[email protected]>
---
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
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 <[email protected]>
---
drivers/clk/qcom/gdsc.c | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)
diff --git a/drivers/clk/qcom/gdsc.c b/drivers/clk/qcom/gdsc.c
index 5358e28122ab..e7a4068b9f39 100644
--- a/drivers/clk/qcom/gdsc.c
+++ b/drivers/clk/qcom/gdsc.c
@@ -557,7 +557,15 @@ void gdsc_unregister(struct gdsc_desc *desc)
*/
int gdsc_gx_do_nothing_enable(struct generic_pm_domain *domain)
{
- /* Do nothing but give genpd the impression that we were successful */
- return 0;
+ struct gdsc *sc = domain_to_gdsc(domain);
+ int ret = 0;
+
+ /* Enable the parent supply, when controlled through the regulator framework. */
+ if (sc->rsupply)
+ ret = regulator_enable(sc->rsupply);
+
+ /* Do nothing with the GDSC itself */
+
+ return ret;
}
EXPORT_SYMBOL_GPL(gdsc_gx_do_nothing_enable);
--
2.25.1
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 <[email protected]>
---
Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 16 ++++++++++++++++
1 file changed, 16 insertions(+)
diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
index f369fa34e00c..c0dd24c9dcb3 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
@@ -74,6 +77,19 @@ required:
- '#reset-cells'
- '#power-domain-cells'
+# Allow either power-domains or vdd-gfx-supply, not both
+oneOf:
+ - required:
+ - power-domains
+ - required:
+ - vdd-gfx-supply
+ - not:
+ anyOf:
+ - required:
+ - power-domains
+ - required:
+ - vdd-gfx-supply
+
additionalProperties: false
examples:
--
2.25.1
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 <[email protected]>
---
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 14327c697116..304c8d79bd31 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.mbn";
+ };
+};
+
+&gpu_smmu {
+ status = "okay";
+};
+
&mdss0 {
status = "okay";
};
--
2.25.1
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.
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
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
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")
Reviewed-by: Dmitry Baryshkov <[email protected]>
Signed-off-by: Bjorn Andersson <[email protected]>
---
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
From: Bjorn Andersson <[email protected]>
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 <[email protected]>
---
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..14327c697116 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-0 = <&qup1_i2c4_state>;
+ pinctrl-names = "default";
+
+ status = "okay";
+
+ vdd_gfx: regulator@39 {
+ compatible = "maxim,max20411";
+ reg = <0x39>;
+
+ regulator-min-microvolt = <800000>;
+ regulator-max-microvolt = <800000>;
+
+ enable-gpios = <&pmm8540a_gpios 2 GPIO_ACTIVE_HIGH>;
+
+ pinctrl-0 = <&max20411_en>;
+ pinctrl-names = "default";
+ };
+};
+
&mdss0 {
status = "okay";
};
@@ -476,6 +496,10 @@ &pcie4_phy {
status = "okay";
};
+&qup1 {
+ status = "okay";
+};
+
&qup2 {
status = "okay";
};
@@ -636,6 +660,14 @@ &xo_board_clk {
/* PINCTRL */
+&pmm8540a_gpios {
+ max20411_en: max20411-en-state {
+ pins = "gpio2";
+ function = "normal";
+ output-enable;
+ };
+};
+
&tlmm {
pcie2a_default: pcie2a-default-state {
clkreq-n-pins {
@@ -728,4 +760,12 @@ wake-n-pins {
bias-pull-up;
};
};
+
+ qup1_i2c4_state: qup1-i2c4-state {
+ pins = "gpio0", "gpio1";
+ function = "qup12";
+
+ drive-strength = <2>;
+ bias-pull-up;
+ };
};
--
2.25.1
On Fri, 22 Dec 2023 at 06:40, Bjorn Andersson <[email protected]> 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 <[email protected]>
> ---
> arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 29 +++++++++++++++++++++++++++++
> 1 file changed, 29 insertions(+)
Reviewed-by: Dmitry Baryshkov <[email protected]>
--
With best wishes
Dmitry
On 22/12/2023 05:39, Bjorn Andersson 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 <[email protected]>
> ---
> Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 16 ++++++++++++++++
> 1 file changed, 16 insertions(+)
>
> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> index f369fa34e00c..c0dd24c9dcb3 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
>
> @@ -74,6 +77,19 @@ required:
> - '#reset-cells'
> - '#power-domain-cells'
>
> +# Allow either power-domains or vdd-gfx-supply, not both
> +oneOf:
> + - required:
> + - power-domains
> + - required:
> + - vdd-gfx-supply
This should be enough, assuming one of them is actually required. The
code. See also:
https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
Best regards,
Krzysztof
On 22.12.2023 09:12, Krzysztof Kozlowski wrote:
> On 22/12/2023 05:39, Bjorn Andersson 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 <[email protected]>
>> ---
>> Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 16 ++++++++++++++++
>> 1 file changed, 16 insertions(+)
>>
>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
>> index f369fa34e00c..c0dd24c9dcb3 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
>>
>> @@ -74,6 +77,19 @@ required:
>> - '#reset-cells'
>> - '#power-domain-cells'
>>
>> +# Allow either power-domains or vdd-gfx-supply, not both
>> +oneOf:
>> + - required:
>> + - power-domains
>> + - required:
>> + - vdd-gfx-supply
>
> This should be enough, assuming one of them is actually required. The
> code. See also:
> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
At least one of them indeed is, though this change is being made
implicitly. No clock controller works with no power FWIW
Konrad
On 22.12.2023 05:39, 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 <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On 22.12.2023 05:39, Bjorn Andersson 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.
>
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
Worth noting the regulator framework will create a virtual supply
for the normal 8280
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On 22.12.2023 05:39, Bjorn Andersson 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")
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On 22.12.2023 05:39, Bjorn Andersson wrote:
> From: Bjorn Andersson <[email protected]>
>
> 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 <[email protected]>
> ---
> &tlmm {
> pcie2a_default: pcie2a-default-state {
> clkreq-n-pins {
> @@ -728,4 +760,12 @@ wake-n-pins {
> bias-pull-up;
> };
> };
> +
> + qup1_i2c4_state: qup1-i2c4-state {
> + pins = "gpio0", "gpio1";
> + function = "qup12";
> +
> + drive-strength = <2>;
unnecessary newline
Konrad
On 22.12.2023 05:39, Bjorn Andersson 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 <[email protected]>
> ---
[...]
> +&gpucc {
> + vdd-gfx-supply = <&vdd_gfx>;
> + status = "okay";
> +};
Already enabled
> +
> +&gmu {
> + status = "okay";
> +};
> +
> +&gpu {
> + status = "okay";
> +
> + zap-shader {
> + memory-region = <&gpu_mem>;
> + firmware-name = "qcom/sa8295p/a690_zap.mbn";
> + };
> +};
> +
> +&gpu_smmu {
> + status = "okay";
> +};
Already enabled
Konrad
On 22.12.2023 05:39, Bjorn Andersson 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")
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
> ---
Reviewed-by: Konrad Dybcio <[email protected]>
Konrad
On Wed, Dec 27, 2023 at 02:09:47AM +0100, Konrad Dybcio wrote:
> On 22.12.2023 05:39, Bjorn Andersson 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 <[email protected]>
> > ---
> [...]
>
> > +&gpucc {
> > + vdd-gfx-supply = <&vdd_gfx>;
> > + status = "okay";
> > +};
> Already enabled
>
No, we're disabling these in sa8540p.dtsi, so they need to be re-enabled
here.
I don't remember if it's because the attempt to bring up gfx.lvl or if
it's the attempt to operate the GPU components without adequate VDD_GFX,
that is causing the issue...but either way, we don't survive boot.
It's possible that we could move the max20411 up to sa8540p.dtsi to
avoid the intermediate disable, but I'm not confident that it's "part of
the platform"...
Regards,
Bjorn
> > +
> > +&gmu {
> > + status = "okay";
> > +};
> > +
> > +&gpu {
> > + status = "okay";
> > +
> > + zap-shader {
> > + memory-region = <&gpu_mem>;
> > + firmware-name = "qcom/sa8295p/a690_zap.mbn";
> > + };
> > +};
> > +
> > +&gpu_smmu {
> > + status = "okay";
> > +};
> Already enabled
>
>
> Konrad
On Fri, Dec 22, 2023 at 09:12:04AM +0100, Krzysztof Kozlowski wrote:
> On 22/12/2023 05:39, Bjorn Andersson 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 <[email protected]>
> > ---
> > Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 16 ++++++++++++++++
> > 1 file changed, 16 insertions(+)
> >
> > diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
> > index f369fa34e00c..c0dd24c9dcb3 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
> >
> > @@ -74,6 +77,19 @@ required:
> > - '#reset-cells'
> > - '#power-domain-cells'
> >
> > +# Allow either power-domains or vdd-gfx-supply, not both
> > +oneOf:
> > + - required:
> > + - power-domains
> > + - required:
> > + - vdd-gfx-supply
>
> This should be enough, assuming one of them is actually required. The
> code. See also:
> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
>
Yes, that would be the correct binding. But the majority of our
DeviceTree source does not specify a power-domain for their gpucc.
While this should be corrected, it seem reasonable to leave this
optional for now.
Regards,
Bjorn
On 27.12.2023 21:30, Bjorn Andersson wrote:
> On Fri, Dec 22, 2023 at 09:12:04AM +0100, Krzysztof Kozlowski wrote:
>> On 22/12/2023 05:39, Bjorn Andersson 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 <[email protected]>
>>> ---
>>> Documentation/devicetree/bindings/clock/qcom,gpucc.yaml | 16 ++++++++++++++++
>>> 1 file changed, 16 insertions(+)
>>>
>>> diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
>>> index f369fa34e00c..c0dd24c9dcb3 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
>>>
>>> @@ -74,6 +77,19 @@ required:
>>> - '#reset-cells'
>>> - '#power-domain-cells'
>>>
>>> +# Allow either power-domains or vdd-gfx-supply, not both
>>> +oneOf:
>>> + - required:
>>> + - power-domains
>>> + - required:
>>> + - vdd-gfx-supply
>>
>> This should be enough, assuming one of them is actually required. The
>> code. See also:
>> https://elixir.bootlin.com/linux/v5.17-rc2/source/Documentation/devicetree/bindings/reserved-memory/reserved-memory.yaml#L91
>>
>
> Yes, that would be the correct binding. But the majority of our
> DeviceTree source does not specify a power-domain for their gpucc.
>
> While this should be corrected, it seem reasonable to leave this
> optional for now.
Moreover, I think it would be reasonable to add power-domains as
required in qcom,gcc.yaml. IIRC all "normal" (not q6) clock controllers
use at least CX+MX, with perhaps more hw-specific domains for some clocks
or GDSCs.
Konrad
On 27.12.2023 21:21, Bjorn Andersson wrote:
> On Wed, Dec 27, 2023 at 02:09:47AM +0100, Konrad Dybcio wrote:
>> On 22.12.2023 05:39, Bjorn Andersson 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 <[email protected]>
>>> ---
>> [...]
>>
>>> +&gpucc {
>>> + vdd-gfx-supply = <&vdd_gfx>;
>>> + status = "okay";
>>> +};
>> Already enabled
>>
>
> No, we're disabling these in sa8540p.dtsi, so they need to be re-enabled
> here.
>
> I don't remember if it's because the attempt to bring up gfx.lvl or if
> it's the attempt to operate the GPU components without adequate VDD_GFX,
> that is causing the issue...but either way, we don't survive boot.
Oh right!
On 8155 touching mmcx, lcx or lmx would kaboom the platform..
>
>
> It's possible that we could move the max20411 up to sa8540p.dtsi to
> avoid the intermediate disable, but I'm not confident that it's "part of
> the platform"...
Yeah, it's probably a question that is impossible to answer, as my
wild assumption is that all designs are ADP-derived anyway..
Konrad
On Fri, 22 Dec 2023 at 05:39, Bjorn Andersson <[email protected]> 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")
> Reviewed-by: Dmitry Baryshkov <[email protected]>
> Signed-off-by: Bjorn Andersson <[email protected]>
I guess it's easier if you funnel this through the soc tree - or you
prefer if I take it through my pmdomain tree?
No matter what, feel free to add:
Acked-by: Ulf Hansson <[email protected]>
Kind regards
Uffe
> ---
> 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
>
On Wed, Dec 27, 2023 at 02:07:52AM +0100, Konrad Dybcio wrote:
> On 22.12.2023 05:39, Bjorn Andersson 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.
> >
> > Reviewed-by: Dmitry Baryshkov <[email protected]>
> > Signed-off-by: Bjorn Andersson <[email protected]>
> > ---
> Worth noting the regulator framework will create a virtual supply
> for the normal 8280
>
You're right. No functional harm, but that's not very nice.
I don't think we have any benefit from having a dummy supply, if the DT
author failed to provide a proper one, so it seems reasonable to switch
gdsc to devm_regulator_get_optional().
Regards,
Bjorn
> Reviewed-by: Konrad Dybcio <[email protected]>
>
> Konrad