[PATCH 01/17] drm/tegra: dc: Add powergate support

[PATCH 01/17] drm/tegra: dc: Add powergate support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Both display controllers are in their own power partition. Currently the
driver relies on the assumption that these partitions are on (which is
the hardware default). However some bootloaders may disable them, so the
driver must make sure to turn them back on to avoid hangs.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c  | 45 ++++++++++++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/tegra/drm.h |  1 +
 2 files changed, 43 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 6553fd238685..4a015232e2e8 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -11,6 +11,8 @@
 #include <linux/debugfs.h>
 #include <linux/reset.h>
 
+#include <soc/tegra/pmc.h>
+
 #include "dc.h"
 #include "drm.h"
 #include "gem.h"
@@ -20,6 +22,7 @@ struct tegra_dc_soc_info {
 	bool supports_cursor;
 	bool supports_block_linear;
 	unsigned int pitch_align;
+	bool has_powergate;
 };
 
 struct tegra_plane {
@@ -1357,6 +1360,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
 	.supports_cursor = false,
 	.supports_block_linear = false,
 	.pitch_align = 8,
+	.has_powergate = false,
 };
 
 static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
@@ -1364,6 +1368,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
 	.supports_cursor = false,
 	.supports_block_linear = false,
 	.pitch_align = 8,
+	.has_powergate = false,
 };
 
 static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
@@ -1371,6 +1376,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
 	.supports_cursor = false,
 	.supports_block_linear = false,
 	.pitch_align = 64,
+	.has_powergate = true,
 };
 
 static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
@@ -1378,6 +1384,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
 	.supports_cursor = true,
 	.supports_block_linear = true,
 	.pitch_align = 64,
+	.has_powergate = true,
 };
 
 static const struct of_device_id tegra_dc_of_match[] = {
@@ -1385,6 +1392,9 @@ static const struct of_device_id tegra_dc_of_match[] = {
 		.compatible = "nvidia,tegra124-dc",
 		.data = &tegra124_dc_soc_info,
 	}, {
+		.compatible = "nvidia,tegra114-dc",
+		.data = &tegra114_dc_soc_info,
+	}, {
 		.compatible = "nvidia,tegra30-dc",
 		.data = &tegra30_dc_soc_info,
 	}, {
@@ -1467,9 +1477,34 @@ static int tegra_dc_probe(struct platform_device *pdev)
 		return PTR_ERR(dc->rst);
 	}
 
-	err = clk_prepare_enable(dc->clk);
-	if (err < 0)
-		return err;
+	if (dc->soc->has_powergate) {
+		if (dc->pipe == 0)
+			dc->powergate = TEGRA_POWERGATE_DIS;
+		else
+			dc->powergate = TEGRA_POWERGATE_DISB;
+
+		err = tegra_powergate_sequence_power_up(dc->powergate, dc->clk,
+							dc->rst);
+		if (err < 0) {
+			dev_err(&pdev->dev, "failed to power partition: %d\n",
+				err);
+			return err;
+		}
+	} else {
+		err = clk_prepare_enable(dc->clk);
+		if (err < 0) {
+			dev_err(&pdev->dev, "failed to enable clock: %d\n",
+				err);
+			return err;
+		}
+
+		err = reset_control_deassert(dc->rst);
+		if (err < 0) {
+			dev_err(&pdev->dev, "failed to deassert reset: %d\n",
+				err);
+			return err;
+		}
+	}
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dc->regs = devm_ioremap_resource(&pdev->dev, regs);
@@ -1523,6 +1558,10 @@ static int tegra_dc_remove(struct platform_device *pdev)
 	}
 
 	reset_control_assert(dc->rst);
+
+	if (dc->soc->has_powergate)
+		tegra_powergate_power_off(dc->powergate);
+
 	clk_disable_unprepare(dc->clk);
 
 	return 0;
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index e89c70fa82d5..b994c017971d 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -101,6 +101,7 @@ struct tegra_dc {
 	spinlock_t lock;
 
 	struct drm_crtc base;
+	int powergate;
 	int pipe;
 
 	struct clk *clk;
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The output is already enabled in .dpms(), doing it in .mode_set() too
can cause noticeable flicker.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/output.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 0c67d7eebc94..6b393cfbb5e7 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -167,12 +167,6 @@ static void tegra_encoder_mode_set(struct drm_encoder *encoder,
 				   struct drm_display_mode *mode,
 				   struct drm_display_mode *adjusted)
 {
-	struct tegra_output *output = encoder_to_output(encoder);
-	int err;
-
-	err = tegra_output_enable(output);
-	if (err < 0)
-		dev_err(encoder->dev->dev, "tegra_output_enable(): %d\n", err);
 }
 
 static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 03/17] drm/tegra: dsi: Make FIFO depths host parameters

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Rather than hardcoding them as macros, make the host and video FIFO
depths parameters so that they can be more easily adjusted if a new
generation of the Tegra SoC changes them.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dsi.c | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index f7874458926a..584b771d8b2f 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -26,9 +26,6 @@
 #include "dsi.h"
 #include "mipi-phy.h"
 
-#define DSI_VIDEO_FIFO_DEPTH (1920 / 4)
-#define DSI_HOST_FIFO_DEPTH 64
-
 struct tegra_dsi {
 	struct host1x_client client;
 	struct tegra_output output;
@@ -54,6 +51,9 @@ struct tegra_dsi {
 
 	struct regulator *vdd;
 	bool enabled;
+
+	unsigned int video_fifo_depth;
+	unsigned int host_fifo_depth;
 };
 
 static inline struct tegra_dsi *
@@ -467,7 +467,7 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
 		DSI_CONTROL_SOURCE(dc->pipe);
 	tegra_dsi_writel(dsi, value, DSI_CONTROL);
 
-	tegra_dsi_writel(dsi, DSI_VIDEO_FIFO_DEPTH, DSI_MAX_THRESHOLD);
+	tegra_dsi_writel(dsi, dsi->video_fifo_depth, DSI_MAX_THRESHOLD);
 
 	value = DSI_HOST_CONTROL_HS | DSI_HOST_CONTROL_CS |
 		DSI_HOST_CONTROL_ECC;
@@ -843,6 +843,8 @@ static int tegra_dsi_probe(struct platform_device *pdev)
 		return -ENOMEM;
 
 	dsi->output.dev = dsi->dev = &pdev->dev;
+	dsi->video_fifo_depth = 1920;
+	dsi->host_fifo_depth = 64;
 
 	err = tegra_output_probe(&dsi->output);
 	if (err < 0)
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v2 04/17] drm/tegra: dsi: Add ganged mode support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Implement ganged mode support for the Tegra DSI driver. The DSI host
controller to gang up with is specified via a phandle in the device tree
and the resolved DSI host controller used for the programming of the
ganged-mode registers.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v2:
- keep track of the number of bytes transferred to/from peripheral
- use newly introduced mipi_dsi_create_packet()
- extract FIFO write into separate function

 .../bindings/gpu/nvidia,tegra20-host1x.txt         |   2 +
 drivers/gpu/drm/tegra/dsi.c                        | 792 ++++++++++++++++++---
 drivers/gpu/drm/tegra/dsi.h                        |  14 +-
 3 files changed, 691 insertions(+), 117 deletions(-)

diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
index b48f4ef31d93..4c32ef0b7db8 100644
--- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
+++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
@@ -191,6 +191,8 @@ of the following host1x client modules:
   - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
   - nvidia,edid: supplies a binary EDID blob
   - nvidia,panel: phandle of a display panel
+  - nvidia,ganged-mode: contains a phandle to a second DSI controller to gang
+    up with in order to support up to 8 data lanes
 
 - sor: serial output resource
 
diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index 584b771d8b2f..8940360ccc9c 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -11,6 +11,7 @@
 #include <linux/host1x.h>
 #include <linux/module.h>
 #include <linux/of.h>
+#include <linux/of_platform.h>
 #include <linux/platform_device.h>
 #include <linux/reset.h>
 
@@ -54,6 +55,10 @@ struct tegra_dsi {
 
 	unsigned int video_fifo_depth;
 	unsigned int host_fifo_depth;
+
+	/* for ganged-mode support */
+	struct tegra_dsi *master;
+	struct tegra_dsi *slave;
 };
 
 static inline struct tegra_dsi *
@@ -318,6 +323,21 @@ static const u32 pkt_seq_video_non_burst_sync_events[NUM_PKT_SEQ] = {
 	[11] = PKT_ID0(MIPI_DSI_BLANKING_PACKET) | PKT_LEN0(4),
 };
 
+static const u32 pkt_seq_command_mode[NUM_PKT_SEQ] = {
+	[ 0] = 0,
+	[ 1] = 0,
+	[ 2] = 0,
+	[ 3] = 0,
+	[ 4] = 0,
+	[ 5] = 0,
+	[ 6] = PKT_ID0(MIPI_DSI_DCS_LONG_WRITE) | PKT_LEN0(3) | PKT_LP,
+	[ 7] = 0,
+	[ 8] = 0,
+	[ 9] = 0,
+	[10] = PKT_ID0(MIPI_DSI_DCS_LONG_WRITE) | PKT_LEN0(5) | PKT_LP,
+	[11] = 0,
+};
+
 static int tegra_dsi_set_phy_timing(struct tegra_dsi *dsi)
 {
 	struct mipi_dphy_timing timing;
@@ -329,7 +349,7 @@ static int tegra_dsi_set_phy_timing(struct tegra_dsi *dsi)
 	if (rate < 0)
 		return rate;
 
-	period = DIV_ROUND_CLOSEST(1000000000UL, rate * 2);
+	period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, rate * 2);
 
 	err = mipi_dphy_timing_get_default(&timing, period);
 	if (err < 0)
@@ -426,26 +446,59 @@ static int tegra_dsi_get_format(enum mipi_dsi_pixel_format format,
 	return 0;
 }
 
-static int tegra_output_dsi_enable(struct tegra_output *output)
+static void tegra_dsi_ganged_enable(struct tegra_dsi *dsi, unsigned int start,
+				    unsigned int size)
+{
+	u32 value;
+
+	tegra_dsi_writel(dsi, start, DSI_GANGED_MODE_START);
+	tegra_dsi_writel(dsi, size << 16 | size, DSI_GANGED_MODE_SIZE);
+
+	value = DSI_GANGED_MODE_CONTROL_ENABLE;
+	tegra_dsi_writel(dsi, value, DSI_GANGED_MODE_CONTROL);
+}
+
+static void tegra_dsi_enable(struct tegra_dsi *dsi)
+{
+	u32 value;
+
+	value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
+	value |= DSI_POWER_CONTROL_ENABLE;
+	tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
+
+	if (dsi->slave)
+		tegra_dsi_enable(dsi->slave);
+}
+
+static unsigned int tegra_dsi_get_lanes(struct tegra_dsi *dsi)
+{
+	if (dsi->master)
+		return dsi->master->lanes + dsi->lanes;
+
+	if (dsi->slave)
+		return dsi->lanes + dsi->slave->lanes;
+
+	return dsi->lanes;
+}
+
+static int tegra_dsi_configure(struct tegra_dsi *dsi, unsigned int pipe,
+			       const struct drm_display_mode *mode)
 {
-	struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
-	struct drm_display_mode *mode = &dc->base.mode;
 	unsigned int hact, hsw, hbp, hfp, i, mul, div;
-	struct tegra_dsi *dsi = to_dsi(output);
 	enum tegra_dsi_format format;
-	unsigned long value;
 	const u32 *pkt_seq;
+	u32 value;
 	int err;
 
-	if (dsi->enabled)
-		return 0;
-
 	if (dsi->flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) {
 		DRM_DEBUG_KMS("Non-burst video mode with sync pulses\n");
 		pkt_seq = pkt_seq_video_non_burst_sync_pulses;
-	} else {
+	} else if (dsi->flags & MIPI_DSI_MODE_VIDEO) {
 		DRM_DEBUG_KMS("Non-burst video mode with sync events\n");
 		pkt_seq = pkt_seq_video_non_burst_sync_events;
+	} else {
+		DRM_DEBUG_KMS("Command mode\n");
+		pkt_seq = pkt_seq_command_mode;
 	}
 
 	err = tegra_dsi_get_muldiv(dsi->format, &mul, &div);
@@ -456,28 +509,29 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
 	if (err < 0)
 		return err;
 
-	err = clk_enable(dsi->clk);
-	if (err < 0)
-		return err;
-
-	reset_control_deassert(dsi->rst);
-
 	value = DSI_CONTROL_CHANNEL(0) | DSI_CONTROL_FORMAT(format) |
 		DSI_CONTROL_LANES(dsi->lanes - 1) |
-		DSI_CONTROL_SOURCE(dc->pipe);
+		DSI_CONTROL_SOURCE(pipe);
 	tegra_dsi_writel(dsi, value, DSI_CONTROL);
 
 	tegra_dsi_writel(dsi, dsi->video_fifo_depth, DSI_MAX_THRESHOLD);
 
-	value = DSI_HOST_CONTROL_HS | DSI_HOST_CONTROL_CS |
-		DSI_HOST_CONTROL_ECC;
+	value = DSI_HOST_CONTROL_HS;
 	tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
 
 	value = tegra_dsi_readl(dsi, DSI_CONTROL);
+
 	if (dsi->flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
 		value |= DSI_CONTROL_HS_CLK_CTRL;
+
 	value &= ~DSI_CONTROL_TX_TRIG(3);
-	value &= ~DSI_CONTROL_DCS_ENABLE;
+
+	/* enable DCS commands for command mode */
+	if (dsi->flags & MIPI_DSI_MODE_VIDEO)
+		value &= ~DSI_CONTROL_DCS_ENABLE;
+	else
+		value |= DSI_CONTROL_DCS_ENABLE;
+
 	value |= DSI_CONTROL_VIDEO_ENABLE;
 	value &= ~DSI_CONTROL_HOST_ENABLE;
 	tegra_dsi_writel(dsi, value, DSI_CONTROL);
@@ -489,28 +543,106 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
 	for (i = 0; i < NUM_PKT_SEQ; i++)
 		tegra_dsi_writel(dsi, pkt_seq[i], DSI_PKT_SEQ_0_LO + i);
 
-	/* horizontal active pixels */
-	hact = mode->hdisplay * mul / div;
+	if (dsi->flags & MIPI_DSI_MODE_VIDEO) {
+		/* horizontal active pixels */
+		hact = mode->hdisplay * mul / div;
+
+		/* horizontal sync width */
+		hsw = (mode->hsync_end - mode->hsync_start) * mul / div;
+		hsw -= 10;
 
-	/* horizontal sync width */
-	hsw = (mode->hsync_end - mode->hsync_start) * mul / div;
-	hsw -= 10;
+		/* horizontal back porch */
+		hbp = (mode->htotal - mode->hsync_end) * mul / div;
+		hbp -= 14;
 
-	/* horizontal back porch */
-	hbp = (mode->htotal - mode->hsync_end) * mul / div;
-	hbp -= 14;
+		/* horizontal front porch */
+		hfp = (mode->hsync_start - mode->hdisplay) * mul / div;
+		hfp -= 8;
+
+		tegra_dsi_writel(dsi, hsw << 16 | 0, DSI_PKT_LEN_0_1);
+		tegra_dsi_writel(dsi, hact << 16 | hbp, DSI_PKT_LEN_2_3);
+		tegra_dsi_writel(dsi, hfp, DSI_PKT_LEN_4_5);
+		tegra_dsi_writel(dsi, 0x0f0f << 16, DSI_PKT_LEN_6_7);
+
+		/* set SOL delay (for non-burst mode only) */
+		tegra_dsi_writel(dsi, 8 * mul / div, DSI_SOL_DELAY);
+
+		/* TODO: implement ganged mode */
+	} else {
+		u16 bytes;
+
+		if (dsi->master || dsi->slave) {
+			/*
+			 * For ganged mode, assume symmetric left-right mode.
+			 */
+			bytes = 1 + (mode->hdisplay / 2) * mul / div;
+		} else {
+			/* 1 byte (DCS command) + pixel data */
+			bytes = 1 + mode->hdisplay * mul / div;
+		}
+
+		tegra_dsi_writel(dsi, 0, DSI_PKT_LEN_0_1);
+		tegra_dsi_writel(dsi, bytes << 16, DSI_PKT_LEN_2_3);
+		tegra_dsi_writel(dsi, bytes << 16, DSI_PKT_LEN_4_5);
+		tegra_dsi_writel(dsi, 0, DSI_PKT_LEN_6_7);
+
+		value = MIPI_DCS_WRITE_MEMORY_START << 8 |
+			MIPI_DCS_WRITE_MEMORY_CONTINUE;
+		tegra_dsi_writel(dsi, value, DSI_DCS_CMDS);
+
+		/* set SOL delay */
+		if (dsi->master || dsi->slave) {
+			unsigned int lanes = tegra_dsi_get_lanes(dsi);
+			unsigned long delay, bclk, bclk_ganged;
+
+			/* SOL to valid, valid to FIFO and FIFO write delay */
+			delay = 4 + 4 + 2;
+			delay = DIV_ROUND_UP(delay * mul, div * lanes);
+			/* FIFO read delay */
+			delay = delay + 6;
+
+			bclk = DIV_ROUND_UP(mode->htotal * mul, div * lanes);
+			bclk_ganged = DIV_ROUND_UP(bclk * lanes / 2, lanes);
+			value = bclk - bclk_ganged + delay + 20;
+		} else {
+			/* TODO: revisit for non-ganged mode */
+			value = 8 * mul / div;
+		}
+
+		tegra_dsi_writel(dsi, value, DSI_SOL_DELAY);
+	}
 
-	/* horizontal front porch */
-	hfp = (mode->hsync_start  - mode->hdisplay) * mul / div;
-	hfp -= 8;
+	if (dsi->slave) {
+		err = tegra_dsi_configure(dsi->slave, pipe, mode);
+		if (err < 0)
+			return err;
+
+		/*
+		 * TODO: Support modes other than symmetrical left-right
+		 * split.
+		 */
+		tegra_dsi_ganged_enable(dsi, 0, mode->hdisplay / 2);
+		tegra_dsi_ganged_enable(dsi->slave, mode->hdisplay / 2,
+					mode->hdisplay / 2);
+	}
+
+	return 0;
+}
+
+static int tegra_output_dsi_enable(struct tegra_output *output)
+{
+	struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
+	const struct drm_display_mode *mode = &dc->base.mode;
+	struct tegra_dsi *dsi = to_dsi(output);
+	u32 value;
+	int err;
 
-	tegra_dsi_writel(dsi, hsw << 16 | 0, DSI_PKT_LEN_0_1);
-	tegra_dsi_writel(dsi, hact << 16 | hbp, DSI_PKT_LEN_2_3);
-	tegra_dsi_writel(dsi, hfp, DSI_PKT_LEN_4_5);
-	tegra_dsi_writel(dsi, 0x0f0f << 16, DSI_PKT_LEN_6_7);
+	if (dsi->enabled)
+		return 0;
 
-	/* set SOL delay */
-	tegra_dsi_writel(dsi, 8 * mul / div, DSI_SOL_DELAY);
+	err = tegra_dsi_configure(dsi, dc->pipe, mode);
+	if (err < 0)
+		return err;
 
 	/* enable display controller */
 	value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS);
@@ -531,28 +663,79 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
 	tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
 
 	/* enable DSI controller */
-	value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
-	value |= DSI_POWER_CONTROL_ENABLE;
-	tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
+	tegra_dsi_enable(dsi);
 
 	dsi->enabled = true;
 
 	return 0;
 }
 
+static int tegra_dsi_wait_idle(struct tegra_dsi *dsi, unsigned long timeout)
+{
+	u32 value;
+
+	timeout = jiffies + msecs_to_jiffies(timeout);
+
+	while (time_before(jiffies, timeout)) {
+		value = tegra_dsi_readl(dsi, DSI_STATUS);
+		if (value & DSI_STATUS_IDLE)
+			return 0;
+
+		usleep_range(1000, 2000);
+	}
+
+	return -ETIMEDOUT;
+}
+
+static void tegra_dsi_video_disable(struct tegra_dsi *dsi)
+{
+	u32 value;
+
+	value = tegra_dsi_readl(dsi, DSI_CONTROL);
+	value &= ~DSI_CONTROL_VIDEO_ENABLE;
+	tegra_dsi_writel(dsi, value, DSI_CONTROL);
+
+	if (dsi->slave)
+		tegra_dsi_video_disable(dsi->slave);
+}
+
+static void tegra_dsi_ganged_disable(struct tegra_dsi *dsi)
+{
+	tegra_dsi_writel(dsi, 0, DSI_GANGED_MODE_START);
+	tegra_dsi_writel(dsi, 0, DSI_GANGED_MODE_SIZE);
+	tegra_dsi_writel(dsi, 0, DSI_GANGED_MODE_CONTROL);
+}
+
+static void tegra_dsi_disable(struct tegra_dsi *dsi)
+{
+	u32 value;
+
+	if (dsi->slave) {
+		tegra_dsi_ganged_disable(dsi->slave);
+		tegra_dsi_ganged_disable(dsi);
+	}
+
+	value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
+	value &= ~DSI_POWER_CONTROL_ENABLE;
+	tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
+
+	if (dsi->slave)
+		tegra_dsi_disable(dsi->slave);
+
+	usleep_range(5000, 10000);
+}
+
 static int tegra_output_dsi_disable(struct tegra_output *output)
 {
 	struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
 	struct tegra_dsi *dsi = to_dsi(output);
 	unsigned long value;
+	int err;
 
 	if (!dsi->enabled)
 		return 0;
 
-	/* disable DSI controller */
-	value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
-	value &= ~DSI_POWER_CONTROL_ENABLE;
-	tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
+	tegra_dsi_video_disable(dsi);
 
 	/*
 	 * The following accesses registers of the display controller, so make
@@ -576,39 +759,68 @@ static int tegra_output_dsi_disable(struct tegra_output *output)
 		tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
 	}
 
-	clk_disable(dsi->clk);
+	err = tegra_dsi_wait_idle(dsi, 100);
+	if (err < 0)
+		dev_dbg(dsi->dev, "failed to idle DSI: %d\n", err);
+
+	tegra_dsi_disable(dsi);
 
 	dsi->enabled = false;
 
 	return 0;
 }
 
+static void tegra_dsi_set_timeout(struct tegra_dsi *dsi, unsigned long bclk,
+				  unsigned int vrefresh)
+{
+	unsigned int timeout;
+	u32 value;
+
+	/* one frame high-speed transmission timeout */
+	timeout = (bclk / vrefresh) / 512;
+	value = DSI_TIMEOUT_LRX(0x2000) | DSI_TIMEOUT_HTX(timeout);
+	tegra_dsi_writel(dsi, value, DSI_TIMEOUT_0);
+
+	/* 2 ms peripheral timeout for panel */
+	timeout = 2 * bclk / 512 * 1000;
+	value = DSI_TIMEOUT_PR(timeout) | DSI_TIMEOUT_TA(0x2000);
+	tegra_dsi_writel(dsi, value, DSI_TIMEOUT_1);
+
+	value = DSI_TALLY_TA(0) | DSI_TALLY_LRX(0) | DSI_TALLY_HTX(0);
+	tegra_dsi_writel(dsi, value, DSI_TO_TALLY);
+
+	if (dsi->slave)
+		tegra_dsi_set_timeout(dsi->slave, bclk, vrefresh);
+}
+
 static int tegra_output_dsi_setup_clock(struct tegra_output *output,
 					struct clk *clk, unsigned long pclk,
 					unsigned int *divp)
 {
 	struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
 	struct drm_display_mode *mode = &dc->base.mode;
-	unsigned int timeout, mul, div, vrefresh;
 	struct tegra_dsi *dsi = to_dsi(output);
-	unsigned long bclk, plld, value;
+	unsigned int mul, div, vrefresh, lanes;
+	unsigned long bclk, plld;
 	int err;
 
+	lanes = tegra_dsi_get_lanes(dsi);
+
 	err = tegra_dsi_get_muldiv(dsi->format, &mul, &div);
 	if (err < 0)
 		return err;
 
-	DRM_DEBUG_KMS("mul: %u, div: %u, lanes: %u\n", mul, div, dsi->lanes);
+	DRM_DEBUG_KMS("mul: %u, div: %u, lanes: %u\n", mul, div, lanes);
 	vrefresh = drm_mode_vrefresh(mode);
 	DRM_DEBUG_KMS("vrefresh: %u\n", vrefresh);
 
 	/* compute byte clock */
-	bclk = (pclk * mul) / (div * dsi->lanes);
+	bclk = (pclk * mul) / (div * lanes);
 
 	/*
 	 * Compute bit clock and round up to the next MHz.
 	 */
-	plld = DIV_ROUND_UP(bclk * 8, 1000000) * 1000000;
+	plld = DIV_ROUND_UP(bclk * 8, USEC_PER_SEC) * USEC_PER_SEC;
 
 	/*
 	 * We divide the frequency by two here, but we make up for that by
@@ -640,25 +852,13 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,
 	 * not working properly otherwise. Perhaps the PLLs cannot generate
 	 * frequencies sufficiently high.
 	 */
-	*divp = ((8 * mul) / (div * dsi->lanes)) - 2;
+	*divp = ((8 * mul) / (div * lanes)) - 2;
 
 	/*
 	 * XXX: Move the below somewhere else so that we don't need to have
 	 * access to the vrefresh in this function?
 	 */
-
-	/* one frame high-speed transmission timeout */
-	timeout = (bclk / vrefresh) / 512;
-	value = DSI_TIMEOUT_LRX(0x2000) | DSI_TIMEOUT_HTX(timeout);
-	tegra_dsi_writel(dsi, value, DSI_TIMEOUT_0);
-
-	/* 2 ms peripheral timeout for panel */
-	timeout = 2 * bclk / 512 * 1000;
-	value = DSI_TIMEOUT_PR(timeout) | DSI_TIMEOUT_TA(0x2000);
-	tegra_dsi_writel(dsi, value, DSI_TIMEOUT_1);
-
-	value = DSI_TALLY_TA(0) | DSI_TALLY_LRX(0) | DSI_TALLY_HTX(0);
-	tegra_dsi_writel(dsi, value, DSI_TO_TALLY);
+	tegra_dsi_set_timeout(dsi, bclk, vrefresh);
 
 	return 0;
 }
@@ -695,7 +895,7 @@ static int tegra_dsi_pad_enable(struct tegra_dsi *dsi)
 
 static int tegra_dsi_pad_calibrate(struct tegra_dsi *dsi)
 {
-	unsigned long value;
+	u32 value;
 
 	tegra_dsi_writel(dsi, 0, DSI_PAD_CONTROL_0);
 	tegra_dsi_writel(dsi, 0, DSI_PAD_CONTROL_1);
@@ -720,14 +920,17 @@ static int tegra_dsi_init(struct host1x_client *client)
 	struct tegra_dsi *dsi = host1x_client_to_dsi(client);
 	int err;
 
-	dsi->output.type = TEGRA_OUTPUT_DSI;
-	dsi->output.dev = client->dev;
-	dsi->output.ops = &dsi_ops;
-
-	err = tegra_output_init(drm, &dsi->output);
-	if (err < 0) {
-		dev_err(client->dev, "output setup failed: %d\n", err);
-		return err;
+	/* Gangsters must not register their own outputs. */
+	if (!dsi->master) {
+		dsi->output.type = TEGRA_OUTPUT_DSI;
+		dsi->output.dev = client->dev;
+		dsi->output.ops = &dsi_ops;
+
+		err = tegra_output_init(drm, &dsi->output);
+		if (err < 0) {
+			dev_err(client->dev, "output setup failed: %d\n", err);
+			return err;
+		}
 	}
 
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
@@ -736,12 +939,6 @@ static int tegra_dsi_init(struct host1x_client *client)
 			dev_err(dsi->dev, "debugfs setup failed: %d\n", err);
 	}
 
-	err = tegra_dsi_pad_calibrate(dsi);
-	if (err < 0) {
-		dev_err(dsi->dev, "MIPI calibration failed: %d\n", err);
-		return err;
-	}
-
 	return 0;
 }
 
@@ -756,16 +953,20 @@ static int tegra_dsi_exit(struct host1x_client *client)
 			dev_err(dsi->dev, "debugfs cleanup failed: %d\n", err);
 	}
 
-	err = tegra_output_disable(&dsi->output);
-	if (err < 0) {
-		dev_err(client->dev, "output failed to disable: %d\n", err);
-		return err;
-	}
-
-	err = tegra_output_exit(&dsi->output);
-	if (err < 0) {
-		dev_err(client->dev, "output cleanup failed: %d\n", err);
-		return err;
+	if (!dsi->master) {
+		err = tegra_output_disable(&dsi->output);
+		if (err < 0) {
+			dev_err(client->dev, "output failed to disable: %d\n",
+				err);
+			return err;
+		}
+
+		err = tegra_output_exit(&dsi->output);
+		if (err < 0) {
+			dev_err(client->dev, "output cleanup failed: %d\n",
+				err);
+			return err;
+		}
 	}
 
 	return 0;
@@ -792,20 +993,324 @@ static int tegra_dsi_setup_clocks(struct tegra_dsi *dsi)
 	return 0;
 }
 
+static const char * const error_report[16] = {
+	"SoT Error",
+	"SoT Sync Error",
+	"EoT Sync Error",
+	"Escape Mode Entry Command Error",
+	"Low-Power Transmit Sync Error",
+	"Peripheral Timeout Error",
+	"False Control Error",
+	"Contention Detected",
+	"ECC Error, single-bit",
+	"ECC Error, multi-bit",
+	"Checksum Error",
+	"DSI Data Type Not Recognized",
+	"DSI VC ID Invalid",
+	"Invalid Transmission Length",
+	"Reserved",
+	"DSI Protocol Violation",
+};
+
+static ssize_t tegra_dsi_read_response(struct tegra_dsi *dsi,
+				       const struct mipi_dsi_msg *msg,
+				       size_t count)
+{
+	u8 *rx = msg->rx_buf;
+	unsigned int i, j, k;
+	size_t size = 0;
+	u16 errors;
+	u32 value;
+
+	/* read and parse packet header */
+	value = tegra_dsi_readl(dsi, DSI_RD_DATA);
+
+	switch (value & 0x3f) {
+	case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
+		errors = (value >> 8) & 0xffff;
+		dev_dbg(dsi->dev, "Acknowledge and error report: %04x\n",
+			errors);
+		for (i = 0; i < ARRAY_SIZE(error_report); i++)
+			if (errors & BIT(i))
+				dev_dbg(dsi->dev, "  %2u: %s\n", i,
+					error_report[i]);
+		break;
+
+	case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
+		rx[0] = (value >> 8) & 0xff;
+		size = 1;
+		break;
+
+	case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
+		rx[0] = (value >>  8) & 0xff;
+		rx[1] = (value >> 16) & 0xff;
+		size = 2;
+		break;
+
+	case MIPI_DSI_RX_DCS_LONG_READ_RESPONSE:
+		size = ((value >> 8) & 0xff00) | ((value >> 8) & 0xff);
+		break;
+
+	case MIPI_DSI_RX_GENERIC_LONG_READ_RESPONSE:
+		size = ((value >> 8) & 0xff00) | ((value >> 8) & 0xff);
+		break;
+
+	default:
+		dev_err(dsi->dev, "unhandled response type: %02x\n",
+			value & 0x3f);
+		return -EPROTO;
+	}
+
+	size = min(size, msg->rx_len);
+
+	if (msg->rx_buf && size > 0) {
+		for (i = 0, j = 0; i < count - 1; i++, j += 4) {
+			u8 *rx = msg->rx_buf + j;
+
+			value = tegra_dsi_readl(dsi, DSI_RD_DATA);
+
+			for (k = 0; k < 4 && (j + k) < msg->rx_len; k++)
+				rx[j + k] = (value >> (k << 3)) & 0xff;
+		}
+	}
+
+	return size;
+}
+
+static int tegra_dsi_transmit(struct tegra_dsi *dsi, unsigned long timeout)
+{
+	tegra_dsi_writel(dsi, DSI_TRIGGER_HOST, DSI_TRIGGER);
+
+	timeout = jiffies + msecs_to_jiffies(timeout);
+
+	while (time_before(jiffies, timeout)) {
+		u32 value = tegra_dsi_readl(dsi, DSI_TRIGGER);
+		if ((value & DSI_TRIGGER_HOST) == 0)
+			return 0;
+
+		usleep_range(1000, 2000);
+	}
+
+	DRM_DEBUG_KMS("timeout waiting for transmission to complete\n");
+	return -ETIMEDOUT;
+}
+
+static int tegra_dsi_wait_for_response(struct tegra_dsi *dsi,
+				       unsigned long timeout)
+{
+	timeout = jiffies + msecs_to_jiffies(250);
+
+	while (time_before(jiffies, timeout)) {
+		u32 value = tegra_dsi_readl(dsi, DSI_STATUS);
+		u8 count = value & 0x1f;
+
+		if (count > 0)
+			return count;
+
+		usleep_range(1000, 2000);
+	}
+
+	DRM_DEBUG_KMS("peripheral returned no data\n");
+	return -ETIMEDOUT;
+}
+
+static void tegra_dsi_writesl(struct tegra_dsi *dsi, unsigned long offset,
+			      const void *buffer, size_t size)
+{
+	const u8 *buf = buffer;
+	size_t i, j;
+	u32 value;
+
+	for (j = 0; j < size; j += 4) {
+		value = 0;
+
+		for (i = 0; i < 4 && j + i < size; i++)
+			value |= buf[j + i] << (i << 3);
+
+		tegra_dsi_writel(dsi, value, DSI_WR_DATA);
+	}
+}
+
+static ssize_t tegra_dsi_host_transfer(struct mipi_dsi_host *host,
+				       const struct mipi_dsi_msg *msg)
+{
+	struct tegra_dsi *dsi = host_to_tegra(host);
+	struct mipi_dsi_packet packet;
+	const u8 *header;
+	size_t count;
+	ssize_t err;
+	u32 value;
+
+	err = mipi_dsi_create_packet(&packet, msg);
+	if (err < 0)
+		return err;
+
+	header = packet.header;
+
+	/* maximum FIFO depth is 1920 words */
+	if (packet.size > dsi->video_fifo_depth * 4)
+		return -ENOSPC;
+
+	/* reset underflow/overflow flags */
+	value = tegra_dsi_readl(dsi, DSI_STATUS);
+	if (value & (DSI_STATUS_UNDERFLOW | DSI_STATUS_OVERFLOW)) {
+		value = DSI_HOST_CONTROL_FIFO_RESET;
+		tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
+		usleep_range(10, 20);
+	}
+
+	value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
+	value |= DSI_POWER_CONTROL_ENABLE;
+	tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
+
+	usleep_range(5000, 10000);
+
+	value = DSI_HOST_CONTROL_CRC_RESET | DSI_HOST_CONTROL_TX_TRIG_HOST |
+		DSI_HOST_CONTROL_CS | DSI_HOST_CONTROL_ECC;
+
+	if ((msg->flags & MIPI_DSI_MSG_USE_LPM) == 0)
+		value |= DSI_HOST_CONTROL_HS;
+
+	/*
+	 * The host FIFO has a maximum of 64 words, so larger transmissions
+	 * need to use the video FIFO.
+	 */
+	if (packet.size > dsi->host_fifo_depth * 4)
+		value |= DSI_HOST_CONTROL_FIFO_SEL;
+
+	tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
+
+	/*
+	 * For reads and messages with explicitly requested ACK, generate a
+	 * BTA sequence after the transmission of the packet.
+	 */
+	if ((msg->flags & MIPI_DSI_MSG_REQ_ACK) ||
+	    (msg->rx_buf && msg->rx_len > 0)) {
+		value = tegra_dsi_readl(dsi, DSI_HOST_CONTROL);
+		value |= DSI_HOST_CONTROL_PKT_BTA;
+		tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
+	}
+
+	value = DSI_CONTROL_LANES(0) | DSI_CONTROL_HOST_ENABLE;
+	tegra_dsi_writel(dsi, value, DSI_CONTROL);
+
+	/* write packet header, ECC is generated by hardware */
+	value = header[2] << 16 | header[1] << 8 | header[0];
+	tegra_dsi_writel(dsi, value, DSI_WR_DATA);
+
+	/* write payload (if any) */
+	if (packet.payload_length > 0)
+		tegra_dsi_writesl(dsi, DSI_WR_DATA, packet.payload,
+				  packet.payload_length);
+
+	err = tegra_dsi_transmit(dsi, 250);
+	if (err < 0)
+		return err;
+
+	if ((msg->flags & MIPI_DSI_MSG_REQ_ACK) ||
+	    (msg->rx_buf && msg->rx_len > 0)) {
+		err = tegra_dsi_wait_for_response(dsi, 250);
+		if (err < 0)
+			return err;
+
+		count = err;
+
+		value = tegra_dsi_readl(dsi, DSI_RD_DATA);
+		switch (value) {
+		case 0x84:
+			/*
+			dev_dbg(dsi->dev, "ACK\n");
+			*/
+			break;
+
+		case 0x87:
+			/*
+			dev_dbg(dsi->dev, "ESCAPE\n");
+			*/
+			break;
+
+		default:
+			dev_err(dsi->dev, "unknown status: %08x\n", value);
+			break;
+		}
+
+		if (count > 1) {
+			err = tegra_dsi_read_response(dsi, msg, count);
+			if (err < 0)
+				dev_err(dsi->dev,
+					"failed to parse response: %zd\n",
+					err);
+			else {
+				/*
+				 * For read commands, return the number of
+				 * bytes returned by the peripheral.
+				 */
+				count = err;
+			}
+		}
+	} else {
+		/*
+		 * For write commands, we have transmitted the 4-byte header
+		 * plus the variable-length payload.
+		 */
+		count = 4 + packet.payload_length;
+	}
+
+	return count;
+}
+
+static int tegra_dsi_ganged_setup(struct tegra_dsi *dsi)
+{
+	struct clk *parent;
+	int err;
+
+	/* make sure both DSI controllers share the same PLL */
+	parent = clk_get_parent(dsi->slave->clk);
+	if (!parent)
+		return -EINVAL;
+
+	err = clk_set_parent(parent, dsi->clk_parent);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
 static int tegra_dsi_host_attach(struct mipi_dsi_host *host,
 				 struct mipi_dsi_device *device)
 {
 	struct tegra_dsi *dsi = host_to_tegra(host);
-	struct tegra_output *output = &dsi->output;
 
 	dsi->flags = device->mode_flags;
 	dsi->format = device->format;
 	dsi->lanes = device->lanes;
 
-	output->panel = of_drm_find_panel(device->dev.of_node);
-	if (output->panel) {
-		if (output->connector.dev)
+	if (dsi->slave) {
+		int err;
+
+		dev_dbg(dsi->dev, "attaching dual-channel device %s\n",
+			dev_name(&device->dev));
+
+		err = tegra_dsi_ganged_setup(dsi);
+		if (err < 0) {
+			dev_err(dsi->dev, "failed to set up ganged mode: %d\n",
+				err);
+			return err;
+		}
+	}
+
+	/*
+	 * Slaves don't have a panel associated with them, so they provide
+	 * merely the second channel.
+	 */
+	if (!dsi->master) {
+		struct tegra_output *output = &dsi->output;
+
+		output->panel = of_drm_find_panel(device->dev.of_node);
+		if (output->panel && output->connector.dev) {
+			drm_panel_attach(output->panel, &output->connector);
 			drm_helper_hpd_irq_event(output->connector.dev);
+		}
 	}
 
 	return 0;
@@ -818,10 +1323,10 @@ static int tegra_dsi_host_detach(struct mipi_dsi_host *host,
 	struct tegra_output *output = &dsi->output;
 
 	if (output->panel && &device->dev == output->panel->dev) {
+		output->panel = NULL;
+
 		if (output->connector.dev)
 			drm_helper_hpd_irq_event(output->connector.dev);
-
-		output->panel = NULL;
 	}
 
 	return 0;
@@ -830,8 +1335,29 @@ static int tegra_dsi_host_detach(struct mipi_dsi_host *host,
 static const struct mipi_dsi_host_ops tegra_dsi_host_ops = {
 	.attach = tegra_dsi_host_attach,
 	.detach = tegra_dsi_host_detach,
+	.transfer = tegra_dsi_host_transfer,
 };
 
+static int tegra_dsi_ganged_probe(struct tegra_dsi *dsi)
+{
+	struct device_node *np;
+
+	np = of_parse_phandle(dsi->dev->of_node, "nvidia,ganged-mode", 0);
+	if (np) {
+		struct platform_device *gangster = of_find_device_by_node(np);
+
+		dsi->slave = platform_get_drvdata(gangster);
+		of_node_put(np);
+
+		if (!dsi->slave)
+			return -EPROBE_DEFER;
+
+		dsi->slave->master = dsi;
+	}
+
+	return 0;
+}
+
 static int tegra_dsi_probe(struct platform_device *pdev)
 {
 	struct tegra_dsi *dsi;
@@ -846,10 +1372,16 @@ static int tegra_dsi_probe(struct platform_device *pdev)
 	dsi->video_fifo_depth = 1920;
 	dsi->host_fifo_depth = 64;
 
+	err = tegra_dsi_ganged_probe(dsi);
+	if (err < 0)
+		return err;
+
 	err = tegra_output_probe(&dsi->output);
 	if (err < 0)
 		return err;
 
+	dsi->output.connector.polled = DRM_CONNECTOR_POLL_HPD;
+
 	/*
 	 * Assume these values by default. When a DSI peripheral driver
 	 * attaches to the DSI host, the parameters will be taken from
@@ -863,68 +1395,83 @@ static int tegra_dsi_probe(struct platform_device *pdev)
 	if (IS_ERR(dsi->rst))
 		return PTR_ERR(dsi->rst);
 
+	err = reset_control_deassert(dsi->rst);
+	if (err < 0) {
+		dev_err(&pdev->dev, "failed to bring DSI out of reset: %d\n",
+			err);
+		return err;
+	}
+
 	dsi->clk = devm_clk_get(&pdev->dev, NULL);
 	if (IS_ERR(dsi->clk)) {
 		dev_err(&pdev->dev, "cannot get DSI clock\n");
-		return PTR_ERR(dsi->clk);
+		err = PTR_ERR(dsi->clk);
+		goto reset;
 	}
 
 	err = clk_prepare_enable(dsi->clk);
 	if (err < 0) {
 		dev_err(&pdev->dev, "cannot enable DSI clock\n");
-		return err;
+		goto reset;
 	}
 
 	dsi->clk_lp = devm_clk_get(&pdev->dev, "lp");
 	if (IS_ERR(dsi->clk_lp)) {
 		dev_err(&pdev->dev, "cannot get low-power clock\n");
-		return PTR_ERR(dsi->clk_lp);
+		err = PTR_ERR(dsi->clk_lp);
+		goto disable_clk;
 	}
 
 	err = clk_prepare_enable(dsi->clk_lp);
 	if (err < 0) {
 		dev_err(&pdev->dev, "cannot enable low-power clock\n");
-		return err;
+		goto disable_clk;
 	}
 
 	dsi->clk_parent = devm_clk_get(&pdev->dev, "parent");
 	if (IS_ERR(dsi->clk_parent)) {
 		dev_err(&pdev->dev, "cannot get parent clock\n");
-		return PTR_ERR(dsi->clk_parent);
-	}
-
-	err = clk_prepare_enable(dsi->clk_parent);
-	if (err < 0) {
-		dev_err(&pdev->dev, "cannot enable parent clock\n");
-		return err;
+		err = PTR_ERR(dsi->clk_parent);
+		goto disable_clk_lp;
 	}
 
 	dsi->vdd = devm_regulator_get(&pdev->dev, "avdd-dsi-csi");
 	if (IS_ERR(dsi->vdd)) {
 		dev_err(&pdev->dev, "cannot get VDD supply\n");
-		return PTR_ERR(dsi->vdd);
+		err = PTR_ERR(dsi->vdd);
+		goto disable_clk_lp;
 	}
 
 	err = regulator_enable(dsi->vdd);
 	if (err < 0) {
 		dev_err(&pdev->dev, "cannot enable VDD supply\n");
-		return err;
+		goto disable_clk_lp;
 	}
 
 	err = tegra_dsi_setup_clocks(dsi);
 	if (err < 0) {
 		dev_err(&pdev->dev, "cannot setup clocks\n");
-		return err;
+		goto disable_vdd;
 	}
 
 	regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
 	dsi->regs = devm_ioremap_resource(&pdev->dev, regs);
-	if (IS_ERR(dsi->regs))
-		return PTR_ERR(dsi->regs);
+	if (IS_ERR(dsi->regs)) {
+		err = PTR_ERR(dsi->regs);
+		goto disable_vdd;
+	}
 
 	dsi->mipi = tegra_mipi_request(&pdev->dev);
-	if (IS_ERR(dsi->mipi))
-		return PTR_ERR(dsi->mipi);
+	if (IS_ERR(dsi->mipi)) {
+		err = PTR_ERR(dsi->mipi);
+		goto disable_vdd;
+	}
+
+	err = tegra_dsi_pad_calibrate(dsi);
+	if (err < 0) {
+		dev_err(dsi->dev, "MIPI calibration failed: %d\n", err);
+		goto mipi_free;
+	}
 
 	dsi->host.ops = &tegra_dsi_host_ops;
 	dsi->host.dev = &pdev->dev;
@@ -932,7 +1479,7 @@ static int tegra_dsi_probe(struct platform_device *pdev)
 	err = mipi_dsi_host_register(&dsi->host);
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register DSI host: %d\n", err);
-		return err;
+		goto mipi_free;
 	}
 
 	INIT_LIST_HEAD(&dsi->client.list);
@@ -943,12 +1490,26 @@ static int tegra_dsi_probe(struct platform_device *pdev)
 	if (err < 0) {
 		dev_err(&pdev->dev, "failed to register host1x client: %d\n",
 			err);
-		return err;
+		goto unregister;
 	}
 
 	platform_set_drvdata(pdev, dsi);
 
 	return 0;
+
+unregister:
+	mipi_dsi_host_unregister(&dsi->host);
+mipi_free:
+	tegra_mipi_free(dsi->mipi);
+disable_vdd:
+	regulator_disable(dsi->vdd);
+disable_clk_lp:
+	clk_disable_unprepare(dsi->clk_lp);
+disable_clk:
+	clk_disable_unprepare(dsi->clk);
+reset:
+	reset_control_assert(dsi->rst);
+	return err;
 }
 
 static int tegra_dsi_remove(struct platform_device *pdev)
@@ -967,7 +1528,6 @@ static int tegra_dsi_remove(struct platform_device *pdev)
 	tegra_mipi_free(dsi->mipi);
 
 	regulator_disable(dsi->vdd);
-	clk_disable_unprepare(dsi->clk_parent);
 	clk_disable_unprepare(dsi->clk_lp);
 	clk_disable_unprepare(dsi->clk);
 	reset_control_assert(dsi->rst);
diff --git a/drivers/gpu/drm/tegra/dsi.h b/drivers/gpu/drm/tegra/dsi.h
index 5ce610d08d77..bad1006a5150 100644
--- a/drivers/gpu/drm/tegra/dsi.h
+++ b/drivers/gpu/drm/tegra/dsi.h
@@ -21,9 +21,16 @@
 #define DSI_INT_STATUS			0x0d
 #define DSI_INT_MASK			0x0e
 #define DSI_HOST_CONTROL		0x0f
+#define DSI_HOST_CONTROL_FIFO_RESET	(1 << 21)
+#define DSI_HOST_CONTROL_CRC_RESET	(1 << 20)
+#define DSI_HOST_CONTROL_TX_TRIG_SOL	(0 << 12)
+#define DSI_HOST_CONTROL_TX_TRIG_FIFO	(1 << 12)
+#define DSI_HOST_CONTROL_TX_TRIG_HOST	(2 << 12)
 #define DSI_HOST_CONTROL_RAW		(1 << 6)
 #define DSI_HOST_CONTROL_HS		(1 << 5)
-#define DSI_HOST_CONTROL_BTA		(1 << 2)
+#define DSI_HOST_CONTROL_FIFO_SEL	(1 << 4)
+#define DSI_HOST_CONTROL_IMM_BTA	(1 << 3)
+#define DSI_HOST_CONTROL_PKT_BTA	(1 << 2)
 #define DSI_HOST_CONTROL_CS		(1 << 1)
 #define DSI_HOST_CONTROL_ECC		(1 << 0)
 #define DSI_CONTROL			0x10
@@ -39,9 +46,13 @@
 #define DSI_SOL_DELAY			0x11
 #define DSI_MAX_THRESHOLD		0x12
 #define DSI_TRIGGER			0x13
+#define DSI_TRIGGER_HOST		(1 << 1)
+#define DSI_TRIGGER_VIDEO		(1 << 0)
 #define DSI_TX_CRC			0x14
 #define DSI_STATUS			0x15
 #define DSI_STATUS_IDLE			(1 << 10)
+#define DSI_STATUS_UNDERFLOW		(1 <<  9)
+#define DSI_STATUS_OVERFLOW		(1 <<  8)
 #define DSI_INIT_SEQ_CONTROL		0x1a
 #define DSI_INIT_SEQ_DATA_0		0x1b
 #define DSI_INIT_SEQ_DATA_1		0x1c
@@ -104,6 +115,7 @@
 #define DSI_PAD_CONTROL_3		0x51
 #define DSI_PAD_CONTROL_4		0x52
 #define DSI_GANGED_MODE_CONTROL		0x53
+#define DSI_GANGED_MODE_CONTROL_ENABLE	(1 << 0)
 #define DSI_GANGED_MODE_START		0x54
 #define DSI_GANGED_MODE_SIZE		0x55
 #define DSI_RAW_DATA_BYTE_COUNT		0x56
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 05/17] drm/tegra: dsi: Set up PHY_TIMING & BTA_TIMING registers earlier

[Thierry Reding]

From: Sean Paul <seanpaul@chromium.org>

Make sure the DSI PHY_TIMING and BTA_TIMING registers are initialized
when the clocks are set up as opposed to when the output is enabled.
This makes sure that the PHY timings are properly set up when the panel
is prepared and that DCS commands sent at that time use the appropriate
timings.

Signed-off-by: Sean Paul <seanpaul@chromium.org>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dsi.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
index 8940360ccc9c..33f67fd601c6 100644
--- a/drivers/gpu/drm/tegra/dsi.c
+++ b/drivers/gpu/drm/tegra/dsi.c
@@ -389,6 +389,9 @@ static int tegra_dsi_set_phy_timing(struct tegra_dsi *dsi)
 		DSI_TIMING_FIELD(timing.tago, period, 1);
 	tegra_dsi_writel(dsi, value, DSI_BTA_TIMING);
 
+	if (dsi->slave)
+		return tegra_dsi_set_phy_timing(dsi->slave);
+
 	return 0;
 }
 
@@ -536,10 +539,6 @@ static int tegra_dsi_configure(struct tegra_dsi *dsi, unsigned int pipe,
 	value &= ~DSI_CONTROL_HOST_ENABLE;
 	tegra_dsi_writel(dsi, value, DSI_CONTROL);
 
-	err = tegra_dsi_set_phy_timing(dsi);
-	if (err < 0)
-		return err;
-
 	for (i = 0; i < NUM_PKT_SEQ; i++)
 		tegra_dsi_writel(dsi, pkt_seq[i], DSI_PKT_SEQ_0_LO + i);
 
@@ -860,6 +859,10 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,
 	 */
 	tegra_dsi_set_timeout(dsi, bclk, vrefresh);
 
+	err = tegra_dsi_set_phy_timing(dsi);
+	if (err < 0)
+		return err;
+
 	return 0;
 }
 
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 06/17] drm/tegra: dc: Add missing call to drm_vblank_on()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

When the CRTC is enabled, make sure the VBLANK machinery is enabled.
Failure to do so will cause drm_vblank_get() to not enable the VBLANK on
the CRTC and VBLANK-synchronized page-flips won't work.

While at it, get rid of the legacy drm_vblank_pre_modeset() and
drm_vblank_post_modeset() calls that are replaced by drm_vblank_on()
and drm_vblank_off().

Reported-by: Alexandre Courbot <acourbot@nvidia.com>
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 4a015232e2e8..4da366a4d78a 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
 
 static void tegra_crtc_disable(struct drm_crtc *crtc)
 {
-	struct tegra_dc *dc = to_tegra_dc(crtc);
 	struct drm_device *drm = crtc->dev;
 	struct drm_plane *plane;
 
@@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
 		}
 	}
 
-	drm_vblank_off(drm, dc->pipe);
+	drm_crtc_vblank_off(crtc);
 }
 
 static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -844,8 +843,6 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
 	u32 value;
 	int err;
 
-	drm_vblank_pre_modeset(crtc->dev, dc->pipe);
-
 	err = tegra_crtc_setup_clk(crtc, mode);
 	if (err) {
 		dev_err(dc->dev, "failed to setup clock for CRTC: %d\n", err);
@@ -946,7 +943,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
 	value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
 	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
 
-	drm_vblank_post_modeset(crtc->dev, dc->pipe);
+	drm_crtc_vblank_on(crtc);
 }
 
 static void tegra_crtc_load_lut(struct drm_crtc *crtc)
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 07/17] drm/tegra: gem: Extract tegra_bo_alloc_object()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This function implements the common buffer object allocation used for
both allocation and import paths.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 73 +++++++++++++++++++++++----------------------
 1 file changed, 38 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index ce023fa3e8ae..75f95e47bff1 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -91,6 +91,36 @@ static const struct host1x_bo_ops tegra_bo_ops = {
 	.kunmap = tegra_bo_kunmap,
 };
 
+static struct tegra_bo *tegra_bo_alloc_object(struct drm_device *drm,
+					      size_t size)
+{
+	struct tegra_bo *bo;
+	int err;
+
+	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
+	if (!bo)
+		return ERR_PTR(-ENOMEM);
+
+	host1x_bo_init(&bo->base, &tegra_bo_ops);
+	size = round_up(size, PAGE_SIZE);
+
+	err = drm_gem_object_init(drm, &bo->gem, size);
+	if (err < 0)
+		goto free;
+
+	err = drm_gem_create_mmap_offset(&bo->gem);
+	if (err < 0)
+		goto release;
+
+	return bo;
+
+release:
+	drm_gem_object_release(&bo->gem);
+free:
+	kfree(bo);
+	return ERR_PTR(err);
+}
+
 static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
 {
 	dma_free_writecombine(drm->dev, bo->gem.size, bo->vaddr, bo->paddr);
@@ -102,12 +132,9 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size,
 	struct tegra_bo *bo;
 	int err;
 
-	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
-	if (!bo)
-		return ERR_PTR(-ENOMEM);
-
-	host1x_bo_init(&bo->base, &tegra_bo_ops);
-	size = round_up(size, PAGE_SIZE);
+	bo = tegra_bo_alloc_object(drm, size);
+	if (IS_ERR(bo))
+		return bo;
 
 	bo->vaddr = dma_alloc_writecombine(drm->dev, size, &bo->paddr,
 					   GFP_KERNEL | __GFP_NOWARN);
@@ -118,14 +145,6 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size,
 		goto err_dma;
 	}
 
-	err = drm_gem_object_init(drm, &bo->gem, size);
-	if (err)
-		goto err_init;
-
-	err = drm_gem_create_mmap_offset(&bo->gem);
-	if (err)
-		goto err_mmap;
-
 	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
 		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
 
@@ -175,28 +194,16 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
 {
 	struct dma_buf_attachment *attach;
 	struct tegra_bo *bo;
-	ssize_t size;
 	int err;
 
-	bo = kzalloc(sizeof(*bo), GFP_KERNEL);
-	if (!bo)
-		return ERR_PTR(-ENOMEM);
-
-	host1x_bo_init(&bo->base, &tegra_bo_ops);
-	size = round_up(buf->size, PAGE_SIZE);
-
-	err = drm_gem_object_init(drm, &bo->gem, size);
-	if (err < 0)
-		goto free;
-
-	err = drm_gem_create_mmap_offset(&bo->gem);
-	if (err < 0)
-		goto release;
+	bo = tegra_bo_alloc_object(drm, buf->size);
+	if (IS_ERR(bo))
+		return bo;
 
 	attach = dma_buf_attach(buf, drm->dev);
 	if (IS_ERR(attach)) {
 		err = PTR_ERR(attach);
-		goto free_mmap;
+		goto free;
 	}
 
 	get_dma_buf(buf);
@@ -228,13 +235,9 @@ detach:
 
 	dma_buf_detach(buf, attach);
 	dma_buf_put(buf);
-free_mmap:
-	drm_gem_free_mmap_offset(&bo->gem);
-release:
-	drm_gem_object_release(&bo->gem);
 free:
+	drm_gem_object_release(&bo->gem);
 	kfree(bo);
-
 	return ERR_PTR(err);
 }
 
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 08/17] drm/tegra: gem: Cleanup tegra_bo_create_with_handle()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

There is only a single location where the function needs to do cleanup.
Skip the error unwinding path and call the cleanup function directly
instead.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 14 ++++++--------
 1 file changed, 6 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 75f95e47bff1..db82c15abc16 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -170,23 +170,21 @@ struct tegra_bo *tegra_bo_create_with_handle(struct drm_file *file,
 					     unsigned int *handle)
 {
 	struct tegra_bo *bo;
-	int ret;
+	int err;
 
 	bo = tegra_bo_create(drm, size, flags);
 	if (IS_ERR(bo))
 		return bo;
 
-	ret = drm_gem_handle_create(file, &bo->gem, handle);
-	if (ret)
-		goto err;
+	err = drm_gem_handle_create(file, &bo->gem, handle);
+	if (err) {
+		tegra_bo_free_object(&bo->gem);
+		return ERR_PTR(err);
+	}
 
 	drm_gem_object_unreference_unlocked(&bo->gem);
 
 	return bo;
-
-err:
-	tegra_bo_free_object(&bo->gem);
-	return ERR_PTR(ret);
 }
 
 static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 09/17] drm/tegra: gem: Remove redundant drm_gem_free_mmap_offset()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The drm_gem_object_release() function already performs this cleanup, so
there is no reason to do it explicitly.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index db82c15abc16..5bc59c5109e0 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -251,9 +251,7 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
 		tegra_bo_destroy(gem->dev, bo);
 	}
 
-	drm_gem_free_mmap_offset(gem);
 	drm_gem_object_release(gem);
-
 	kfree(bo);
 }
 
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 10/17] drm/tegra: gem: Use dma_mmap_writecombine()

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Use the existing API rather than open-coding equivalent functionality
in the driver.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 5bc59c5109e0..b513df33f17a 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -310,6 +310,7 @@ const struct vm_operations_struct tegra_bo_vm_ops = {
 
 int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
 {
+	unsigned long vm_pgoff = vma->vm_pgoff;
 	struct drm_gem_object *gem;
 	struct tegra_bo *bo;
 	int ret;
@@ -321,12 +322,19 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
 	gem = vma->vm_private_data;
 	bo = to_tegra_bo(gem);
 
-	ret = remap_pfn_range(vma, vma->vm_start, bo->paddr >> PAGE_SHIFT,
-			      vma->vm_end - vma->vm_start, vma->vm_page_prot);
-	if (ret)
+	vma->vm_flags &= ~VM_PFNMAP;
+	vma->vm_pgoff = 0;
+
+	ret = dma_mmap_writecombine(gem->dev->dev, vma, bo->vaddr, bo->paddr,
+				    gem->size);
+	if (ret) {
 		drm_gem_vm_close(vma);
+		return ret;
+	}
+
+	vma->vm_pgoff = vm_pgoff;
 
-	return ret;
+	return 0;
 }
 
 static struct sg_table *
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH v5 11/17] drm/tegra: Add IOMMU support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

When an IOMMU device is available on the platform bus, allocate an IOMMU
domain and attach the display controllers to it. The display controllers
can then scan out non-contiguous buffers by mapping them through the
IOMMU.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
Changes in v5:
- fix PRIME export of physically non-contiguous buffers
- refactor and fix error cleanup paths
- fix checking for failed allocations
- fix build warnings on 64-bit

Changes in v4:
- error out if IOMMU attachment fails

Changes in v3:
- create write-combined mappings for mmap()
- keep non-IOMMU fallback path working
- no longer rely on iommu_attach()

Changes in v2:
- don't pass GFP_KERNEL to drm_gem_get_pages()

 drivers/gpu/drm/tegra/dc.c  |  17 +++
 drivers/gpu/drm/tegra/drm.c |  18 +++
 drivers/gpu/drm/tegra/drm.h |   5 +
 drivers/gpu/drm/tegra/fb.c  |  16 ++-
 drivers/gpu/drm/tegra/gem.c | 277 ++++++++++++++++++++++++++++++++++++++------
 drivers/gpu/drm/tegra/gem.h |   6 +
 6 files changed, 302 insertions(+), 37 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 4da366a4d78a..a1e7962ccbe2 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -9,6 +9,7 @@
 
 #include <linux/clk.h>
 #include <linux/debugfs.h>
+#include <linux/iommu.h>
 #include <linux/reset.h>
 
 #include <soc/tegra/pmc.h>
@@ -1287,6 +1288,17 @@ static int tegra_dc_init(struct host1x_client *client)
 	struct tegra_drm *tegra = drm->dev_private;
 	int err;
 
+	if (tegra->domain) {
+		err = iommu_attach_device(tegra->domain, dc->dev);
+		if (err < 0) {
+			dev_err(dc->dev, "failed to attach to domain: %d\n",
+				err);
+			return err;
+		}
+
+		dc->domain = tegra->domain;
+	}
+
 	drm_crtc_init(drm, &dc->base, &tegra_crtc_funcs);
 	drm_mode_crtc_set_gamma_size(&dc->base, 256);
 	drm_crtc_helper_add(&dc->base, &tegra_crtc_helper_funcs);
@@ -1344,6 +1356,11 @@ static int tegra_dc_exit(struct host1x_client *client)
 		return err;
 	}
 
+	if (dc->domain) {
+		iommu_detach_device(dc->domain, dc->dev);
+		dc->domain = NULL;
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/tegra/drm.c b/drivers/gpu/drm/tegra/drm.c
index 59736bb810cd..ce5123c2c3c5 100644
--- a/drivers/gpu/drm/tegra/drm.c
+++ b/drivers/gpu/drm/tegra/drm.c
@@ -8,6 +8,7 @@
  */
 
 #include <linux/host1x.h>
+#include <linux/iommu.h>
 
 #include "drm.h"
 #include "gem.h"
@@ -33,6 +34,17 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
 	if (!tegra)
 		return -ENOMEM;
 
+	if (iommu_present(&platform_bus_type)) {
+		tegra->domain = iommu_domain_alloc(&platform_bus_type);
+		if (IS_ERR(tegra->domain)) {
+			kfree(tegra);
+			return PTR_ERR(tegra->domain);
+		}
+
+		DRM_DEBUG("IOMMU context initialized\n");
+		drm_mm_init(&tegra->mm, 0, SZ_2G);
+	}
+
 	mutex_init(&tegra->clients_lock);
 	INIT_LIST_HEAD(&tegra->clients);
 	drm->dev_private = tegra;
@@ -71,6 +83,7 @@ static int tegra_drm_load(struct drm_device *drm, unsigned long flags)
 static int tegra_drm_unload(struct drm_device *drm)
 {
 	struct host1x_device *device = to_host1x_device(drm->dev);
+	struct tegra_drm *tegra = drm->dev_private;
 	int err;
 
 	drm_kms_helper_poll_fini(drm);
@@ -82,6 +95,11 @@ static int tegra_drm_unload(struct drm_device *drm)
 	if (err < 0)
 		return err;
 
+	if (tegra->domain) {
+		iommu_domain_free(tegra->domain);
+		drm_mm_takedown(&tegra->mm);
+	}
+
 	return 0;
 }
 
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index b994c017971d..2fdae404e719 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -39,6 +39,9 @@ struct tegra_fbdev {
 struct tegra_drm {
 	struct drm_device *drm;
 
+	struct iommu_domain *domain;
+	struct drm_mm mm;
+
 	struct mutex clients_lock;
 	struct list_head clients;
 
@@ -121,6 +124,8 @@ struct tegra_dc {
 	struct drm_pending_vblank_event *event;
 
 	const struct tegra_dc_soc_info *soc;
+
+	struct iommu_domain *domain;
 };
 
 static inline struct tegra_dc *
diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index 3513d12d5aa1..c5fa3c4b2ed5 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -65,8 +65,12 @@ static void tegra_fb_destroy(struct drm_framebuffer *framebuffer)
 	for (i = 0; i < fb->num_planes; i++) {
 		struct tegra_bo *bo = fb->planes[i];
 
-		if (bo)
+		if (bo) {
+			if (bo->pages && bo->vaddr)
+				vunmap(bo->vaddr);
+
 			drm_gem_object_unreference_unlocked(&bo->gem);
+		}
 	}
 
 	drm_framebuffer_cleanup(framebuffer);
@@ -254,6 +258,16 @@ static int tegra_fbdev_probe(struct drm_fb_helper *helper,
 	offset = info->var.xoffset * bytes_per_pixel +
 		 info->var.yoffset * fb->pitches[0];
 
+	if (bo->pages) {
+		bo->vaddr = vmap(bo->pages, bo->num_pages, VM_MAP,
+				 pgprot_writecombine(PAGE_KERNEL));
+		if (!bo->vaddr) {
+			dev_err(drm->dev, "failed to vmap() framebuffer\n");
+			err = -ENOMEM;
+			goto destroy;
+		}
+	}
+
 	drm->mode_config.fb_base = (resource_size_t)bo->paddr;
 	info->screen_base = (void __iomem *)bo->vaddr + offset;
 	info->screen_size = size;
diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index b513df33f17a..8b1095d05c58 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -14,6 +14,7 @@
  */
 
 #include <linux/dma-buf.h>
+#include <linux/iommu.h>
 #include <drm/tegra_drm.h>
 
 #include "drm.h"
@@ -91,6 +92,88 @@ static const struct host1x_bo_ops tegra_bo_ops = {
 	.kunmap = tegra_bo_kunmap,
 };
 
+/*
+ * A generic iommu_map_sg() function is being reviewed and will hopefully be
+ * merged soon. At that point this function can be dropped in favour of the
+ * one provided by the IOMMU API.
+ */
+static ssize_t __iommu_map_sg(struct iommu_domain *domain, unsigned long iova,
+			      struct scatterlist *sg, unsigned int nents,
+			      int prot)
+{
+	struct scatterlist *s;
+	size_t offset = 0;
+	unsigned int i;
+	int err;
+
+	for_each_sg(sg, s, nents, i) {
+		phys_addr_t phys = page_to_phys(sg_page(s));
+		size_t length = s->offset + s->length;
+
+		err = iommu_map(domain, iova + offset, phys, length, prot);
+		if (err < 0) {
+			iommu_unmap(domain, iova, offset);
+			return err;
+		}
+
+		offset += length;
+	}
+
+	return offset;
+}
+
+static int tegra_bo_iommu_map(struct tegra_drm *tegra, struct tegra_bo *bo)
+{
+	int prot = IOMMU_READ | IOMMU_WRITE;
+	ssize_t err;
+
+	if (bo->mm)
+		return -EBUSY;
+
+	bo->mm = kzalloc(sizeof(*bo->mm), GFP_KERNEL);
+	if (!bo->mm)
+		return -ENOMEM;
+
+	err = drm_mm_insert_node_generic(&tegra->mm, bo->mm, bo->gem.size,
+					 PAGE_SIZE, 0, 0, 0);
+	if (err < 0) {
+		dev_err(tegra->drm->dev, "out of I/O virtual memory: %zd\n",
+			err);
+		goto free;
+	}
+
+	bo->paddr = bo->mm->start;
+
+	err = __iommu_map_sg(tegra->domain, bo->paddr, bo->sgt->sgl,
+			     bo->sgt->nents, prot);
+	if (err < 0) {
+		dev_err(tegra->drm->dev, "failed to map buffer: %zd\n", err);
+		goto remove;
+	}
+
+	bo->size = err;
+
+	return 0;
+
+remove:
+	drm_mm_remove_node(bo->mm);
+free:
+	kfree(bo->mm);
+	return err;
+}
+
+static int tegra_bo_iommu_unmap(struct tegra_drm *tegra, struct tegra_bo *bo)
+{
+	if (!bo->mm)
+		return 0;
+
+	iommu_unmap(tegra->domain, bo->paddr, bo->size);
+	drm_mm_remove_node(bo->mm);
+	kfree(bo->mm);
+
+	return 0;
+}
+
 static struct tegra_bo *tegra_bo_alloc_object(struct drm_device *drm,
 					      size_t size)
 {
@@ -121,9 +204,64 @@ free:
 	return ERR_PTR(err);
 }
 
-static void tegra_bo_destroy(struct drm_device *drm, struct tegra_bo *bo)
+static void tegra_bo_free(struct drm_device *drm, struct tegra_bo *bo)
 {
-	dma_free_writecombine(drm->dev, bo->gem.size, bo->vaddr, bo->paddr);
+	if (bo->pages) {
+		drm_gem_put_pages(&bo->gem, bo->pages, true, true);
+		sg_free_table(bo->sgt);
+		kfree(bo->sgt);
+	} else {
+		dma_free_writecombine(drm->dev, bo->gem.size, bo->vaddr,
+				      bo->paddr);
+	}
+}
+
+static int tegra_bo_get_pages(struct drm_device *drm, struct tegra_bo *bo,
+			      size_t size)
+{
+	bo->pages = drm_gem_get_pages(&bo->gem);
+	if (IS_ERR(bo->pages))
+		return PTR_ERR(bo->pages);
+
+	bo->num_pages = size >> PAGE_SHIFT;
+
+	bo->sgt = drm_prime_pages_to_sg(bo->pages, bo->num_pages);
+	if (IS_ERR(bo->sgt)) {
+		drm_gem_put_pages(&bo->gem, bo->pages, false, false);
+		return PTR_ERR(bo->sgt);
+	}
+
+	return 0;
+}
+
+static int tegra_bo_alloc(struct drm_device *drm, struct tegra_bo *bo,
+			  size_t size)
+{
+	struct tegra_drm *tegra = drm->dev_private;
+	int err;
+
+	if (tegra->domain) {
+		err = tegra_bo_get_pages(drm, bo, size);
+		if (err < 0)
+			return err;
+
+		err = tegra_bo_iommu_map(tegra, bo);
+		if (err < 0) {
+			tegra_bo_free(drm, bo);
+			return err;
+		}
+	} else {
+		bo->vaddr = dma_alloc_writecombine(drm->dev, size, &bo->paddr,
+						   GFP_KERNEL | __GFP_NOWARN);
+		if (!bo->vaddr) {
+			dev_err(drm->dev,
+				"failed to allocate buffer of size %zu\n",
+				size);
+			return -ENOMEM;
+		}
+	}
+
+	return 0;
 }
 
 struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size,
@@ -136,14 +274,9 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size,
 	if (IS_ERR(bo))
 		return bo;
 
-	bo->vaddr = dma_alloc_writecombine(drm->dev, size, &bo->paddr,
-					   GFP_KERNEL | __GFP_NOWARN);
-	if (!bo->vaddr) {
-		dev_err(drm->dev, "failed to allocate buffer with size %u\n",
-			size);
-		err = -ENOMEM;
-		goto err_dma;
-	}
+	err = tegra_bo_alloc(drm, bo, size);
+	if (err < 0)
+		goto release;
 
 	if (flags & DRM_TEGRA_GEM_CREATE_TILED)
 		bo->tiling.mode = TEGRA_BO_TILING_MODE_TILED;
@@ -153,13 +286,9 @@ struct tegra_bo *tegra_bo_create(struct drm_device *drm, unsigned int size,
 
 	return bo;
 
-err_mmap:
+release:
 	drm_gem_object_release(&bo->gem);
-err_init:
-	tegra_bo_destroy(drm, bo);
-err_dma:
 	kfree(bo);
-
 	return ERR_PTR(err);
 }
 
@@ -190,6 +319,7 @@ struct tegra_bo *tegra_bo_create_with_handle(struct drm_file *file,
 static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
 					struct dma_buf *buf)
 {
+	struct tegra_drm *tegra = drm->dev_private;
 	struct dma_buf_attachment *attach;
 	struct tegra_bo *bo;
 	int err;
@@ -217,12 +347,19 @@ static struct tegra_bo *tegra_bo_import(struct drm_device *drm,
 		goto detach;
 	}
 
-	if (bo->sgt->nents > 1) {
-		err = -EINVAL;
-		goto detach;
+	if (tegra->domain) {
+		err = tegra_bo_iommu_map(tegra, bo);
+		if (err < 0)
+			goto detach;
+	} else {
+		if (bo->sgt->nents > 1) {
+			err = -EINVAL;
+			goto detach;
+		}
+
+		bo->paddr = sg_dma_address(bo->sgt->sgl);
 	}
 
-	bo->paddr = sg_dma_address(bo->sgt->sgl);
 	bo->gem.import_attach = attach;
 
 	return bo;
@@ -241,14 +378,18 @@ free:
 
 void tegra_bo_free_object(struct drm_gem_object *gem)
 {
+	struct tegra_drm *tegra = gem->dev->dev_private;
 	struct tegra_bo *bo = to_tegra_bo(gem);
 
+	if (tegra->domain)
+		tegra_bo_iommu_unmap(tegra, bo);
+
 	if (gem->import_attach) {
 		dma_buf_unmap_attachment(gem->import_attach, bo->sgt,
 					 DMA_TO_DEVICE);
 		drm_prime_gem_destroy(gem, NULL);
 	} else {
-		tegra_bo_destroy(gem->dev, bo);
+		tegra_bo_free(gem->dev, bo);
 	}
 
 	drm_gem_object_release(gem);
@@ -303,14 +444,44 @@ int tegra_bo_dumb_map_offset(struct drm_file *file, struct drm_device *drm,
 	return 0;
 }
 
+static int tegra_bo_fault(struct vm_area_struct *vma, struct vm_fault *vmf)
+{
+	struct drm_gem_object *gem = vma->vm_private_data;
+	struct tegra_bo *bo = to_tegra_bo(gem);
+	struct page *page;
+	pgoff_t offset;
+	int err;
+
+	if (!bo->pages)
+		return VM_FAULT_SIGBUS;
+
+	offset = ((unsigned long)vmf->virtual_address - vma->vm_start) >> PAGE_SHIFT;
+	page = bo->pages[offset];
+
+	err = vm_insert_page(vma, (unsigned long)vmf->virtual_address, page);
+	switch (err) {
+	case -EAGAIN:
+	case 0:
+	case -ERESTARTSYS:
+	case -EINTR:
+	case -EBUSY:
+		return VM_FAULT_NOPAGE;
+
+	case -ENOMEM:
+		return VM_FAULT_OOM;
+	}
+
+	return VM_FAULT_SIGBUS;
+}
+
 const struct vm_operations_struct tegra_bo_vm_ops = {
+	.fault = tegra_bo_fault,
 	.open = drm_gem_vm_open,
 	.close = drm_gem_vm_close,
 };
 
 int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
 {
-	unsigned long vm_pgoff = vma->vm_pgoff;
 	struct drm_gem_object *gem;
 	struct tegra_bo *bo;
 	int ret;
@@ -322,17 +493,28 @@ int tegra_drm_mmap(struct file *file, struct vm_area_struct *vma)
 	gem = vma->vm_private_data;
 	bo = to_tegra_bo(gem);
 
-	vma->vm_flags &= ~VM_PFNMAP;
-	vma->vm_pgoff = 0;
+	if (!bo->pages) {
+		unsigned long vm_pgoff = vma->vm_pgoff;
 
-	ret = dma_mmap_writecombine(gem->dev->dev, vma, bo->vaddr, bo->paddr,
-				    gem->size);
-	if (ret) {
-		drm_gem_vm_close(vma);
-		return ret;
-	}
+		vma->vm_flags &= ~VM_PFNMAP;
+		vma->vm_pgoff = 0;
 
-	vma->vm_pgoff = vm_pgoff;
+		ret = dma_mmap_writecombine(gem->dev->dev, vma, bo->vaddr,
+					    bo->paddr, gem->size);
+		if (ret) {
+			drm_gem_vm_close(vma);
+			return ret;
+		}
+
+		vma->vm_pgoff = vm_pgoff;
+	} else {
+		pgprot_t prot = vm_get_page_prot(vma->vm_flags);
+
+		vma->vm_flags |= VM_MIXEDMAP;
+		vma->vm_flags &= ~VM_PFNMAP;
+
+		vma->vm_page_prot = pgprot_writecombine(prot);
+	}
 
 	return 0;
 }
@@ -349,21 +531,44 @@ tegra_gem_prime_map_dma_buf(struct dma_buf_attachment *attach,
 	if (!sgt)
 		return NULL;
 
-	if (sg_alloc_table(sgt, 1, GFP_KERNEL)) {
-		kfree(sgt);
-		return NULL;
-	}
+	if (bo->pages) {
+		struct scatterlist *sg;
+		unsigned int i;
+
+		if (sg_alloc_table(sgt, bo->num_pages, GFP_KERNEL))
+			goto free;
 
-	sg_dma_address(sgt->sgl) = bo->paddr;
-	sg_dma_len(sgt->sgl) = gem->size;
+		for_each_sg(sgt->sgl, sg, bo->num_pages, i)
+			sg_set_page(sg, bo->pages[i], PAGE_SIZE, 0);
+
+		if (dma_map_sg(attach->dev, sgt->sgl, sgt->nents, dir) == 0)
+			goto free;
+	} else {
+		if (sg_alloc_table(sgt, 1, GFP_KERNEL))
+			goto free;
+
+		sg_dma_address(sgt->sgl) = bo->paddr;
+		sg_dma_len(sgt->sgl) = gem->size;
+	}
 
 	return sgt;
+
+free:
+	sg_free_table(sgt);
+	kfree(sgt);
+	return NULL;
 }
 
 static void tegra_gem_prime_unmap_dma_buf(struct dma_buf_attachment *attach,
 					  struct sg_table *sgt,
 					  enum dma_data_direction dir)
 {
+	struct drm_gem_object *gem = attach->dmabuf->priv;
+	struct tegra_bo *bo = to_tegra_bo(gem);
+
+	if (bo->pages)
+		dma_unmap_sg(attach->dev, sgt->sgl, sgt->nents, dir);
+
 	sg_free_table(sgt);
 	kfree(sgt);
 }
diff --git a/drivers/gpu/drm/tegra/gem.h b/drivers/gpu/drm/tegra/gem.h
index 6538b56780c2..3dd4165f812a 100644
--- a/drivers/gpu/drm/tegra/gem.h
+++ b/drivers/gpu/drm/tegra/gem.h
@@ -38,6 +38,12 @@ struct tegra_bo {
 	dma_addr_t paddr;
 	void *vaddr;
 
+	struct drm_mm_node *mm;
+	unsigned long num_pages;
+	struct page **pages;
+	/* size of IOMMU mapping */
+	size_t size;
+
 	struct tegra_bo_tiling tiling;
 };
 
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 12/17] drm/tegra: dc: Factor out DC, window and cursor commit

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The sequence to commit changes to the DC, window or cursor configuration
is repetitive and can be extracted into separate functions for ease of
use.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c | 52 +++++++++++++++++++++++++---------------------
 1 file changed, 28 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index a1e7962ccbe2..3a6038a53fdb 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -36,6 +36,26 @@ static inline struct tegra_plane *to_tegra_plane(struct drm_plane *plane)
 	return container_of(plane, struct tegra_plane, base);
 }
 
+static void tegra_dc_window_commit(struct tegra_dc *dc, unsigned int index)
+{
+	u32 value = WIN_A_ACT_REQ << index;
+
+	tegra_dc_writel(dc, value << 8, DC_CMD_STATE_CONTROL);
+	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
+}
+
+static void tegra_dc_cursor_commit(struct tegra_dc *dc)
+{
+	tegra_dc_writel(dc, CURSOR_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
+	tegra_dc_writel(dc, CURSOR_ACT_REQ, DC_CMD_STATE_CONTROL);
+}
+
+static void tegra_dc_commit(struct tegra_dc *dc)
+{
+	tegra_dc_writel(dc, GENERAL_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
+	tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
+}
+
 static unsigned int tegra_dc_format(uint32_t format, uint32_t *swap)
 {
 	/* assume no swapping of fetched data */
@@ -307,8 +327,7 @@ static int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 		break;
 	}
 
-	tegra_dc_writel(dc, WIN_A_UPDATE << index, DC_CMD_STATE_CONTROL);
-	tegra_dc_writel(dc, WIN_A_ACT_REQ << index, DC_CMD_STATE_CONTROL);
+	tegra_dc_window_commit(dc, index);
 
 	return 0;
 }
@@ -379,8 +398,7 @@ static int tegra_plane_disable(struct drm_plane *plane)
 	value &= ~WIN_ENABLE;
 	tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
 
-	tegra_dc_writel(dc, WIN_A_UPDATE << p->index, DC_CMD_STATE_CONTROL);
-	tegra_dc_writel(dc, WIN_A_ACT_REQ << p->index, DC_CMD_STATE_CONTROL);
+	tegra_dc_window_commit(dc, p->index);
 
 	return 0;
 }
@@ -517,10 +535,8 @@ static int tegra_dc_set_base(struct tegra_dc *dc, int x, int y,
 	tegra_dc_writel(dc, h_offset, DC_WINBUF_ADDR_H_OFFSET);
 	tegra_dc_writel(dc, v_offset, DC_WINBUF_ADDR_V_OFFSET);
 
-	value = GENERAL_UPDATE | WIN_A_UPDATE;
-	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
-
 	value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
+	tegra_dc_writel(dc, value << 8, DC_CMD_STATE_CONTROL);
 	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
 
 	return 0;
@@ -625,11 +641,8 @@ static int tegra_dc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file,
 		tegra_dc_writel(dc, value, DC_DISP_DISP_WIN_OPTIONS);
 	}
 
-	tegra_dc_writel(dc, CURSOR_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
-	tegra_dc_writel(dc, CURSOR_ACT_REQ, DC_CMD_STATE_CONTROL);
-
-	tegra_dc_writel(dc, GENERAL_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
-	tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
+	tegra_dc_cursor_commit(dc);
+	tegra_dc_commit(dc);
 
 	return 0;
 }
@@ -645,12 +658,9 @@ static int tegra_dc_cursor_move(struct drm_crtc *crtc, int x, int y)
 	value = ((y & 0x3fff) << 16) | (x & 0x3fff);
 	tegra_dc_writel(dc, value, DC_DISP_CURSOR_POSITION);
 
-	tegra_dc_writel(dc, CURSOR_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
-	tegra_dc_writel(dc, CURSOR_ACT_REQ, DC_CMD_STATE_CONTROL);
-
+	tegra_dc_cursor_commit(dc);
 	/* XXX: only required on generations earlier than Tegra124? */
-	tegra_dc_writel(dc, GENERAL_ACT_REQ << 8, DC_CMD_STATE_CONTROL);
-	tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
+	tegra_dc_commit(dc);
 
 	return 0;
 }
@@ -936,15 +946,9 @@ static void tegra_crtc_prepare(struct drm_crtc *crtc)
 static void tegra_crtc_commit(struct drm_crtc *crtc)
 {
 	struct tegra_dc *dc = to_tegra_dc(crtc);
-	unsigned long value;
-
-	value = GENERAL_UPDATE | WIN_A_UPDATE;
-	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
-
-	value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
-	tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
 
 	drm_crtc_vblank_on(crtc);
+	tegra_dc_commit(dc);
 }
 
 static void tegra_crtc_load_lut(struct drm_crtc *crtc)
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 13/17] drm/tegra: dc: Registers are 32 bits wide

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Using an unsigned long type will cause these variables to become 64-bit
on 64-bit SoCs. In practice this should always work, but there's no need
for carrying around the additional 32 bits.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c  |  2 +-
 drivers/gpu/drm/tegra/drm.h | 11 +++++------
 2 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 3a6038a53fdb..13f8cc0e5324 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -1002,7 +1002,7 @@ static int tegra_dc_show_regs(struct seq_file *s, void *data)
 	struct tegra_dc *dc = node->info_ent->data;
 
 #define DUMP_REG(name)						\
-	seq_printf(s, "%-40s %#05x %08lx\n", #name, name,	\
+	seq_printf(s, "%-40s %#05x %08x\n", #name, name,	\
 		   tegra_dc_readl(dc, name))
 
 	DUMP_REG(DC_CMD_GENERAL_INCR_SYNCPT);
diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
index 2fdae404e719..601989bd3549 100644
--- a/drivers/gpu/drm/tegra/drm.h
+++ b/drivers/gpu/drm/tegra/drm.h
@@ -139,16 +139,15 @@ static inline struct tegra_dc *to_tegra_dc(struct drm_crtc *crtc)
 	return crtc ? container_of(crtc, struct tegra_dc, base) : NULL;
 }
 
-static inline void tegra_dc_writel(struct tegra_dc *dc, unsigned long value,
-				   unsigned long reg)
+static inline void tegra_dc_writel(struct tegra_dc *dc, u32 value,
+				   unsigned long offset)
 {
-	writel(value, dc->regs + (reg << 2));
+	writel(value, dc->regs + (offset << 2));
 }
 
-static inline unsigned long tegra_dc_readl(struct tegra_dc *dc,
-					   unsigned long reg)
+static inline u32 tegra_dc_readl(struct tegra_dc *dc, unsigned long offset)
 {
-	return readl(dc->regs + (reg << 2));
+	return readl(dc->regs + (offset << 2));
 }
 
 struct tegra_dc_window {
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 14/17] drm/tegra: dc: Universal plane support

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

This allows the primary plane and cursor to be exposed as regular
DRM/KMS planes, which is a prerequisite for atomic modesetting and gives
userspace more flexibility over controlling them.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/dc.c | 488 ++++++++++++++++++++++++++++++---------------
 1 file changed, 331 insertions(+), 157 deletions(-)

diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
index 13f8cc0e5324..791cdd3c95c3 100644
--- a/drivers/gpu/drm/tegra/dc.c
+++ b/drivers/gpu/drm/tegra/dc.c
@@ -332,11 +332,255 @@ static int tegra_dc_setup_window(struct tegra_dc *dc, unsigned int index,
 	return 0;
 }
 
-static int tegra_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
-			      struct drm_framebuffer *fb, int crtc_x,
-			      int crtc_y, unsigned int crtc_w,
-			      unsigned int crtc_h, uint32_t src_x,
-			      uint32_t src_y, uint32_t src_w, uint32_t src_h)
+static int tegra_window_plane_disable(struct drm_plane *plane)
+{
+	struct tegra_dc *dc = to_tegra_dc(plane->crtc);
+	struct tegra_plane *p = to_tegra_plane(plane);
+	u32 value;
+
+	if (!plane->crtc)
+		return 0;
+
+	value = WINDOW_A_SELECT << p->index;
+	tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
+
+	value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS);
+	value &= ~WIN_ENABLE;
+	tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
+
+	tegra_dc_window_commit(dc, p->index);
+
+	return 0;
+}
+
+static void tegra_plane_destroy(struct drm_plane *plane)
+{
+	struct tegra_plane *p = to_tegra_plane(plane);
+
+	drm_plane_cleanup(plane);
+	kfree(p);
+}
+
+static const u32 tegra_primary_plane_formats[] = {
+	DRM_FORMAT_XBGR8888,
+	DRM_FORMAT_XRGB8888,
+	DRM_FORMAT_RGB565,
+};
+
+static int tegra_primary_plane_update(struct drm_plane *plane,
+				      struct drm_crtc *crtc,
+				      struct drm_framebuffer *fb, int crtc_x,
+				      int crtc_y, unsigned int crtc_w,
+				      unsigned int crtc_h, uint32_t src_x,
+				      uint32_t src_y, uint32_t src_w,
+				      uint32_t src_h)
+{
+	struct tegra_bo *bo = tegra_fb_get_plane(fb, 0);
+	struct tegra_plane *p = to_tegra_plane(plane);
+	struct tegra_dc *dc = to_tegra_dc(crtc);
+	struct tegra_dc_window window;
+	int err;
+
+	memset(&window, 0, sizeof(window));
+	window.src.x = src_x >> 16;
+	window.src.y = src_y >> 16;
+	window.src.w = src_w >> 16;
+	window.src.h = src_h >> 16;
+	window.dst.x = crtc_x;
+	window.dst.y = crtc_y;
+	window.dst.w = crtc_w;
+	window.dst.h = crtc_h;
+	window.format = tegra_dc_format(fb->pixel_format, &window.swap);
+	window.bits_per_pixel = fb->bits_per_pixel;
+	window.bottom_up = tegra_fb_is_bottom_up(fb);
+
+	err = tegra_fb_get_tiling(fb, &window.tiling);
+	if (err < 0)
+		return err;
+
+	window.base[0] = bo->paddr + fb->offsets[0];
+	window.stride[0] = fb->pitches[0];
+
+	err = tegra_dc_setup_window(dc, p->index, &window);
+	if (err < 0)
+		return err;
+
+	return 0;
+}
+
+static void tegra_primary_plane_destroy(struct drm_plane *plane)
+{
+	tegra_window_plane_disable(plane);
+	tegra_plane_destroy(plane);
+}
+
+static const struct drm_plane_funcs tegra_primary_plane_funcs = {
+	.update_plane = tegra_primary_plane_update,
+	.disable_plane = tegra_window_plane_disable,
+	.destroy = tegra_primary_plane_destroy,
+};
+
+static struct drm_plane *tegra_dc_primary_plane_create(struct drm_device *drm,
+						       struct tegra_dc *dc)
+{
+	struct tegra_plane *plane;
+	unsigned int num_formats;
+	const u32 *formats;
+	int err;
+
+	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
+
+	num_formats = ARRAY_SIZE(tegra_primary_plane_formats);
+	formats = tegra_primary_plane_formats;
+
+	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
+				       &tegra_primary_plane_funcs, formats,
+				       num_formats, DRM_PLANE_TYPE_PRIMARY);
+	if (err < 0) {
+		kfree(plane);
+		return ERR_PTR(err);
+	}
+
+	return &plane->base;
+}
+
+static const u32 tegra_cursor_plane_formats[] = {
+	DRM_FORMAT_RGBA8888,
+};
+
+static int tegra_cursor_plane_update(struct drm_plane *plane,
+				     struct drm_crtc *crtc,
+				     struct drm_framebuffer *fb, int crtc_x,
+				     int crtc_y, unsigned int crtc_w,
+				     unsigned int crtc_h, uint32_t src_x,
+				     uint32_t src_y, uint32_t src_w,
+				     uint32_t src_h)
+{
+	struct tegra_bo *bo = tegra_fb_get_plane(fb, 0);
+	struct tegra_dc *dc = to_tegra_dc(crtc);
+	u32 value = CURSOR_CLIP_DISPLAY;
+
+	/* scaling not supported for cursor */
+	if ((src_w >> 16 != crtc_w) || (src_h >> 16 != crtc_h))
+		return -EINVAL;
+
+	/* only square cursors supported */
+	if (src_w != src_h)
+		return -EINVAL;
+
+	switch (crtc_w) {
+	case 32:
+		value |= CURSOR_SIZE_32x32;
+		break;
+
+	case 64:
+		value |= CURSOR_SIZE_64x64;
+		break;
+
+	case 128:
+		value |= CURSOR_SIZE_128x128;
+		break;
+
+	case 256:
+		value |= CURSOR_SIZE_256x256;
+		break;
+
+	default:
+		return -EINVAL;
+	}
+
+	value |= (bo->paddr >> 10) & 0x3fffff;
+	tegra_dc_writel(dc, value, DC_DISP_CURSOR_START_ADDR);
+
+#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
+	value = (bo->paddr >> 32) & 0x3;
+	tegra_dc_writel(dc, value, DC_DISP_CURSOR_START_ADDR_HI);
+#endif
+
+	/* enable cursor and set blend mode */
+	value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS);
+	value |= CURSOR_ENABLE;
+	tegra_dc_writel(dc, value, DC_DISP_DISP_WIN_OPTIONS);
+
+	value = tegra_dc_readl(dc, DC_DISP_BLEND_CURSOR_CONTROL);
+	value &= ~CURSOR_DST_BLEND_MASK;
+	value &= ~CURSOR_SRC_BLEND_MASK;
+	value |= CURSOR_MODE_NORMAL;
+	value |= CURSOR_DST_BLEND_NEG_K1_TIMES_SRC;
+	value |= CURSOR_SRC_BLEND_K1_TIMES_SRC;
+	value |= CURSOR_ALPHA;
+	tegra_dc_writel(dc, value, DC_DISP_BLEND_CURSOR_CONTROL);
+
+	/* position the cursor */
+	value = (crtc_y & 0x3fff) << 16 | (crtc_x & 0x3fff);
+	tegra_dc_writel(dc, value, DC_DISP_CURSOR_POSITION);
+
+	/* apply changes */
+	tegra_dc_cursor_commit(dc);
+	tegra_dc_commit(dc);
+
+	return 0;
+}
+
+static int tegra_cursor_plane_disable(struct drm_plane *plane)
+{
+	struct tegra_dc *dc = to_tegra_dc(plane->crtc);
+	u32 value;
+
+	if (!plane->crtc)
+		return 0;
+
+	value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS);
+	value &= ~CURSOR_ENABLE;
+	tegra_dc_writel(dc, value, DC_DISP_DISP_WIN_OPTIONS);
+
+	tegra_dc_cursor_commit(dc);
+	tegra_dc_commit(dc);
+
+	return 0;
+}
+
+static const struct drm_plane_funcs tegra_cursor_plane_funcs = {
+	.update_plane = tegra_cursor_plane_update,
+	.disable_plane = tegra_cursor_plane_disable,
+	.destroy = tegra_plane_destroy,
+};
+
+static struct drm_plane *tegra_dc_cursor_plane_create(struct drm_device *drm,
+						      struct tegra_dc *dc)
+{
+	struct tegra_plane *plane;
+	unsigned int num_formats;
+	const u32 *formats;
+	int err;
+
+	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
+
+	num_formats = ARRAY_SIZE(tegra_cursor_plane_formats);
+	formats = tegra_cursor_plane_formats;
+
+	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
+				       &tegra_cursor_plane_funcs, formats,
+				       num_formats, DRM_PLANE_TYPE_CURSOR);
+	if (err < 0) {
+		kfree(plane);
+		return ERR_PTR(err);
+	}
+
+	return &plane->base;
+}
+
+static int tegra_overlay_plane_update(struct drm_plane *plane,
+				      struct drm_crtc *crtc,
+				      struct drm_framebuffer *fb, int crtc_x,
+				      int crtc_y, unsigned int crtc_w,
+				      unsigned int crtc_h, uint32_t src_x,
+				      uint32_t src_y, uint32_t src_w,
+				      uint32_t src_h)
 {
 	struct tegra_plane *p = to_tegra_plane(plane);
 	struct tegra_dc *dc = to_tegra_dc(crtc);
@@ -382,43 +626,19 @@ static int tegra_plane_update(struct drm_plane *plane, struct drm_crtc *crtc,
 	return tegra_dc_setup_window(dc, p->index, &window);
 }
 
-static int tegra_plane_disable(struct drm_plane *plane)
-{
-	struct tegra_dc *dc = to_tegra_dc(plane->crtc);
-	struct tegra_plane *p = to_tegra_plane(plane);
-	unsigned long value;
-
-	if (!plane->crtc)
-		return 0;
-
-	value = WINDOW_A_SELECT << p->index;
-	tegra_dc_writel(dc, value, DC_CMD_DISPLAY_WINDOW_HEADER);
-
-	value = tegra_dc_readl(dc, DC_WIN_WIN_OPTIONS);
-	value &= ~WIN_ENABLE;
-	tegra_dc_writel(dc, value, DC_WIN_WIN_OPTIONS);
-
-	tegra_dc_window_commit(dc, p->index);
-
-	return 0;
-}
-
-static void tegra_plane_destroy(struct drm_plane *plane)
+static void tegra_overlay_plane_destroy(struct drm_plane *plane)
 {
-	struct tegra_plane *p = to_tegra_plane(plane);
-
-	tegra_plane_disable(plane);
-	drm_plane_cleanup(plane);
-	kfree(p);
+	tegra_window_plane_disable(plane);
+	tegra_plane_destroy(plane);
 }
 
-static const struct drm_plane_funcs tegra_plane_funcs = {
-	.update_plane = tegra_plane_update,
-	.disable_plane = tegra_plane_disable,
-	.destroy = tegra_plane_destroy,
+static const struct drm_plane_funcs tegra_overlay_plane_funcs = {
+	.update_plane = tegra_overlay_plane_update,
+	.disable_plane = tegra_window_plane_disable,
+	.destroy = tegra_overlay_plane_destroy,
 };
 
-static const uint32_t plane_formats[] = {
+static const uint32_t tegra_overlay_plane_formats[] = {
 	DRM_FORMAT_XBGR8888,
 	DRM_FORMAT_XRGB8888,
 	DRM_FORMAT_RGB565,
@@ -428,27 +648,44 @@ static const uint32_t plane_formats[] = {
 	DRM_FORMAT_YUV422,
 };
 
-static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
+static struct drm_plane *tegra_dc_overlay_plane_create(struct drm_device *drm,
+						       struct tegra_dc *dc,
+						       unsigned int index)
 {
-	unsigned int i;
-	int err = 0;
+	struct tegra_plane *plane;
+	unsigned int num_formats;
+	const u32 *formats;
+	int err;
 
-	for (i = 0; i < 2; i++) {
-		struct tegra_plane *plane;
+	plane = kzalloc(sizeof(*plane), GFP_KERNEL);
+	if (!plane)
+		return ERR_PTR(-ENOMEM);
 
-		plane = kzalloc(sizeof(*plane), GFP_KERNEL);
-		if (!plane)
-			return -ENOMEM;
+	plane->index = index;
 
-		plane->index = 1 + i;
+	num_formats = ARRAY_SIZE(tegra_overlay_plane_formats);
+	formats = tegra_overlay_plane_formats;
 
-		err = drm_plane_init(drm, &plane->base, 1 << dc->pipe,
-				     &tegra_plane_funcs, plane_formats,
-				     ARRAY_SIZE(plane_formats), false);
-		if (err < 0) {
-			kfree(plane);
-			return err;
-		}
+	err = drm_universal_plane_init(drm, &plane->base, 1 << dc->pipe,
+				       &tegra_overlay_plane_funcs, formats,
+				       num_formats, DRM_PLANE_TYPE_OVERLAY);
+	if (err < 0) {
+		kfree(plane);
+		return ERR_PTR(err);
+	}
+
+	return &plane->base;
+}
+
+static int tegra_dc_add_planes(struct drm_device *drm, struct tegra_dc *dc)
+{
+	struct drm_plane *plane;
+	unsigned int i;
+
+	for (i = 0; i < 2; i++) {
+		plane = tegra_dc_overlay_plane_create(drm, dc, 1 + i);
+		if (IS_ERR(plane))
+			return PTR_ERR(plane);
 	}
 
 	return 0;
@@ -568,103 +805,6 @@ void tegra_dc_disable_vblank(struct tegra_dc *dc)
 	spin_unlock_irqrestore(&dc->lock, flags);
 }
 
-static int tegra_dc_cursor_set2(struct drm_crtc *crtc, struct drm_file *file,
-				uint32_t handle, uint32_t width,
-				uint32_t height, int32_t hot_x, int32_t hot_y)
-{
-	unsigned long value = CURSOR_CLIP_DISPLAY;
-	struct tegra_dc *dc = to_tegra_dc(crtc);
-	struct drm_gem_object *gem;
-	struct tegra_bo *bo = NULL;
-
-	if (!dc->soc->supports_cursor)
-		return -ENXIO;
-
-	if (width != height)
-		return -EINVAL;
-
-	switch (width) {
-	case 32:
-		value |= CURSOR_SIZE_32x32;
-		break;
-
-	case 64:
-		value |= CURSOR_SIZE_64x64;
-		break;
-
-	case 128:
-		value |= CURSOR_SIZE_128x128;
-
-	case 256:
-		value |= CURSOR_SIZE_256x256;
-		break;
-
-	default:
-		return -EINVAL;
-	}
-
-	if (handle) {
-		gem = drm_gem_object_lookup(crtc->dev, file, handle);
-		if (!gem)
-			return -ENOENT;
-
-		bo = to_tegra_bo(gem);
-	}
-
-	if (bo) {
-		unsigned long addr = (bo->paddr & 0xfffffc00) >> 10;
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		unsigned long high = (bo->paddr & 0xfffffffc) >> 32;
-#endif
-
-		tegra_dc_writel(dc, value | addr, DC_DISP_CURSOR_START_ADDR);
-
-#ifdef CONFIG_ARCH_DMA_ADDR_T_64BIT
-		tegra_dc_writel(dc, high, DC_DISP_CURSOR_START_ADDR_HI);
-#endif
-
-		value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS);
-		value |= CURSOR_ENABLE;
-		tegra_dc_writel(dc, value, DC_DISP_DISP_WIN_OPTIONS);
-
-		value = tegra_dc_readl(dc, DC_DISP_BLEND_CURSOR_CONTROL);
-		value &= ~CURSOR_DST_BLEND_MASK;
-		value &= ~CURSOR_SRC_BLEND_MASK;
-		value |= CURSOR_MODE_NORMAL;
-		value |= CURSOR_DST_BLEND_NEG_K1_TIMES_SRC;
-		value |= CURSOR_SRC_BLEND_K1_TIMES_SRC;
-		value |= CURSOR_ALPHA;
-		tegra_dc_writel(dc, value, DC_DISP_BLEND_CURSOR_CONTROL);
-	} else {
-		value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS);
-		value &= ~CURSOR_ENABLE;
-		tegra_dc_writel(dc, value, DC_DISP_DISP_WIN_OPTIONS);
-	}
-
-	tegra_dc_cursor_commit(dc);
-	tegra_dc_commit(dc);
-
-	return 0;
-}
-
-static int tegra_dc_cursor_move(struct drm_crtc *crtc, int x, int y)
-{
-	struct tegra_dc *dc = to_tegra_dc(crtc);
-	unsigned long value;
-
-	if (!dc->soc->supports_cursor)
-		return -ENXIO;
-
-	value = ((y & 0x3fff) << 16) | (x & 0x3fff);
-	tegra_dc_writel(dc, value, DC_DISP_CURSOR_POSITION);
-
-	tegra_dc_cursor_commit(dc);
-	/* XXX: only required on generations earlier than Tegra124? */
-	tegra_dc_commit(dc);
-
-	return 0;
-}
-
 static void tegra_dc_finish_page_flip(struct tegra_dc *dc)
 {
 	struct drm_device *drm = dc->base.dev;
@@ -741,8 +881,6 @@ static void tegra_dc_destroy(struct drm_crtc *crtc)
 }
 
 static const struct drm_crtc_funcs tegra_crtc_funcs = {
-	.cursor_set2 = tegra_dc_cursor_set2,
-	.cursor_move = tegra_dc_cursor_move,
 	.page_flip = tegra_dc_page_flip,
 	.set_config = drm_crtc_helper_set_config,
 	.destroy = tegra_dc_destroy,
@@ -750,12 +888,13 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
 
 static void tegra_crtc_disable(struct drm_crtc *crtc)
 {
+	struct tegra_dc *dc = to_tegra_dc(crtc);
 	struct drm_device *drm = crtc->dev;
 	struct drm_plane *plane;
 
 	drm_for_each_legacy_plane(plane, &drm->mode_config.plane_list) {
 		if (plane->crtc == crtc) {
-			tegra_plane_disable(plane);
+			tegra_window_plane_disable(plane);
 			plane->crtc = NULL;
 
 			if (plane->fb) {
@@ -766,6 +905,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
 	}
 
 	drm_crtc_vblank_off(crtc);
+	tegra_dc_commit(dc);
 }
 
 static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
@@ -1290,6 +1430,8 @@ static int tegra_dc_init(struct host1x_client *client)
 	struct drm_device *drm = dev_get_drvdata(client->parent);
 	struct tegra_dc *dc = host1x_client_to_dc(client);
 	struct tegra_drm *tegra = drm->dev_private;
+	struct drm_plane *primary = NULL;
+	struct drm_plane *cursor = NULL;
 	int err;
 
 	if (tegra->domain) {
@@ -1303,7 +1445,25 @@ static int tegra_dc_init(struct host1x_client *client)
 		dc->domain = tegra->domain;
 	}
 
-	drm_crtc_init(drm, &dc->base, &tegra_crtc_funcs);
+	primary = tegra_dc_primary_plane_create(drm, dc);
+	if (IS_ERR(primary)) {
+		err = PTR_ERR(primary);
+		goto cleanup;
+	}
+
+	if (dc->soc->supports_cursor) {
+		cursor = tegra_dc_cursor_plane_create(drm, dc);
+		if (IS_ERR(cursor)) {
+			err = PTR_ERR(cursor);
+			goto cleanup;
+		}
+	}
+
+	err = drm_crtc_init_with_planes(drm, &dc->base, primary, cursor,
+					&tegra_crtc_funcs);
+	if (err < 0)
+		goto cleanup;
+
 	drm_mode_crtc_set_gamma_size(&dc->base, 256);
 	drm_crtc_helper_add(&dc->base, &tegra_crtc_helper_funcs);
 
@@ -1317,12 +1477,12 @@ static int tegra_dc_init(struct host1x_client *client)
 	err = tegra_dc_rgb_init(drm, dc);
 	if (err < 0 && err != -ENODEV) {
 		dev_err(dc->dev, "failed to initialize RGB output: %d\n", err);
-		return err;
+		goto cleanup;
 	}
 
 	err = tegra_dc_add_planes(drm, dc);
 	if (err < 0)
-		return err;
+		goto cleanup;
 
 	if (IS_ENABLED(CONFIG_DEBUG_FS)) {
 		err = tegra_dc_debugfs_init(dc, drm->primary);
@@ -1335,10 +1495,24 @@ static int tegra_dc_init(struct host1x_client *client)
 	if (err < 0) {
 		dev_err(dc->dev, "failed to request IRQ#%u: %d\n", dc->irq,
 			err);
-		return err;
+		goto cleanup;
 	}
 
 	return 0;
+
+cleanup:
+	if (cursor)
+		drm_plane_cleanup(cursor);
+
+	if (primary)
+		drm_plane_cleanup(primary);
+
+	if (tegra->domain) {
+		iommu_detach_device(tegra->domain, dc->dev);
+		dc->domain = NULL;
+	}
+
+	return err;
 }
 
 static int tegra_dc_exit(struct host1x_client *client)
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 15/17] drm/tegra: Fix potential bug on driver unload

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

The HDMI hotplug signal may toggle after the DRM driver has been
unloaded. Make sure not to call into DRM if that's the case.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/output.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
index 6b393cfbb5e7..def74914dd72 100644
--- a/drivers/gpu/drm/tegra/output.c
+++ b/drivers/gpu/drm/tegra/output.c
@@ -181,7 +181,8 @@ static irqreturn_t hpd_irq(int irq, void *data)
 {
 	struct tegra_output *output = data;
 
-	drm_helper_hpd_irq_event(output->connector.dev);
+	if (output->connector.dev)
+		drm_helper_hpd_irq_event(output->connector.dev);
 
 	return IRQ_HANDLED;
 }
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 16/17] drm/tegra: gem: dumb: pitch and size are outputs

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB
IOCTL, only the width, height, bpp and flags parameters are inputs. The
caller is not guaranteed to zero out or set handle, pitch and size, so
the driver must not treat these values as possible inputs.

Fixes a bug where running the Weston compositor on Tegra DRM would cause
an attempt to allocate a 3 GiB framebuffer to be allocated.

Fixes: de2ba664c30f ("gpu: host1x: drm: Add memory manager and fb")
Cc: stable@vger.kernel.org
Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/gem.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
index 8b1095d05c58..8348783f7d64 100644
--- a/drivers/gpu/drm/tegra/gem.c
+++ b/drivers/gpu/drm/tegra/gem.c
@@ -399,16 +399,12 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
 int tegra_bo_dumb_create(struct drm_file *file, struct drm_device *drm,
 			 struct drm_mode_create_dumb *args)
 {
-	int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
+	unsigned int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
 	struct tegra_drm *tegra = drm->dev_private;
 	struct tegra_bo *bo;
 
-	min_pitch = round_up(min_pitch, tegra->pitch_align);
-	if (args->pitch < min_pitch)
-		args->pitch = min_pitch;
-
-	if (args->size < args->pitch * args->height)
-		args->size = args->pitch * args->height;
+	args->pitch = round_up(min_pitch, tegra->pitch_align);
+	args->size = args->pitch * args->height;
 
 	bo = tegra_bo_create_with_handle(file, drm, args->size, 0,
 					 &args->handle);
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

[PATCH 17/17] drm/tegra: fb: Do not destroy framebuffer

[Thierry Reding]

From: Thierry Reding <treding@nvidia.com>

Drop a reference instead of directly calling the framebuffer .destroy()
callback at fbdev free time. This is necessary to make sure the object
isn't destroyed if anyone else still has a reference.

Signed-off-by: Thierry Reding <treding@nvidia.com>
---
 drivers/gpu/drm/tegra/fb.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
index c5fa3c4b2ed5..17a29971a7ee 100644
--- a/drivers/gpu/drm/tegra/fb.c
+++ b/drivers/gpu/drm/tegra/fb.c
@@ -355,7 +355,7 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
 
 	if (fbdev->fb) {
 		drm_framebuffer_unregister_private(&fbdev->fb->base);
-		tegra_fb_destroy(&fbdev->fb->base);
+		drm_framebuffer_unreference(&fbdev->fb->base);
 	}
 
 	drm_fb_helper_fini(&fbdev->base);
-- 
2.1.2

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/17] drm/tegra: gem: dumb: pitch and size are outputs

[Daniel Vetter]

On Mon, Nov 03, 2014 at 10:27:47AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB
> IOCTL, only the width, height, bpp and flags parameters are inputs. The
> caller is not guaranteed to zero out or set handle, pitch and size, so
> the driver must not treat these values as possible inputs.
> 
> Fixes a bug where running the Weston compositor on Tegra DRM would cause
> an attempt to allocate a 3 GiB framebuffer to be allocated.
> 
> Fixes: de2ba664c30f ("gpu: host1x: drm: Add memory manager and fb")
> Cc: stable@vger.kernel.org
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Shouldn't we also clear these fields in the drm core ioctl code? This
is indeed surprising (yay for lacking input validation!), doing this
mistake in each driver won't scale ...
-Daniel

> ---
>  drivers/gpu/drm/tegra/gem.c | 10 +++-------
>  1 file changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/gpu/drm/tegra/gem.c b/drivers/gpu/drm/tegra/gem.c
> index 8b1095d05c58..8348783f7d64 100644
> --- a/drivers/gpu/drm/tegra/gem.c
> +++ b/drivers/gpu/drm/tegra/gem.c
> @@ -399,16 +399,12 @@ void tegra_bo_free_object(struct drm_gem_object *gem)
>  int tegra_bo_dumb_create(struct drm_file *file, struct drm_device *drm,
>  			 struct drm_mode_create_dumb *args)
>  {
> -	int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
> +	unsigned int min_pitch = DIV_ROUND_UP(args->width * args->bpp, 8);
>  	struct tegra_drm *tegra = drm->dev_private;
>  	struct tegra_bo *bo;
>  
> -	min_pitch = round_up(min_pitch, tegra->pitch_align);
> -	if (args->pitch < min_pitch)
> -		args->pitch = min_pitch;
> -
> -	if (args->size < args->pitch * args->height)
> -		args->size = args->pitch * args->height;
> +	args->pitch = round_up(min_pitch, tegra->pitch_align);
> +	args->size = args->pitch * args->height;
>  
>  	bo = tegra_bo_create_with_handle(file, drm, args->size, 0,
>  					 &args->handle);
> -- 
> 2.1.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/17] drm/tegra: fb: Do not destroy framebuffer

[Daniel Vetter]

On Mon, Nov 03, 2014 at 10:27:48AM +0100, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> Drop a reference instead of directly calling the framebuffer .destroy()
> callback at fbdev free time. This is necessary to make sure the object
> isn't destroyed if anyone else still has a reference.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/fb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> index c5fa3c4b2ed5..17a29971a7ee 100644
> --- a/drivers/gpu/drm/tegra/fb.c
> +++ b/drivers/gpu/drm/tegra/fb.c
> @@ -355,7 +355,7 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
>  
>  	if (fbdev->fb) {
>  		drm_framebuffer_unregister_private(&fbdev->fb->base);
> -		tegra_fb_destroy(&fbdev->fb->base);
> +		drm_framebuffer_unreference(&fbdev->fb->base);

Yeah this is better since you have a free-standing fb pointer. I think
most kms drivers copied this stuff from i915, which just embedded the
framebuffer. And then calling unref obviously is a bad idea since the
kfree will blow up.

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
>  	}
>  
>  	drm_fb_helper_fini(&fbdev->base);
> -- 
> 2.1.2
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 16/17] drm/tegra: gem: dumb: pitch and size are outputs

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1290 bytes --]

On Mon, Nov 03, 2014 at 10:51:42AM +0100, Daniel Vetter wrote:
> On Mon, Nov 03, 2014 at 10:27:47AM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > When creating a dumb buffer object using the DRM_IOCTL_MODE_CREATE_DUMB
> > IOCTL, only the width, height, bpp and flags parameters are inputs. The
> > caller is not guaranteed to zero out or set handle, pitch and size, so
> > the driver must not treat these values as possible inputs.
> > 
> > Fixes a bug where running the Weston compositor on Tegra DRM would cause
> > an attempt to allocate a 3 GiB framebuffer to be allocated.
> > 
> > Fixes: de2ba664c30f ("gpu: host1x: drm: Add memory manager and fb")
> > Cc: stable@vger.kernel.org
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> 
> Shouldn't we also clear these fields in the drm core ioctl code? This
> is indeed surprising (yay for lacking input validation!), doing this
> mistake in each driver won't scale ...

They are clearly documented as being outputs in the drm_mode_create_dumb
struct (include/uapi/drm/drm_mode.h), so this was really just me being
stupid a couple of year ago.

But yes, validating the input in the core sounds like a good idea to
avoid this in other drivers in the future.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/17] drm/tegra: dc: Add powergate support

[Sean Paul]

On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Both display controllers are in their own power partition. Currently the
> driver relies on the assumption that these partitions are on (which is
> the hardware default). However some bootloaders may disable them, so the
> driver must make sure to turn them back on to avoid hangs.
>

A bug in our firmware caused the host1x block to be in a bad state on
boot such that we had to pulse the reset control in kernel probe to
recover. I'm not sure how paranoid you want to be here, but something
to keep in mind.

Sean


> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dc.c  | 45 ++++++++++++++++++++++++++++++++++++++++++---
>  drivers/gpu/drm/tegra/drm.h |  1 +
>  2 files changed, 43 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 6553fd238685..4a015232e2e8 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -11,6 +11,8 @@
>  #include <linux/debugfs.h>
>  #include <linux/reset.h>
>
> +#include <soc/tegra/pmc.h>
> +
>  #include "dc.h"
>  #include "drm.h"
>  #include "gem.h"
> @@ -20,6 +22,7 @@ struct tegra_dc_soc_info {
>         bool supports_cursor;
>         bool supports_block_linear;
>         unsigned int pitch_align;
> +       bool has_powergate;
>  };
>
>  struct tegra_plane {
> @@ -1357,6 +1360,7 @@ static const struct tegra_dc_soc_info tegra20_dc_soc_info = {
>         .supports_cursor = false,
>         .supports_block_linear = false,
>         .pitch_align = 8,
> +       .has_powergate = false,
>  };
>
>  static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
> @@ -1364,6 +1368,7 @@ static const struct tegra_dc_soc_info tegra30_dc_soc_info = {
>         .supports_cursor = false,
>         .supports_block_linear = false,
>         .pitch_align = 8,
> +       .has_powergate = false,
>  };
>
>  static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
> @@ -1371,6 +1376,7 @@ static const struct tegra_dc_soc_info tegra114_dc_soc_info = {
>         .supports_cursor = false,
>         .supports_block_linear = false,
>         .pitch_align = 64,
> +       .has_powergate = true,
>  };
>
>  static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
> @@ -1378,6 +1384,7 @@ static const struct tegra_dc_soc_info tegra124_dc_soc_info = {
>         .supports_cursor = true,
>         .supports_block_linear = true,
>         .pitch_align = 64,
> +       .has_powergate = true,
>  };
>
>  static const struct of_device_id tegra_dc_of_match[] = {
> @@ -1385,6 +1392,9 @@ static const struct of_device_id tegra_dc_of_match[] = {
>                 .compatible = "nvidia,tegra124-dc",
>                 .data = &tegra124_dc_soc_info,
>         }, {
> +               .compatible = "nvidia,tegra114-dc",
> +               .data = &tegra114_dc_soc_info,
> +       }, {
>                 .compatible = "nvidia,tegra30-dc",
>                 .data = &tegra30_dc_soc_info,
>         }, {
> @@ -1467,9 +1477,34 @@ static int tegra_dc_probe(struct platform_device *pdev)
>                 return PTR_ERR(dc->rst);
>         }
>
> -       err = clk_prepare_enable(dc->clk);
> -       if (err < 0)
> -               return err;
> +       if (dc->soc->has_powergate) {
> +               if (dc->pipe == 0)
> +                       dc->powergate = TEGRA_POWERGATE_DIS;
> +               else
> +                       dc->powergate = TEGRA_POWERGATE_DISB;
> +
> +               err = tegra_powergate_sequence_power_up(dc->powergate, dc->clk,
> +                                                       dc->rst);
> +               if (err < 0) {
> +                       dev_err(&pdev->dev, "failed to power partition: %d\n",
> +                               err);
> +                       return err;
> +               }
> +       } else {
> +               err = clk_prepare_enable(dc->clk);
> +               if (err < 0) {
> +                       dev_err(&pdev->dev, "failed to enable clock: %d\n",
> +                               err);
> +                       return err;
> +               }
> +
> +               err = reset_control_deassert(dc->rst);
> +               if (err < 0) {
> +                       dev_err(&pdev->dev, "failed to deassert reset: %d\n",
> +                               err);
> +                       return err;
> +               }
> +       }
>
>         regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         dc->regs = devm_ioremap_resource(&pdev->dev, regs);
> @@ -1523,6 +1558,10 @@ static int tegra_dc_remove(struct platform_device *pdev)
>         }
>
>         reset_control_assert(dc->rst);
> +
> +       if (dc->soc->has_powergate)
> +               tegra_powergate_power_off(dc->powergate);
> +
>         clk_disable_unprepare(dc->clk);
>
>         return 0;
> diff --git a/drivers/gpu/drm/tegra/drm.h b/drivers/gpu/drm/tegra/drm.h
> index e89c70fa82d5..b994c017971d 100644
> --- a/drivers/gpu/drm/tegra/drm.h
> +++ b/drivers/gpu/drm/tegra/drm.h
> @@ -101,6 +101,7 @@ struct tegra_dc {
>         spinlock_t lock;
>
>         struct drm_crtc base;
> +       int powergate;
>         int pipe;
>
>         struct clk *clk;
> --
> 2.1.2
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Sean Paul]

On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> The output is already enabled in .dpms(), doing it in .mode_set() too
> can cause noticeable flicker.
>

I think this should be coupled with "drm/tegra: DPMS off/on in encoder
prepare/commit" that I sent earlier this week. Without it, the driver
can get into a state where connector status is on, but the output is
disabled.

Sean


> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/output.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index 0c67d7eebc94..6b393cfbb5e7 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -167,12 +167,6 @@ static void tegra_encoder_mode_set(struct drm_encoder *encoder,
>                                    struct drm_display_mode *mode,
>                                    struct drm_display_mode *adjusted)
>  {
> -       struct tegra_output *output = encoder_to_output(encoder);
> -       int err;
> -
> -       err = tegra_output_enable(output);
> -       if (err < 0)
> -               dev_err(encoder->dev->dev, "tegra_output_enable(): %d\n", err);
>  }
>
>  static const struct drm_encoder_helper_funcs encoder_helper_funcs = {
> --
> 2.1.2
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/17] drm/tegra: dsi: Make FIFO depths host parameters

[Sean Paul]

On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Rather than hardcoding them as macros, make the host and video FIFO
> depths parameters so that they can be more easily adjusted if a new
> generation of the Tegra SoC changes them.
>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dsi.c | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
> index f7874458926a..584b771d8b2f 100644
> --- a/drivers/gpu/drm/tegra/dsi.c
> +++ b/drivers/gpu/drm/tegra/dsi.c
> @@ -26,9 +26,6 @@
>  #include "dsi.h"
>  #include "mipi-phy.h"
>
> -#define DSI_VIDEO_FIFO_DEPTH (1920 / 4)
> -#define DSI_HOST_FIFO_DEPTH 64
> -
>  struct tegra_dsi {
>         struct host1x_client client;
>         struct tegra_output output;
> @@ -54,6 +51,9 @@ struct tegra_dsi {
>
>         struct regulator *vdd;
>         bool enabled;
> +
> +       unsigned int video_fifo_depth;
> +       unsigned int host_fifo_depth;
>  };
>
>  static inline struct tegra_dsi *
> @@ -467,7 +467,7 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
>                 DSI_CONTROL_SOURCE(dc->pipe);
>         tegra_dsi_writel(dsi, value, DSI_CONTROL);
>
> -       tegra_dsi_writel(dsi, DSI_VIDEO_FIFO_DEPTH, DSI_MAX_THRESHOLD);
> +       tegra_dsi_writel(dsi, dsi->video_fifo_depth, DSI_MAX_THRESHOLD);
>
>         value = DSI_HOST_CONTROL_HS | DSI_HOST_CONTROL_CS |
>                 DSI_HOST_CONTROL_ECC;
> @@ -843,6 +843,8 @@ static int tegra_dsi_probe(struct platform_device *pdev)
>                 return -ENOMEM;
>
>         dsi->output.dev = dsi->dev = &pdev->dev;
> +       dsi->video_fifo_depth = 1920;

This is not functionally equivalent to what was previously being set
(1920/4). Perhaps calling that out in the commit message might be
appropriate?

Sean

> +       dsi->host_fifo_depth = 64;
>
>         err = tegra_output_probe(&dsi->output);
>         if (err < 0)
> --
> 2.1.2
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 04/17] drm/tegra: dsi: Add ganged mode support

[Sean Paul]

On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> Implement ganged mode support for the Tegra DSI driver. The DSI host
> controller to gang up with is specified via a phandle in the device tree
> and the resolved DSI host controller used for the programming of the
> ganged-mode registers.
>

There's a lot in here that is not specifically ganging-support, such
as adding the transfer callback and command mode, as well as pulling
out functionality into helper functions. It might make things a little
clearer to split this up into a few patches. I'll leave that up to
you.

At any rate, aside from the tiny nit I picked below (which you can
feel free to ignore),

Reviewed-by: Sean Paul <seanpaul@chromium.org>



> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
> Changes in v2:
> - keep track of the number of bytes transferred to/from peripheral
> - use newly introduced mipi_dsi_create_packet()
> - extract FIFO write into separate function
>
>  .../bindings/gpu/nvidia,tegra20-host1x.txt         |   2 +
>  drivers/gpu/drm/tegra/dsi.c                        | 792 ++++++++++++++++++---
>  drivers/gpu/drm/tegra/dsi.h                        |  14 +-
>  3 files changed, 691 insertions(+), 117 deletions(-)
>
> diff --git a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> index b48f4ef31d93..4c32ef0b7db8 100644
> --- a/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> +++ b/Documentation/devicetree/bindings/gpu/nvidia,tegra20-host1x.txt
> @@ -191,6 +191,8 @@ of the following host1x client modules:
>    - nvidia,hpd-gpio: specifies a GPIO used for hotplug detection
>    - nvidia,edid: supplies a binary EDID blob
>    - nvidia,panel: phandle of a display panel
> +  - nvidia,ganged-mode: contains a phandle to a second DSI controller to gang
> +    up with in order to support up to 8 data lanes
>
>  - sor: serial output resource
>
> diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
> index 584b771d8b2f..8940360ccc9c 100644
> --- a/drivers/gpu/drm/tegra/dsi.c
> +++ b/drivers/gpu/drm/tegra/dsi.c
> @@ -11,6 +11,7 @@
>  #include <linux/host1x.h>
>  #include <linux/module.h>
>  #include <linux/of.h>
> +#include <linux/of_platform.h>
>  #include <linux/platform_device.h>
>  #include <linux/reset.h>
>
> @@ -54,6 +55,10 @@ struct tegra_dsi {
>
>         unsigned int video_fifo_depth;
>         unsigned int host_fifo_depth;
> +
> +       /* for ganged-mode support */
> +       struct tegra_dsi *master;
> +       struct tegra_dsi *slave;
>  };
>
>  static inline struct tegra_dsi *
> @@ -318,6 +323,21 @@ static const u32 pkt_seq_video_non_burst_sync_events[NUM_PKT_SEQ] = {
>         [11] = PKT_ID0(MIPI_DSI_BLANKING_PACKET) | PKT_LEN0(4),
>  };
>
> +static const u32 pkt_seq_command_mode[NUM_PKT_SEQ] = {
> +       [ 0] = 0,
> +       [ 1] = 0,
> +       [ 2] = 0,
> +       [ 3] = 0,
> +       [ 4] = 0,
> +       [ 5] = 0,
> +       [ 6] = PKT_ID0(MIPI_DSI_DCS_LONG_WRITE) | PKT_LEN0(3) | PKT_LP,
> +       [ 7] = 0,
> +       [ 8] = 0,
> +       [ 9] = 0,
> +       [10] = PKT_ID0(MIPI_DSI_DCS_LONG_WRITE) | PKT_LEN0(5) | PKT_LP,
> +       [11] = 0,
> +};
> +
>  static int tegra_dsi_set_phy_timing(struct tegra_dsi *dsi)
>  {
>         struct mipi_dphy_timing timing;
> @@ -329,7 +349,7 @@ static int tegra_dsi_set_phy_timing(struct tegra_dsi *dsi)
>         if (rate < 0)
>                 return rate;
>
> -       period = DIV_ROUND_CLOSEST(1000000000UL, rate * 2);
> +       period = DIV_ROUND_CLOSEST(NSEC_PER_SEC, rate * 2);
>
>         err = mipi_dphy_timing_get_default(&timing, period);
>         if (err < 0)
> @@ -426,26 +446,59 @@ static int tegra_dsi_get_format(enum mipi_dsi_pixel_format format,
>         return 0;
>  }
>
> -static int tegra_output_dsi_enable(struct tegra_output *output)
> +static void tegra_dsi_ganged_enable(struct tegra_dsi *dsi, unsigned int start,
> +                                   unsigned int size)
> +{
> +       u32 value;
> +
> +       tegra_dsi_writel(dsi, start, DSI_GANGED_MODE_START);
> +       tegra_dsi_writel(dsi, size << 16 | size, DSI_GANGED_MODE_SIZE);

You might want to add "size = size & 0xFFFF;" before performing this write.

> +
> +       value = DSI_GANGED_MODE_CONTROL_ENABLE;
> +       tegra_dsi_writel(dsi, value, DSI_GANGED_MODE_CONTROL);
> +}
> +
> +static void tegra_dsi_enable(struct tegra_dsi *dsi)
> +{
> +       u32 value;
> +
> +       value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
> +       value |= DSI_POWER_CONTROL_ENABLE;
> +       tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
> +
> +       if (dsi->slave)
> +               tegra_dsi_enable(dsi->slave);
> +}
> +
> +static unsigned int tegra_dsi_get_lanes(struct tegra_dsi *dsi)
> +{
> +       if (dsi->master)
> +               return dsi->master->lanes + dsi->lanes;
> +
> +       if (dsi->slave)
> +               return dsi->lanes + dsi->slave->lanes;
> +
> +       return dsi->lanes;
> +}
> +
> +static int tegra_dsi_configure(struct tegra_dsi *dsi, unsigned int pipe,
> +                              const struct drm_display_mode *mode)
>  {
> -       struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
> -       struct drm_display_mode *mode = &dc->base.mode;
>         unsigned int hact, hsw, hbp, hfp, i, mul, div;
> -       struct tegra_dsi *dsi = to_dsi(output);
>         enum tegra_dsi_format format;
> -       unsigned long value;
>         const u32 *pkt_seq;
> +       u32 value;
>         int err;
>
> -       if (dsi->enabled)
> -               return 0;
> -
>         if (dsi->flags & MIPI_DSI_MODE_VIDEO_SYNC_PULSE) {
>                 DRM_DEBUG_KMS("Non-burst video mode with sync pulses\n");
>                 pkt_seq = pkt_seq_video_non_burst_sync_pulses;
> -       } else {
> +       } else if (dsi->flags & MIPI_DSI_MODE_VIDEO) {
>                 DRM_DEBUG_KMS("Non-burst video mode with sync events\n");
>                 pkt_seq = pkt_seq_video_non_burst_sync_events;
> +       } else {
> +               DRM_DEBUG_KMS("Command mode\n");
> +               pkt_seq = pkt_seq_command_mode;
>         }
>
>         err = tegra_dsi_get_muldiv(dsi->format, &mul, &div);
> @@ -456,28 +509,29 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
>         if (err < 0)
>                 return err;
>
> -       err = clk_enable(dsi->clk);
> -       if (err < 0)
> -               return err;
> -
> -       reset_control_deassert(dsi->rst);
> -
>         value = DSI_CONTROL_CHANNEL(0) | DSI_CONTROL_FORMAT(format) |
>                 DSI_CONTROL_LANES(dsi->lanes - 1) |
> -               DSI_CONTROL_SOURCE(dc->pipe);
> +               DSI_CONTROL_SOURCE(pipe);
>         tegra_dsi_writel(dsi, value, DSI_CONTROL);
>
>         tegra_dsi_writel(dsi, dsi->video_fifo_depth, DSI_MAX_THRESHOLD);
>
> -       value = DSI_HOST_CONTROL_HS | DSI_HOST_CONTROL_CS |
> -               DSI_HOST_CONTROL_ECC;
> +       value = DSI_HOST_CONTROL_HS;
>         tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
>
>         value = tegra_dsi_readl(dsi, DSI_CONTROL);
> +
>         if (dsi->flags & MIPI_DSI_CLOCK_NON_CONTINUOUS)
>                 value |= DSI_CONTROL_HS_CLK_CTRL;
> +
>         value &= ~DSI_CONTROL_TX_TRIG(3);
> -       value &= ~DSI_CONTROL_DCS_ENABLE;
> +
> +       /* enable DCS commands for command mode */
> +       if (dsi->flags & MIPI_DSI_MODE_VIDEO)
> +               value &= ~DSI_CONTROL_DCS_ENABLE;
> +       else
> +               value |= DSI_CONTROL_DCS_ENABLE;
> +
>         value |= DSI_CONTROL_VIDEO_ENABLE;
>         value &= ~DSI_CONTROL_HOST_ENABLE;
>         tegra_dsi_writel(dsi, value, DSI_CONTROL);
> @@ -489,28 +543,106 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
>         for (i = 0; i < NUM_PKT_SEQ; i++)
>                 tegra_dsi_writel(dsi, pkt_seq[i], DSI_PKT_SEQ_0_LO + i);
>
> -       /* horizontal active pixels */
> -       hact = mode->hdisplay * mul / div;
> +       if (dsi->flags & MIPI_DSI_MODE_VIDEO) {
> +               /* horizontal active pixels */
> +               hact = mode->hdisplay * mul / div;
> +
> +               /* horizontal sync width */
> +               hsw = (mode->hsync_end - mode->hsync_start) * mul / div;
> +               hsw -= 10;
>
> -       /* horizontal sync width */
> -       hsw = (mode->hsync_end - mode->hsync_start) * mul / div;
> -       hsw -= 10;
> +               /* horizontal back porch */
> +               hbp = (mode->htotal - mode->hsync_end) * mul / div;
> +               hbp -= 14;
>
> -       /* horizontal back porch */
> -       hbp = (mode->htotal - mode->hsync_end) * mul / div;
> -       hbp -= 14;
> +               /* horizontal front porch */
> +               hfp = (mode->hsync_start - mode->hdisplay) * mul / div;
> +               hfp -= 8;
> +
> +               tegra_dsi_writel(dsi, hsw << 16 | 0, DSI_PKT_LEN_0_1);
> +               tegra_dsi_writel(dsi, hact << 16 | hbp, DSI_PKT_LEN_2_3);
> +               tegra_dsi_writel(dsi, hfp, DSI_PKT_LEN_4_5);
> +               tegra_dsi_writel(dsi, 0x0f0f << 16, DSI_PKT_LEN_6_7);
> +
> +               /* set SOL delay (for non-burst mode only) */
> +               tegra_dsi_writel(dsi, 8 * mul / div, DSI_SOL_DELAY);
> +
> +               /* TODO: implement ganged mode */
> +       } else {
> +               u16 bytes;
> +
> +               if (dsi->master || dsi->slave) {
> +                       /*
> +                        * For ganged mode, assume symmetric left-right mode.
> +                        */
> +                       bytes = 1 + (mode->hdisplay / 2) * mul / div;
> +               } else {
> +                       /* 1 byte (DCS command) + pixel data */
> +                       bytes = 1 + mode->hdisplay * mul / div;
> +               }
> +
> +               tegra_dsi_writel(dsi, 0, DSI_PKT_LEN_0_1);
> +               tegra_dsi_writel(dsi, bytes << 16, DSI_PKT_LEN_2_3);
> +               tegra_dsi_writel(dsi, bytes << 16, DSI_PKT_LEN_4_5);
> +               tegra_dsi_writel(dsi, 0, DSI_PKT_LEN_6_7);
> +
> +               value = MIPI_DCS_WRITE_MEMORY_START << 8 |
> +                       MIPI_DCS_WRITE_MEMORY_CONTINUE;
> +               tegra_dsi_writel(dsi, value, DSI_DCS_CMDS);
> +
> +               /* set SOL delay */
> +               if (dsi->master || dsi->slave) {
> +                       unsigned int lanes = tegra_dsi_get_lanes(dsi);
> +                       unsigned long delay, bclk, bclk_ganged;
> +
> +                       /* SOL to valid, valid to FIFO and FIFO write delay */
> +                       delay = 4 + 4 + 2;
> +                       delay = DIV_ROUND_UP(delay * mul, div * lanes);
> +                       /* FIFO read delay */
> +                       delay = delay + 6;
> +
> +                       bclk = DIV_ROUND_UP(mode->htotal * mul, div * lanes);
> +                       bclk_ganged = DIV_ROUND_UP(bclk * lanes / 2, lanes);
> +                       value = bclk - bclk_ganged + delay + 20;
> +               } else {
> +                       /* TODO: revisit for non-ganged mode */
> +                       value = 8 * mul / div;
> +               }
> +
> +               tegra_dsi_writel(dsi, value, DSI_SOL_DELAY);
> +       }
>
> -       /* horizontal front porch */
> -       hfp = (mode->hsync_start  - mode->hdisplay) * mul / div;
> -       hfp -= 8;
> +       if (dsi->slave) {
> +               err = tegra_dsi_configure(dsi->slave, pipe, mode);
> +               if (err < 0)
> +                       return err;
> +
> +               /*
> +                * TODO: Support modes other than symmetrical left-right
> +                * split.
> +                */
> +               tegra_dsi_ganged_enable(dsi, 0, mode->hdisplay / 2);
> +               tegra_dsi_ganged_enable(dsi->slave, mode->hdisplay / 2,
> +                                       mode->hdisplay / 2);
> +       }
> +
> +       return 0;
> +}
> +
> +static int tegra_output_dsi_enable(struct tegra_output *output)
> +{
> +       struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
> +       const struct drm_display_mode *mode = &dc->base.mode;
> +       struct tegra_dsi *dsi = to_dsi(output);
> +       u32 value;
> +       int err;
>
> -       tegra_dsi_writel(dsi, hsw << 16 | 0, DSI_PKT_LEN_0_1);
> -       tegra_dsi_writel(dsi, hact << 16 | hbp, DSI_PKT_LEN_2_3);
> -       tegra_dsi_writel(dsi, hfp, DSI_PKT_LEN_4_5);
> -       tegra_dsi_writel(dsi, 0x0f0f << 16, DSI_PKT_LEN_6_7);
> +       if (dsi->enabled)
> +               return 0;
>
> -       /* set SOL delay */
> -       tegra_dsi_writel(dsi, 8 * mul / div, DSI_SOL_DELAY);
> +       err = tegra_dsi_configure(dsi, dc->pipe, mode);
> +       if (err < 0)
> +               return err;
>
>         /* enable display controller */
>         value = tegra_dc_readl(dc, DC_DISP_DISP_WIN_OPTIONS);
> @@ -531,28 +663,79 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
>         tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
>
>         /* enable DSI controller */
> -       value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
> -       value |= DSI_POWER_CONTROL_ENABLE;
> -       tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
> +       tegra_dsi_enable(dsi);
>
>         dsi->enabled = true;
>
>         return 0;
>  }
>
> +static int tegra_dsi_wait_idle(struct tegra_dsi *dsi, unsigned long timeout)
> +{
> +       u32 value;
> +
> +       timeout = jiffies + msecs_to_jiffies(timeout);
> +
> +       while (time_before(jiffies, timeout)) {
> +               value = tegra_dsi_readl(dsi, DSI_STATUS);
> +               if (value & DSI_STATUS_IDLE)
> +                       return 0;
> +
> +               usleep_range(1000, 2000);
> +       }
> +
> +       return -ETIMEDOUT;
> +}
> +
> +static void tegra_dsi_video_disable(struct tegra_dsi *dsi)
> +{
> +       u32 value;
> +
> +       value = tegra_dsi_readl(dsi, DSI_CONTROL);
> +       value &= ~DSI_CONTROL_VIDEO_ENABLE;
> +       tegra_dsi_writel(dsi, value, DSI_CONTROL);
> +
> +       if (dsi->slave)
> +               tegra_dsi_video_disable(dsi->slave);
> +}
> +
> +static void tegra_dsi_ganged_disable(struct tegra_dsi *dsi)
> +{
> +       tegra_dsi_writel(dsi, 0, DSI_GANGED_MODE_START);
> +       tegra_dsi_writel(dsi, 0, DSI_GANGED_MODE_SIZE);
> +       tegra_dsi_writel(dsi, 0, DSI_GANGED_MODE_CONTROL);
> +}
> +
> +static void tegra_dsi_disable(struct tegra_dsi *dsi)
> +{
> +       u32 value;
> +
> +       if (dsi->slave) {
> +               tegra_dsi_ganged_disable(dsi->slave);
> +               tegra_dsi_ganged_disable(dsi);
> +       }
> +
> +       value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
> +       value &= ~DSI_POWER_CONTROL_ENABLE;
> +       tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
> +
> +       if (dsi->slave)
> +               tegra_dsi_disable(dsi->slave);
> +
> +       usleep_range(5000, 10000);
> +}
> +
>  static int tegra_output_dsi_disable(struct tegra_output *output)
>  {
>         struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
>         struct tegra_dsi *dsi = to_dsi(output);
>         unsigned long value;
> +       int err;
>
>         if (!dsi->enabled)
>                 return 0;
>
> -       /* disable DSI controller */
> -       value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
> -       value &= ~DSI_POWER_CONTROL_ENABLE;
> -       tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
> +       tegra_dsi_video_disable(dsi);
>
>         /*
>          * The following accesses registers of the display controller, so make
> @@ -576,39 +759,68 @@ static int tegra_output_dsi_disable(struct tegra_output *output)
>                 tegra_dc_writel(dc, GENERAL_ACT_REQ, DC_CMD_STATE_CONTROL);
>         }
>
> -       clk_disable(dsi->clk);
> +       err = tegra_dsi_wait_idle(dsi, 100);
> +       if (err < 0)
> +               dev_dbg(dsi->dev, "failed to idle DSI: %d\n", err);
> +
> +       tegra_dsi_disable(dsi);
>
>         dsi->enabled = false;
>
>         return 0;
>  }
>
> +static void tegra_dsi_set_timeout(struct tegra_dsi *dsi, unsigned long bclk,
> +                                 unsigned int vrefresh)
> +{
> +       unsigned int timeout;
> +       u32 value;
> +
> +       /* one frame high-speed transmission timeout */
> +       timeout = (bclk / vrefresh) / 512;
> +       value = DSI_TIMEOUT_LRX(0x2000) | DSI_TIMEOUT_HTX(timeout);
> +       tegra_dsi_writel(dsi, value, DSI_TIMEOUT_0);
> +
> +       /* 2 ms peripheral timeout for panel */
> +       timeout = 2 * bclk / 512 * 1000;
> +       value = DSI_TIMEOUT_PR(timeout) | DSI_TIMEOUT_TA(0x2000);
> +       tegra_dsi_writel(dsi, value, DSI_TIMEOUT_1);
> +
> +       value = DSI_TALLY_TA(0) | DSI_TALLY_LRX(0) | DSI_TALLY_HTX(0);
> +       tegra_dsi_writel(dsi, value, DSI_TO_TALLY);
> +
> +       if (dsi->slave)
> +               tegra_dsi_set_timeout(dsi->slave, bclk, vrefresh);
> +}
> +
>  static int tegra_output_dsi_setup_clock(struct tegra_output *output,
>                                         struct clk *clk, unsigned long pclk,
>                                         unsigned int *divp)
>  {
>         struct tegra_dc *dc = to_tegra_dc(output->encoder.crtc);
>         struct drm_display_mode *mode = &dc->base.mode;
> -       unsigned int timeout, mul, div, vrefresh;
>         struct tegra_dsi *dsi = to_dsi(output);
> -       unsigned long bclk, plld, value;
> +       unsigned int mul, div, vrefresh, lanes;
> +       unsigned long bclk, plld;
>         int err;
>
> +       lanes = tegra_dsi_get_lanes(dsi);
> +
>         err = tegra_dsi_get_muldiv(dsi->format, &mul, &div);
>         if (err < 0)
>                 return err;
>
> -       DRM_DEBUG_KMS("mul: %u, div: %u, lanes: %u\n", mul, div, dsi->lanes);
> +       DRM_DEBUG_KMS("mul: %u, div: %u, lanes: %u\n", mul, div, lanes);
>         vrefresh = drm_mode_vrefresh(mode);
>         DRM_DEBUG_KMS("vrefresh: %u\n", vrefresh);
>
>         /* compute byte clock */
> -       bclk = (pclk * mul) / (div * dsi->lanes);
> +       bclk = (pclk * mul) / (div * lanes);
>
>         /*
>          * Compute bit clock and round up to the next MHz.
>          */
> -       plld = DIV_ROUND_UP(bclk * 8, 1000000) * 1000000;
> +       plld = DIV_ROUND_UP(bclk * 8, USEC_PER_SEC) * USEC_PER_SEC;
>
>         /*
>          * We divide the frequency by two here, but we make up for that by
> @@ -640,25 +852,13 @@ static int tegra_output_dsi_setup_clock(struct tegra_output *output,
>          * not working properly otherwise. Perhaps the PLLs cannot generate
>          * frequencies sufficiently high.
>          */
> -       *divp = ((8 * mul) / (div * dsi->lanes)) - 2;
> +       *divp = ((8 * mul) / (div * lanes)) - 2;
>
>         /*
>          * XXX: Move the below somewhere else so that we don't need to have
>          * access to the vrefresh in this function?
>          */
> -
> -       /* one frame high-speed transmission timeout */
> -       timeout = (bclk / vrefresh) / 512;
> -       value = DSI_TIMEOUT_LRX(0x2000) | DSI_TIMEOUT_HTX(timeout);
> -       tegra_dsi_writel(dsi, value, DSI_TIMEOUT_0);
> -
> -       /* 2 ms peripheral timeout for panel */
> -       timeout = 2 * bclk / 512 * 1000;
> -       value = DSI_TIMEOUT_PR(timeout) | DSI_TIMEOUT_TA(0x2000);
> -       tegra_dsi_writel(dsi, value, DSI_TIMEOUT_1);
> -
> -       value = DSI_TALLY_TA(0) | DSI_TALLY_LRX(0) | DSI_TALLY_HTX(0);
> -       tegra_dsi_writel(dsi, value, DSI_TO_TALLY);
> +       tegra_dsi_set_timeout(dsi, bclk, vrefresh);
>
>         return 0;
>  }
> @@ -695,7 +895,7 @@ static int tegra_dsi_pad_enable(struct tegra_dsi *dsi)
>
>  static int tegra_dsi_pad_calibrate(struct tegra_dsi *dsi)
>  {
> -       unsigned long value;
> +       u32 value;
>
>         tegra_dsi_writel(dsi, 0, DSI_PAD_CONTROL_0);
>         tegra_dsi_writel(dsi, 0, DSI_PAD_CONTROL_1);
> @@ -720,14 +920,17 @@ static int tegra_dsi_init(struct host1x_client *client)
>         struct tegra_dsi *dsi = host1x_client_to_dsi(client);
>         int err;
>
> -       dsi->output.type = TEGRA_OUTPUT_DSI;
> -       dsi->output.dev = client->dev;
> -       dsi->output.ops = &dsi_ops;
> -
> -       err = tegra_output_init(drm, &dsi->output);
> -       if (err < 0) {
> -               dev_err(client->dev, "output setup failed: %d\n", err);
> -               return err;
> +       /* Gangsters must not register their own outputs. */
> +       if (!dsi->master) {
> +               dsi->output.type = TEGRA_OUTPUT_DSI;
> +               dsi->output.dev = client->dev;
> +               dsi->output.ops = &dsi_ops;
> +
> +               err = tegra_output_init(drm, &dsi->output);
> +               if (err < 0) {
> +                       dev_err(client->dev, "output setup failed: %d\n", err);
> +                       return err;
> +               }
>         }
>
>         if (IS_ENABLED(CONFIG_DEBUG_FS)) {
> @@ -736,12 +939,6 @@ static int tegra_dsi_init(struct host1x_client *client)
>                         dev_err(dsi->dev, "debugfs setup failed: %d\n", err);
>         }
>
> -       err = tegra_dsi_pad_calibrate(dsi);
> -       if (err < 0) {
> -               dev_err(dsi->dev, "MIPI calibration failed: %d\n", err);
> -               return err;
> -       }
> -
>         return 0;
>  }
>
> @@ -756,16 +953,20 @@ static int tegra_dsi_exit(struct host1x_client *client)
>                         dev_err(dsi->dev, "debugfs cleanup failed: %d\n", err);
>         }
>
> -       err = tegra_output_disable(&dsi->output);
> -       if (err < 0) {
> -               dev_err(client->dev, "output failed to disable: %d\n", err);
> -               return err;
> -       }
> -
> -       err = tegra_output_exit(&dsi->output);
> -       if (err < 0) {
> -               dev_err(client->dev, "output cleanup failed: %d\n", err);
> -               return err;
> +       if (!dsi->master) {
> +               err = tegra_output_disable(&dsi->output);
> +               if (err < 0) {
> +                       dev_err(client->dev, "output failed to disable: %d\n",
> +                               err);
> +                       return err;
> +               }
> +
> +               err = tegra_output_exit(&dsi->output);
> +               if (err < 0) {
> +                       dev_err(client->dev, "output cleanup failed: %d\n",
> +                               err);
> +                       return err;
> +               }
>         }
>
>         return 0;
> @@ -792,20 +993,324 @@ static int tegra_dsi_setup_clocks(struct tegra_dsi *dsi)
>         return 0;
>  }
>
> +static const char * const error_report[16] = {
> +       "SoT Error",
> +       "SoT Sync Error",
> +       "EoT Sync Error",
> +       "Escape Mode Entry Command Error",
> +       "Low-Power Transmit Sync Error",
> +       "Peripheral Timeout Error",
> +       "False Control Error",
> +       "Contention Detected",
> +       "ECC Error, single-bit",
> +       "ECC Error, multi-bit",
> +       "Checksum Error",
> +       "DSI Data Type Not Recognized",
> +       "DSI VC ID Invalid",
> +       "Invalid Transmission Length",
> +       "Reserved",
> +       "DSI Protocol Violation",
> +};
> +
> +static ssize_t tegra_dsi_read_response(struct tegra_dsi *dsi,
> +                                      const struct mipi_dsi_msg *msg,
> +                                      size_t count)
> +{
> +       u8 *rx = msg->rx_buf;
> +       unsigned int i, j, k;
> +       size_t size = 0;
> +       u16 errors;
> +       u32 value;
> +
> +       /* read and parse packet header */
> +       value = tegra_dsi_readl(dsi, DSI_RD_DATA);
> +
> +       switch (value & 0x3f) {
> +       case MIPI_DSI_RX_ACKNOWLEDGE_AND_ERROR_REPORT:
> +               errors = (value >> 8) & 0xffff;
> +               dev_dbg(dsi->dev, "Acknowledge and error report: %04x\n",
> +                       errors);
> +               for (i = 0; i < ARRAY_SIZE(error_report); i++)
> +                       if (errors & BIT(i))
> +                               dev_dbg(dsi->dev, "  %2u: %s\n", i,
> +                                       error_report[i]);
> +               break;
> +
> +       case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_1BYTE:
> +               rx[0] = (value >> 8) & 0xff;
> +               size = 1;
> +               break;
> +
> +       case MIPI_DSI_RX_DCS_SHORT_READ_RESPONSE_2BYTE:
> +               rx[0] = (value >>  8) & 0xff;
> +               rx[1] = (value >> 16) & 0xff;
> +               size = 2;
> +               break;
> +
> +       case MIPI_DSI_RX_DCS_LONG_READ_RESPONSE:
> +               size = ((value >> 8) & 0xff00) | ((value >> 8) & 0xff);
> +               break;
> +
> +       case MIPI_DSI_RX_GENERIC_LONG_READ_RESPONSE:
> +               size = ((value >> 8) & 0xff00) | ((value >> 8) & 0xff);
> +               break;
> +
> +       default:
> +               dev_err(dsi->dev, "unhandled response type: %02x\n",
> +                       value & 0x3f);
> +               return -EPROTO;
> +       }
> +
> +       size = min(size, msg->rx_len);
> +
> +       if (msg->rx_buf && size > 0) {
> +               for (i = 0, j = 0; i < count - 1; i++, j += 4) {
> +                       u8 *rx = msg->rx_buf + j;
> +
> +                       value = tegra_dsi_readl(dsi, DSI_RD_DATA);
> +
> +                       for (k = 0; k < 4 && (j + k) < msg->rx_len; k++)
> +                               rx[j + k] = (value >> (k << 3)) & 0xff;
> +               }
> +       }
> +
> +       return size;
> +}
> +
> +static int tegra_dsi_transmit(struct tegra_dsi *dsi, unsigned long timeout)
> +{
> +       tegra_dsi_writel(dsi, DSI_TRIGGER_HOST, DSI_TRIGGER);
> +
> +       timeout = jiffies + msecs_to_jiffies(timeout);
> +
> +       while (time_before(jiffies, timeout)) {
> +               u32 value = tegra_dsi_readl(dsi, DSI_TRIGGER);
> +               if ((value & DSI_TRIGGER_HOST) == 0)
> +                       return 0;
> +
> +               usleep_range(1000, 2000);
> +       }
> +
> +       DRM_DEBUG_KMS("timeout waiting for transmission to complete\n");
> +       return -ETIMEDOUT;
> +}
> +
> +static int tegra_dsi_wait_for_response(struct tegra_dsi *dsi,
> +                                      unsigned long timeout)
> +{
> +       timeout = jiffies + msecs_to_jiffies(250);
> +
> +       while (time_before(jiffies, timeout)) {
> +               u32 value = tegra_dsi_readl(dsi, DSI_STATUS);
> +               u8 count = value & 0x1f;
> +
> +               if (count > 0)
> +                       return count;
> +
> +               usleep_range(1000, 2000);
> +       }
> +
> +       DRM_DEBUG_KMS("peripheral returned no data\n");
> +       return -ETIMEDOUT;
> +}
> +
> +static void tegra_dsi_writesl(struct tegra_dsi *dsi, unsigned long offset,
> +                             const void *buffer, size_t size)
> +{
> +       const u8 *buf = buffer;
> +       size_t i, j;
> +       u32 value;
> +
> +       for (j = 0; j < size; j += 4) {
> +               value = 0;
> +
> +               for (i = 0; i < 4 && j + i < size; i++)
> +                       value |= buf[j + i] << (i << 3);
> +
> +               tegra_dsi_writel(dsi, value, DSI_WR_DATA);
> +       }
> +}
> +
> +static ssize_t tegra_dsi_host_transfer(struct mipi_dsi_host *host,
> +                                      const struct mipi_dsi_msg *msg)
> +{
> +       struct tegra_dsi *dsi = host_to_tegra(host);
> +       struct mipi_dsi_packet packet;
> +       const u8 *header;
> +       size_t count;
> +       ssize_t err;
> +       u32 value;
> +
> +       err = mipi_dsi_create_packet(&packet, msg);
> +       if (err < 0)
> +               return err;
> +
> +       header = packet.header;
> +
> +       /* maximum FIFO depth is 1920 words */
> +       if (packet.size > dsi->video_fifo_depth * 4)
> +               return -ENOSPC;
> +
> +       /* reset underflow/overflow flags */
> +       value = tegra_dsi_readl(dsi, DSI_STATUS);
> +       if (value & (DSI_STATUS_UNDERFLOW | DSI_STATUS_OVERFLOW)) {
> +               value = DSI_HOST_CONTROL_FIFO_RESET;
> +               tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
> +               usleep_range(10, 20);
> +       }
> +
> +       value = tegra_dsi_readl(dsi, DSI_POWER_CONTROL);
> +       value |= DSI_POWER_CONTROL_ENABLE;
> +       tegra_dsi_writel(dsi, value, DSI_POWER_CONTROL);
> +
> +       usleep_range(5000, 10000);
> +
> +       value = DSI_HOST_CONTROL_CRC_RESET | DSI_HOST_CONTROL_TX_TRIG_HOST |
> +               DSI_HOST_CONTROL_CS | DSI_HOST_CONTROL_ECC;
> +
> +       if ((msg->flags & MIPI_DSI_MSG_USE_LPM) == 0)
> +               value |= DSI_HOST_CONTROL_HS;
> +
> +       /*
> +        * The host FIFO has a maximum of 64 words, so larger transmissions
> +        * need to use the video FIFO.
> +        */
> +       if (packet.size > dsi->host_fifo_depth * 4)
> +               value |= DSI_HOST_CONTROL_FIFO_SEL;
> +
> +       tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
> +
> +       /*
> +        * For reads and messages with explicitly requested ACK, generate a
> +        * BTA sequence after the transmission of the packet.
> +        */
> +       if ((msg->flags & MIPI_DSI_MSG_REQ_ACK) ||
> +           (msg->rx_buf && msg->rx_len > 0)) {
> +               value = tegra_dsi_readl(dsi, DSI_HOST_CONTROL);
> +               value |= DSI_HOST_CONTROL_PKT_BTA;
> +               tegra_dsi_writel(dsi, value, DSI_HOST_CONTROL);
> +       }
> +
> +       value = DSI_CONTROL_LANES(0) | DSI_CONTROL_HOST_ENABLE;
> +       tegra_dsi_writel(dsi, value, DSI_CONTROL);
> +
> +       /* write packet header, ECC is generated by hardware */
> +       value = header[2] << 16 | header[1] << 8 | header[0];
> +       tegra_dsi_writel(dsi, value, DSI_WR_DATA);
> +
> +       /* write payload (if any) */
> +       if (packet.payload_length > 0)
> +               tegra_dsi_writesl(dsi, DSI_WR_DATA, packet.payload,
> +                                 packet.payload_length);
> +
> +       err = tegra_dsi_transmit(dsi, 250);
> +       if (err < 0)
> +               return err;
> +
> +       if ((msg->flags & MIPI_DSI_MSG_REQ_ACK) ||
> +           (msg->rx_buf && msg->rx_len > 0)) {
> +               err = tegra_dsi_wait_for_response(dsi, 250);
> +               if (err < 0)
> +                       return err;
> +
> +               count = err;
> +
> +               value = tegra_dsi_readl(dsi, DSI_RD_DATA);
> +               switch (value) {
> +               case 0x84:
> +                       /*
> +                       dev_dbg(dsi->dev, "ACK\n");
> +                       */
> +                       break;
> +
> +               case 0x87:
> +                       /*
> +                       dev_dbg(dsi->dev, "ESCAPE\n");
> +                       */
> +                       break;
> +
> +               default:
> +                       dev_err(dsi->dev, "unknown status: %08x\n", value);
> +                       break;
> +               }
> +
> +               if (count > 1) {
> +                       err = tegra_dsi_read_response(dsi, msg, count);
> +                       if (err < 0)
> +                               dev_err(dsi->dev,
> +                                       "failed to parse response: %zd\n",
> +                                       err);
> +                       else {
> +                               /*
> +                                * For read commands, return the number of
> +                                * bytes returned by the peripheral.
> +                                */
> +                               count = err;
> +                       }
> +               }
> +       } else {
> +               /*
> +                * For write commands, we have transmitted the 4-byte header
> +                * plus the variable-length payload.
> +                */
> +               count = 4 + packet.payload_length;
> +       }
> +
> +       return count;
> +}
> +
> +static int tegra_dsi_ganged_setup(struct tegra_dsi *dsi)
> +{
> +       struct clk *parent;
> +       int err;
> +
> +       /* make sure both DSI controllers share the same PLL */
> +       parent = clk_get_parent(dsi->slave->clk);
> +       if (!parent)
> +               return -EINVAL;
> +
> +       err = clk_set_parent(parent, dsi->clk_parent);
> +       if (err < 0)
> +               return err;
> +
> +       return 0;
> +}
> +
>  static int tegra_dsi_host_attach(struct mipi_dsi_host *host,
>                                  struct mipi_dsi_device *device)
>  {
>         struct tegra_dsi *dsi = host_to_tegra(host);
> -       struct tegra_output *output = &dsi->output;
>
>         dsi->flags = device->mode_flags;
>         dsi->format = device->format;
>         dsi->lanes = device->lanes;
>
> -       output->panel = of_drm_find_panel(device->dev.of_node);
> -       if (output->panel) {
> -               if (output->connector.dev)
> +       if (dsi->slave) {
> +               int err;
> +
> +               dev_dbg(dsi->dev, "attaching dual-channel device %s\n",
> +                       dev_name(&device->dev));
> +
> +               err = tegra_dsi_ganged_setup(dsi);
> +               if (err < 0) {
> +                       dev_err(dsi->dev, "failed to set up ganged mode: %d\n",
> +                               err);
> +                       return err;
> +               }
> +       }
> +
> +       /*
> +        * Slaves don't have a panel associated with them, so they provide
> +        * merely the second channel.
> +        */
> +       if (!dsi->master) {
> +               struct tegra_output *output = &dsi->output;
> +
> +               output->panel = of_drm_find_panel(device->dev.of_node);
> +               if (output->panel && output->connector.dev) {
> +                       drm_panel_attach(output->panel, &output->connector);
>                         drm_helper_hpd_irq_event(output->connector.dev);
> +               }
>         }
>
>         return 0;
> @@ -818,10 +1323,10 @@ static int tegra_dsi_host_detach(struct mipi_dsi_host *host,
>         struct tegra_output *output = &dsi->output;
>
>         if (output->panel && &device->dev == output->panel->dev) {
> +               output->panel = NULL;
> +
>                 if (output->connector.dev)
>                         drm_helper_hpd_irq_event(output->connector.dev);
> -
> -               output->panel = NULL;
>         }
>
>         return 0;
> @@ -830,8 +1335,29 @@ static int tegra_dsi_host_detach(struct mipi_dsi_host *host,
>  static const struct mipi_dsi_host_ops tegra_dsi_host_ops = {
>         .attach = tegra_dsi_host_attach,
>         .detach = tegra_dsi_host_detach,
> +       .transfer = tegra_dsi_host_transfer,
>  };
>
> +static int tegra_dsi_ganged_probe(struct tegra_dsi *dsi)
> +{
> +       struct device_node *np;
> +
> +       np = of_parse_phandle(dsi->dev->of_node, "nvidia,ganged-mode", 0);
> +       if (np) {
> +               struct platform_device *gangster = of_find_device_by_node(np);
> +
> +               dsi->slave = platform_get_drvdata(gangster);
> +               of_node_put(np);
> +
> +               if (!dsi->slave)
> +                       return -EPROBE_DEFER;
> +
> +               dsi->slave->master = dsi;
> +       }
> +
> +       return 0;
> +}
> +
>  static int tegra_dsi_probe(struct platform_device *pdev)
>  {
>         struct tegra_dsi *dsi;
> @@ -846,10 +1372,16 @@ static int tegra_dsi_probe(struct platform_device *pdev)
>         dsi->video_fifo_depth = 1920;
>         dsi->host_fifo_depth = 64;
>
> +       err = tegra_dsi_ganged_probe(dsi);
> +       if (err < 0)
> +               return err;
> +
>         err = tegra_output_probe(&dsi->output);
>         if (err < 0)
>                 return err;
>
> +       dsi->output.connector.polled = DRM_CONNECTOR_POLL_HPD;
> +
>         /*
>          * Assume these values by default. When a DSI peripheral driver
>          * attaches to the DSI host, the parameters will be taken from
> @@ -863,68 +1395,83 @@ static int tegra_dsi_probe(struct platform_device *pdev)
>         if (IS_ERR(dsi->rst))
>                 return PTR_ERR(dsi->rst);
>
> +       err = reset_control_deassert(dsi->rst);
> +       if (err < 0) {
> +               dev_err(&pdev->dev, "failed to bring DSI out of reset: %d\n",
> +                       err);
> +               return err;
> +       }
> +
>         dsi->clk = devm_clk_get(&pdev->dev, NULL);
>         if (IS_ERR(dsi->clk)) {
>                 dev_err(&pdev->dev, "cannot get DSI clock\n");
> -               return PTR_ERR(dsi->clk);
> +               err = PTR_ERR(dsi->clk);
> +               goto reset;
>         }
>
>         err = clk_prepare_enable(dsi->clk);
>         if (err < 0) {
>                 dev_err(&pdev->dev, "cannot enable DSI clock\n");
> -               return err;
> +               goto reset;
>         }
>
>         dsi->clk_lp = devm_clk_get(&pdev->dev, "lp");
>         if (IS_ERR(dsi->clk_lp)) {
>                 dev_err(&pdev->dev, "cannot get low-power clock\n");
> -               return PTR_ERR(dsi->clk_lp);
> +               err = PTR_ERR(dsi->clk_lp);
> +               goto disable_clk;
>         }
>
>         err = clk_prepare_enable(dsi->clk_lp);
>         if (err < 0) {
>                 dev_err(&pdev->dev, "cannot enable low-power clock\n");
> -               return err;
> +               goto disable_clk;
>         }
>
>         dsi->clk_parent = devm_clk_get(&pdev->dev, "parent");
>         if (IS_ERR(dsi->clk_parent)) {
>                 dev_err(&pdev->dev, "cannot get parent clock\n");
> -               return PTR_ERR(dsi->clk_parent);
> -       }
> -
> -       err = clk_prepare_enable(dsi->clk_parent);
> -       if (err < 0) {
> -               dev_err(&pdev->dev, "cannot enable parent clock\n");
> -               return err;
> +               err = PTR_ERR(dsi->clk_parent);
> +               goto disable_clk_lp;
>         }
>
>         dsi->vdd = devm_regulator_get(&pdev->dev, "avdd-dsi-csi");
>         if (IS_ERR(dsi->vdd)) {
>                 dev_err(&pdev->dev, "cannot get VDD supply\n");
> -               return PTR_ERR(dsi->vdd);
> +               err = PTR_ERR(dsi->vdd);
> +               goto disable_clk_lp;
>         }
>
>         err = regulator_enable(dsi->vdd);
>         if (err < 0) {
>                 dev_err(&pdev->dev, "cannot enable VDD supply\n");
> -               return err;
> +               goto disable_clk_lp;
>         }
>
>         err = tegra_dsi_setup_clocks(dsi);
>         if (err < 0) {
>                 dev_err(&pdev->dev, "cannot setup clocks\n");
> -               return err;
> +               goto disable_vdd;
>         }
>
>         regs = platform_get_resource(pdev, IORESOURCE_MEM, 0);
>         dsi->regs = devm_ioremap_resource(&pdev->dev, regs);
> -       if (IS_ERR(dsi->regs))
> -               return PTR_ERR(dsi->regs);
> +       if (IS_ERR(dsi->regs)) {
> +               err = PTR_ERR(dsi->regs);
> +               goto disable_vdd;
> +       }
>
>         dsi->mipi = tegra_mipi_request(&pdev->dev);
> -       if (IS_ERR(dsi->mipi))
> -               return PTR_ERR(dsi->mipi);
> +       if (IS_ERR(dsi->mipi)) {
> +               err = PTR_ERR(dsi->mipi);
> +               goto disable_vdd;
> +       }
> +
> +       err = tegra_dsi_pad_calibrate(dsi);
> +       if (err < 0) {
> +               dev_err(dsi->dev, "MIPI calibration failed: %d\n", err);
> +               goto mipi_free;
> +       }
>
>         dsi->host.ops = &tegra_dsi_host_ops;
>         dsi->host.dev = &pdev->dev;
> @@ -932,7 +1479,7 @@ static int tegra_dsi_probe(struct platform_device *pdev)
>         err = mipi_dsi_host_register(&dsi->host);
>         if (err < 0) {
>                 dev_err(&pdev->dev, "failed to register DSI host: %d\n", err);
> -               return err;
> +               goto mipi_free;
>         }
>
>         INIT_LIST_HEAD(&dsi->client.list);
> @@ -943,12 +1490,26 @@ static int tegra_dsi_probe(struct platform_device *pdev)
>         if (err < 0) {
>                 dev_err(&pdev->dev, "failed to register host1x client: %d\n",
>                         err);
> -               return err;
> +               goto unregister;
>         }
>
>         platform_set_drvdata(pdev, dsi);
>
>         return 0;
> +
> +unregister:
> +       mipi_dsi_host_unregister(&dsi->host);
> +mipi_free:
> +       tegra_mipi_free(dsi->mipi);
> +disable_vdd:
> +       regulator_disable(dsi->vdd);
> +disable_clk_lp:
> +       clk_disable_unprepare(dsi->clk_lp);
> +disable_clk:
> +       clk_disable_unprepare(dsi->clk);
> +reset:
> +       reset_control_assert(dsi->rst);
> +       return err;
>  }
>
>  static int tegra_dsi_remove(struct platform_device *pdev)
> @@ -967,7 +1528,6 @@ static int tegra_dsi_remove(struct platform_device *pdev)
>         tegra_mipi_free(dsi->mipi);
>
>         regulator_disable(dsi->vdd);
> -       clk_disable_unprepare(dsi->clk_parent);
>         clk_disable_unprepare(dsi->clk_lp);
>         clk_disable_unprepare(dsi->clk);
>         reset_control_assert(dsi->rst);
> diff --git a/drivers/gpu/drm/tegra/dsi.h b/drivers/gpu/drm/tegra/dsi.h
> index 5ce610d08d77..bad1006a5150 100644
> --- a/drivers/gpu/drm/tegra/dsi.h
> +++ b/drivers/gpu/drm/tegra/dsi.h
> @@ -21,9 +21,16 @@
>  #define DSI_INT_STATUS                 0x0d
>  #define DSI_INT_MASK                   0x0e
>  #define DSI_HOST_CONTROL               0x0f
> +#define DSI_HOST_CONTROL_FIFO_RESET    (1 << 21)
> +#define DSI_HOST_CONTROL_CRC_RESET     (1 << 20)
> +#define DSI_HOST_CONTROL_TX_TRIG_SOL   (0 << 12)
> +#define DSI_HOST_CONTROL_TX_TRIG_FIFO  (1 << 12)
> +#define DSI_HOST_CONTROL_TX_TRIG_HOST  (2 << 12)
>  #define DSI_HOST_CONTROL_RAW           (1 << 6)
>  #define DSI_HOST_CONTROL_HS            (1 << 5)
> -#define DSI_HOST_CONTROL_BTA           (1 << 2)
> +#define DSI_HOST_CONTROL_FIFO_SEL      (1 << 4)
> +#define DSI_HOST_CONTROL_IMM_BTA       (1 << 3)
> +#define DSI_HOST_CONTROL_PKT_BTA       (1 << 2)
>  #define DSI_HOST_CONTROL_CS            (1 << 1)
>  #define DSI_HOST_CONTROL_ECC           (1 << 0)
>  #define DSI_CONTROL                    0x10
> @@ -39,9 +46,13 @@
>  #define DSI_SOL_DELAY                  0x11
>  #define DSI_MAX_THRESHOLD              0x12
>  #define DSI_TRIGGER                    0x13
> +#define DSI_TRIGGER_HOST               (1 << 1)
> +#define DSI_TRIGGER_VIDEO              (1 << 0)
>  #define DSI_TX_CRC                     0x14
>  #define DSI_STATUS                     0x15
>  #define DSI_STATUS_IDLE                        (1 << 10)
> +#define DSI_STATUS_UNDERFLOW           (1 <<  9)
> +#define DSI_STATUS_OVERFLOW            (1 <<  8)
>  #define DSI_INIT_SEQ_CONTROL           0x1a
>  #define DSI_INIT_SEQ_DATA_0            0x1b
>  #define DSI_INIT_SEQ_DATA_1            0x1c
> @@ -104,6 +115,7 @@
>  #define DSI_PAD_CONTROL_3              0x51
>  #define DSI_PAD_CONTROL_4              0x52
>  #define DSI_GANGED_MODE_CONTROL                0x53
> +#define DSI_GANGED_MODE_CONTROL_ENABLE (1 << 0)
>  #define DSI_GANGED_MODE_START          0x54
>  #define DSI_GANGED_MODE_SIZE           0x55
>  #define DSI_RAW_DATA_BYTE_COUNT                0x56
> --
> 2.1.2
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/17] drm/tegra: dc: Add missing call to drm_vblank_on()

[Sean Paul]

On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> When the CRTC is enabled, make sure the VBLANK machinery is enabled.
> Failure to do so will cause drm_vblank_get() to not enable the VBLANK on
> the CRTC and VBLANK-synchronized page-flips won't work.
>
> While at it, get rid of the legacy drm_vblank_pre_modeset() and
> drm_vblank_post_modeset() calls that are replaced by drm_vblank_on()
> and drm_vblank_off().
>
> Reported-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/dc.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> index 4a015232e2e8..4da366a4d78a 100644
> --- a/drivers/gpu/drm/tegra/dc.c
> +++ b/drivers/gpu/drm/tegra/dc.c
> @@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
>
>  static void tegra_crtc_disable(struct drm_crtc *crtc)
>  {
> -       struct tegra_dc *dc = to_tegra_dc(crtc);
>         struct drm_device *drm = crtc->dev;
>         struct drm_plane *plane;
>
> @@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
>                 }
>         }
>
> -       drm_vblank_off(drm, dc->pipe);
> +       drm_crtc_vblank_off(crtc);
>  }
>
>  static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
> @@ -844,8 +843,6 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
>         u32 value;
>         int err;
>
> -       drm_vblank_pre_modeset(crtc->dev, dc->pipe);
> -

Should you replace this with a call to drm_crtc_vblank_off in
prepare()? AFAICT, crtc_funcs->disable() isn't guaranteed to be called
before modeset, and it's unclear to me whether the vblank counter will
be reset in prepare.

Sean



>         err = tegra_crtc_setup_clk(crtc, mode);
>         if (err) {
>                 dev_err(dc->dev, "failed to setup clock for CRTC: %d\n", err);
> @@ -946,7 +943,7 @@ static void tegra_crtc_commit(struct drm_crtc *crtc)
>         value = GENERAL_ACT_REQ | WIN_A_ACT_REQ;
>         tegra_dc_writel(dc, value, DC_CMD_STATE_CONTROL);
>
> -       drm_vblank_post_modeset(crtc->dev, dc->pipe);
> +       drm_crtc_vblank_on(crtc);
>  }
>
>  static void tegra_crtc_load_lut(struct drm_crtc *crtc)
> --
> 2.1.2
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/17] drm/tegra: dc: Add missing call to drm_vblank_on()

[Daniel Vetter]

On Mon, Nov 03, 2014 at 01:45:56PM -0500, Sean Paul wrote:
> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > When the CRTC is enabled, make sure the VBLANK machinery is enabled.
> > Failure to do so will cause drm_vblank_get() to not enable the VBLANK on
> > the CRTC and VBLANK-synchronized page-flips won't work.
> >
> > While at it, get rid of the legacy drm_vblank_pre_modeset() and
> > drm_vblank_post_modeset() calls that are replaced by drm_vblank_on()
> > and drm_vblank_off().
> >
> > Reported-by: Alexandre Courbot <acourbot@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/dc.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index 4a015232e2e8..4da366a4d78a 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
> >
> >  static void tegra_crtc_disable(struct drm_crtc *crtc)
> >  {
> > -       struct tegra_dc *dc = to_tegra_dc(crtc);
> >         struct drm_device *drm = crtc->dev;
> >         struct drm_plane *plane;
> >
> > @@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
> >                 }
> >         }
> >
> > -       drm_vblank_off(drm, dc->pipe);
> > +       drm_crtc_vblank_off(crtc);
> >  }
> >
> >  static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
> > @@ -844,8 +843,6 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
> >         u32 value;
> >         int err;
> >
> > -       drm_vblank_pre_modeset(crtc->dev, dc->pipe);
> > -
> 
> Should you replace this with a call to drm_crtc_vblank_off in
> prepare()? AFAICT, crtc_funcs->disable() isn't guaranteed to be called
> before modeset, and it's unclear to me whether the vblank counter will
> be reset in prepare.

->disable is only called when fully disabling the crtc and should be
implemented in terms of ->prepare and additional release any crtc related
resources acquire. Examples include unpinning the framebuffer or releasing
shared resources like dplls.

Shutting down the vblank logic should definitely happen in the prepare
logic, which should be shared with the dpms off logic.

/me has looked a few too many times too closely at the crtc helpers ;-)

Aside if you'll read the atomic helpers that should be a lot clearer - the
corresponding code even explains what exact callback is called under which
conditions. And it's all in one place thanks to the less insane calling
sequence compared to crtc helpers. Hint, hint, ...
Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/17] drm/tegra: dc: Add missing call to drm_vblank_on()

[Alexandre Courbot]

On 11/03/2014 06:27 PM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
>
> When the CRTC is enabled, make sure the VBLANK machinery is enabled.
> Failure to do so will cause drm_vblank_get() to not enable the VBLANK on
> the CRTC and VBLANK-synchronized page-flips won't work.
>
> While at it, get rid of the legacy drm_vblank_pre_modeset() and
> drm_vblank_post_modeset() calls that are replaced by drm_vblank_on()
> and drm_vblank_off().
>
> Reported-by: Alexandre Courbot <acourbot@nvidia.com>
> Signed-off-by: Thierry Reding <treding@nvidia.com>

Tested-by: Alexandre Courbot <acourbot@nvidia.com>

I also think this patch should go for 3.18 since even kmscube won't work 
without it.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/17] drm/tegra: Fix potential bug on driver unload

[Andrzej Hajda]

Hi Thierry,

Just passing by.

On 11/03/2014 10:27 AM, Thierry Reding wrote:
> From: Thierry Reding <treding@nvidia.com>
> 
> The HDMI hotplug signal may toggle after the DRM driver has been
> unloaded. Make sure not to call into DRM if that's the case.
> 
> Signed-off-by: Thierry Reding <treding@nvidia.com>
> ---
>  drivers/gpu/drm/tegra/output.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> index 6b393cfbb5e7..def74914dd72 100644
> --- a/drivers/gpu/drm/tegra/output.c
> +++ b/drivers/gpu/drm/tegra/output.c
> @@ -181,7 +181,8 @@ static irqreturn_t hpd_irq(int irq, void *data)
>  {
>  	struct tegra_output *output = data;
>  
> -	drm_helper_hpd_irq_event(output->connector.dev);
> +	if (output->connector.dev)
> +		drm_helper_hpd_irq_event(output->connector.dev);
>  
>  	return IRQ_HANDLED;
>  }
> 

Since output->connector.dev is not synchronized between irq and other
code this patch do not solves the issue, it just decreases chances of
the disaster.

Regards
Andrzej
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 15/17] drm/tegra: Fix potential bug on driver unload

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1635 bytes --]

On Tue, Nov 04, 2014 at 11:59:46AM +0100, Andrzej Hajda wrote:
> Hi Thierry,
> 
> Just passing by.
> 
> On 11/03/2014 10:27 AM, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > The HDMI hotplug signal may toggle after the DRM driver has been
> > unloaded. Make sure not to call into DRM if that's the case.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/output.c | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/output.c b/drivers/gpu/drm/tegra/output.c
> > index 6b393cfbb5e7..def74914dd72 100644
> > --- a/drivers/gpu/drm/tegra/output.c
> > +++ b/drivers/gpu/drm/tegra/output.c
> > @@ -181,7 +181,8 @@ static irqreturn_t hpd_irq(int irq, void *data)
> >  {
> >  	struct tegra_output *output = data;
> >  
> > -	drm_helper_hpd_irq_event(output->connector.dev);
> > +	if (output->connector.dev)
> > +		drm_helper_hpd_irq_event(output->connector.dev);
> >  
> >  	return IRQ_HANDLED;
> >  }
> > 
> 
> Since output->connector.dev is not synchronized between irq and other
> code this patch do not solves the issue, it just decreases chances of
> the disaster.

You're right. I guess in addition to this we could call enable_irq()
when the connector is bound to the DRM device and disable_irq() when it
is unbound. Actually, that should even allow drm_helper_hpd_irq_event()
to be called unconditionally because .dev could not be NULL in that
case.

Upon closer inspection it seems like the majority of drivers would fall
prey to this particular race.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 01/17] drm/tegra: dc: Add powergate support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1015 bytes --]

On Mon, Nov 03, 2014 at 12:15:38PM -0500, Sean Paul wrote:
> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Both display controllers are in their own power partition. Currently the
> > driver relies on the assumption that these partitions are on (which is
> > the hardware default). However some bootloaders may disable them, so the
> > driver must make sure to turn them back on to avoid hangs.
> >
> 
> A bug in our firmware caused the host1x block to be in a bad state on
> boot such that we had to pulse the reset control in kernel probe to
> recover. I'm not sure how paranoid you want to be here, but something
> to keep in mind.

I think I'd rather not do that. The reason is simple: eventually what
I'd like to do is implement a way for firmware to hand over display to
the kernel so we can seamlessly transition graphics. If we pulse reset
for any of the blocks we can no longer do that.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 06/17] drm/tegra: dc: Add missing call to drm_vblank_on()

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2057 bytes --]

On Mon, Nov 03, 2014 at 01:45:56PM -0500, Sean Paul wrote:
> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > When the CRTC is enabled, make sure the VBLANK machinery is enabled.
> > Failure to do so will cause drm_vblank_get() to not enable the VBLANK on
> > the CRTC and VBLANK-synchronized page-flips won't work.
> >
> > While at it, get rid of the legacy drm_vblank_pre_modeset() and
> > drm_vblank_post_modeset() calls that are replaced by drm_vblank_on()
> > and drm_vblank_off().
> >
> > Reported-by: Alexandre Courbot <acourbot@nvidia.com>
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/dc.c | 7 ++-----
> >  1 file changed, 2 insertions(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/dc.c b/drivers/gpu/drm/tegra/dc.c
> > index 4a015232e2e8..4da366a4d78a 100644
> > --- a/drivers/gpu/drm/tegra/dc.c
> > +++ b/drivers/gpu/drm/tegra/dc.c
> > @@ -739,7 +739,6 @@ static const struct drm_crtc_funcs tegra_crtc_funcs = {
> >
> >  static void tegra_crtc_disable(struct drm_crtc *crtc)
> >  {
> > -       struct tegra_dc *dc = to_tegra_dc(crtc);
> >         struct drm_device *drm = crtc->dev;
> >         struct drm_plane *plane;
> >
> > @@ -755,7 +754,7 @@ static void tegra_crtc_disable(struct drm_crtc *crtc)
> >                 }
> >         }
> >
> > -       drm_vblank_off(drm, dc->pipe);
> > +       drm_crtc_vblank_off(crtc);
> >  }
> >
> >  static bool tegra_crtc_mode_fixup(struct drm_crtc *crtc,
> > @@ -844,8 +843,6 @@ static int tegra_crtc_mode_set(struct drm_crtc *crtc,
> >         u32 value;
> >         int err;
> >
> > -       drm_vblank_pre_modeset(crtc->dev, dc->pipe);
> > -
> 
> Should you replace this with a call to drm_crtc_vblank_off in
> prepare()? AFAICT, crtc_funcs->disable() isn't guaranteed to be called
> before modeset, and it's unclear to me whether the vblank counter will
> be reset in prepare.

Done. Thanks,

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 695 bytes --]

On Mon, Nov 03, 2014 at 12:18:30PM -0500, Sean Paul wrote:
> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > The output is already enabled in .dpms(), doing it in .mode_set() too
> > can cause noticeable flicker.
> >
> 
> I think this should be coupled with "drm/tegra: DPMS off/on in encoder
> prepare/commit" that I sent earlier this week. Without it, the driver
> can get into a state where connector status is on, but the output is
> disabled.

I'm not sure I exactly understand which problem that patch fixes, but
I'll give it some testing to see if it doesn't break anything.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Sean Paul]

On Tue, Nov 4, 2014 at 9:50 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> On Mon, Nov 03, 2014 at 12:18:30PM -0500, Sean Paul wrote:
>> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> > From: Thierry Reding <treding@nvidia.com>
>> >
>> > The output is already enabled in .dpms(), doing it in .mode_set() too
>> > can cause noticeable flicker.
>> >
>>
>> I think this should be coupled with "drm/tegra: DPMS off/on in encoder
>> prepare/commit" that I sent earlier this week. Without it, the driver
>> can get into a state where connector status is on, but the output is
>> disabled.
>
> I'm not sure I exactly understand which problem that patch fixes, but
> I'll give it some testing to see if it doesn't break anything.
>

I'll try to explain it more clearly.

The problem occurs when userspace does set_property(dpms_on), then modeset.

When the set_property ioctl to set dpms = on is called, the tegra
driver goes through drm_helper_connector_dpms(). At this point,
because the connector has not participated in a modeset,
connector->encoder == NULL. In drm_helper_connector_dpms(), we set
connector->dpms = DRM_MODE_DPMS_ON, and skip everything else because
!encoder && !crtc.

When modeset is called, we go through drm_crtc_helper_set_config().
This function will populate connector->encoder with the appropriate
encoder and call drm_crtc_helper_set_mode(), which goes through the
prepare/mode_set/commit calls. Finally, drm_crtc_helper_set_config()
iterates through the connectors involved in the modeset and calls
their dpms() func. In our case, as above, we go through
drm_helper_connector_dpms(), but it just exits early because mode ==
connector->dpms.

So we end up in a state where connector->dpms == DRM_MODE_DPMS_ON, and
the output, as well as any connected panel, has never been enabled.
The reason is that they're only enabled in encoder_funcs->dpms(),
which is never called in the above scenario.

I hope that makes more sense :-)

Sean


> Thierry
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2405 bytes --]

On Tue, Nov 04, 2014 at 10:11:22AM -0500, Sean Paul wrote:
> On Tue, Nov 4, 2014 at 9:50 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > On Mon, Nov 03, 2014 at 12:18:30PM -0500, Sean Paul wrote:
> >> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> >> > From: Thierry Reding <treding@nvidia.com>
> >> >
> >> > The output is already enabled in .dpms(), doing it in .mode_set() too
> >> > can cause noticeable flicker.
> >> >
> >>
> >> I think this should be coupled with "drm/tegra: DPMS off/on in encoder
> >> prepare/commit" that I sent earlier this week. Without it, the driver
> >> can get into a state where connector status is on, but the output is
> >> disabled.
> >
> > I'm not sure I exactly understand which problem that patch fixes, but
> > I'll give it some testing to see if it doesn't break anything.
> >
> 
> I'll try to explain it more clearly.
> 
> The problem occurs when userspace does set_property(dpms_on), then modeset.
> 
> When the set_property ioctl to set dpms = on is called, the tegra
> driver goes through drm_helper_connector_dpms(). At this point,
> because the connector has not participated in a modeset,
> connector->encoder == NULL. In drm_helper_connector_dpms(), we set
> connector->dpms = DRM_MODE_DPMS_ON, and skip everything else because
> !encoder && !crtc.
> 
> When modeset is called, we go through drm_crtc_helper_set_config().
> This function will populate connector->encoder with the appropriate
> encoder and call drm_crtc_helper_set_mode(), which goes through the
> prepare/mode_set/commit calls. Finally, drm_crtc_helper_set_config()
> iterates through the connectors involved in the modeset and calls
> their dpms() func. In our case, as above, we go through
> drm_helper_connector_dpms(), but it just exits early because mode ==
> connector->dpms.
> 
> So we end up in a state where connector->dpms == DRM_MODE_DPMS_ON, and
> the output, as well as any connected panel, has never been enabled.
> The reason is that they're only enabled in encoder_funcs->dpms(),
> which is never called in the above scenario.
> 
> I hope that makes more sense :-)

Yes, perfect sense. It's somewhat unfortunate that the helpers even
allow this to happen. It doesn't seem like it's only the Tegra driver
using it wrongly. Anyway, I've applied this to my tree.

Thanks,
Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 03/17] drm/tegra: dsi: Make FIFO depths host parameters

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 2172 bytes --]

On Mon, Nov 03, 2014 at 12:20:24PM -0500, Sean Paul wrote:
> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Rather than hardcoding them as macros, make the host and video FIFO
> > depths parameters so that they can be more easily adjusted if a new
> > generation of the Tegra SoC changes them.
> >
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/dsi.c | 10 ++++++----
> >  1 file changed, 6 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
> > index f7874458926a..584b771d8b2f 100644
> > --- a/drivers/gpu/drm/tegra/dsi.c
> > +++ b/drivers/gpu/drm/tegra/dsi.c
> > @@ -26,9 +26,6 @@
> >  #include "dsi.h"
> >  #include "mipi-phy.h"
> >
> > -#define DSI_VIDEO_FIFO_DEPTH (1920 / 4)
> > -#define DSI_HOST_FIFO_DEPTH 64
> > -
> >  struct tegra_dsi {
> >         struct host1x_client client;
> >         struct tegra_output output;
> > @@ -54,6 +51,9 @@ struct tegra_dsi {
> >
> >         struct regulator *vdd;
> >         bool enabled;
> > +
> > +       unsigned int video_fifo_depth;
> > +       unsigned int host_fifo_depth;
> >  };
> >
> >  static inline struct tegra_dsi *
> > @@ -467,7 +467,7 @@ static int tegra_output_dsi_enable(struct tegra_output *output)
> >                 DSI_CONTROL_SOURCE(dc->pipe);
> >         tegra_dsi_writel(dsi, value, DSI_CONTROL);
> >
> > -       tegra_dsi_writel(dsi, DSI_VIDEO_FIFO_DEPTH, DSI_MAX_THRESHOLD);
> > +       tegra_dsi_writel(dsi, dsi->video_fifo_depth, DSI_MAX_THRESHOLD);
> >
> >         value = DSI_HOST_CONTROL_HS | DSI_HOST_CONTROL_CS |
> >                 DSI_HOST_CONTROL_ECC;
> > @@ -843,6 +843,8 @@ static int tegra_dsi_probe(struct platform_device *pdev)
> >                 return -ENOMEM;
> >
> >         dsi->output.dev = dsi->dev = &pdev->dev;
> > +       dsi->video_fifo_depth = 1920;
> 
> This is not functionally equivalent to what was previously being set
> (1920/4). Perhaps calling that out in the commit message might be
> appropriate?

Done.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH v2 04/17] drm/tegra: dsi: Add ganged mode support

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1706 bytes --]

On Mon, Nov 03, 2014 at 01:30:38PM -0500, Sean Paul wrote:
> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > From: Thierry Reding <treding@nvidia.com>
> >
> > Implement ganged mode support for the Tegra DSI driver. The DSI host
> > controller to gang up with is specified via a phandle in the device tree
> > and the resolved DSI host controller used for the programming of the
> > ganged-mode registers.
> >
> 
> There's a lot in here that is not specifically ganging-support, such
> as adding the transfer callback and command mode, as well as pulling
> out functionality into helper functions. It might make things a little
> clearer to split this up into a few patches. I'll leave that up to
> you.

I think I tried to do that a while back, but things got really
complicated so I abandonned that effort. I'll give it another shot and
see what I can come up with.

> > diff --git a/drivers/gpu/drm/tegra/dsi.c b/drivers/gpu/drm/tegra/dsi.c
[...]
> > -static int tegra_output_dsi_enable(struct tegra_output *output)
> > +static void tegra_dsi_ganged_enable(struct tegra_dsi *dsi, unsigned int start,
> > +                                   unsigned int size)
> > +{
> > +       u32 value;
> > +
> > +       tegra_dsi_writel(dsi, start, DSI_GANGED_MODE_START);
> > +       tegra_dsi_writel(dsi, size << 16 | size, DSI_GANGED_MODE_SIZE);
> 
> You might want to add "size = size & 0xFFFF;" before performing this write.

Actually according to register documentation the mask even needs to be
0x1fff, so that's a good idea. Alternatively I guess we could check the
size earlier to make sure that we can actually support it.

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 17/17] drm/tegra: fb: Do not destroy framebuffer

[Thierry Reding]

[-- Attachment #1.1: Type: text/plain, Size: 1804 bytes --]

On Mon, Nov 03, 2014 at 10:53:42AM +0100, Daniel Vetter wrote:
> On Mon, Nov 03, 2014 at 10:27:48AM +0100, Thierry Reding wrote:
> > From: Thierry Reding <treding@nvidia.com>
> > 
> > Drop a reference instead of directly calling the framebuffer .destroy()
> > callback at fbdev free time. This is necessary to make sure the object
> > isn't destroyed if anyone else still has a reference.
> > 
> > Signed-off-by: Thierry Reding <treding@nvidia.com>
> > ---
> >  drivers/gpu/drm/tegra/fb.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/gpu/drm/tegra/fb.c b/drivers/gpu/drm/tegra/fb.c
> > index c5fa3c4b2ed5..17a29971a7ee 100644
> > --- a/drivers/gpu/drm/tegra/fb.c
> > +++ b/drivers/gpu/drm/tegra/fb.c
> > @@ -355,7 +355,7 @@ static void tegra_fbdev_free(struct tegra_fbdev *fbdev)
> >  
> >  	if (fbdev->fb) {
> >  		drm_framebuffer_unregister_private(&fbdev->fb->base);
> > -		tegra_fb_destroy(&fbdev->fb->base);
> > +		drm_framebuffer_unreference(&fbdev->fb->base);
> 
> Yeah this is better since you have a free-standing fb pointer. I think
> most kms drivers copied this stuff from i915, which just embedded the
> framebuffer. And then calling unref obviously is a bad idea since the
> kfree will blow up.
> 
> Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>

I just noticed that drm_framebuffer_remove() is actually more
appropriate here. Just unreferencing will trigger the WARN_ON in
drm_mode_config_cleanup(). Removing the framebuffer also has the
advantage that any users are forcibly disabled, which gets rid of
some annoying IOMMU faults.

Does the Reviewed-by still apply if I do this on top:

-		drm_framebuffer_unreference(&fbdev->fb->base);
+		drm_framebuffer_remove(&fbdev->fb->base);

?

Thierry

[-- Attachment #1.2: Type: application/pgp-signature, Size: 819 bytes --]

[-- Attachment #2: Type: text/plain, Size: 159 bytes --]

_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: [PATCH 02/17] drm/tegra: Do not enable output on .mode_set()

[Daniel Vetter]

On Tue, Nov 04, 2014 at 04:26:12PM +0100, Thierry Reding wrote:
> On Tue, Nov 04, 2014 at 10:11:22AM -0500, Sean Paul wrote:
> > On Tue, Nov 4, 2014 at 9:50 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > > On Mon, Nov 03, 2014 at 12:18:30PM -0500, Sean Paul wrote:
> > >> On Mon, Nov 3, 2014 at 4:27 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
> > >> > From: Thierry Reding <treding@nvidia.com>
> > >> >
> > >> > The output is already enabled in .dpms(), doing it in .mode_set() too
> > >> > can cause noticeable flicker.
> > >> >
> > >>
> > >> I think this should be coupled with "drm/tegra: DPMS off/on in encoder
> > >> prepare/commit" that I sent earlier this week. Without it, the driver
> > >> can get into a state where connector status is on, but the output is
> > >> disabled.
> > >
> > > I'm not sure I exactly understand which problem that patch fixes, but
> > > I'll give it some testing to see if it doesn't break anything.
> > >
> > 
> > I'll try to explain it more clearly.
> > 
> > The problem occurs when userspace does set_property(dpms_on), then modeset.
> > 
> > When the set_property ioctl to set dpms = on is called, the tegra
> > driver goes through drm_helper_connector_dpms(). At this point,
> > because the connector has not participated in a modeset,
> > connector->encoder == NULL. In drm_helper_connector_dpms(), we set
> > connector->dpms = DRM_MODE_DPMS_ON, and skip everything else because
> > !encoder && !crtc.
> > 
> > When modeset is called, we go through drm_crtc_helper_set_config().
> > This function will populate connector->encoder with the appropriate
> > encoder and call drm_crtc_helper_set_mode(), which goes through the
> > prepare/mode_set/commit calls. Finally, drm_crtc_helper_set_config()
> > iterates through the connectors involved in the modeset and calls
> > their dpms() func. In our case, as above, we go through
> > drm_helper_connector_dpms(), but it just exits early because mode ==
> > connector->dpms.
> > 
> > So we end up in a state where connector->dpms == DRM_MODE_DPMS_ON, and
> > the output, as well as any connected panel, has never been enabled.
> > The reason is that they're only enabled in encoder_funcs->dpms(),
> > which is never called in the above scenario.
> > 
> > I hope that makes more sense :-)
> 
> Yes, perfect sense. It's somewhat unfortunate that the helpers even
> allow this to happen. It doesn't seem like it's only the Tegra driver
> using it wrongly. Anyway, I've applied this to my tree.

The helper support for dpms is completely tacked onto the side and yeah
you get to do all the hard work of keeping track of state in the driver.

For modesets atomic should improve on the situation, except that I didn't
write any helpers to implement DPMS. Oops, totally missed that entry
point.

So maybe we should write new helpers to implement connector->dpms on top
of atomic which are actually sane?
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel