diff mbox

drm/tegra: dsi: Enhance runtime power management

Message ID 20160817160141.20676-1-thierry.reding@gmail.com
State Deferred
Headers show

Commit Message

Thierry Reding Aug. 17, 2016, 4:01 p.m. UTC 
From: Thierry Reding <treding@nvidia.com>

The MIPI DSI output on Tegra SoCs requires some external logic to
calibrate the MIPI pads before a video signal can be transmitted. This
MIPI calibration logic requires to be powered on while the MIPI pads are
being used, which is currently done as part of the DSI driver's probe
implementation.

This is suboptimal because it will leave the MIPI calibration logic
powered up even if the DSI output is never used.

On Tegra114 and earlier this behaviour also causes the driver to hang
while trying to power up the MIPI calibration logic because the power
partition that contains the MIPI calibration logic will be powered on
by the display controller at output pipeline configuration time. Thus
the power up sequence for the MIPI calibration logic happens before
it's power partition is guaranteed to be enabled.

Fix this by splitting up the API into a request/free pair of functions
that manage the runtime dependency between the DSI and the calibration
modules (no registers are accessed) and a set of enable, calibrate and
disable functions that program the MIPI calibration logic at points in
time where the power partition is really enabled.

While at it, make sure that the runtime power management also works in
ganged mode, which is currently also broken.

Reported-by: Jonathan Hunter <jonathanh@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dsi.c | 43 ++++++++++++++++++++++++++-----
 drivers/gpu/host1x/mipi.c   | 63 ++++++++++++++++++++++-----------------------
 include/linux/host1x.h      |  2 ++
 3 files changed, 69 insertions(+), 39 deletions(-)

Comments

Jon Hunter Aug. 24, 2016, 1:34 p.m. UTC | #1 
On 17/08/16 17:01, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The MIPI DSI output on Tegra SoCs requires some external logic to
> calibrate the MIPI pads before a video signal can be transmitted. This
> MIPI calibration logic requires to be powered on while the MIPI pads are
> being used, which is currently done as part of the DSI driver's probe
> implementation.
> 
> This is suboptimal because it will leave the MIPI calibration logic
> powered up even if the DSI output is never used.
> 
> On Tegra114 and earlier this behaviour also causes the driver to hang
> while trying to power up the MIPI calibration logic because the power
> partition that contains the MIPI calibration logic will be powered on
> by the display controller at output pipeline configuration time. Thus
> the power up sequence for the MIPI calibration logic happens before
> it's power partition is guaranteed to be enabled.
> 
> Fix this by splitting up the API into a request/free pair of functions
> that manage the runtime dependency between the DSI and the calibration
> modules (no registers are accessed) and a set of enable, calibrate and
> disable functions that program the MIPI calibration logic at points in
> time where the power partition is really enabled.
> 
> While at it, make sure that the runtime power management also works in
> ganged mode, which is currently also broken.
> 
> Reported-by: Jonathan Hunter <jonathanh@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

This patch along with the fix from Vince [0] fixes the hang during boot
on Tegra114. So ...

Tested-by: Jon Hunter <jonathanh@nvidia.com>

Cheers
Jon

[0] http://marc.info/?l=linux-kernel&m=144017122430016&w=2
diff mbox

Patch

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index 83f519aee29b..a95f10f05a3c 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -841,6 +841,21 @@  static const struct drm_encoder_funcs tegra_dsi_encoder_funcs = {
 	.destroy = tegra_output_encoder_destroy,
 };
 
+static void tegra_dsi_unprepare(struct tegra_dsi *dsi)
+{
+	int err;
+
+	if (dsi->slave)
+		tegra_dsi_unprepare(dsi->slave);
+
+	err = tegra_mipi_disable(dsi->mipi);
+	if (err < 0)
+		dev_err(dsi->dev, "failed to disable MIPI calibration: %d\n",
+			err);
+
+	pm_runtime_put(dsi->dev);
+}
+
 static void tegra_dsi_encoder_disable(struct drm_encoder *encoder)
 {
 	struct tegra_output *output = encoder_to_output(encoder);
@@ -877,7 +892,26 @@  static void tegra_dsi_encoder_disable(struct drm_encoder *encoder)
 
 	tegra_dsi_disable(dsi);
 
-	pm_runtime_put(dsi->dev);
+	tegra_dsi_unprepare(dsi);
+}
+
+static void tegra_dsi_prepare(struct tegra_dsi *dsi)
+{
+	int err;
+
+	pm_runtime_get_sync(dsi->dev);
+
+	err = tegra_mipi_enable(dsi->mipi);
+	if (err < 0)
+		dev_err(dsi->dev, "failed to enable MIPI calibration: %d\n",
+			err);
+
+	err = tegra_dsi_pad_calibrate(dsi);
+	if (err < 0)
+		dev_err(dsi->dev, "MIPI calibration failed: %d\n", err);
+
+	if (dsi->slave)
+		tegra_dsi_prepare(dsi->slave);
 }
 
 static void tegra_dsi_encoder_enable(struct drm_encoder *encoder)
@@ -888,13 +922,8 @@  static void tegra_dsi_encoder_enable(struct drm_encoder *encoder)
 	struct tegra_dsi *dsi = to_dsi(output);
 	struct tegra_dsi_state *state;
 	u32 value;
-	int err;
-
-	pm_runtime_get_sync(dsi->dev);
 
-	err = tegra_dsi_pad_calibrate(dsi);
-	if (err < 0)
-		dev_err(dsi->dev, "MIPI calibration failed: %d\n", err);
+	tegra_dsi_prepare(dsi);
 
 	state = tegra_dsi_get_state(dsi);
 
diff --git a/drivers/gpu/host1x/mipi.c b/drivers/gpu/host1x/mipi.c
index 52a6fd224127..e00809d996a2 100644
--- a/drivers/gpu/host1x/mipi.c
+++ b/drivers/gpu/host1x/mipi.c
@@ -242,20 +242,6 @@  struct tegra_mipi_device *tegra_mipi_request(struct device *device)
 	dev->pads = args.args[0];
 	dev->device = device;
 
-	mutex_lock(&dev->mipi->lock);
-
-	if (dev->mipi->usage_count++ == 0) {
-		err = tegra_mipi_power_up(dev->mipi);
-		if (err < 0) {
-			dev_err(dev->mipi->dev,
-				"failed to power up MIPI bricks: %d\n",
-				err);
-			return ERR_PTR(err);
-		}
-	}
-
-	mutex_unlock(&dev->mipi->lock);
-
 	return dev;
 
 put:
@@ -270,29 +256,42 @@  EXPORT_SYMBOL(tegra_mipi_request);
 
 void tegra_mipi_free(struct tegra_mipi_device *device)
 {
-	int err;
+	platform_device_put(device->pdev);
+	kfree(device);
+}
+EXPORT_SYMBOL(tegra_mipi_free);
 
-	mutex_lock(&device->mipi->lock);
+int tegra_mipi_enable(struct tegra_mipi_device *dev)
+{
+	int err = 0;
 
-	if (--device->mipi->usage_count == 0) {
-		err = tegra_mipi_power_down(device->mipi);
-		if (err < 0) {
-			/*
-			 * Not much that can be done here, so an error message
-			 * will have to do.
-			 */
-			dev_err(device->mipi->dev,
-				"failed to power down MIPI bricks: %d\n",
-				err);
-		}
-	}
+	mutex_lock(&dev->mipi->lock);
 
-	mutex_unlock(&device->mipi->lock);
+	if (dev->mipi->usage_count++ == 0)
+		err = tegra_mipi_power_up(dev->mipi);
+
+	mutex_unlock(&dev->mipi->lock);
+
+	return err;
 
-	platform_device_put(device->pdev);
-	kfree(device);
 }
-EXPORT_SYMBOL(tegra_mipi_free);
+EXPORT_SYMBOL(tegra_mipi_enable);
+
+int tegra_mipi_disable(struct tegra_mipi_device *dev)
+{
+	int err = 0;
+
+	mutex_lock(&dev->mipi->lock);
+
+	if (--dev->mipi->usage_count == 0)
+		err = tegra_mipi_power_down(dev->mipi);
+
+	mutex_unlock(&dev->mipi->lock);
+
+	return err;
+
+}
+EXPORT_SYMBOL(tegra_mipi_disable);
 
 static int tegra_mipi_wait(struct tegra_mipi *mipi)
 {
diff --git a/include/linux/host1x.h b/include/linux/host1x.h
index d2ba7d334039..1ffbf2a8cb99 100644
--- a/include/linux/host1x.h
+++ b/include/linux/host1x.h
@@ -304,6 +304,8 @@  struct tegra_mipi_device;
 
 struct tegra_mipi_device *tegra_mipi_request(struct device *device);
 void tegra_mipi_free(struct tegra_mipi_device *device);
+int tegra_mipi_enable(struct tegra_mipi_device *device);
+int tegra_mipi_disable(struct tegra_mipi_device *device);
 int tegra_mipi_calibrate(struct tegra_mipi_device *device);
 
 #endif