[RFC PATCH 00/10] drm/i915/bxt: add dsi transcoders

[RFC PATCH 00/10] drm/i915/bxt: add dsi transcoders

[Jani Nikula]

Here's my first attempt at adding bxt dsi transcoder support. Patch 8 is
the real deal, everything else is almost trivial. It was painful to try
ensure we really aren't indexing any regular transcoder registers with
the dsi transcoder enumerations, but I think I got them all. It's fairly
straightforward to check everything I changed; much harder to try to
figure out what I missed...

This is all untested. Paraphrasing Bob Marley, no hardware, no cry.

BR,
Jani.


Jani Nikula (10):
  drm/i915: add for_each_port_masked macro
  drm/i915: make transcoder_name return a string
  drm/i915/dsi: refactor dsi get hw state readout
  drm/i915/bxt: fix dsi hw state pipe readout
  drm/i915: split get/set pipe timings to pipe and transcoder parts
  drm/i915: split set pipeconf to pipe and transcoder parts
  drm/i915: abstract get config for cpu transcoder
  drm/i915/bxt: add dsi transcoders
  drm/i915/dsi: use the BIT macro for clarity
  drm/i915/bxt: allow dsi on any pipe

 drivers/gpu/drm/i915/i915_drv.h            |  31 ++++-
 drivers/gpu/drm/i915/i915_reg.h            |   1 +
 drivers/gpu/drm/i915/intel_ddi.c           |   6 +
 drivers/gpu/drm/i915/intel_display.c       | 210 ++++++++++++++++++++++-------
 drivers/gpu/drm/i915/intel_drv.h           |   3 +-
 drivers/gpu/drm/i915/intel_dsi.c           |  71 ++++++----
 drivers/gpu/drm/i915/intel_dsi.h           |   4 +-
 drivers/gpu/drm/i915/intel_fifo_underrun.c |   6 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c    |   6 +
 9 files changed, 256 insertions(+), 82 deletions(-)

-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 01/10] drm/i915: add for_each_port_masked macro

[Jani Nikula]

Same as for_each_dsi_port, but for general use. Leave the
for_each_dsi_port version around as an "alias" for now to not cause too
much churn. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h  | 4 ++++
 drivers/gpu/drm/i915/intel_dsi.h | 4 +---
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 80b14f1ba302..8ef3c88d0ed2 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -274,6 +274,10 @@ struct i915_hotplug {
 	     (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];	\
 	     (__s)++)
 
+#define for_each_port_masked(__port, __ports_mask) \
+	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
+		for_each_if ((__ports_mask) & (1 << (__port)))
+
 #define for_each_crtc(dev, crtc) \
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
index 92f39227b361..0b5e0b8ac08d 100644
--- a/drivers/gpu/drm/i915/intel_dsi.h
+++ b/drivers/gpu/drm/i915/intel_dsi.h
@@ -117,9 +117,7 @@ static inline struct intel_dsi_host *to_intel_dsi_host(struct mipi_dsi_host *h)
 	return container_of(h, struct intel_dsi_host, base);
 }
 
-#define for_each_dsi_port(__port, __ports_mask) \
-	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
-		for_each_if ((__ports_mask) & (1 << (__port)))
+#define for_each_dsi_port(__port, __ports_mask) for_each_port_masked(__port, __ports_mask)
 
 static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
 {
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 02/10] drm/i915: make transcoder_name return a string

[Jani Nikula]

Nicer for eDP (actually "eDP" instead of "D"), and makes future
expansion for DSI transcoders easier.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h            | 17 ++++++++++++++++-
 drivers/gpu/drm/i915/intel_display.c       |  4 ++--
 drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
 3 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 8ef3c88d0ed2..5e4a42d996d8 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -125,7 +125,22 @@ enum transcoder {
 	TRANSCODER_EDP,
 	I915_MAX_TRANSCODERS
 };
-#define transcoder_name(t) ((t) + 'A')
+
+static inline const char *transcoder_name(enum transcoder transcoder)
+{
+	switch (transcoder) {
+	case TRANSCODER_A:
+		return "A";
+	case TRANSCODER_B:
+		return "B";
+	case TRANSCODER_C:
+		return "C";
+	case TRANSCODER_EDP:
+		return "eDP";
+	default:
+		return "<invalid>";
+	}
+}
 
 /*
  * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index ce55f0b683c6..7977a818326d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12096,7 +12096,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
 	DRM_DEBUG_KMS("[CRTC:%d]%s config %p for pipe %c\n", crtc->base.base.id,
 		      context, pipe_config, pipe_name(crtc->pipe));
 
-	DRM_DEBUG_KMS("cpu_transcoder: %c\n", transcoder_name(pipe_config->cpu_transcoder));
+	DRM_DEBUG_KMS("cpu_transcoder: %s\n", transcoder_name(pipe_config->cpu_transcoder));
 	DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
 		      pipe_config->pipe_bpp, pipe_config->dither);
 	DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
@@ -16227,7 +16227,7 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
 	}
 
 	for (i = 0; i < error->num_transcoders; i++) {
-		err_printf(m, "CPU transcoder: %c\n",
+		err_printf(m, "CPU transcoder: %s\n",
 			   transcoder_name(error->transcoder[i].cpu_transcoder));
 		err_printf(m, "  Power: %s\n",
 			   onoff(error->transcoder[i].power_domain_on));
diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
index bda526660e20..19e50fdf9a91 100644
--- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
+++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
@@ -212,7 +212,7 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc)
 	I915_WRITE(SERR_INT, SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
 	POSTING_READ(SERR_INT);
 
-	DRM_ERROR("pch fifo underrun on pch transcoder %c\n",
+	DRM_ERROR("pch fifo underrun on pch transcoder %s\n",
 		  transcoder_name(pch_transcoder));
 }
 
@@ -235,7 +235,7 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
 
 		if (old && I915_READ(SERR_INT) &
 		    SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) {
-			DRM_ERROR("uncleared pch fifo underrun on pch transcoder %c\n",
+			DRM_ERROR("uncleared pch fifo underrun on pch transcoder %s\n",
 				  transcoder_name(pch_transcoder));
 		}
 	}
@@ -386,7 +386,7 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
 {
 	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
 						  false))
-		DRM_ERROR("PCH transcoder %c FIFO underrun\n",
+		DRM_ERROR("PCH transcoder %s FIFO underrun\n",
 			  transcoder_name(pch_transcoder));
 }
 
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 03/10] drm/i915/dsi: refactor dsi get hw state readout

[Jani Nikula]

Make the code easier to read and update. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 37 +++++++++++++++++++------------------
 1 file changed, 19 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 01b8e9f4c272..6574b9ac3698 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -667,7 +667,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	enum intel_display_power_domain power_domain;
 	enum port port;
-	bool ret;
+	bool active = false;
 
 	DRM_DEBUG_KMS("\n");
 
@@ -675,38 +675,39 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 
-	ret = false;
-
 	/* XXX: this only works for one DSI output */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
 			BXT_MIPI_PORT_CTRL(port) : MIPI_PORT_CTRL(port);
-		u32 dpi_enabled, func;
-
-		func = I915_READ(MIPI_DSI_FUNC_PRG(port));
-		dpi_enabled = I915_READ(ctrl_reg) & DPI_ENABLE;
+		bool enabled = I915_READ(ctrl_reg) & DPI_ENABLE;
 
 		/* Due to some hardware limitations on BYT, MIPI Port C DPI
 		 * Enable bit does not get set. To check whether DSI Port C
 		 * was enabled in BIOS, check the Pipe B enable bit
 		 */
 		if (IS_VALLEYVIEW(dev) && port == PORT_C)
-			dpi_enabled = I915_READ(PIPECONF(PIPE_B)) &
-							PIPECONF_ENABLE;
+			enabled = I915_READ(PIPECONF(PIPE_B)) & PIPECONF_ENABLE;
 
-		if (dpi_enabled || (func & CMD_MODE_DATA_WIDTH_MASK)) {
-			if (I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY) {
-				*pipe = port == PORT_A ? PIPE_A : PIPE_B;
-				ret = true;
-
-				goto out;
-			}
+		/* Try command mode if video mode not enabled */
+		if (!enabled) {
+			u32 tmp = I915_READ(MIPI_DSI_FUNC_PRG(port));
+			enabled = tmp & CMD_MODE_DATA_WIDTH_MASK;
 		}
+
+		if (!enabled)
+			continue;
+
+		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY))
+			continue;
+
+		*pipe = port == PORT_A ? PIPE_A : PIPE_B;
+		active = true;
+		break;
 	}
-out:
+
 	intel_display_power_put(dev_priv, power_domain);
 
-	return ret;
+	return active;
 }
 
 static void intel_dsi_get_config(struct intel_encoder *encoder,
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 04/10] drm/i915/bxt: fix dsi hw state pipe readout

[Jani Nikula]

BXT isn't as limited as BYT and CHT regarding DSI pipes and ports.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_reg.h  |  1 +
 drivers/gpu/drm/i915/intel_dsi.c | 14 +++++++++++++-
 2 files changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 7dfc4007f3fa..a5035991dd57 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -8144,6 +8144,7 @@ enum skl_disp_power_wells {
 #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
 #define  RGB_FLIP_TO_BGR				(1 << 2)
 
+#define  BXT_PIPE_SELECT_SHIFT				7
 #define  BXT_PIPE_SELECT_MASK				(7 << 7)
 #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 6574b9ac3698..73c15210fdb1 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -700,7 +700,19 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY))
 			continue;
 
-		*pipe = port == PORT_A ? PIPE_A : PIPE_B;
+		if (IS_BROXTON(dev_priv)) {
+			u32 tmp = I915_READ(MIPI_CTRL(port));
+			tmp &= BXT_PIPE_SELECT_MASK;
+			tmp >>= BXT_PIPE_SELECT_SHIFT;
+
+			if (WARN_ON(tmp > PIPE_C))
+				continue;
+
+			*pipe = tmp;
+		} else {
+			*pipe = port == PORT_A ? PIPE_A : PIPE_B;
+		}
+
 		active = true;
 		break;
 	}
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 05/10] drm/i915: split get/set pipe timings to pipe and transcoder parts

[Jani Nikula]

Prep work for DSI transcoders. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7977a818326d..4bfad0199232 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -7648,11 +7648,10 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
 	crtc_state->dpll_hw_state.dpll = dpll;
 }
 
-static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
+static void intel_set_trans_timings(struct intel_crtc *intel_crtc)
 {
 	struct drm_device *dev = intel_crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
 	uint32_t crtc_vtotal, crtc_vblank_end;
@@ -7699,6 +7698,16 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 	I915_WRITE(VSYNC(cpu_transcoder),
 		   (adjusted_mode->crtc_vsync_start - 1) |
 		   ((adjusted_mode->crtc_vsync_end - 1) << 16));
+}
+
+static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
+{
+	struct drm_device *dev = intel_crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum pipe pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
+
+	intel_set_trans_timings(intel_crtc);
 
 	/* Workaround: when the EDP input selection is B, the VTOTAL_B must be
 	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
@@ -7716,8 +7725,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 		   (intel_crtc->config->pipe_src_h - 1));
 }
 
-static void intel_get_pipe_timings(struct intel_crtc *crtc,
-				   struct intel_crtc_state *pipe_config)
+static void intel_get_trans_timings(struct intel_crtc *crtc,
+				    struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -7749,6 +7758,16 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
 		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
 		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
 	}
+}
+
+static void intel_get_pipe_timings(struct intel_crtc *crtc,
+				   struct intel_crtc_state *pipe_config)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	uint32_t tmp;
+
+	intel_get_trans_timings(crtc, pipe_config);
 
 	tmp = I915_READ(PIPESRC(crtc->pipe));
 	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 06/10] drm/i915: split set pipeconf to pipe and transcoder parts

[Jani Nikula]

Prep work for DSI transcoders. No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4bfad0199232..e485c1f9ca2b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -8748,14 +8748,13 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
 	}
 }
 
-static void haswell_set_pipeconf(struct drm_crtc *crtc)
+static void haswell_set_transconf(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
-	uint32_t val;
+	u32 val;
 
 	val = 0;
 
@@ -8769,6 +8768,17 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
 
 	I915_WRITE(PIPECONF(cpu_transcoder), val);
 	POSTING_READ(PIPECONF(cpu_transcoder));
+}
+
+static void haswell_set_pipeconf(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	enum pipe pipe = intel_crtc->pipe;
+	u32 val;
+
+	haswell_set_transconf(crtc);
 
 	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
 	POSTING_READ(GAMMA_MODE(intel_crtc->pipe));
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 07/10] drm/i915: abstract get config for cpu transcoder

[Jani Nikula]

Makes it neater to add the same for DSI transcoder. No functional
changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 83 ++++++++++++++++++++----------------
 1 file changed, 47 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e485c1f9ca2b..842467528892 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9893,6 +9893,49 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
 	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
 }
 
+static bool hsw_get_trans_config(struct intel_crtc *crtc,
+				 struct intel_crtc_state *pipe_config,
+				 unsigned long *power_domain_mask)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum intel_display_power_domain power_domain;
+	u32 tmp;
+
+	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
+
+	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
+	if (tmp & TRANS_DDI_FUNC_ENABLE) {
+		enum pipe trans_edp_pipe;
+		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
+		default:
+			WARN(1, "unknown pipe linked to edp transcoder\n");
+		case TRANS_DDI_EDP_INPUT_A_ONOFF:
+		case TRANS_DDI_EDP_INPUT_A_ON:
+			trans_edp_pipe = PIPE_A;
+			break;
+		case TRANS_DDI_EDP_INPUT_B_ONOFF:
+			trans_edp_pipe = PIPE_B;
+			break;
+		case TRANS_DDI_EDP_INPUT_C_ONOFF:
+			trans_edp_pipe = PIPE_C;
+			break;
+		}
+
+		if (trans_edp_pipe == crtc->pipe)
+			pipe_config->cpu_transcoder = TRANSCODER_EDP;
+	}
+
+	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
+	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+		return false;
+	*power_domain_mask |= BIT(power_domain);
+
+	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
+
+	return tmp & PIPECONF_ENABLE;
+}
+
 static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 				       struct intel_crtc_state *pipe_config)
 {
@@ -9943,48 +9986,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	enum intel_display_power_domain power_domain;
 	unsigned long power_domain_mask;
-	uint32_t tmp;
-	bool ret;
+	bool active;
 
 	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
 	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
 		return false;
 	power_domain_mask = BIT(power_domain);
 
-	ret = false;
-
-	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
 	pipe_config->shared_dpll = NULL;
 
-	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
-	if (tmp & TRANS_DDI_FUNC_ENABLE) {
-		enum pipe trans_edp_pipe;
-		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
-		default:
-			WARN(1, "unknown pipe linked to edp transcoder\n");
-		case TRANS_DDI_EDP_INPUT_A_ONOFF:
-		case TRANS_DDI_EDP_INPUT_A_ON:
-			trans_edp_pipe = PIPE_A;
-			break;
-		case TRANS_DDI_EDP_INPUT_B_ONOFF:
-			trans_edp_pipe = PIPE_B;
-			break;
-		case TRANS_DDI_EDP_INPUT_C_ONOFF:
-			trans_edp_pipe = PIPE_C;
-			break;
-		}
-
-		if (trans_edp_pipe == crtc->pipe)
-			pipe_config->cpu_transcoder = TRANSCODER_EDP;
-	}
-
-	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
-		goto out;
-	power_domain_mask |= BIT(power_domain);
+	active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
 
-	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
-	if (!(tmp & PIPECONF_ENABLE))
+	if (!active)
 		goto out;
 
 	haswell_get_ddi_port_state(crtc, pipe_config);
@@ -10020,13 +10033,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->pixel_multiplier = 1;
 	}
 
-	ret = true;
-
 out:
 	for_each_power_domain(power_domain, power_domain_mask)
 		intel_display_power_put(dev_priv, power_domain);
 
-	return ret;
+	return active;
 }
 
 static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 08/10] drm/i915/bxt: add dsi transcoders

[Jani Nikula]

The BXT display connections have DSI transcoders A and C that can be
muxed to any pipe, not unlike the eDP transcoder. Add the notion of DSI
transcoders.

The "normal" transcoders A, B and C are not used with BXT DSI, so care
must be taken to avoid accessing those registers with DSI transcoders in
the hardware state readout, modeset, and generally everywhere.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 10 ++++
 drivers/gpu/drm/i915/intel_ddi.c        |  6 +++
 drivers/gpu/drm/i915/intel_display.c    | 90 +++++++++++++++++++++++++++++----
 drivers/gpu/drm/i915/intel_drv.h        |  3 +-
 drivers/gpu/drm/i915/intel_dsi.c        |  9 ++++
 drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++
 6 files changed, 112 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 5e4a42d996d8..0ddd0f492ca9 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -123,6 +123,8 @@ enum transcoder {
 	TRANSCODER_B,
 	TRANSCODER_C,
 	TRANSCODER_EDP,
+	TRANSCODER_DSI_A,
+	TRANSCODER_DSI_C,
 	I915_MAX_TRANSCODERS
 };
 
@@ -137,11 +139,17 @@ static inline const char *transcoder_name(enum transcoder transcoder)
 		return "C";
 	case TRANSCODER_EDP:
 		return "eDP";
+	case TRANSCODER_DSI_A:
+		return "DSI A";
+	case TRANSCODER_DSI_C:
+		return "DSI C";
 	default:
 		return "<invalid>";
 	}
 }
 
+#define IS_DSI_TRANSCODER(t) ((t) == TRANSCODER_DSI_A || (t) == TRANSCODER_DSI_C)
+
 /*
  * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
  * number of planes per CRTC.  Not all platforms really have this many planes,
@@ -192,6 +200,8 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_TRANSCODER_B,
 	POWER_DOMAIN_TRANSCODER_C,
 	POWER_DOMAIN_TRANSCODER_EDP,
+	POWER_DOMAIN_TRANSCODER_DSI_A,
+	POWER_DOMAIN_TRANSCODER_DSI_C,
 	POWER_DOMAIN_PORT_DDI_A_LANES,
 	POWER_DOMAIN_PORT_DDI_B_LANES,
 	POWER_DOMAIN_PORT_DDI_C_LANES,
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 91654ffc3a42..8726711864eb 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1061,6 +1061,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
 	uint32_t temp;
 
 	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
+		WARN_ON(IS_DSI_TRANSCODER(cpu_transcoder));
+
 		temp = TRANS_MSA_SYNC_CLK;
 		switch (intel_crtc->config->pipe_bpp) {
 		case 18:
@@ -1942,6 +1944,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	struct intel_hdmi *intel_hdmi;
 	u32 temp, flags = 0;
 
+	/* XXX: DSI transcoder paranoia */
+	if (WARN_ON(IS_DSI_TRANSCODER(cpu_transcoder)))
+		return;
+
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (temp & TRANS_DDI_PHSYNC)
 		flags |= DRM_MODE_FLAG_PHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 842467528892..f1d0a868e80c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4906,7 +4906,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 
 	intel_set_pipe_timings(intel_crtc);
 
-	if (intel_crtc->config->cpu_transcoder != TRANSCODER_EDP) {
+	if (intel_crtc->config->cpu_transcoder < TRANSCODER_EDP) {
 		I915_WRITE(PIPE_MULT(intel_crtc->config->cpu_transcoder),
 			   intel_crtc->config->pixel_multiplier - 1);
 	}
@@ -4957,7 +4957,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 		dev_priv->display.initial_watermarks(pipe_config);
 	else
 		intel_update_watermarks(crtc);
-	intel_enable_pipe(intel_crtc);
+
+	/* XXX: Do the pipe assertions at the right place for BXT DSI. */
+	if (!intel_crtc->config->has_dsi_encoder)
+		intel_enable_pipe(intel_crtc);
 
 	if (intel_crtc->config->has_pch_encoder)
 		lpt_pch_enable(crtc);
@@ -5090,7 +5093,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	drm_crtc_vblank_off(crtc);
 	assert_vblank_disabled(crtc);
 
-	intel_disable_pipe(intel_crtc);
+	/* XXX: Do the pipe assertions at the right place for BXT DSI. */
+	if (!intel_crtc->config->has_dsi_encoder)
+		intel_disable_pipe(intel_crtc);
 
 	if (intel_crtc->config->dp_encoder_is_mst)
 		intel_ddi_set_vc_payload_alloc(crtc, false);
@@ -7707,7 +7712,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
 	enum pipe pipe = intel_crtc->pipe;
 	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 
-	intel_set_trans_timings(intel_crtc);
+	if (!IS_DSI_TRANSCODER(cpu_transcoder))
+		intel_set_trans_timings(intel_crtc);
 
 	/* Workaround: when the EDP input selection is B, the VTOTAL_B must be
 	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
@@ -7765,9 +7771,11 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	uint32_t tmp;
 
-	intel_get_trans_timings(crtc, pipe_config);
+	if (!IS_DSI_TRANSCODER(cpu_transcoder))
+		intel_get_trans_timings(crtc, pipe_config);
 
 	tmp = I915_READ(PIPESRC(crtc->pipe));
 	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
@@ -8776,9 +8784,11 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum pipe pipe = intel_crtc->pipe;
+	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
 	u32 val;
 
-	haswell_set_transconf(crtc);
+	if (!IS_DSI_TRANSCODER(cpu_transcoder))
+		haswell_set_transconf(crtc);
 
 	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
 	POSTING_READ(GAMMA_MODE(intel_crtc->pipe));
@@ -9936,6 +9946,47 @@ static bool hsw_get_trans_config(struct intel_crtc *crtc,
 	return tmp & PIPECONF_ENABLE;
 }
 
+static bool bxt_get_dsi_trans_config(struct intel_crtc *crtc,
+				     struct intel_crtc_state *pipe_config,
+				     unsigned long *power_domain_mask)
+{
+	struct drm_device *dev = crtc->base.dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	enum intel_display_power_domain power_domain;
+	enum port port;
+	enum transcoder cpu_transcoder;
+	u32 tmp;
+
+	pipe_config->has_dsi_encoder = false;
+
+	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
+		if (port == PORT_A)
+			cpu_transcoder = TRANSCODER_DSI_A;
+		else
+			cpu_transcoder = TRANSCODER_DSI_C;
+
+		power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
+		if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+			continue;
+		*power_domain_mask |= BIT(power_domain);
+
+		/* XXX: this works for video mode only */
+		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
+		if (!(tmp & DPI_ENABLE))
+			continue;
+
+		tmp = I915_READ(MIPI_CTRL(port));
+		if ((tmp & BXT_PIPE_SELECT_MASK) != BXT_PIPE_SELECT(crtc->pipe))
+			continue;
+
+		pipe_config->cpu_transcoder = cpu_transcoder;
+		pipe_config->has_dsi_encoder = true;
+		break;
+	}
+
+	return pipe_config->has_dsi_encoder;
+}
+
 static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
 				       struct intel_crtc_state *pipe_config)
 {
@@ -9997,10 +10048,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 
 	active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
 
+	if (IS_BROXTON(dev_priv)) {
+		bxt_get_dsi_trans_config(crtc, pipe_config, &power_domain_mask);
+		WARN_ON(active && pipe_config->has_dsi_encoder);
+		if (pipe_config->has_dsi_encoder)
+			active = true;
+	}
+
 	if (!active)
 		goto out;
 
-	haswell_get_ddi_port_state(crtc, pipe_config);
+	if (!pipe_config->has_dsi_encoder)
+		haswell_get_ddi_port_state(crtc, pipe_config);
 
 	intel_get_pipe_timings(crtc, pipe_config);
 
@@ -10026,7 +10085,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
 		pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
 			(I915_READ(IPS_CTL) & IPS_ENABLE);
 
-	if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
+	if (pipe_config->cpu_transcoder < TRANSCODER_EDP) {
 		pipe_config->pixel_multiplier =
 			I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
 	} else {
@@ -15516,10 +15575,18 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	i915_reg_t reg = PIPECONF(crtc->config->cpu_transcoder);
+	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
 
-	/* Clear any frame start delays used for debugging left by the BIOS */
-	I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
+	if (!IS_DSI_TRANSCODER(cpu_transcoder)) {
+		i915_reg_t reg = PIPECONF(cpu_transcoder);
+
+		/*
+		 * Clear any frame start delays used for debugging left by the
+		 * BIOS.
+		 */
+		I915_WRITE(reg,
+			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
+	}
 
 	/* restore vblank interrupts to correct state */
 	drm_crtc_vblank_reset(&crtc->base);
@@ -16195,6 +16262,7 @@ intel_display_capture_error_state(struct drm_device *dev)
 			error->pipe[i].stat = I915_READ(PIPESTAT(i));
 	}
 
+	/* Note: this does not include DSI transcoders. */
 	error->num_transcoders = INTEL_INFO(dev)->num_pipes;
 	if (HAS_DDI(dev_priv->dev))
 		error->num_transcoders++; /* Account for eDP. */
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 02b3d22862a1..6e8c1386129e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -436,7 +436,8 @@ struct intel_crtc_state {
 	bool has_infoframe;
 
 	/* CPU Transcoder for the pipe. Currently this can only differ from the
-	 * pipe on Haswell (where we have a special eDP transcoder). */
+	 * pipe on Haswell and later (where we have a special eDP transcoder)
+	 * and Broxton (where we have special DSI transcoders). */
 	enum transcoder cpu_transcoder;
 
 	/*
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 73c15210fdb1..47fd0296a05a 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -268,6 +268,7 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
 static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 				     struct intel_crtc_state *pipe_config)
 {
+	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
 	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
 						   base);
 	struct intel_connector *intel_connector = intel_dsi->attached_connector;
@@ -284,6 +285,14 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
 	/* DSI uses short packets for sync events, so clear mode flags for DSI */
 	adjusted_mode->flags = 0;
 
+	if (IS_BROXTON(dev_priv)) {
+		/* Dual link goes to DSI transcoder A. */
+		if (intel_dsi->ports == BIT(PORT_C))
+			pipe_config->cpu_transcoder = TRANSCODER_DSI_C;
+		else
+			pipe_config->cpu_transcoder = TRANSCODER_DSI_A;
+	}
+
 	return true;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 2e88a5e06884..d189a0012277 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -89,6 +89,10 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "TRANSCODER_C";
 	case POWER_DOMAIN_TRANSCODER_EDP:
 		return "TRANSCODER_EDP";
+	case POWER_DOMAIN_TRANSCODER_DSI_A:
+		return "TRANSCODER_DSI_A";
+	case POWER_DOMAIN_TRANSCODER_DSI_C:
+		return "TRANSCODER_DSI_C";
 	case POWER_DOMAIN_PORT_DDI_A_LANES:
 		return "PORT_DDI_A_LANES";
 	case POWER_DOMAIN_PORT_DDI_B_LANES:
@@ -419,6 +423,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
 	BIT(POWER_DOMAIN_PIPE_A) |			\
 	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_DSI_A) |		\
+	BIT(POWER_DOMAIN_TRANSCODER_DSI_C) |		\
 	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
 	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
 	BIT(POWER_DOMAIN_PORT_DSI) |			\
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 09/10] drm/i915/dsi: use the BIT macro for clarity

[Jani Nikula]

No functional changes.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 47fd0296a05a..a249549a1d86 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -412,7 +412,7 @@ static void intel_dsi_port_enable(struct intel_encoder *encoder)
 		temp &= ~LANE_CONFIGURATION_MASK;
 		temp &= ~DUAL_LINK_MODE_MASK;
 
-		if (intel_dsi->ports == ((1 << PORT_A) | (1 << PORT_C))) {
+		if (intel_dsi->ports == (BIT(PORT_A) | BIT(PORT_C))) {
 			temp |= (intel_dsi->dual_link - 1)
 						<< DUAL_LINK_MODE_SHIFT;
 			temp |= intel_crtc->pipe ?
@@ -1185,15 +1185,15 @@ void intel_dsi_init(struct drm_device *dev)
 
 	/* Pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI port C */
 	if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIA) {
-		intel_encoder->crtc_mask = (1 << PIPE_A);
-		intel_dsi->ports = (1 << PORT_A);
+		intel_encoder->crtc_mask = BIT(PIPE_A);
+		intel_dsi->ports = BIT(PORT_A);
 	} else if (dev_priv->vbt.dsi.port == DVO_PORT_MIPIC) {
-		intel_encoder->crtc_mask = (1 << PIPE_B);
-		intel_dsi->ports = (1 << PORT_C);
+		intel_encoder->crtc_mask = BIT(PIPE_B);
+		intel_dsi->ports = BIT(PORT_C);
 	}
 
 	if (dev_priv->vbt.dsi.config->dual_link)
-		intel_dsi->ports = ((1 << PORT_A) | (1 << PORT_C));
+		intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C);
 
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

[RFC PATCH 10/10] drm/i915/bxt: allow dsi on any pipe

[Jani Nikula]

BXT isn't as limited as BYT and CHT regarding DSI pipes and ports.

Signed-off-by: Jani Nikula <jani.nikula@intel.com>
---
 drivers/gpu/drm/i915/intel_dsi.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index a249549a1d86..dbdd5e7d9ef8 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1195,6 +1195,9 @@ void intel_dsi_init(struct drm_device *dev)
 	if (dev_priv->vbt.dsi.config->dual_link)
 		intel_dsi->ports = BIT(PORT_A) | BIT(PORT_C);
 
+	if (IS_BROXTON(dev_priv))
+		intel_encoder->crtc_mask = BIT(PIPE_A) | BIT(PIPE_B) | BIT(PIPE_C);
+
 	/* Create a DSI host (and a device) for each port. */
 	for_each_dsi_port(port, intel_dsi->ports) {
 		struct intel_dsi_host *host;
-- 
2.1.4

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

✗ Fi.CI.BAT: failure for drm/i915/bxt: add dsi transcoders

[Patchwork]

== Series Details ==

Series: drm/i915/bxt: add dsi transcoders
URL   : https://patchwork.freedesktop.org/series/4483/
State : failure

== Summary ==

Series 4483v1 drm/i915/bxt: add dsi transcoders
http://patchwork.freedesktop.org/api/1.0/series/4483/revisions/1/mbox/

Test drv_module_reload_basic:
                skip       -> PASS       (bdw-nuci7)
Test gem_ringfill:
        Subgroup basic-default-s3:
                pass       -> DMESG-WARN (bsw-nuc-2)
Test kms_flip:
        Subgroup basic-flip-vs-modeset:
                pass       -> DMESG-WARN (skl-i5k-2)
        Subgroup basic-flip-vs-wf_vblank:
                dmesg-warn -> PASS       (hsw-brixbox)
Test kms_pipe_crc_basic:
        Subgroup read-crc-pipe-c:
                fail       -> PASS       (hsw-brixbox)
        Subgroup suspend-read-crc-pipe-b:
                dmesg-warn -> PASS       (skl-nuci5)
Test pm_rpm:
        Subgroup basic-pci-d3-state:
                pass       -> DMESG-WARN (snb-dellxps)
        Subgroup basic-rte:
                dmesg-warn -> PASS       (snb-dellxps)

bdw-nuci7        total:194  pass:182  dwarn:0   dfail:0   fail:0   skip:12 
bdw-ultra        total:194  pass:173  dwarn:0   dfail:0   fail:0   skip:21 
bsw-nuc-2        total:194  pass:156  dwarn:1   dfail:0   fail:0   skip:37 
byt-nuc          total:194  pass:155  dwarn:4   dfail:0   fail:0   skip:35 
hsw-brixbox      total:194  pass:172  dwarn:0   dfail:0   fail:0   skip:22 
hsw-gt2          total:194  pass:177  dwarn:0   dfail:0   fail:0   skip:17 
ivb-t430s        total:194  pass:169  dwarn:0   dfail:0   fail:0   skip:25 
skl-i5k-2        total:194  pass:170  dwarn:1   dfail:0   fail:0   skip:23 
skl-i7k-2        total:194  pass:171  dwarn:0   dfail:0   fail:0   skip:23 
skl-nuci5        total:194  pass:183  dwarn:0   dfail:0   fail:0   skip:11 
snb-dellxps      total:194  pass:159  dwarn:1   dfail:0   fail:0   skip:34 

Results at /archive/results/CI_IGT_test/Patchwork_1608/

fc881ebd9c3c26919c7d1113f8bf7014e1a05563 drm-intel-nightly: 2016y-03m-15d-13h-10m-41s UTC integration manifest
f8674383de51fd39abf64633a2e37e278108f687 drm/i915/bxt: allow dsi on any pipe
bd00d25cfe9977c2826db780539e7a01e680a252 drm/i915/dsi: use the BIT macro for clarity
45676697ab1cdb4c674ed5deb0202984ed8a6c86 drm/i915/bxt: add dsi transcoders
bb910bf0aa8396510f7b47e178824161aa50f576 drm/i915: abstract get config for cpu transcoder
329cad09ea7efc6c284ce178ddca72c0c5133209 drm/i915: split set pipeconf to pipe and transcoder parts
88ba0136f2b4789e82fdb8efbf2c1d6d1c083cfb drm/i915: split get/set pipe timings to pipe and transcoder parts
16ca06fb41bfa6f7bb44f64ba133314fdf2e81bd drm/i915/bxt: fix dsi hw state pipe readout
5bebc9533048bbed094ff3861471447b1635896f drm/i915/dsi: refactor dsi get hw state readout
0d8cd289aa10e9b83200941306af5ab868ed4188 drm/i915: make transcoder_name return a string
cebc90ba5af275ac8189b62919edbdc075b4dcb7 drm/i915: add for_each_port_masked macro

_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 02/10] drm/i915: make transcoder_name return a string

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:10PM +0200, Jani Nikula wrote:
> Nicer for eDP (actually "eDP" instead of "D"), and makes future
> expansion for DSI transcoders easier.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h            | 17 ++++++++++++++++-
>  drivers/gpu/drm/i915/intel_display.c       |  4 ++--
>  drivers/gpu/drm/i915/intel_fifo_underrun.c |  6 +++---
>  3 files changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 8ef3c88d0ed2..5e4a42d996d8 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -125,7 +125,22 @@ enum transcoder {
>  	TRANSCODER_EDP,
>  	I915_MAX_TRANSCODERS
>  };
> -#define transcoder_name(t) ((t) + 'A')
> +
> +static inline const char *transcoder_name(enum transcoder transcoder)
> +{
> +	switch (transcoder) {
> +	case TRANSCODER_A:
> +		return "A";
> +	case TRANSCODER_B:
> +		return "B";
> +	case TRANSCODER_C:
> +		return "C";
> +	case TRANSCODER_EDP:
> +		return "eDP";

It's all caps "EDP" in Bspec.

with that
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>


> +	default:
> +		return "<invalid>";
> +	}
> +}
>  
>  /*
>   * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index ce55f0b683c6..7977a818326d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -12096,7 +12096,7 @@ static void intel_dump_pipe_config(struct intel_crtc *crtc,
>  	DRM_DEBUG_KMS("[CRTC:%d]%s config %p for pipe %c\n", crtc->base.base.id,
>  		      context, pipe_config, pipe_name(crtc->pipe));
>  
> -	DRM_DEBUG_KMS("cpu_transcoder: %c\n", transcoder_name(pipe_config->cpu_transcoder));
> +	DRM_DEBUG_KMS("cpu_transcoder: %s\n", transcoder_name(pipe_config->cpu_transcoder));
>  	DRM_DEBUG_KMS("pipe bpp: %i, dithering: %i\n",
>  		      pipe_config->pipe_bpp, pipe_config->dither);
>  	DRM_DEBUG_KMS("fdi/pch: %i, lanes: %i, gmch_m: %u, gmch_n: %u, link_m: %u, link_n: %u, tu: %u\n",
> @@ -16227,7 +16227,7 @@ intel_display_print_error_state(struct drm_i915_error_state_buf *m,
>  	}
>  
>  	for (i = 0; i < error->num_transcoders; i++) {
> -		err_printf(m, "CPU transcoder: %c\n",
> +		err_printf(m, "CPU transcoder: %s\n",
>  			   transcoder_name(error->transcoder[i].cpu_transcoder));
>  		err_printf(m, "  Power: %s\n",
>  			   onoff(error->transcoder[i].power_domain_on));
> diff --git a/drivers/gpu/drm/i915/intel_fifo_underrun.c b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> index bda526660e20..19e50fdf9a91 100644
> --- a/drivers/gpu/drm/i915/intel_fifo_underrun.c
> +++ b/drivers/gpu/drm/i915/intel_fifo_underrun.c
> @@ -212,7 +212,7 @@ static void cpt_check_pch_fifo_underruns(struct intel_crtc *crtc)
>  	I915_WRITE(SERR_INT, SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder));
>  	POSTING_READ(SERR_INT);
>  
> -	DRM_ERROR("pch fifo underrun on pch transcoder %c\n",
> +	DRM_ERROR("pch fifo underrun on pch transcoder %s\n",
>  		  transcoder_name(pch_transcoder));
>  }
>  
> @@ -235,7 +235,7 @@ static void cpt_set_fifo_underrun_reporting(struct drm_device *dev,
>  
>  		if (old && I915_READ(SERR_INT) &
>  		    SERR_INT_TRANS_FIFO_UNDERRUN(pch_transcoder)) {
> -			DRM_ERROR("uncleared pch fifo underrun on pch transcoder %c\n",
> +			DRM_ERROR("uncleared pch fifo underrun on pch transcoder %s\n",
>  				  transcoder_name(pch_transcoder));
>  		}
>  	}
> @@ -386,7 +386,7 @@ void intel_pch_fifo_underrun_irq_handler(struct drm_i915_private *dev_priv,
>  {
>  	if (intel_set_pch_fifo_underrun_reporting(dev_priv, pch_transcoder,
>  						  false))
> -		DRM_ERROR("PCH transcoder %c FIFO underrun\n",
> +		DRM_ERROR("PCH transcoder %s FIFO underrun\n",
>  			  transcoder_name(pch_transcoder));

We're a bit lazy with the PCH transcoders. Sometimes we use enum
transcoder sometimes enum pipe. In practice it doesn't make any
difference though since it's a 1:1 mapping. But at some point I
was arguing with myself whether I should try to change it all to
use one or the other, or even introduce a separate pch_transcoder
enum for clarity.

>  }
>  
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 05/10] drm/i915: split get/set pipe timings to pipe and transcoder parts

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:13PM +0200, Jani Nikula wrote:
> Prep work for DSI transcoders. No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++++++++++++----
>  1 file changed, 23 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7977a818326d..4bfad0199232 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -7648,11 +7648,10 @@ static void i8xx_compute_dpll(struct intel_crtc *crtc,
>  	crtc_state->dpll_hw_state.dpll = dpll;
>  }
>  
> -static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> +static void intel_set_trans_timings(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_device *dev = intel_crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	enum pipe pipe = intel_crtc->pipe;
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  	const struct drm_display_mode *adjusted_mode = &intel_crtc->config->base.adjusted_mode;
>  	uint32_t crtc_vtotal, crtc_vblank_end;
> @@ -7699,6 +7698,16 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  	I915_WRITE(VSYNC(cpu_transcoder),
>  		   (adjusted_mode->crtc_vsync_start - 1) |
>  		   ((adjusted_mode->crtc_vsync_end - 1) << 16));
> +}
> +
> +static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
> +{
> +	struct drm_device *dev = intel_crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum pipe pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> +
> +	intel_set_trans_timings(intel_crtc);
>  
>  	/* Workaround: when the EDP input selection is B, the VTOTAL_B must be
>  	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is

You split seems to be in the wrong place, as in we just want to split
out the PIPESRC thing from the rest. I'd just call them
set_pipe_timings() and set_pipe_src_size(). And we'll need to call the
latter even with DSI.

> @@ -7716,8 +7725,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  		   (intel_crtc->config->pipe_src_h - 1));
>  }
>  
> -static void intel_get_pipe_timings(struct intel_crtc *crtc,
> -				   struct intel_crtc_state *pipe_config)
> +static void intel_get_trans_timings(struct intel_crtc *crtc,
> +				    struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> @@ -7749,6 +7758,16 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  		pipe_config->base.adjusted_mode.crtc_vtotal += 1;
>  		pipe_config->base.adjusted_mode.crtc_vblank_end += 1;
>  	}
> +}
> +
> +static void intel_get_pipe_timings(struct intel_crtc *crtc,
> +				   struct intel_crtc_state *pipe_config)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	uint32_t tmp;
> +
> +	intel_get_trans_timings(crtc, pipe_config);
>  
>  	tmp = I915_READ(PIPESRC(crtc->pipe));
>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 01/10] drm/i915: add for_each_port_masked macro

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:09PM +0200, Jani Nikula wrote:
> Same as for_each_dsi_port, but for general use. Leave the
> for_each_dsi_port version around as an "alias" for now to not cause too
> much churn. No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/i915_drv.h  | 4 ++++
>  drivers/gpu/drm/i915/intel_dsi.h | 4 +---
>  2 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 80b14f1ba302..8ef3c88d0ed2 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -274,6 +274,10 @@ struct i915_hotplug {
>  	     (__s) < INTEL_INFO(__dev_priv)->num_sprites[(__p)];	\
>  	     (__s)++)
>  
> +#define for_each_port_masked(__port, __ports_mask) \
> +	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
> +		for_each_if ((__ports_mask) & (1 << (__port)))
> +
>  #define for_each_crtc(dev, crtc) \
>  	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.h b/drivers/gpu/drm/i915/intel_dsi.h
> index 92f39227b361..0b5e0b8ac08d 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.h
> +++ b/drivers/gpu/drm/i915/intel_dsi.h
> @@ -117,9 +117,7 @@ static inline struct intel_dsi_host *to_intel_dsi_host(struct mipi_dsi_host *h)
>  	return container_of(h, struct intel_dsi_host, base);
>  }
>  
> -#define for_each_dsi_port(__port, __ports_mask) \
> -	for ((__port) = PORT_A; (__port) < I915_MAX_PORTS; (__port)++)	\
> -		for_each_if ((__ports_mask) & (1 << (__port)))
> +#define for_each_dsi_port(__port, __ports_mask) for_each_port_masked(__port, __ports_mask)
>  
>  static inline struct intel_dsi *enc_to_intel_dsi(struct drm_encoder *encoder)
>  {
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 03/10] drm/i915/dsi: refactor dsi get hw state readout

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:11PM +0200, Jani Nikula wrote:
> Make the code easier to read and update. No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>

The logic looks intact
Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

> ---
>  drivers/gpu/drm/i915/intel_dsi.c | 37 +++++++++++++++++++------------------
>  1 file changed, 19 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 01b8e9f4c272..6574b9ac3698 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -667,7 +667,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  	struct drm_device *dev = encoder->base.dev;
>  	enum intel_display_power_domain power_domain;
>  	enum port port;
> -	bool ret;
> +	bool active = false;
>  
>  	DRM_DEBUG_KMS("\n");
>  
> @@ -675,38 +675,39 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
>  
> -	ret = false;
> -
>  	/* XXX: this only works for one DSI output */
>  	for_each_dsi_port(port, intel_dsi->ports) {
>  		i915_reg_t ctrl_reg = IS_BROXTON(dev) ?
>  			BXT_MIPI_PORT_CTRL(port) : MIPI_PORT_CTRL(port);
> -		u32 dpi_enabled, func;
> -
> -		func = I915_READ(MIPI_DSI_FUNC_PRG(port));
> -		dpi_enabled = I915_READ(ctrl_reg) & DPI_ENABLE;
> +		bool enabled = I915_READ(ctrl_reg) & DPI_ENABLE;
>  
>  		/* Due to some hardware limitations on BYT, MIPI Port C DPI
>  		 * Enable bit does not get set. To check whether DSI Port C
>  		 * was enabled in BIOS, check the Pipe B enable bit
>  		 */
>  		if (IS_VALLEYVIEW(dev) && port == PORT_C)
> -			dpi_enabled = I915_READ(PIPECONF(PIPE_B)) &
> -							PIPECONF_ENABLE;
> +			enabled = I915_READ(PIPECONF(PIPE_B)) & PIPECONF_ENABLE;
>  
> -		if (dpi_enabled || (func & CMD_MODE_DATA_WIDTH_MASK)) {
> -			if (I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY) {
> -				*pipe = port == PORT_A ? PIPE_A : PIPE_B;
> -				ret = true;
> -
> -				goto out;
> -			}
> +		/* Try command mode if video mode not enabled */
> +		if (!enabled) {
> +			u32 tmp = I915_READ(MIPI_DSI_FUNC_PRG(port));
> +			enabled = tmp & CMD_MODE_DATA_WIDTH_MASK;
>  		}
> +
> +		if (!enabled)
> +			continue;
> +
> +		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY))
> +			continue;
> +
> +		*pipe = port == PORT_A ? PIPE_A : PIPE_B;
> +		active = true;
> +		break;
>  	}
> -out:
> +
>  	intel_display_power_put(dev_priv, power_domain);
>  
> -	return ret;
> +	return active;
>  }
>  
>  static void intel_dsi_get_config(struct intel_encoder *encoder,
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 04/10] drm/i915/bxt: fix dsi hw state pipe readout

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:12PM +0200, Jani Nikula wrote:
> BXT isn't as limited as BYT and CHT regarding DSI pipes and ports.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>  drivers/gpu/drm/i915/intel_dsi.c | 14 +++++++++++++-
>  2 files changed, 14 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
> index 7dfc4007f3fa..a5035991dd57 100644
> --- a/drivers/gpu/drm/i915/i915_reg.h
> +++ b/drivers/gpu/drm/i915/i915_reg.h
> @@ -8144,6 +8144,7 @@ enum skl_disp_power_wells {
>  #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
>  #define  RGB_FLIP_TO_BGR				(1 << 2)
>  
> +#define  BXT_PIPE_SELECT_SHIFT				7
>  #define  BXT_PIPE_SELECT_MASK				(7 << 7)
>  #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)

These defines look a little misplaced, but that's separate topic.

Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

>  
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 6574b9ac3698..73c15210fdb1 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -700,7 +700,19 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>  		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY))
>  			continue;
>  
> -		*pipe = port == PORT_A ? PIPE_A : PIPE_B;
> +		if (IS_BROXTON(dev_priv)) {
> +			u32 tmp = I915_READ(MIPI_CTRL(port));
> +			tmp &= BXT_PIPE_SELECT_MASK;
> +			tmp >>= BXT_PIPE_SELECT_SHIFT;
> +
> +			if (WARN_ON(tmp > PIPE_C))
> +				continue;
> +
> +			*pipe = tmp;
> +		} else {
> +			*pipe = port == PORT_A ? PIPE_A : PIPE_B;
> +		}
> +
>  		active = true;
>  		break;
>  	}
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 06/10] drm/i915: split set pipeconf to pipe and transcoder parts

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:14PM +0200, Jani Nikula wrote:
> Prep work for DSI transcoders. No functional changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 +++++++++++++---
>  1 file changed, 13 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4bfad0199232..e485c1f9ca2b 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8748,14 +8748,13 @@ static void intel_set_pipe_csc(struct drm_crtc *crtc)
>  	}
>  }
>  
> -static void haswell_set_pipeconf(struct drm_crtc *crtc)
> +static void haswell_set_transconf(struct drm_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> -	enum pipe pipe = intel_crtc->pipe;
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
> -	uint32_t val;
> +	u32 val;
>  
>  	val = 0;
>  
> @@ -8769,6 +8768,17 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  
>  	I915_WRITE(PIPECONF(cpu_transcoder), val);
>  	POSTING_READ(PIPECONF(cpu_transcoder));
> +}
> +
> +static void haswell_set_pipeconf(struct drm_crtc *crtc)

The name is maybe a bit confusing if it won't actually set PIPECONF.

Dunnoe, maybe we should just have set_pipeconf,
set_pipe_gamma, set_pipemisc?

> +{
> +	struct drm_device *dev = crtc->dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> +	enum pipe pipe = intel_crtc->pipe;
> +	u32 val;
> +
> +	haswell_set_transconf(crtc);
>  
>  	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
>  	POSTING_READ(GAMMA_MODE(intel_crtc->pipe));
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 07/10] drm/i915: abstract get config for cpu transcoder

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:15PM +0200, Jani Nikula wrote:
> Makes it neater to add the same for DSI transcoder. No functional
> changes.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 83 ++++++++++++++++++++----------------
>  1 file changed, 47 insertions(+), 36 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index e485c1f9ca2b..842467528892 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -9893,6 +9893,49 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>  	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
>  }
>  
> +static bool hsw_get_trans_config(struct intel_crtc *crtc,
> +				 struct intel_crtc_state *pipe_config,
> +				 unsigned long *power_domain_mask)
> +{

To be consistent with existing practices we should probably call this 
..._get_hw_state, or maybe ..._get_transcoder_state (if we want to
follow the get_ddi_port_state() below).

> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum intel_display_power_domain power_domain;
> +	u32 tmp;
> +
> +	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> +
> +	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> +	if (tmp & TRANS_DDI_FUNC_ENABLE) {

I guess as a followup we should do the
intel_display_power_get_if_enabled() for the edp transcoder already
before this read. Just need to be careful that we don't grab it twice
then.

> +		enum pipe trans_edp_pipe;
> +		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
> +		default:
> +			WARN(1, "unknown pipe linked to edp transcoder\n");
> +		case TRANS_DDI_EDP_INPUT_A_ONOFF:
> +		case TRANS_DDI_EDP_INPUT_A_ON:
> +			trans_edp_pipe = PIPE_A;
> +			break;
> +		case TRANS_DDI_EDP_INPUT_B_ONOFF:
> +			trans_edp_pipe = PIPE_B;
> +			break;
> +		case TRANS_DDI_EDP_INPUT_C_ONOFF:
> +			trans_edp_pipe = PIPE_C;
> +			break;
> +		}
> +
> +		if (trans_edp_pipe == crtc->pipe)
> +			pipe_config->cpu_transcoder = TRANSCODER_EDP;
> +	}
> +
> +	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> +		return false;
> +	*power_domain_mask |= BIT(power_domain);
> +
> +	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> +
> +	return tmp & PIPECONF_ENABLE;
> +}
> +
>  static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *pipe_config)
>  {
> @@ -9943,48 +9986,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	enum intel_display_power_domain power_domain;
>  	unsigned long power_domain_mask;
> -	uint32_t tmp;
> -	bool ret;
> +	bool active;
>  
>  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>  		return false;
>  	power_domain_mask = BIT(power_domain);
>  
> -	ret = false;
> -
> -	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
>  	pipe_config->shared_dpll = NULL;
>  
> -	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> -	if (tmp & TRANS_DDI_FUNC_ENABLE) {
> -		enum pipe trans_edp_pipe;
> -		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
> -		default:
> -			WARN(1, "unknown pipe linked to edp transcoder\n");
> -		case TRANS_DDI_EDP_INPUT_A_ONOFF:
> -		case TRANS_DDI_EDP_INPUT_A_ON:
> -			trans_edp_pipe = PIPE_A;
> -			break;
> -		case TRANS_DDI_EDP_INPUT_B_ONOFF:
> -			trans_edp_pipe = PIPE_B;
> -			break;
> -		case TRANS_DDI_EDP_INPUT_C_ONOFF:
> -			trans_edp_pipe = PIPE_C;
> -			break;
> -		}
> -
> -		if (trans_edp_pipe == crtc->pipe)
> -			pipe_config->cpu_transcoder = TRANSCODER_EDP;
> -	}
> -
> -	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
> -	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> -		goto out;
> -	power_domain_mask |= BIT(power_domain);
> +	active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
>  
> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> -	if (!(tmp & PIPECONF_ENABLE))
> +	if (!active)
>  		goto out;
>  
>  	haswell_get_ddi_port_state(crtc, pipe_config);
> @@ -10020,13 +10033,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->pixel_multiplier = 1;
>  	}
>  
> -	ret = true;
> -
>  out:
>  	for_each_power_domain(power_domain, power_domain_mask)
>  		intel_display_power_put(dev_priv, power_domain);
>  
> -	return ret;
> +	return active;
>  }
>  
>  static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 08/10] drm/i915/bxt: add dsi transcoders

[Ville Syrjälä]

On Tue, Mar 15, 2016 at 09:51:16PM +0200, Jani Nikula wrote:
> The BXT display connections have DSI transcoders A and C that can be
> muxed to any pipe, not unlike the eDP transcoder. Add the notion of DSI
> transcoders.
> 
> The "normal" transcoders A, B and C are not used with BXT DSI, so care
> must be taken to avoid accessing those registers with DSI transcoders in
> the hardware state readout, modeset, and generally everywhere.
> 
> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 10 ++++
>  drivers/gpu/drm/i915/intel_ddi.c        |  6 +++
>  drivers/gpu/drm/i915/intel_display.c    | 90 +++++++++++++++++++++++++++++----
>  drivers/gpu/drm/i915/intel_drv.h        |  3 +-
>  drivers/gpu/drm/i915/intel_dsi.c        |  9 ++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c |  6 +++
>  6 files changed, 112 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 5e4a42d996d8..0ddd0f492ca9 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -123,6 +123,8 @@ enum transcoder {
>  	TRANSCODER_B,
>  	TRANSCODER_C,
>  	TRANSCODER_EDP,
> +	TRANSCODER_DSI_A,
> +	TRANSCODER_DSI_C,
>  	I915_MAX_TRANSCODERS
>  };
>  
> @@ -137,11 +139,17 @@ static inline const char *transcoder_name(enum transcoder transcoder)
>  		return "C";
>  	case TRANSCODER_EDP:
>  		return "eDP";
> +	case TRANSCODER_DSI_A:
> +		return "DSI A";
> +	case TRANSCODER_DSI_C:
> +		return "DSI C";
>  	default:
>  		return "<invalid>";
>  	}
>  }
>  
> +#define IS_DSI_TRANSCODER(t) ((t) == TRANSCODER_DSI_A || (t) == TRANSCODER_DSI_C)
> +
>  /*
>   * I915_MAX_PLANES in the enum below is the maximum (across all platforms)
>   * number of planes per CRTC.  Not all platforms really have this many planes,
> @@ -192,6 +200,8 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_TRANSCODER_B,
>  	POWER_DOMAIN_TRANSCODER_C,
>  	POWER_DOMAIN_TRANSCODER_EDP,
> +	POWER_DOMAIN_TRANSCODER_DSI_A,
> +	POWER_DOMAIN_TRANSCODER_DSI_C,
>  	POWER_DOMAIN_PORT_DDI_A_LANES,
>  	POWER_DOMAIN_PORT_DDI_B_LANES,
>  	POWER_DOMAIN_PORT_DDI_C_LANES,
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 91654ffc3a42..8726711864eb 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1061,6 +1061,8 @@ void intel_ddi_set_pipe_settings(struct drm_crtc *crtc)
>  	uint32_t temp;
>  
>  	if (type == INTEL_OUTPUT_DISPLAYPORT || type == INTEL_OUTPUT_EDP || type == INTEL_OUTPUT_DP_MST) {
> +		WARN_ON(IS_DSI_TRANSCODER(cpu_transcoder));

That all caps IS_DSI_TRANSCODER doesn't really agree with me for some
reason. static inline with lower caps name maybe?

> +
>  		temp = TRANS_MSA_SYNC_CLK;
>  		switch (intel_crtc->config->pipe_bpp) {
>  		case 18:
> @@ -1942,6 +1944,10 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	struct intel_hdmi *intel_hdmi;
>  	u32 temp, flags = 0;
>  
> +	/* XXX: DSI transcoder paranoia */
> +	if (WARN_ON(IS_DSI_TRANSCODER(cpu_transcoder)))
> +		return;
> +
>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	if (temp & TRANS_DDI_PHSYNC)
>  		flags |= DRM_MODE_FLAG_PHSYNC;
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 842467528892..f1d0a868e80c 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4906,7 +4906,7 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  
>  	intel_set_pipe_timings(intel_crtc);
>  
> -	if (intel_crtc->config->cpu_transcoder != TRANSCODER_EDP) {
> +	if (intel_crtc->config->cpu_transcoder < TRANSCODER_EDP) {
>  		I915_WRITE(PIPE_MULT(intel_crtc->config->cpu_transcoder),
>  			   intel_crtc->config->pixel_multiplier - 1);
>  	}
> @@ -4957,7 +4957,10 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
>  		dev_priv->display.initial_watermarks(pipe_config);
>  	else
>  		intel_update_watermarks(crtc);
> -	intel_enable_pipe(intel_crtc);
> +
> +	/* XXX: Do the pipe assertions at the right place for BXT DSI. */
> +	if (!intel_crtc->config->has_dsi_encoder)
> +		intel_enable_pipe(intel_crtc);
>  
>  	if (intel_crtc->config->has_pch_encoder)
>  		lpt_pch_enable(crtc);
> @@ -5090,7 +5093,9 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
>  	drm_crtc_vblank_off(crtc);
>  	assert_vblank_disabled(crtc);
>  
> -	intel_disable_pipe(intel_crtc);
> +	/* XXX: Do the pipe assertions at the right place for BXT DSI. */
> +	if (!intel_crtc->config->has_dsi_encoder)
> +		intel_disable_pipe(intel_crtc);
>  
>  	if (intel_crtc->config->dp_encoder_is_mst)
>  		intel_ddi_set_vc_payload_alloc(crtc, false);
> @@ -7707,7 +7712,8 @@ static void intel_set_pipe_timings(struct intel_crtc *intel_crtc)
>  	enum pipe pipe = intel_crtc->pipe;
>  	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  
> -	intel_set_trans_timings(intel_crtc);
> +	if (!IS_DSI_TRANSCODER(cpu_transcoder))
> +		intel_set_trans_timings(intel_crtc);

I think we should put all the DSI checks to the top level
.crtc_enable/disable hooks. Will need to do the function splitting a bit
differently perhaps, but I've already commented on those. The main
benefit would be a more immediate view on the modeset sequence without
having to go through all the functions that get called. Also I would
expect it would then be easier to start shifting stuff into the encoder
hooks if we choose to go that route.

>  
>  	/* Workaround: when the EDP input selection is B, the VTOTAL_B must be
>  	 * programmed with the VTOTAL_EDP value. Same for VTOTAL_C. This is
> @@ -7765,9 +7771,11 @@ static void intel_get_pipe_timings(struct intel_crtc *crtc,
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>  	uint32_t tmp;
>  
> -	intel_get_trans_timings(crtc, pipe_config);
> +	if (!IS_DSI_TRANSCODER(cpu_transcoder))
> +		intel_get_trans_timings(crtc, pipe_config);
>  
>  	tmp = I915_READ(PIPESRC(crtc->pipe));
>  	pipe_config->pipe_src_h = (tmp & 0xffff) + 1;
> @@ -8776,9 +8784,11 @@ static void haswell_set_pipeconf(struct drm_crtc *crtc)
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  	enum pipe pipe = intel_crtc->pipe;
> +	enum transcoder cpu_transcoder = intel_crtc->config->cpu_transcoder;
>  	u32 val;
>  
> -	haswell_set_transconf(crtc);
> +	if (!IS_DSI_TRANSCODER(cpu_transcoder))
> +		haswell_set_transconf(crtc);
>  
>  	I915_WRITE(GAMMA_MODE(intel_crtc->pipe), GAMMA_MODE_MODE_8BIT);
>  	POSTING_READ(GAMMA_MODE(intel_crtc->pipe));
> @@ -9936,6 +9946,47 @@ static bool hsw_get_trans_config(struct intel_crtc *crtc,
>  	return tmp & PIPECONF_ENABLE;
>  }
>  
> +static bool bxt_get_dsi_trans_config(struct intel_crtc *crtc,
> +				     struct intel_crtc_state *pipe_config,
> +				     unsigned long *power_domain_mask)
> +{
> +	struct drm_device *dev = crtc->base.dev;
> +	struct drm_i915_private *dev_priv = dev->dev_private;
> +	enum intel_display_power_domain power_domain;
> +	enum port port;
> +	enum transcoder cpu_transcoder;
> +	u32 tmp;
> +
> +	pipe_config->has_dsi_encoder = false;
> +
> +	for_each_port_masked(port, BIT(PORT_A) | BIT(PORT_C)) {
> +		if (port == PORT_A)
> +			cpu_transcoder = TRANSCODER_DSI_A;
> +		else
> +			cpu_transcoder = TRANSCODER_DSI_C;
> +
> +		power_domain = POWER_DOMAIN_TRANSCODER(cpu_transcoder);
> +		if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> +			continue;
> +		*power_domain_mask |= BIT(power_domain);
> +
> +		/* XXX: this works for video mode only */
> +		tmp = I915_READ(BXT_MIPI_PORT_CTRL(port));
> +		if (!(tmp & DPI_ENABLE))
> +			continue;
> +
> +		tmp = I915_READ(MIPI_CTRL(port));
> +		if ((tmp & BXT_PIPE_SELECT_MASK) != BXT_PIPE_SELECT(crtc->pipe))
> +			continue;
> +
> +		pipe_config->cpu_transcoder = cpu_transcoder;
> +		pipe_config->has_dsi_encoder = true;
> +		break;
> +	}
> +
> +	return pipe_config->has_dsi_encoder;
> +}
> +
>  static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>  				       struct intel_crtc_state *pipe_config)
>  {
> @@ -9997,10 +10048,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  
>  	active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
>  
> +	if (IS_BROXTON(dev_priv)) {
> +		bxt_get_dsi_trans_config(crtc, pipe_config, &power_domain_mask);
> +		WARN_ON(active && pipe_config->has_dsi_encoder);
> +		if (pipe_config->has_dsi_encoder)
> +			active = true;
> +	}
> +
>  	if (!active)
>  		goto out;
>  
> -	haswell_get_ddi_port_state(crtc, pipe_config);
> +	if (!pipe_config->has_dsi_encoder)
> +		haswell_get_ddi_port_state(crtc, pipe_config);
>  
>  	intel_get_pipe_timings(crtc, pipe_config);
>  
> @@ -10026,7 +10085,7 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>  		pipe_config->ips_enabled = hsw_crtc_supports_ips(crtc) &&
>  			(I915_READ(IPS_CTL) & IPS_ENABLE);
>  
> -	if (pipe_config->cpu_transcoder != TRANSCODER_EDP) {
> +	if (pipe_config->cpu_transcoder < TRANSCODER_EDP) {
>  		pipe_config->pixel_multiplier =
>  			I915_READ(PIPE_MULT(pipe_config->cpu_transcoder)) + 1;
>  	} else {
> @@ -15516,10 +15575,18 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  {
>  	struct drm_device *dev = crtc->base.dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
> -	i915_reg_t reg = PIPECONF(crtc->config->cpu_transcoder);
> +	enum transcoder cpu_transcoder = crtc->config->cpu_transcoder;
>  
> -	/* Clear any frame start delays used for debugging left by the BIOS */
> -	I915_WRITE(reg, I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
> +	if (!IS_DSI_TRANSCODER(cpu_transcoder)) {
> +		i915_reg_t reg = PIPECONF(cpu_transcoder);
> +
> +		/*
> +		 * Clear any frame start delays used for debugging left by the
> +		 * BIOS.
> +		 */
> +		I915_WRITE(reg,
> +			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
> +	}
>  
>  	/* restore vblank interrupts to correct state */
>  	drm_crtc_vblank_reset(&crtc->base);
> @@ -16195,6 +16262,7 @@ intel_display_capture_error_state(struct drm_device *dev)
>  			error->pipe[i].stat = I915_READ(PIPESTAT(i));
>  	}
>  
> +	/* Note: this does not include DSI transcoders. */
>  	error->num_transcoders = INTEL_INFO(dev)->num_pipes;
>  	if (HAS_DDI(dev_priv->dev))
>  		error->num_transcoders++; /* Account for eDP. */
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 02b3d22862a1..6e8c1386129e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -436,7 +436,8 @@ struct intel_crtc_state {
>  	bool has_infoframe;
>  
>  	/* CPU Transcoder for the pipe. Currently this can only differ from the
> -	 * pipe on Haswell (where we have a special eDP transcoder). */
> +	 * pipe on Haswell and later (where we have a special eDP transcoder)
> +	 * and Broxton (where we have special DSI transcoders). */
>  	enum transcoder cpu_transcoder;
>  
>  	/*
> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
> index 73c15210fdb1..47fd0296a05a 100644
> --- a/drivers/gpu/drm/i915/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/intel_dsi.c
> @@ -268,6 +268,7 @@ static inline bool is_cmd_mode(struct intel_dsi *intel_dsi)
>  static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *pipe_config)
>  {
> +	struct drm_i915_private *dev_priv = encoder->base.dev->dev_private;
>  	struct intel_dsi *intel_dsi = container_of(encoder, struct intel_dsi,
>  						   base);
>  	struct intel_connector *intel_connector = intel_dsi->attached_connector;
> @@ -284,6 +285,14 @@ static bool intel_dsi_compute_config(struct intel_encoder *encoder,
>  	/* DSI uses short packets for sync events, so clear mode flags for DSI */
>  	adjusted_mode->flags = 0;
>  
> +	if (IS_BROXTON(dev_priv)) {
> +		/* Dual link goes to DSI transcoder A. */
> +		if (intel_dsi->ports == BIT(PORT_C))
> +			pipe_config->cpu_transcoder = TRANSCODER_DSI_C;
> +		else
> +			pipe_config->cpu_transcoder = TRANSCODER_DSI_A;
> +	}
> +
>  	return true;
>  }
>  
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 2e88a5e06884..d189a0012277 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -89,6 +89,10 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>  		return "TRANSCODER_C";
>  	case POWER_DOMAIN_TRANSCODER_EDP:
>  		return "TRANSCODER_EDP";
> +	case POWER_DOMAIN_TRANSCODER_DSI_A:
> +		return "TRANSCODER_DSI_A";
> +	case POWER_DOMAIN_TRANSCODER_DSI_C:
> +		return "TRANSCODER_DSI_C";
>  	case POWER_DOMAIN_PORT_DDI_A_LANES:
>  		return "PORT_DDI_A_LANES";
>  	case POWER_DOMAIN_PORT_DDI_B_LANES:
> @@ -419,6 +423,8 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
>  	BIT(POWER_DOMAIN_PIPE_A) |			\
>  	BIT(POWER_DOMAIN_TRANSCODER_EDP) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_DSI_A) |		\
> +	BIT(POWER_DOMAIN_TRANSCODER_DSI_C) |		\
>  	BIT(POWER_DOMAIN_PIPE_A_PANEL_FITTER) |		\
>  	BIT(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
>  	BIT(POWER_DOMAIN_PORT_DSI) |			\
> -- 
> 2.1.4

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 07/10] drm/i915: abstract get config for cpu transcoder

[Jani Nikula]

On Wed, 16 Mar 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> [ text/plain ]
> On Tue, Mar 15, 2016 at 09:51:15PM +0200, Jani Nikula wrote:
>> Makes it neater to add the same for DSI transcoder. No functional
>> changes.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/intel_display.c | 83 ++++++++++++++++++++----------------
>>  1 file changed, 47 insertions(+), 36 deletions(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index e485c1f9ca2b..842467528892 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -9893,6 +9893,49 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
>>  	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
>>  }
>>  
>> +static bool hsw_get_trans_config(struct intel_crtc *crtc,
>> +				 struct intel_crtc_state *pipe_config,
>> +				 unsigned long *power_domain_mask)
>> +{
>
> To be consistent with existing practices we should probably call this 
> ..._get_hw_state, or maybe ..._get_transcoder_state (if we want to
> follow the get_ddi_port_state() below).
>
>> +	struct drm_device *dev = crtc->base.dev;
>> +	struct drm_i915_private *dev_priv = dev->dev_private;
>> +	enum intel_display_power_domain power_domain;
>> +	u32 tmp;
>> +
>> +	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
>> +
>> +	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
>> +	if (tmp & TRANS_DDI_FUNC_ENABLE) {
>
> I guess as a followup we should do the
> intel_display_power_get_if_enabled() for the edp transcoder already
> before this read. Just need to be careful that we don't grab it twice
> then.

I checked this while writing the code, and IIUC
TRANS_DDI_FUNC_CTL(TRANSCODER_EDP) is in always on power.

BR,
Jani.

>
>> +		enum pipe trans_edp_pipe;
>> +		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
>> +		default:
>> +			WARN(1, "unknown pipe linked to edp transcoder\n");
>> +		case TRANS_DDI_EDP_INPUT_A_ONOFF:
>> +		case TRANS_DDI_EDP_INPUT_A_ON:
>> +			trans_edp_pipe = PIPE_A;
>> +			break;
>> +		case TRANS_DDI_EDP_INPUT_B_ONOFF:
>> +			trans_edp_pipe = PIPE_B;
>> +			break;
>> +		case TRANS_DDI_EDP_INPUT_C_ONOFF:
>> +			trans_edp_pipe = PIPE_C;
>> +			break;
>> +		}
>> +
>> +		if (trans_edp_pipe == crtc->pipe)
>> +			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>> +	}
>> +
>> +	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
>> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>> +		return false;
>> +	*power_domain_mask |= BIT(power_domain);
>> +
>> +	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> +
>> +	return tmp & PIPECONF_ENABLE;
>> +}
>> +
>>  static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
>>  				       struct intel_crtc_state *pipe_config)
>>  {
>> @@ -9943,48 +9986,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>  	struct drm_i915_private *dev_priv = dev->dev_private;
>>  	enum intel_display_power_domain power_domain;
>>  	unsigned long power_domain_mask;
>> -	uint32_t tmp;
>> -	bool ret;
>> +	bool active;
>>  
>>  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
>>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>>  		return false;
>>  	power_domain_mask = BIT(power_domain);
>>  
>> -	ret = false;
>> -
>> -	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
>>  	pipe_config->shared_dpll = NULL;
>>  
>> -	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
>> -	if (tmp & TRANS_DDI_FUNC_ENABLE) {
>> -		enum pipe trans_edp_pipe;
>> -		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
>> -		default:
>> -			WARN(1, "unknown pipe linked to edp transcoder\n");
>> -		case TRANS_DDI_EDP_INPUT_A_ONOFF:
>> -		case TRANS_DDI_EDP_INPUT_A_ON:
>> -			trans_edp_pipe = PIPE_A;
>> -			break;
>> -		case TRANS_DDI_EDP_INPUT_B_ONOFF:
>> -			trans_edp_pipe = PIPE_B;
>> -			break;
>> -		case TRANS_DDI_EDP_INPUT_C_ONOFF:
>> -			trans_edp_pipe = PIPE_C;
>> -			break;
>> -		}
>> -
>> -		if (trans_edp_pipe == crtc->pipe)
>> -			pipe_config->cpu_transcoder = TRANSCODER_EDP;
>> -	}
>> -
>> -	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
>> -	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
>> -		goto out;
>> -	power_domain_mask |= BIT(power_domain);
>> +	active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
>>  
>> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
>> -	if (!(tmp & PIPECONF_ENABLE))
>> +	if (!active)
>>  		goto out;
>>  
>>  	haswell_get_ddi_port_state(crtc, pipe_config);
>> @@ -10020,13 +10033,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
>>  		pipe_config->pixel_multiplier = 1;
>>  	}
>>  
>> -	ret = true;
>> -
>>  out:
>>  	for_each_power_domain(power_domain, power_domain_mask)
>>  		intel_display_power_put(dev_priv, power_domain);
>>  
>> -	return ret;
>> +	return active;
>>  }
>>  
>>  static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
>> -- 
>> 2.1.4

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 07/10] drm/i915: abstract get config for cpu transcoder

[Ville Syrjälä]

On Wed, Mar 16, 2016 at 05:26:24PM +0200, Jani Nikula wrote:
> On Wed, 16 Mar 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> > [ text/plain ]
> > On Tue, Mar 15, 2016 at 09:51:15PM +0200, Jani Nikula wrote:
> >> Makes it neater to add the same for DSI transcoder. No functional
> >> changes.
> >> 
> >> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
> >> ---
> >>  drivers/gpu/drm/i915/intel_display.c | 83 ++++++++++++++++++++----------------
> >>  1 file changed, 47 insertions(+), 36 deletions(-)
> >> 
> >> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >> index e485c1f9ca2b..842467528892 100644
> >> --- a/drivers/gpu/drm/i915/intel_display.c
> >> +++ b/drivers/gpu/drm/i915/intel_display.c
> >> @@ -9893,6 +9893,49 @@ static void haswell_get_ddi_pll(struct drm_i915_private *dev_priv,
> >>  	pipe_config->shared_dpll = intel_get_shared_dpll_by_id(dev_priv, id);
> >>  }
> >>  
> >> +static bool hsw_get_trans_config(struct intel_crtc *crtc,
> >> +				 struct intel_crtc_state *pipe_config,
> >> +				 unsigned long *power_domain_mask)
> >> +{
> >
> > To be consistent with existing practices we should probably call this 
> > ..._get_hw_state, or maybe ..._get_transcoder_state (if we want to
> > follow the get_ddi_port_state() below).
> >
> >> +	struct drm_device *dev = crtc->base.dev;
> >> +	struct drm_i915_private *dev_priv = dev->dev_private;
> >> +	enum intel_display_power_domain power_domain;
> >> +	u32 tmp;
> >> +
> >> +	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> >> +
> >> +	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> >> +	if (tmp & TRANS_DDI_FUNC_ENABLE) {
> >
> > I guess as a followup we should do the
> > intel_display_power_get_if_enabled() for the edp transcoder already
> > before this read. Just need to be careful that we don't grab it twice
> > then.
> 
> I checked this while writing the code, and IIUC
> TRANS_DDI_FUNC_CTL(TRANSCODER_EDP) is in always on power.

True, but it does look a bit odd to the eye on the first glance to read
the reg just before grabbing the power reference.

> 
> BR,
> Jani.
> 
> >
> >> +		enum pipe trans_edp_pipe;
> >> +		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
> >> +		default:
> >> +			WARN(1, "unknown pipe linked to edp transcoder\n");
> >> +		case TRANS_DDI_EDP_INPUT_A_ONOFF:
> >> +		case TRANS_DDI_EDP_INPUT_A_ON:
> >> +			trans_edp_pipe = PIPE_A;
> >> +			break;
> >> +		case TRANS_DDI_EDP_INPUT_B_ONOFF:
> >> +			trans_edp_pipe = PIPE_B;
> >> +			break;
> >> +		case TRANS_DDI_EDP_INPUT_C_ONOFF:
> >> +			trans_edp_pipe = PIPE_C;
> >> +			break;
> >> +		}
> >> +
> >> +		if (trans_edp_pipe == crtc->pipe)
> >> +			pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >> +	}
> >> +
> >> +	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
> >> +	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> >> +		return false;
> >> +	*power_domain_mask |= BIT(power_domain);
> >> +
> >> +	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> >> +
> >> +	return tmp & PIPECONF_ENABLE;
> >> +}
> >> +
> >>  static void haswell_get_ddi_port_state(struct intel_crtc *crtc,
> >>  				       struct intel_crtc_state *pipe_config)
> >>  {
> >> @@ -9943,48 +9986,18 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >>  	struct drm_i915_private *dev_priv = dev->dev_private;
> >>  	enum intel_display_power_domain power_domain;
> >>  	unsigned long power_domain_mask;
> >> -	uint32_t tmp;
> >> -	bool ret;
> >> +	bool active;
> >>  
> >>  	power_domain = POWER_DOMAIN_PIPE(crtc->pipe);
> >>  	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> >>  		return false;
> >>  	power_domain_mask = BIT(power_domain);
> >>  
> >> -	ret = false;
> >> -
> >> -	pipe_config->cpu_transcoder = (enum transcoder) crtc->pipe;
> >>  	pipe_config->shared_dpll = NULL;
> >>  
> >> -	tmp = I915_READ(TRANS_DDI_FUNC_CTL(TRANSCODER_EDP));
> >> -	if (tmp & TRANS_DDI_FUNC_ENABLE) {
> >> -		enum pipe trans_edp_pipe;
> >> -		switch (tmp & TRANS_DDI_EDP_INPUT_MASK) {
> >> -		default:
> >> -			WARN(1, "unknown pipe linked to edp transcoder\n");
> >> -		case TRANS_DDI_EDP_INPUT_A_ONOFF:
> >> -		case TRANS_DDI_EDP_INPUT_A_ON:
> >> -			trans_edp_pipe = PIPE_A;
> >> -			break;
> >> -		case TRANS_DDI_EDP_INPUT_B_ONOFF:
> >> -			trans_edp_pipe = PIPE_B;
> >> -			break;
> >> -		case TRANS_DDI_EDP_INPUT_C_ONOFF:
> >> -			trans_edp_pipe = PIPE_C;
> >> -			break;
> >> -		}
> >> -
> >> -		if (trans_edp_pipe == crtc->pipe)
> >> -			pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >> -	}
> >> -
> >> -	power_domain = POWER_DOMAIN_TRANSCODER(pipe_config->cpu_transcoder);
> >> -	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
> >> -		goto out;
> >> -	power_domain_mask |= BIT(power_domain);
> >> +	active = hsw_get_trans_config(crtc, pipe_config, &power_domain_mask);
> >>  
> >> -	tmp = I915_READ(PIPECONF(pipe_config->cpu_transcoder));
> >> -	if (!(tmp & PIPECONF_ENABLE))
> >> +	if (!active)
> >>  		goto out;
> >>  
> >>  	haswell_get_ddi_port_state(crtc, pipe_config);
> >> @@ -10020,13 +10033,11 @@ static bool haswell_get_pipe_config(struct intel_crtc *crtc,
> >>  		pipe_config->pixel_multiplier = 1;
> >>  	}
> >>  
> >> -	ret = true;
> >> -
> >>  out:
> >>  	for_each_power_domain(power_domain, power_domain_mask)
> >>  		intel_display_power_put(dev_priv, power_domain);
> >>  
> >> -	return ret;
> >> +	return active;
> >>  }
> >>  
> >>  static void i845_update_cursor(struct drm_crtc *crtc, u32 base,
> >> -- 
> >> 2.1.4
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [RFC PATCH 04/10] drm/i915/bxt: fix dsi hw state pipe readout

[Jani Nikula]

On Wed, 16 Mar 2016, Ville Syrjälä <ville.syrjala@linux.intel.com> wrote:
> On Tue, Mar 15, 2016 at 09:51:12PM +0200, Jani Nikula wrote:
>> BXT isn't as limited as BYT and CHT regarding DSI pipes and ports.
>> 
>> Signed-off-by: Jani Nikula <jani.nikula@intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_reg.h  |  1 +
>>  drivers/gpu/drm/i915/intel_dsi.c | 14 +++++++++++++-
>>  2 files changed, 14 insertions(+), 1 deletion(-)
>> 
>> diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
>> index 7dfc4007f3fa..a5035991dd57 100644
>> --- a/drivers/gpu/drm/i915/i915_reg.h
>> +++ b/drivers/gpu/drm/i915/i915_reg.h
>> @@ -8144,6 +8144,7 @@ enum skl_disp_power_wells {
>>  #define  READ_REQUEST_PRIORITY_HIGH			(3 << 3)
>>  #define  RGB_FLIP_TO_BGR				(1 << 2)
>>  
>> +#define  BXT_PIPE_SELECT_SHIFT				7
>>  #define  BXT_PIPE_SELECT_MASK				(7 << 7)
>>  #define  BXT_PIPE_SELECT(pipe)				((pipe) << 7)
>
> These defines look a little misplaced, but that's separate topic.
>
> Reviewed-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pushed patches 1-4 to drm-intel-next-queued, as they are mostly
harmless. Fixed the eDP string to match spec in the earlier patch.

BR,
Jani.

>
>>  
>> diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
>> index 6574b9ac3698..73c15210fdb1 100644
>> --- a/drivers/gpu/drm/i915/intel_dsi.c
>> +++ b/drivers/gpu/drm/i915/intel_dsi.c
>> @@ -700,7 +700,19 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
>>  		if (!(I915_READ(MIPI_DEVICE_READY(port)) & DEVICE_READY))
>>  			continue;
>>  
>> -		*pipe = port == PORT_A ? PIPE_A : PIPE_B;
>> +		if (IS_BROXTON(dev_priv)) {
>> +			u32 tmp = I915_READ(MIPI_CTRL(port));
>> +			tmp &= BXT_PIPE_SELECT_MASK;
>> +			tmp >>= BXT_PIPE_SELECT_SHIFT;
>> +
>> +			if (WARN_ON(tmp > PIPE_C))
>> +				continue;
>> +
>> +			*pipe = tmp;
>> +		} else {
>> +			*pipe = port == PORT_A ? PIPE_A : PIPE_B;
>> +		}
>> +
>>  		active = true;
>>  		break;
>>  	}
>> -- 
>> 2.1.4

-- 
Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx