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

[PATCH v4 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>
---
Changes in v4:
- Updated qcom,gpucc.yaml binding check that both power-domains and
  vdd-gfx-supply isn't used together. Updated related comment as well.
- Link to v3: https://lore.kernel.org/r/20240123-sa8295p-gpu-v3-0-d5b4474c8f33@quicinc.com

Changes in v3:
- Removed one unnecessary empty line in DT node
- Rebased series to v6.8-rc1
- Link to v2: https://lore.kernel.org/r/20231220-sa8295p-gpu-v2-0-4763246b72c0@quicinc.com

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/20231220-sa8295p-gpu-v1-0-d8cdf2257f97@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      |  9 +++
 arch/arm64/boot/dts/qcom/sa8295p-adp.dts           | 68 ++++++++++++++++++++++
 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, 92 insertions(+), 3 deletions(-)
---
base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
change-id: 20231220-sa8295p-gpu-51c5f343e3ec

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

[PATCH v4 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 | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml b/Documentation/devicetree/bindings/clock/qcom,gpucc.yaml
index f369fa34e00c..f57aceddac6b 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,12 @@ required:
   - '#reset-cells'
   - '#power-domain-cells'
 
+# Require that power-domains and vdd-gfx-supply are not both present
+not:
+  required:
+    - power-domains
+    - vdd-gfx-supply
+
 additionalProperties: false
 
 examples:

-- 
2.25.1

[PATCH v4 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().

Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 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

[PATCH v4 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.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
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

[PATCH v4 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")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Acked-by: Ulf Hansson <ulf.hansson@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
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

[PATCH v4 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")
Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
---
 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

[PATCH v4 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 | 39 ++++++++++++++++++++++++++++++++
 1 file changed, 39 insertions(+)

diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
index fd253942e5e5..bd0962f39fc5 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,11 @@ 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

[PATCH v4 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.

Reviewed-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>
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 bd0962f39fc5..78e933c42c31 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

[PATCH v4 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 e6cf3e5d63c3..43cd31f30fd2 100644
--- a/arch/arm64/configs/defconfig
+++ b/arch/arm64/configs/defconfig
@@ -752,6 +752,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

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

[Bjorn Andersson]

On Thu, Jan 25, 2024 at 01:05:06PM -0800, Bjorn Andersson wrote:
> 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>

Sorry, I forgot to change the subject prefix of the rpmhpd patch, and
pick up Konrad's two r-b.

Regards,
Bjorn

> ---
> Changes in v4:
> - Updated qcom,gpucc.yaml binding check that both power-domains and
>   vdd-gfx-supply isn't used together. Updated related comment as well.
> - Link to v3: https://lore.kernel.org/r/20240123-sa8295p-gpu-v3-0-d5b4474c8f33@quicinc.com
> 
> Changes in v3:
> - Removed one unnecessary empty line in DT node
> - Rebased series to v6.8-rc1
> - Link to v2: https://lore.kernel.org/r/20231220-sa8295p-gpu-v2-0-4763246b72c0@quicinc.com
> 
> 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/20231220-sa8295p-gpu-v1-0-d8cdf2257f97@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      |  9 +++
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dts           | 68 ++++++++++++++++++++++
>  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, 92 insertions(+), 3 deletions(-)
> ---
> base-commit: 6613476e225e090cc9aad49be7fa504e290dd33d
> change-id: 20231220-sa8295p-gpu-51c5f343e3ec
> 
> Best regards,
> -- 
> Bjorn Andersson <quic_bjorande@quicinc.com>
> 

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

[Dmitry Baryshkov]

On Thu, 25 Jan 2024 at 23:06, 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>

This doesn't match the From header.

> ---
>  arch/arm64/boot/dts/qcom/sa8295p-adp.dts | 39 ++++++++++++++++++++++++++++++++
>  1 file changed, 39 insertions(+)
>
> diff --git a/arch/arm64/boot/dts/qcom/sa8295p-adp.dts b/arch/arm64/boot/dts/qcom/sa8295p-adp.dts
> index fd253942e5e5..bd0962f39fc5 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,11 @@ 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
>
>


-- 
With best wishes
Dmitry

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

[Krzysztof Kozlowski]

On 25/01/2024 22:05, 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 <quic_bjorande@quicinc.com>
> ---

Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org>

Best regards,
Krzysztof

Re: (subset) [PATCH v4 0/8] arm64: dts: qcom: sa8295p: Enable GPU

[Bjorn Andersson]

On Thu, 25 Jan 2024 13:05:06 -0800, Bjorn Andersson wrote:
> 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.
> 
> [...]

Applied, thanks!

[8/8] arm64: defconfig: Enable MAX20411 regulator driver
      commit: 42945eb663d88471a0c394d9f466401b1a8d791f

Best regards,
-- 
Bjorn Andersson <andersson@kernel.org>

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

[Johan Hovold]

On Thu, Jan 25, 2024 at 01:05:08PM -0800, 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().
> 
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> ---
>  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);

This patch (and series) is now in mainline as commit 9187ebb954ab ("clk:
qcom: gdsc: Enable supply reglator in GPU GX handler") and appears to be
involved in triggering the below lockdep splat on boot of the Lenovo
ThinkPad X13s.

Adding Ulf and the MSM DRM devs as well in case I blamed the wrong
change here.

Johan


[    5.849106] ======================================================
[    5.849111] WARNING: possible circular locking dependency detected
[    5.849115] 6.8.0 #111 Not tainted
[    5.849119] ------------------------------------------------------
[    5.849123] kworker/u32:2/66 is trying to acquire lock:
[    5.849128] ffffaffb0ca9b4f0 (regulator_list_mutex){+.+.}-{3:3}, at: regulator_lock_dependent+0x54/0x27c
[    5.849148] 
               but task is already holding lock:
[    5.849152] ffffaffad8f33a00 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x18/0x24
[    5.849165] 
               which lock already depends on the new lock.

[    5.849170] 
               the existing dependency chain (in reverse order) is:
[    5.849175] 
               -> #3 (&genpd->mlock){+.+.}-{3:3}:
[    5.849182]        lock_acquire+0x68/0x84
[    5.849190]        __mutex_lock+0xa0/0x840
[    5.849197]        mutex_lock_nested+0x24/0x30
[    5.849201]        genpd_lock_mtx+0x18/0x24
[    5.849206]        genpd_runtime_resume+0x104/0x2f4
[    5.849211]        __rpm_callback+0x48/0x1a8
[    5.849218]        rpm_callback+0x68/0x74
[    5.849223]        rpm_resume+0x444/0x638
[    5.849228]        __pm_runtime_resume+0x5c/0xbc
[    5.849233]        device_link_add+0x680/0x6e8
[    5.849239]        arm_smmu_probe_device+0x2a4/0x3e4
[    5.849245]        __iommu_probe_device+0x108/0x430
[    5.849250]        iommu_probe_device+0x3c/0x80
[    5.849255]        of_iommu_configure+0x170/0x25c
[    5.849260]        of_dma_configure_id+0x10c/0x340
[    5.849265]        platform_dma_configure+0x78/0xbc
[    5.849270]        really_probe+0x74/0x388
[    5.849276]        __driver_probe_device+0x7c/0x160
[    5.849281]        driver_probe_device+0x40/0x114
[    5.849287]        __driver_attach+0xfc/0x208
[    5.849292]        bus_for_each_dev+0x74/0xd0
[    5.849296]        driver_attach+0x24/0x30
[    5.849301]        bus_add_driver+0x110/0x214
[    5.849306]        driver_register+0x60/0x128
[    5.849312]        __platform_driver_register+0x28/0x34
[    5.849316]        panel_edp_init+0x20/0x1000 [panel_edp]
[    5.849329]        panel_edp_init+0xc8/0x1000 [panel_edp]
[    5.849337]        do_one_initcall+0x74/0x344
[    5.849342]        do_init_module+0x5c/0x1f8
[    5.849347]        load_module+0x1c9c/0x1e18
[    5.849351]        init_module_from_file+0x84/0xc0
[    5.849356]        idempotent_init_module+0x180/0x250
[    5.849360]        __arm64_sys_finit_module+0x68/0xcc
[    5.849365]        invoke_syscall+0x48/0x114
[    5.849370]        el0_svc_common.constprop.0+0xc0/0xe0
[    5.849376]        do_el0_svc+0x1c/0x28
[    5.849381]        el0_svc+0x48/0x114
[    5.849386]        el0t_64_sync_handler+0xc0/0xc4
[    5.849391]        el0t_64_sync+0x190/0x194
[    5.849395] 
               -> #2 (dpm_list_mtx){+.+.}-{3:3}:
[    5.849403]        lock_acquire+0x68/0x84
[    5.849408]        __mutex_lock+0xa0/0x840
[    5.849412]        mutex_lock_nested+0x24/0x30
[    5.849416]        device_pm_lock+0x1c/0x28
[    5.849422]        device_link_add+0x274/0x6e8
[    5.849426]        fw_devlink_create_devlink+0x118/0x2fc
[    5.849431]        __fw_devlink_link_to_consumers.isra.0+0x50/0x104
[    5.849437]        device_add+0x744/0x7b0
[    5.849441]        of_device_add+0x44/0x60
[    5.849446]        of_platform_device_create_pdata+0x98/0x140
[    5.849451]        of_platform_bus_create+0x1b4/0x4b4
[    5.849455]        of_platform_populate+0x58/0x150
[    5.849459]        of_platform_default_populate_init+0xd8/0xf0
[    5.849466]        do_one_initcall+0x74/0x344
[    5.849470]        kernel_init_freeable+0x244/0x350
[    5.849476]        kernel_init+0x20/0x1d8
[    5.849480]        ret_from_fork+0x10/0x20
[    5.849484] 
               -> #1 (device_links_lock){+.+.}-{3:3}:
[    5.849492]        lock_acquire+0x68/0x84
[    5.849497]        __mutex_lock+0xa0/0x840
[    5.849501]        mutex_lock_nested+0x24/0x30
[    5.849505]        device_link_remove+0x38/0x94
[    5.849509]        _regulator_put.part.0+0x168/0x190
[    5.849515]        regulator_bulk_free+0x64/0x90
[    5.849521]        devm_regulator_bulk_release+0x1c/0x28
[    5.849526]        release_nodes+0x5c/0x90
[    5.849532]        devres_release_group+0xc8/0x134
[    5.849537]        i2c_device_probe+0x138/0x2e8 [i2c_core]
[    5.849549]        really_probe+0xc0/0x388
[    5.849554]        __driver_probe_device+0x7c/0x160
[    5.849559]        driver_probe_device+0x40/0x114
[    5.849565]        __device_attach_driver+0xbc/0x158
[    5.849571]        bus_for_each_drv+0x84/0xe0
[    5.849576]        __device_attach_async_helper+0xb0/0x10c
[    5.849582]        async_run_entry_fn+0x34/0x14c
[    5.849588]        process_one_work+0x220/0x634
[    5.849595]        worker_thread+0x268/0x3a8
[    5.849599]        kthread+0x124/0x128
[    5.849603]        ret_from_fork+0x10/0x20
[    5.849608] 
               -> #0 (regulator_list_mutex){+.+.}-{3:3}:
[    5.849615]        __lock_acquire+0x130c/0x2064
[    5.849621]        lock_acquire.part.0+0xc8/0x20c
[    5.849626]        lock_acquire+0x68/0x84
[    5.849631]        __mutex_lock+0xa0/0x840
[    5.849636]        mutex_lock_nested+0x24/0x30
[    5.849640]        regulator_lock_dependent+0x54/0x27c
[    5.849646]        regulator_enable+0x34/0xd0
[    5.849651]        gdsc_gx_do_nothing_enable+0x18/0x2c
[    5.849659]        _genpd_power_on+0x94/0x17c
[    5.849664]        genpd_power_on.part.0+0xa4/0x1a8
[    5.849669]        genpd_runtime_resume+0x120/0x2f4
[    5.849673]        __rpm_callback+0x48/0x1a8
[    5.849678]        rpm_callback+0x68/0x74
[    5.849683]        rpm_resume+0x584/0x638
[    5.849688]        __pm_runtime_resume+0x5c/0xbc
[    5.849693]        a6xx_gmu_resume+0x70/0xcf8 [msm]
[    5.849722]        a6xx_gmu_pm_resume+0x3c/0x284 [msm]
[    5.849745]        adreno_runtime_resume+0x28/0x34 [msm]
[    5.849769]        pm_generic_runtime_resume+0x2c/0x44
[    5.849774]        __rpm_callback+0x48/0x1a8
[    5.849779]        rpm_callback+0x68/0x74
[    5.849783]        rpm_resume+0x444/0x638
[    5.849788]        __pm_runtime_resume+0x5c/0xbc
[    5.849793]        adreno_load_gpu+0x78/0x238 [msm]
[    5.849816]        msm_open+0x114/0x128 [msm]
[    5.849843]        drm_file_alloc+0x184/0x2a8 [drm]
[    5.849876]        drm_client_init+0x7c/0x10c [drm]
[    5.849900]        msm_fbdev_setup+0x84/0x150 [msm]
[    5.849929]        msm_drm_bind+0x264/0x420 [msm]
[    5.849950]        try_to_bring_up_aggregate_device+0x1ec/0x2f4
[    5.849962]        __component_add+0xa8/0x194
[    5.849965]        component_add+0x14/0x20
[    5.849967]        dp_display_probe_tail+0x4c/0xac [msm]
[    5.850000]        dp_auxbus_done_probe+0x14/0x20 [msm]
[    5.850023]        dp_aux_ep_probe+0x4c/0xf0 [drm_dp_aux_bus]
[    5.850030]        really_probe+0xc0/0x388
[    5.850034]        __driver_probe_device+0x7c/0x160
[    5.850038]        driver_probe_device+0x40/0x114
[    5.850042]        __device_attach_driver+0xbc/0x158
[    5.850046]        bus_for_each_drv+0x84/0xe0
[    5.850050]        __device_attach+0xa8/0x1d4
[    5.850053]        device_initial_probe+0x14/0x20
[    5.850058]        bus_probe_device+0xb0/0xb4
[    5.850061]        device_add+0x5b8/0x7b0
[    5.850065]        device_register+0x20/0x30
[    5.850068]        of_dp_aux_populate_bus+0xcc/0x1a0 [drm_dp_aux_bus]
[    5.850074]        devm_of_dp_aux_populate_bus+0x18/0x80 [drm_dp_aux_bus]
[    5.850080]        dp_display_probe+0x2a4/0x454 [msm]
[    5.850098]        platform_probe+0x68/0xd8
[    5.850102]        really_probe+0xc0/0x388
[    5.850106]        __driver_probe_device+0x7c/0x160
[    5.850109]        driver_probe_device+0x40/0x114
[    5.850113]        __device_attach_driver+0xbc/0x158
[    5.850117]        bus_for_each_drv+0x84/0xe0
[    5.850121]        __device_attach+0xa8/0x1d4
[    5.850125]        device_initial_probe+0x14/0x20
[    5.850129]        bus_probe_device+0xb0/0xb4
[    5.850133]        deferred_probe_work_func+0xa0/0xf4
[    5.850137]        process_one_work+0x220/0x634
[    5.850141]        worker_thread+0x268/0x3a8
[    5.850144]        kthread+0x124/0x128
[    5.850147]        ret_from_fork+0x10/0x20
[    5.850151] 
               other info that might help us debug this:

[    5.850154] Chain exists of:
                 regulator_list_mutex --> dpm_list_mtx --> &genpd->mlock

[    5.850163]  Possible unsafe locking scenario:

[    5.850166]        CPU0                    CPU1
[    5.850168]        ----                    ----
[    5.850171]   lock(&genpd->mlock);
[    5.850174]                                lock(dpm_list_mtx);
[    5.850178]                                lock(&genpd->mlock);
[    5.850182]   lock(regulator_list_mutex);
[    5.850185] 
                *** DEADLOCK ***
[    5.850188] 8 locks held by kworker/u32:2/66:
[    5.850191]  #0: ffff2be180020948 ((wq_completion)events_unbound){+.+.}-{0:0}, at: process_one_work+0x1a0/0x634
[    5.850199]  #1: ffff800081033de0 (deferred_probe_work){+.+.}-{0:0}, at: process_one_work+0x1c8/0x634
[    5.850207]  #2: ffff2be1849e38f8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x38/0x1d4
[    5.850215]  #3: ffff2be183c550e8 (&dev->mutex){....}-{3:3}, at: __device_attach+0x38/0x1d4
[    5.850223]  #4: ffffaffb0caa59e8 (component_mutex){+.+.}-{3:3}, at: __component_add+0x60/0x194
[    5.850231]  #5: ffffaffad96ee178 (init_lock){+.+.}-{3:3}, at: msm_open+0x38/0x128 [msm]
[    5.850252]  #6: ffff2be18d6536d8 (&a6xx_gpu->gmu.lock){+.+.}-{3:3}, at: a6xx_gmu_pm_resume+0x34/0x284 [msm]
[    5.850274]  #7: ffffaffad8f33a00 (&genpd->mlock){+.+.}-{3:3}, at: genpd_lock_mtx+0x18/0x24
[    5.850282] 
               stack backtrace:
[    5.850286] CPU: 6 PID: 66 Comm: kworker/u32:2 Not tainted 6.8.0 #111
[    5.850291] Hardware name: LENOVO 21BYZ9SRUS/21BYZ9SRUS, BIOS N3HET87W (1.59 ) 12/05/2023
[    5.850296] Workqueue: events_unbound deferred_probe_work_func
[    5.850301] Call trace:
[    5.850303]  dump_backtrace+0x9c/0x11c
[    5.850307]  show_stack+0x18/0x24
[    5.850312]  dump_stack_lvl+0x90/0xd0
[    5.850316]  dump_stack+0x18/0x24
[    5.850320]  print_circular_bug+0x290/0x370
[    5.850324]  check_noncircular+0x15c/0x170
[    5.850327]  __lock_acquire+0x130c/0x2064
[    5.850331]  lock_acquire.part.0+0xc8/0x20c
[    5.850335]  lock_acquire+0x68/0x84
[    5.850339]  __mutex_lock+0xa0/0x840
[    5.850342]  mutex_lock_nested+0x24/0x30
[    5.850345]  regulator_lock_dependent+0x54/0x27c
[    5.850349]  regulator_enable+0x34/0xd0
[    5.850352]  gdsc_gx_do_nothing_enable+0x18/0x2c
[    5.850356]  _genpd_power_on+0x94/0x17c
[    5.850360]  genpd_power_on.part.0+0xa4/0x1a8
[    5.850363]  genpd_runtime_resume+0x120/0x2f4
[    5.850366]  __rpm_callback+0x48/0x1a8
[    5.850370]  rpm_callback+0x68/0x74
[    5.850374]  rpm_resume+0x584/0x638
[    5.850377]  __pm_runtime_resume+0x5c/0xbc
[    5.850381]  a6xx_gmu_resume+0x70/0xcf8 [msm]
[    5.850398]  a6xx_gmu_pm_resume+0x3c/0x284 [msm]
[    5.850416]  adreno_runtime_resume+0x28/0x34 [msm]
[    5.850433]  pm_generic_runtime_resume+0x2c/0x44
[    5.850437]  __rpm_callback+0x48/0x1a8
[    5.850440]  rpm_callback+0x68/0x74
[    5.850443]  rpm_resume+0x444/0x638
[    5.850447]  __pm_runtime_resume+0x5c/0xbc
[    5.850450]  adreno_load_gpu+0x78/0x238 [msm]
[    5.850467]  msm_open+0x114/0x128 [msm]
[    5.850485]  drm_file_alloc+0x184/0x2a8 [drm]
[    5.850504]  drm_client_init+0x7c/0x10c [drm]
[    5.850522]  msm_fbdev_setup+0x84/0x150 [msm]
[    5.850540]  msm_drm_bind+0x264/0x420 [msm]
[    5.850557]  try_to_bring_up_aggregate_device+0x1ec/0x2f4
[    5.850562]  __component_add+0xa8/0x194
[    5.850566]  component_add+0x14/0x20
[    5.850570]  dp_display_probe_tail+0x4c/0xac [msm]
[    5.850587]  dp_auxbus_done_probe+0x14/0x20 [msm]
[    5.850605]  dp_aux_ep_probe+0x4c/0xf0 [drm_dp_aux_bus]
[    5.850611]  really_probe+0xc0/0x388
[    5.850615]  __driver_probe_device+0x7c/0x160
[    5.850619]  driver_probe_device+0x40/0x114
[    5.850624]  __device_attach_driver+0xbc/0x158
[    5.850628]  bus_for_each_drv+0x84/0xe0
[    5.850631]  __device_attach+0xa8/0x1d4
[    5.850635]  device_initial_probe+0x14/0x20
[    5.850639]  bus_probe_device+0xb0/0xb4
[    5.850642]  device_add+0x5b8/0x7b0
[    5.850645]  device_register+0x20/0x30
[    5.850648]  of_dp_aux_populate_bus+0xcc/0x1a0 [drm_dp_aux_bus]
[    5.850654]  devm_of_dp_aux_populate_bus+0x18/0x80 [drm_dp_aux_bus]
[    5.850660]  dp_display_probe+0x2a4/0x454 [msm]
[    5.850678]  platform_probe+0x68/0xd8
[    5.850681]  really_probe+0xc0/0x388
[    5.850684]  __driver_probe_device+0x7c/0x160
[    5.850688]  driver_probe_device+0x40/0x114
[    5.850692]  __device_attach_driver+0xbc/0x158
[    5.850696]  bus_for_each_drv+0x84/0xe0
[    5.850699]  __device_attach+0xa8/0x1d4
[    5.850703]  device_initial_probe+0x14/0x20
[    5.850707]  bus_probe_device+0xb0/0xb4
[    5.850710]  deferred_probe_work_func+0xa0/0xf4
[    5.850714]  process_one_work+0x220/0x634
[    5.850717]  worker_thread+0x268/0x3a8
[    5.850720]  kthread+0x124/0x128
[    5.850723]  ret_from_fork+0x10/0x20

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

[Johan Hovold]

On Fri, Mar 22, 2024 at 06:00:58PM +0100, Johan Hovold wrote:
> On Thu, Jan 25, 2024 at 01:05:08PM -0800, 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().
> > 
> > Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> > Signed-off-by: Bjorn Andersson <quic_bjorande@quicinc.com>
> > ---
> >  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);
> 
> This patch (and series) is now in mainline as commit 9187ebb954ab ("clk:
> qcom: gdsc: Enable supply reglator in GPU GX handler") and appears to be
> involved in triggering the below lockdep splat on boot of the Lenovo
> ThinkPad X13s.
> 
> Adding Ulf and the MSM DRM devs as well in case I blamed the wrong
> change here.

I've now verified that applying this series on top of 6.8 also triggers
the lockdep splat even if it is possible that it only exposed an
existing issue.

This is still a regression and also prevents using lockdep on these
platforms, which can lead to further locking issues being introduced
until this is fixed:

#regzbot ^introduced: 9187ebb954ab

Johan

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

[Johan Hovold]

On Thu, Jan 25, 2024 at 01:05:09PM -0800, 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 <dmitry.baryshkov@linaro.org>
> Reviewed-by: Konrad Dybcio <konrad.dybcio@linaro.org>
> 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",

This change now triggers warnings on SC8280XP which does not have this
supply:

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

I've sent a change to start treating this optional supply as truly
optional here (even if it has not shown up in lore yet):

	https://lore.kernel.org/r/20240325081957.10946-1-johan+linaro@kernel.org

But why are we still using the same compatible string for sc8280xp and
sa8540p and sa8295p if they differ in such a way?

Shouldn't these structures be different for the two classes of SoCs,
which would avoid such issues and which would allow us to continue to
warn if the supply is missing on a sa8540p derivative platforms where it
appears to be required.

Johan