[PATCH v1 0/6] drm/msm: Improved a6xx GMU reset

[PATCH v1 0/6] drm/msm: Improved a6xx GMU reset

[Jordan Crouse]

This is a stack of changes for 5.1 (if I'm not already too late). The bulk of
the changes implement a better GMU reset sequence using the new gpucc power
domain added in 5.0. If a GMU fault occurs during runtime we try to do a
standard GPU recovery and if the fault happens during GMU start then try to
fail somewhat gracefully than BUG_ON.

There will be a DT change to go along with this, but we can send that along
after the core code is merged. The downside for not having the domain
properly listed is that the runtime reset sequence probably won't work which
is no worse than it is today.

Jordan Crouse (6):
  drm/msm/a6xx: Remove unwanted regulator code
  dt-bindings: drm/msm/a6xx: Add GX power-domain for GMU bindings
  drm/msm/gpu: Attach to the GPU GX power domain
  drm/msm/a6xx: Make GMU reset useful
  msm/drm/a6xx: Turn off the GMU if resume fails
  drm/msm/a6xx: Remove an unused struct member

 .../devicetree/bindings/display/msm/gmu.txt        |  10 +-
 drivers/gpu/drm/msm/adreno/a6xx_gmu.c              | 200 +++++++++++++--------
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h              |   9 +-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c              |  20 +--
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h              |   3 +-
 drivers/gpu/drm/msm/adreno/adreno_device.c         |   1 +
 6 files changed, 144 insertions(+), 99 deletions(-)

-- 
2.7.4

[PATCH v1 1/6] drm/msm/a6xx: Remove unwanted regulator code

[Jordan Crouse]

The GMU code currently has some misguided code to try to work around
a hardware quirk that requires the power domains on the GPU be
collapsed in a certain order. Upcoming patches will do this the
right way so get rid of the unused and unwanted regulator
code.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 4 ----
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 2 --
 2 files changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index ce1b3cc..f1baf64f 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -671,9 +671,6 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu)
 	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS3_DRV0_STATUS, val,
 		(val & 1), 100, 1000);
 
-	/* Force off the GX GSDC */
-	regulator_force_disable(gmu->gx);
-
 	/* Disable the resources */
 	clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
 	pm_runtime_put_sync(gmu->dev);
@@ -1203,7 +1200,6 @@ int a6xx_gmu_probe(struct a6xx_gpu *a6xx_gpu, struct device_node *node)
 	gmu->idle_level = GMU_IDLE_STATE_ACTIVE;
 
 	pm_runtime_enable(gmu->dev);
-	gmu->gx = devm_regulator_get(gmu->dev, "vdd");
 
 	/* Get the list of clocks */
 	ret = a6xx_gmu_clocks_probe(gmu);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index c721d91..8081083 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -52,8 +52,6 @@ struct a6xx_gmu {
 	int hfi_irq;
 	int gmu_irq;
 
-	struct regulator *gx;
-
 	struct iommu_domain *domain;
 	u64 uncached_iova_base;
 
-- 
2.7.4

[PATCH v1 2/6] dt-bindings: drm/msm/a6xx: Add GX power-domain for GMU bindings

[Jordan Crouse]

The GMU should have two power domains defined: "cx" and "gx". "cx" is the
actual power domain for the device and "gx" will be attached at runtime
to manage reference counting on the GPU device in case of a GMU crash.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 Documentation/devicetree/bindings/display/msm/gmu.txt | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/Documentation/devicetree/bindings/display/msm/gmu.txt b/Documentation/devicetree/bindings/display/msm/gmu.txt
index 3439b38..90af5b0 100644
--- a/Documentation/devicetree/bindings/display/msm/gmu.txt
+++ b/Documentation/devicetree/bindings/display/msm/gmu.txt
@@ -24,7 +24,10 @@ Required properties:
    * "cxo"
    * "axi"
    * "mnoc"
-- power-domains: should be <&clock_gpucc GPU_CX_GDSC>
+- power-domains: should be:
+	<&clock_gpucc GPU_CX_GDSC>
+	<&clock_gpucc GPU_GX_GDSC>
+- power-domain-names: Matching names for the power domains
 - iommus: phandle to the adreno iommu
 - operating-points-v2: phandle to the OPP operating points
 
@@ -51,7 +54,10 @@ Example:
 			<&gcc GCC_GPU_MEMNOC_GFX_CLK>;
 		clock-names = "gmu", "cxo", "axi", "memnoc";
 
-		power-domains = <&gpucc GPU_CX_GDSC>;
+		power-domains = <&gpucc GPU_CX_GDSC>,
+				<&gpucc GPU_GX_GDSC>;
+		power-domain-names = "cx", "gx";
+
 		iommus = <&adreno_smmu 5>;
 
 		operating-points-v2 = <&gmu_opp_table>;
-- 
2.7.4

[PATCH v1 3/6] drm/msm/gpu: Attach to the GPU GX power domain

[Jordan Crouse]

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 <jcrouse@codeaurora.org>
---

 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(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index f1baf64f..a527c50 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -2,6 +2,7 @@
 /* Copyright (c) 2017-2018 The Linux Foundation. All rights reserved. */
 
 #include <linux/clk.h>
+#include <linux/pm_domain.h>
 #include <linux/pm_opp.h>
 #include <soc/qcom/cmd-db.h>
 
@@ -671,6 +672,16 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu)
 	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS3_DRV0_STATUS, val,
 		(val & 1), 100, 1000);
 
+	/*
+	 * Depending on the state of the GMU at this point the GX domain might
+	 * have been left on. Hardware sequencing rules state that the GX has to
+	 * be turned off before the CX domain so this is that one time that
+	 * that calling pm_runtime_put_sync() is expected to do something useful
+	 * (turn off the headswitch)
+	 */
+	if (!IS_ERR(gmu->gxpd))
+		pm_runtime_put_sync(gmu->gxpd);
+
 	/* Disable the resources */
 	clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
 	pm_runtime_put_sync(gmu->dev);
@@ -732,6 +743,14 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 	/* Set the GPU to the highest power frequency */
 	__a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
 
+	/*
+	 * "enable" the GX power domain which won't actually do anything but it
+	 * will make sure that the refcounting is correct in case we need to
+	 * bring down the GX after a GMU failure
+	 */
+	if (!IS_ERR(gmu->gxpd))
+		pm_runtime_get(gmu->gxpd);
+
 out:
 	/* Make sure to turn off the boot OOB request on error */
 	if (ret)
@@ -803,6 +822,14 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
 	/* Tell RPMh to power off the GPU */
 	a6xx_rpmh_stop(gmu);
 
+	/*
+	 * Mark the GPU power domain as off. During the shutdown process the GMU
+	 * should actually turn off the power so this is really just a
+	 * houskeeping step
+	 */
+	if (!IS_ERR(gmu->gxpd))
+		pm_runtime_put_sync(gmu->gxpd);
+
 	clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
 
 	pm_runtime_put_sync(gmu->dev);
@@ -1172,9 +1199,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);
 
@@ -1233,6 +1266,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;
 
+	/*
+	 * 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");
+
 	/* Get the power levels for the GMU and GPU */
 	a6xx_gmu_pwrlevels_probe(gmu);
 
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 8081083..078d418 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -55,6 +55,8 @@ struct a6xx_gmu {
 	struct iommu_domain *domain;
 	u64 uncached_iova_base;
 
+	struct device *gxpd;
+
 	int idle_level;
 
 	struct a6xx_gmu_bo *hfi;
-- 
2.7.4

[PATCH v1 4/6] drm/msm/a6xx: Make GMU reset useful

[Jordan Crouse]

Now that the GX domain is sorted we can wire up a working GMU reset.
IF a GMU hang was detected then try to forcefully shut down the GMU
in the power down sequence which should ensure that it can recover
normally on the next power up.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c | 109 ++++++++++++++++------------------
 drivers/gpu/drm/msm/adreno/a6xx_gmu.h |   4 +-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c |   2 +-
 drivers/gpu/drm/msm/adreno/a6xx_gpu.h |   3 +-
 4 files changed, 55 insertions(+), 63 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index a527c50..e16d55d 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -9,6 +9,24 @@
 #include "a6xx_gpu.h"
 #include "a6xx_gmu.xml.h"
 
+static void a6xx_gmu_fault(struct a6xx_gmu *gmu)
+{
+	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+	struct msm_gpu *gpu = &adreno_gpu->base;
+	struct drm_device *dev = gpu->dev;
+	struct msm_drm_private *priv = dev->dev_private;
+
+	/* FIXME: add a banner here */
+	gmu->hung = true;
+
+	/* Turn off the hangcheck timer while we are resetting */
+	del_timer(&gpu->hangcheck_timer);
+
+	/* Queue the GPU handler because we need to treat this as a recovery */
+	queue_work(priv->wq, &gpu->recover_work);
+}
+
 static irqreturn_t a6xx_gmu_irq(int irq, void *data)
 {
 	struct a6xx_gmu *gmu = data;
@@ -20,8 +38,7 @@ static irqreturn_t a6xx_gmu_irq(int irq, void *data)
 	if (status & A6XX_GMU_AO_HOST_INTERRUPT_STATUS_WDOG_BITE) {
 		dev_err_ratelimited(gmu->dev, "GMU watchdog expired\n");
 
-		/* Temporary until we can recover safely */
-		BUG();
+		a6xx_gmu_fault(gmu);
 	}
 
 	if (status &  A6XX_GMU_AO_HOST_INTERRUPT_STATUS_HOST_AHB_BUS_ERROR)
@@ -45,8 +62,7 @@ static irqreturn_t a6xx_hfi_irq(int irq, void *data)
 	if (status & A6XX_GMU_GMU2HOST_INTR_INFO_CM3_FAULT) {
 		dev_err_ratelimited(gmu->dev, "GMU firmware fault\n");
 
-		/* Temporary until we can recover safely */
-		BUG();
+		a6xx_gmu_fault(gmu);
 	}
 
 	return IRQ_HANDLED;
@@ -156,10 +172,8 @@ static bool a6xx_gmu_check_idle_level(struct a6xx_gmu *gmu)
 }
 
 /* Wait for the GMU to get to its most idle state */
-int a6xx_gmu_wait_for_idle(struct a6xx_gpu *a6xx_gpu)
+int a6xx_gmu_wait_for_idle(struct a6xx_gmu *gmu)
 {
-	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
-
 	return spin_until(a6xx_gmu_check_idle_level(gmu));
 }
 
@@ -558,7 +572,7 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, unsigned int state)
 		if (!rpmh_init) {
 			a6xx_gmu_rpmh_init(gmu);
 			rpmh_init = true;
-		} else if (state != GMU_RESET) {
+		} else {
 			ret = a6xx_rpmh_start(gmu);
 			if (ret)
 				return ret;
@@ -647,10 +661,9 @@ static void a6xx_gmu_irq_disable(struct a6xx_gmu *gmu)
 	gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~0);
 }
 
-int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu)
+/* Force the GMU off in case it isn't responsive */
+static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
 {
-	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
-	int ret;
 	u32 val;
 
 	/* Flush all the queues */
@@ -671,44 +684,6 @@ int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu)
 		(val & 1), 100, 10000);
 	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS3_DRV0_STATUS, val,
 		(val & 1), 100, 1000);
-
-	/*
-	 * Depending on the state of the GMU at this point the GX domain might
-	 * have been left on. Hardware sequencing rules state that the GX has to
-	 * be turned off before the CX domain so this is that one time that
-	 * that calling pm_runtime_put_sync() is expected to do something useful
-	 * (turn off the headswitch)
-	 */
-	if (!IS_ERR(gmu->gxpd))
-		pm_runtime_put_sync(gmu->gxpd);
-
-	/* Disable the resources */
-	clk_bulk_disable_unprepare(gmu->nr_clocks, gmu->clocks);
-	pm_runtime_put_sync(gmu->dev);
-
-	/* Re-enable the resources */
-	pm_runtime_get_sync(gmu->dev);
-
-	/* Use a known rate to bring up the GMU */
-	clk_set_rate(gmu->core_clk, 200000000);
-	ret = clk_bulk_prepare_enable(gmu->nr_clocks, gmu->clocks);
-	if (ret)
-		goto out;
-
-	a6xx_gmu_irq_enable(gmu);
-
-	ret = a6xx_gmu_fw_start(gmu, GMU_RESET);
-	if (!ret)
-		ret = a6xx_hfi_start(gmu, GMU_COLD_BOOT);
-
-	/* Set the GPU back to the highest power frequency */
-	__a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
-
-out:
-	if (ret)
-		a6xx_gmu_clear_oob(gmu, GMU_OOB_BOOT_SLUMBER);
-
-	return ret;
 }
 
 int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
@@ -719,6 +694,8 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 	if (WARN(!gmu->mmio, "The GMU is not set up yet\n"))
 		return 0;
 
+	gmu->hung = false;
+
 	/* Turn on the resources */
 	pm_runtime_get_sync(gmu->dev);
 
@@ -774,9 +751,9 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu)
 	return true;
 }
 
-int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
+/* Gracefully try to shut down the GMU and by extension the GPU */
+static void a6xx_gmu_shutdown(struct a6xx_gmu *gmu)
 {
-	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
 	u32 val;
 
 	/*
@@ -786,10 +763,13 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
 	val = gmu_read(gmu, REG_A6XX_GPU_GMU_CX_GMU_RPMH_POWER_STATE);
 
 	if (val != 0xf) {
-		int ret = a6xx_gmu_wait_for_idle(a6xx_gpu);
+		int ret = a6xx_gmu_wait_for_idle(gmu);
 
-		/* Temporary until we can recover safely */
-		BUG_ON(ret);
+		/* If the GMU isn't responding assume it is hung */
+		if (ret) {
+			a6xx_gmu_force_off(gmu);
+			return;
+		}
 
 		/* tell the GMU we want to slumber */
 		a6xx_gmu_notify_slumber(gmu);
@@ -821,11 +801,26 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
 
 	/* Tell RPMh to power off the GPU */
 	a6xx_rpmh_stop(gmu);
+}
+
+
+int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
+{
+	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
+
+	/*
+	 * Force the GMU off if we detected a hang, otherwise try to shut it
+	 * down gracefully
+	 */
+	if (gmu->hung)
+		a6xx_gmu_force_off(gmu);
+	else
+		a6xx_gmu_shutdown(gmu);
 
 	/*
-	 * Mark the GPU power domain as off. During the shutdown process the GMU
-	 * should actually turn off the power so this is really just a
-	 * houskeeping step
+	 * Make sure the GX domain is off before turning off the GMU (CX)
+	 * domain. Usually the GMU does this but only if the shutdown sequence
+	 * was successful
 	 */
 	if (!IS_ERR(gmu->gxpd))
 		pm_runtime_put_sync(gmu->gxpd);
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index 078d418..c5b1887 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -27,9 +27,6 @@ struct a6xx_gmu_bo {
 /* the GMU is coming up for the first time or back from a power collapse */
 #define GMU_COLD_BOOT 1
 
-/* The GMU is being soft reset after a fault */
-#define GMU_RESET 2
-
 /*
  * These define the level of control that the GMU has - the higher the number
  * the more things that the GMU hardware controls on its own.
@@ -79,6 +76,7 @@ struct a6xx_gmu {
 	struct a6xx_hfi_queue queues[2];
 
 	struct tasklet_struct hfi_tasklet;
+	bool hung;
 };
 
 static inline u32 gmu_read(struct a6xx_gmu *gmu, u32 offset)
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index fefe773..f76d8cd 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -698,7 +698,7 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
 	 * Make sure the GMU is idle before continuing (because some transitions
 	 * may use VBIF
 	 */
-	a6xx_gmu_wait_for_idle(a6xx_gpu);
+	a6xx_gmu_wait_for_idle(&a6xx_gpu->gmu);
 
 	/* Clear the VBIF pipe before shutting down */
 	/* FIXME: This accesses the GPU - do we need to make sure it is on? */
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
index 528a4cf..b46279e 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.h
@@ -46,9 +46,8 @@ struct a6xx_gpu {
 int a6xx_gmu_resume(struct a6xx_gpu *gpu);
 int a6xx_gmu_stop(struct a6xx_gpu *gpu);
 
-int a6xx_gmu_wait_for_idle(struct a6xx_gpu *gpu);
+int a6xx_gmu_wait_for_idle(struct a6xx_gmu *gmu);
 
-int a6xx_gmu_reset(struct a6xx_gpu *a6xx_gpu);
 bool a6xx_gmu_isidle(struct a6xx_gmu *gmu);
 
 int a6xx_gmu_set_oob(struct a6xx_gmu *gmu, enum a6xx_gmu_oob_state state);
-- 
2.7.4

[PATCH v1 5/6] msm/drm/a6xx: Turn off the GMU if resume fails

[Jordan Crouse]

Currently if the GMU resume function fails all we try to do is clear the
BOOT_SLUMBER oob which usually times out and ends up in a cycle of death.
If the resume function fails at any point remove any RPMh votes that might
have been added and try to shut down the GMU hardware cleanly.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gmu.c      | 82 +++++++++++++++++++-----------
 drivers/gpu/drm/msm/adreno/a6xx_gpu.c      | 20 ++------
 drivers/gpu/drm/msm/adreno/adreno_device.c |  1 +
 3 files changed, 58 insertions(+), 45 deletions(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
index e16d55d..2e89ca3 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.c
@@ -638,20 +638,6 @@ static int a6xx_gmu_fw_start(struct a6xx_gmu *gmu, unsigned int state)
 	 A6XX_GMU_AO_HOST_INTERRUPT_STATUS_HOST_AHB_BUS_ERROR | \
 	 A6XX_GMU_AO_HOST_INTERRUPT_STATUS_FENCE_ERR)
 
-static void a6xx_gmu_irq_enable(struct a6xx_gmu *gmu)
-{
-	gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0);
-	gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_CLR, ~0);
-
-	gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_MASK,
-		~A6XX_GMU_IRQ_MASK);
-	gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK,
-		~A6XX_HFI_IRQ_MASK);
-
-	enable_irq(gmu->gmu_irq);
-	enable_irq(gmu->hfi_irq);
-}
-
 static void a6xx_gmu_irq_disable(struct a6xx_gmu *gmu)
 {
 	disable_irq(gmu->gmu_irq);
@@ -661,11 +647,24 @@ static void a6xx_gmu_irq_disable(struct a6xx_gmu *gmu)
 	gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~0);
 }
 
-/* Force the GMU off in case it isn't responsive */
-static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
+static void a6xx_gmu_rpmh_off(struct a6xx_gmu *gmu)
 {
 	u32 val;
 
+	/* Make sure there are no outstanding RPMh votes */
+	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS0_DRV0_STATUS, val,
+		(val & 1), 100, 10000);
+	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS1_DRV0_STATUS, val,
+		(val & 1), 100, 10000);
+	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS2_DRV0_STATUS, val,
+		(val & 1), 100, 10000);
+	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS3_DRV0_STATUS, val,
+		(val & 1), 100, 1000);
+}
+
+/* Force the GMU off in case it isn't responsive */
+static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
+{
 	/* Flush all the queues */
 	a6xx_hfi_stop(gmu);
 
@@ -676,14 +675,7 @@ static void a6xx_gmu_force_off(struct a6xx_gmu *gmu)
 	a6xx_sptprac_disable(gmu);
 
 	/* Make sure there are no outstanding RPMh votes */
-	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS0_DRV0_STATUS, val,
-		(val & 1), 100, 10000);
-	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS1_DRV0_STATUS, val,
-		(val & 1), 100, 10000);
-	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS2_DRV0_STATUS, val,
-		(val & 1), 100, 10000);
-	gmu_poll_timeout(gmu, REG_A6XX_RSCC_TCS3_DRV0_STATUS, val,
-		(val & 1), 100, 1000);
+	a6xx_gmu_rpmh_off(gmu);
 }
 
 int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
@@ -702,10 +694,15 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 	/* Use a known rate to bring up the GMU */
 	clk_set_rate(gmu->core_clk, 200000000);
 	ret = clk_bulk_prepare_enable(gmu->nr_clocks, gmu->clocks);
-	if (ret)
-		goto out;
+	if (ret) {
+		pm_runtime_put(gmu->dev);
+		return ret;
+	}
 
-	a6xx_gmu_irq_enable(gmu);
+	/* Enable the GMU interrupt */
+	gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_CLR, ~0);
+	gmu_write(gmu, REG_A6XX_GMU_AO_HOST_INTERRUPT_MASK, ~A6XX_GMU_IRQ_MASK);
+	enable_irq(gmu->gmu_irq);
 
 	/* Check to see if we are doing a cold or warm boot */
 	status = gmu_read(gmu, REG_A6XX_GMU_GENERAL_7) == 1 ?
@@ -716,6 +713,16 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 		goto out;
 
 	ret = a6xx_hfi_start(gmu, status);
+	if (ret)
+		goto out;
+
+	/*
+	 * Turn on the GMU firmware fault interrupt after we know the boot
+	 * sequence is successful
+	 */
+	gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_CLR, ~0);
+	gmu_write(gmu, REG_A6XX_GMU_GMU2HOST_INTR_MASK, ~A6XX_HFI_IRQ_MASK);
+	enable_irq(gmu->hfi_irq);
 
 	/* Set the GPU to the highest power frequency */
 	__a6xx_gmu_set_freq(gmu, gmu->nr_gpu_freqs - 1);
@@ -729,9 +736,12 @@ int a6xx_gmu_resume(struct a6xx_gpu *a6xx_gpu)
 		pm_runtime_get(gmu->gxpd);
 
 out:
-	/* Make sure to turn off the boot OOB request on error */
-	if (ret)
-		a6xx_gmu_clear_oob(gmu, GMU_OOB_BOOT_SLUMBER);
+	/* On failure, shut down the GMU to leave it in a good state */
+	if (ret) {
+		disable_irq(gmu->gmu_irq);
+		a6xx_rpmh_stop(gmu);
+		pm_runtime_put(gmu->dev);
+	}
 
 	return ret;
 }
@@ -754,6 +764,9 @@ bool a6xx_gmu_isidle(struct a6xx_gmu *gmu)
 /* Gracefully try to shut down the GMU and by extension the GPU */
 static void a6xx_gmu_shutdown(struct a6xx_gmu *gmu)
 {
+	struct a6xx_gpu *a6xx_gpu = container_of(gmu, struct a6xx_gpu, gmu);
+	struct adreno_gpu *adreno_gpu = &a6xx_gpu->base;
+	struct msm_gpu *gpu = &adreno_gpu->base;
 	u32 val;
 
 	/*
@@ -771,6 +784,12 @@ static void a6xx_gmu_shutdown(struct a6xx_gmu *gmu)
 			return;
 		}
 
+		/* Clear the VBIF pipe before shutting down */
+		gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0xf);
+		spin_until((gpu_read(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL1) & 0xf)
+			== 0xf);
+		gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0);
+
 		/* tell the GMU we want to slumber */
 		a6xx_gmu_notify_slumber(gmu);
 
@@ -808,6 +827,9 @@ int a6xx_gmu_stop(struct a6xx_gpu *a6xx_gpu)
 {
 	struct a6xx_gmu *gmu = &a6xx_gpu->gmu;
 
+	if (!pm_runtime_active(gmu->dev))
+		return 0;
+
 	/*
 	 * Force the GMU off if we detected a hang, otherwise try to shut it
 	 * down gracefully
diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
index f76d8cd..576559a 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gpu.c
@@ -678,13 +678,15 @@ static int a6xx_pm_resume(struct msm_gpu *gpu)
 	struct a6xx_gpu *a6xx_gpu = to_a6xx_gpu(adreno_gpu);
 	int ret;
 
-	ret = a6xx_gmu_resume(a6xx_gpu);
-
 	gpu->needs_hw_init = true;
 
+	ret = a6xx_gmu_resume(a6xx_gpu);
+	if (ret)
+		return ret;
+
 	msm_gpu_resume_devfreq(gpu);
 
-	return ret;
+	return 0;
 }
 
 static int a6xx_pm_suspend(struct msm_gpu *gpu)
@@ -694,18 +696,6 @@ static int a6xx_pm_suspend(struct msm_gpu *gpu)
 
 	devfreq_suspend_device(gpu->devfreq.devfreq);
 
-	/*
-	 * Make sure the GMU is idle before continuing (because some transitions
-	 * may use VBIF
-	 */
-	a6xx_gmu_wait_for_idle(&a6xx_gpu->gmu);
-
-	/* Clear the VBIF pipe before shutting down */
-	/* FIXME: This accesses the GPU - do we need to make sure it is on? */
-	gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0xf);
-	spin_until((gpu_read(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL1) & 0xf) == 0xf);
-	gpu_write(gpu, REG_A6XX_VBIF_XIN_HALT_CTRL0, 0);
-
 	return a6xx_gmu_stop(a6xx_gpu);
 }
 
diff --git a/drivers/gpu/drm/msm/adreno/adreno_device.c b/drivers/gpu/drm/msm/adreno/adreno_device.c
index 714ed65..0d87db7 100644
--- a/drivers/gpu/drm/msm/adreno/adreno_device.c
+++ b/drivers/gpu/drm/msm/adreno/adreno_device.c
@@ -229,6 +229,7 @@ struct msm_gpu *adreno_load_gpu(struct drm_device *dev)
 
 	ret = pm_runtime_get_sync(&pdev->dev);
 	if (ret < 0) {
+		pm_runtime_put_sync(&pdev->dev);
 		DRM_DEV_ERROR(dev->dev, "Couldn't power up the GPU: %d\n", ret);
 		return NULL;
 	}
-- 
2.7.4

[PATCH v1 6/6] drm/msm/a6xx: Remove an unused struct member

[Jordan Crouse]

The HFI tasklet was removed in df0dff1 ("drm/msm/a6xx: Poll for HFI
responses") but the tasklet_struct was accidentally left behind.

Signed-off-by: Jordan Crouse <jcrouse@codeaurora.org>
---

 drivers/gpu/drm/msm/adreno/a6xx_gmu.h | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
index c5b1887..bedd8e6 100644
--- a/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
+++ b/drivers/gpu/drm/msm/adreno/a6xx_gmu.h
@@ -75,7 +75,6 @@ struct a6xx_gmu {
 
 	struct a6xx_hfi_queue queues[2];
 
-	struct tasklet_struct hfi_tasklet;
 	bool hung;
 };
 
-- 
2.7.4

Re: [PATCH v1 2/6] dt-bindings: drm/msm/a6xx: Add GX power-domain for GMU bindings

[Rob Herring]

On Mon, Feb 4, 2019 at 10:15 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
>
> The GMU should have two power domains defined: "cx" and "gx". "cx" is the
> actual power domain for the device and "gx" will be attached at runtime
> to manage reference counting on the GPU device in case of a GMU crash.

power-domains are supposed to be actual regions on a chip die which
can be power gated. However, they are often abused by being defined in
terms of kernel PM domains which are not always the same thing. This
description sounds like the latter case.

Rob

Re: [PATCH v1 2/6] dt-bindings: drm/msm/a6xx: Add GX power-domain for GMU bindings

[Rob Clark]

On Sun, Feb 17, 2019 at 4:08 PM Rob Herring <robh+dt@kernel.org> wrote:
>
> On Mon, Feb 4, 2019 at 10:15 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
> >
> > The GMU should have two power domains defined: "cx" and "gx". "cx" is the
> > actual power domain for the device and "gx" will be attached at runtime
> > to manage reference counting on the GPU device in case of a GMU crash.
>
> power-domains are supposed to be actual regions on a chip die which
> can be power gated. However, they are often abused by being defined in
> terms of kernel PM domains which are not always the same thing. This
> description sounds like the latter case.
>

iirc (and Jordan can correct me), this arrangement was needed because
normally the GMU does the GPU power control (except for if we manage
to crash it and need to reset the GMU)..

so maybe not 100% about the actual regions on chip die which can be
gated.. but it is a reality of how hw + fw + sw fit together..

BR,
-R

Re: [PATCH v1 2/6] dt-bindings: drm/msm/a6xx: Add GX power-domain for GMU bindings

[Jordan Crouse]

On Sun, Feb 17, 2019 at 05:43:16PM -0500, Rob Clark wrote:
> On Sun, Feb 17, 2019 at 4:08 PM Rob Herring <robh+dt@kernel.org> wrote:
> >
> > On Mon, Feb 4, 2019 at 10:15 AM Jordan Crouse <jcrouse@codeaurora.org> wrote:
> > >
> > > The GMU should have two power domains defined: "cx" and "gx". "cx" is the
> > > actual power domain for the device and "gx" will be attached at runtime
> > > to manage reference counting on the GPU device in case of a GMU crash.
> >
> > power-domains are supposed to be actual regions on a chip die which
> > can be power gated. However, they are often abused by being defined in
> > terms of kernel PM domains which are not always the same thing. This
> > description sounds like the latter case.
> >
> 
> iirc (and Jordan can correct me), this arrangement was needed because
> normally the GMU does the GPU power control (except for if we manage
> to crash it and need to reset the GMU)..
> 
> so maybe not 100% about the actual regions on chip die which can be
> gated.. but it is a reality of how hw + fw + sw fit together..

Ack - forgot to add Stephen who knows about this.

Rob is correct. The GX domain is real but it is normally controlled by an
off-CPU microcontroller. The CPU needs to get involved only when the
microcontroller crashes and we need to get things back to normal.  So the
description of it being an actual region on a chip die is accurate.

It sounds like I need to describe the hardware better in the bindings document.

Jordan

-- 
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project