msm: clk: Define a special power domain for SDM845 GX

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +0 -0

        /* Disable the resources */
        clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
        pm_runtime_put_sync(gmu->dev);

In very extreme cases an individual gdsc may wish to override the power domain enable or disable callback functions for their own purposes. Only set the generic gdsc callback if the function pointers are not already set.

Signed-off-by Jordan Crouse <[email protected]>

---
  drivers/clk/qcom/gdsc.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

drivers/clk/qcom/gdsc.c +4 -2

index b6adca1f3918..7b55368b9a9c 100644
@@ -394,8 +394,10 @@ static int gdsc_init(struct gdsc *sc)
  	else
  		gdsc_clear_mem_on(sc);
 
-	sc->pd.power_off = gdsc_disable;
-	sc->pd.power_on = gdsc_enable;
+	if (!sc->pd.power_off)
+		sc->pd.power_off = gdsc_disable;
+	if (!sc->pd.power_on)
+		sc->pd.power_on = gdsc_enable;
  	pm_genpd_init(&sc->pd, NULL, !on);
 
  	return 0;

99.999% of the time during normal operation the GMU is responsible for power and clock control on the GX domain and the CPU remains blissfully unaware. However, there is one situation where the CPU needs to get involved:

The power sequencing rules dictate that the GX needs to be turned off before the CX so that the CX can be turned on before the GX during power up. During normal operation when the CPU is taking down the CX domain a stop command is sent to the GMU which turns off the GX domain and then the CPU handles the CX domain.

But if the GMU happened to be unresponsive while the GX domain was left then the CPU will need to step in and turn off the GX domain

before resetting the CX and rebooting the GMU. This unfortunately means that the CPU needs to be marginally aware of the GX domain even though it is expected to usually keep its hands off.

To support this we create a semi-disabled GX power domain that does nothing to the hardware on power up but tries to shut it down normally on power down. In this method the reference counting is correct and we can step in with the pm_runtime_put() at the right time during the failure path.

This patch sets up the connection to the GX power domain and does the magic to "enable" and disable it at the right points.

Signed-off-by Jordan Crouse <[email protected]>

---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 41 ++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  2 ++
 2 files changed, 42 insertions(+), 1 deletion(-)

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +7 -1

index 51493f409358..ca71709efc94 100644
@@ -1142,9 +1169,15 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
        if (IS_ERR_OR_NULL(gmu->mmio))
                return;
 
-       pm_runtime_disable(gmu->dev);
        a6xx_gmu_stop(a6xx_gpu);
 
+       pm_runtime_disable(gmu->dev);
+
+       if (!IS_ERR(gmu->gxpd)) {
+               pm_runtime_disable(gmu->gxpd);
+               dev_pm_domain_detach(gmu->gxpd, false);
+       }
+
        a6xx_gmu_irq_disable(gmu);
        a6xx_gmu_memory_free(gmu, gmu->hfi);
 

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +1 -0

+
        /* Get the power levels for the GMU and GPU */
        a6xx_gmu_pwrlevels_probe(gmu);
 

Quoting Jordan Crouse (2018-11-19 15:47:06)

Quoted 5 lines

99.999% of the time during normal operation the GMU is responsible for power and clock control on the GX domain and the CPU remains blissfully unaware. However, there is one situation where the CPU needs to get involved:

Quoted 8 lines

The power sequencing rules dictate that the GX needs to be turned off before the CX so that the CX can be turned on before the GX during power up. During normal operation when the CPU is taking down the CX domain a stop command is sent to the GMU which turns off the GX domain and then the CPU handles the CX domain.

But if the GMU happened to be unresponsive while the GX domain was left then the CPU will need to step in and turn off the GX domain

^ left on?

Quoted 10 lines

before resetting the CX and rebooting the GMU. This unfortunately means that the CPU needs to be marginally aware of the GX domain even though it is expected to usually keep its hands off.

To support this we create a semi-disabled GX power domain that does nothing to the hardware on power up but tries to shut it down normally on power down. In this method the reference counting is correct and we can step in with the pm_runtime_put() at the right time during the failure path.

Quoted 4 lines

This patch sets up the connection to the GX power domain and does the magic to "enable" and disable it at the right points.

Signed-off-by Jordan Crouse <[email protected]>

The pm_runtime gymnastics is scary! But I'm willing to go with it.

Quoted 30 lines

---
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 41 ++++++++++++++++++++++++++-
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |  2 ++
 2 files changed, 42 insertions(+), 1 deletion(-)

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +7 -1

index 51493f409358..ca71709efc94 100644
@@ -1142,9 +1169,15 @@ void a6xx_gmu_remove(struct a6xx_gpu *a6xx_gpu)
        if (IS_ERR_OR_NULL(gmu->mmio))
                return;
 
-       pm_runtime_disable(gmu->dev);
        a6xx_gmu_stop(a6xx_gpu);
 
+       pm_runtime_disable(gmu->dev);
+
+       if (!IS_ERR(gmu->gxpd)) {
+               pm_runtime_disable(gmu->gxpd);
+               dev_pm_domain_detach(gmu->gxpd, false);
+       }
+
        a6xx_gmu_irq_disable(gmu);
        a6xx_gmu_memory_free(gmu, gmu->hfi);
 

@@ -1203,6 +1236,12 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
        if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0)
                goto err;
 

drivers/gpu/drm/msm/adreno/a6xx_gmu.c int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) · line 1243

Quoted 5 lines

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +5 -0

+       /*
+        * Get a link to the GX power domain to reset the GPU in case of GMU
+        * crash
+        */
+       gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");

Are there a6xx devices that don't have this genpd? Just curious if we could always assume that if it's an a6xx gpu then this must be there and we can always call pm_runtime_get/put on it and avoid the if (!IS_ERR()) checks.

+

Quoted 3 lines

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +0 -0

        /* Get the power levels for the GMU and GPU */
        a6xx_gmu_pwrlevels_probe(gmu);
 

Quoted 5 lines

Quoting Jordan Crouse (2018-11-19 15:47:06)

Quoted 4 lines

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +0 -0

@@ -1203,6 +1236,12 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
        if (gmu->hfi_irq < 0 || gmu->gmu_irq < 0)
                goto err;
 

Quoted 10 lines

drivers/gpu/drm/msm/adreno/a6xx_gmu.c int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node) · line 1243

Quoted 5 lines

drivers/gpu/drm/msm/adreno/a6xx_gmu.c +5 -0

+       /*
+        * Get a link to the GX power domain to reset the GPU in case of GMU
+        * crash
+        */
+       gmu->gxpd = dev_pm_domain_attach_by_name(gmu->dev, "gx");

Are there a6xx devices that don't have this genpd? Just curious if we could always assume that if it's an a6xx gpu then this must be there and we can always call pm_runtime_get/put on it and avoid the if (!IS_ERR()) checks.

Mainly I was trying to be backwards compatible with the device tree. I didn't want to derail various development efforts. I don't mind the checks (though I wish that pm_runtime_* was error tolerant) but folks really hate them we can replace this with an error return and force people to update their device trees.

The GPU GX domain on SDM845 is nominally managed by the GMU microcontroller but there are certain circumstances when the CPU needs to be sure that the GX headswitch is off.

This RFC series adds a special modification for the GX power domain that always returns success for domain power on and uses the default gdsc functions for power down.

With this, we should be able to remove most of GX clocks from the gpucc and have a relatively clean way of handling the hardware workaround.

This is based on the series from [1] with some slight changes for the current for-next from Andy.

[1] https://patchwork.kernel.org/patch/10563887/

Jordan Crouse (4):
drm/msm/a6xx: Remove unwanted regulator code
clk: qcom: gdsc: Don't override existing gdsc pd functions
clk: qcom: Add a dummy enable function for GX gdsc drm/msm/gpu: Attach to the GPU GX power domain

Quoting Jordan Crouse (2018-11-19 15:47:02)

Quoted 14 lines

The GPU GX domain on SDM845 is nominally managed by the GMU microcontroller but there are certain circumstances when the CPU needs to be sure that the GX headswitch is off.

This RFC series adds a special modification for the GX power domain that always returns success for domain power on and uses the default gdsc functions for power down.

With this, we should be able to remove most of GX clocks from the gpucc and have a relatively clean way of handling the hardware workaround.

This is based on the series from [1] with some slight changes for the current for-next from Andy.

Quoted 7 lines

[1] https://patchwork.kernel.org/patch/10563887/

Jordan Crouse (4):
drm/msm/a6xx: Remove unwanted regulator code
clk: qcom: gdsc: Don't override existing gdsc pd functions
clk: qcom: Add a dummy enable function for GX gdsc drm/msm/gpu: Attach to the GPU GX power domain

The clk bits look good to me, and would simplify the gpucc patches sitting on the list. Can we roll those into or on top of the GPU clk driver patches and split them from the GPU driver parts? I think the DTS will be blocked on everything coming together, so it should be OK to let the DTS bits come in and meet up with drm bits and clk bits all from different trees.