All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup
@ 2017-10-27 19:31 Ville Syrjala
  2017-10-27 19:31 ` [PATCH v2 1/6] drm/i915: Populate output_types from .get_config() Ville Syrjala
                   ` (8 more replies)
  0 siblings, 9 replies; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

I pushed the first four patches of the last series, and split out
populating output_types in .get_config() to a separate patch. Hence
a new series with the leftovers.

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Ville Syrjälä (6):
  drm/i915: Populate output_types from .get_config()
  drm/i915: Stop frobbing with DDI encoder->type
  drm/i915: Nuke intel_ddi_get_encoder_port()
  drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
  drm/i915: Pass a crtc state to ddi post_disable from MST code
  drm/i915: Use intel_ddi_get_config() for MST

 drivers/gpu/drm/i915/i915_debugfs.c   |   2 +-
 drivers/gpu/drm/i915/intel_crt.c      |   2 +
 drivers/gpu/drm/i915/intel_ddi.c      | 149 +++++++++++++++++++---------------
 drivers/gpu/drm/i915/intel_display.c  |  16 ++--
 drivers/gpu/drm/i915/intel_dp.c       |  24 +++---
 drivers/gpu/drm/i915/intel_dp_mst.c   |  54 +-----------
 drivers/gpu/drm/i915/intel_drv.h      |  10 +--
 drivers/gpu/drm/i915/intel_dsi.c      |   2 +
 drivers/gpu/drm/i915/intel_dvo.c      |   2 +
 drivers/gpu/drm/i915/intel_hdmi.c     |  12 +--
 drivers/gpu/drm/i915/intel_lvds.c     |   2 +
 drivers/gpu/drm/i915/intel_opregion.c |   4 +-
 drivers/gpu/drm/i915/intel_sdvo.c     |   2 +
 drivers/gpu/drm/i915/intel_tv.c       |   2 +
 14 files changed, 132 insertions(+), 151 deletions(-)

-- 
2.13.6

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* [PATCH v2 1/6] drm/i915: Populate output_types from .get_config()
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
@ 2017-10-27 19:31 ` Ville Syrjala
  2017-10-27 19:31 ` [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Rather than having the caller of .get_config() set output_types based on
encoder->type, let's just have .get_config() itself populate
output_types. This way we are isolated from encoder->type, which won't
be useable for this purpose anyway soon (at least for DDI encoders).

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     |  2 ++
 drivers/gpu/drm/i915/intel_ddi.c     | 11 +++++++++++
 drivers/gpu/drm/i915/intel_display.c |  5 +----
 drivers/gpu/drm/i915/intel_dp.c      |  5 +++++
 drivers/gpu/drm/i915/intel_dp_mst.c  |  2 ++
 drivers/gpu/drm/i915/intel_dsi.c     |  2 ++
 drivers/gpu/drm/i915/intel_dvo.c     |  2 ++
 drivers/gpu/drm/i915/intel_hdmi.c    |  2 ++
 drivers/gpu/drm/i915/intel_lvds.c    |  2 ++
 drivers/gpu/drm/i915/intel_sdvo.c    |  2 ++
 drivers/gpu/drm/i915/intel_tv.c      |  2 ++
 11 files changed, 33 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 437339f5d098..9c000ac612da 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -119,6 +119,8 @@ static unsigned int intel_crt_get_flags(struct intel_encoder *encoder)
 static void intel_crt_get_config(struct intel_encoder *encoder,
 				 struct intel_crtc_state *pipe_config)
 {
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
+
 	pipe_config->base.adjusted_mode.flags |= intel_crt_get_flags(encoder);
 
 	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 9c118e5305f7..7e0b1a02912a 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2595,12 +2595,23 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			pipe_config->hdmi_high_tmds_clock_ratio = true;
 		/* fall through */
 	case TRANS_DDI_MODE_SELECT_DVI:
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
 		pipe_config->lane_count = 4;
 		break;
 	case TRANS_DDI_MODE_SELECT_FDI:
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_ANALOG);
 		break;
 	case TRANS_DDI_MODE_SELECT_DP_SST:
+		if (encoder->type == INTEL_OUTPUT_EDP)
+			pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
+		else
+			pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
+		pipe_config->lane_count =
+			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
+		intel_dp_get_m_n(intel_crtc, pipe_config);
+		break;
 	case TRANS_DDI_MODE_SELECT_DP_MST:
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
 		pipe_config->lane_count =
 			((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
 		intel_dp_get_m_n(intel_crtc, pipe_config);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a369b35d044d..8f769e9b9342 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11562,10 +11562,8 @@ verify_crtc_state(struct drm_crtc *crtc,
 				"Encoder connected to wrong pipe %c\n",
 				pipe_name(pipe));
 
-		if (active) {
-			pipe_config->output_types |= 1 << encoder->type;
+		if (active)
 			encoder->get_config(encoder, pipe_config);
-		}
 	}
 
 	intel_crtc_compute_pixel_rate(pipe_config);
@@ -14960,7 +14958,6 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 			crtc_state = to_intel_crtc_state(crtc->base.state);
 
 			encoder->base.crtc = &crtc->base;
-			crtc_state->output_types |= 1 << encoder->type;
 			encoder->get_config(encoder, crtc_state);
 		} else {
 			encoder->base.crtc = NULL;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 8d25a019b772..30688a5d680d 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2621,6 +2621,11 @@ static void intel_dp_get_config(struct intel_encoder *encoder,
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct intel_crtc *crtc = to_intel_crtc(encoder->base.crtc);
 
+	if (encoder->type == INTEL_OUTPUT_EDP)
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_EDP);
+	else
+		pipe_config->output_types |= BIT(INTEL_OUTPUT_DP);
+
 	tmp = I915_READ(intel_dp->output_reg);
 
 	pipe_config->has_audio = tmp & DP_AUDIO_OUTPUT_ENABLE && port != PORT_A;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index c34ffa959e90..210ad0580a66 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -291,6 +291,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	u32 temp, flags = 0;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
+
 	pipe_config->has_audio =
 		intel_ddi_is_audio_enabled(dev_priv, crtc);
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index 83f15848098a..2bff7ab25bf3 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -1243,6 +1243,8 @@ static void intel_dsi_get_config(struct intel_encoder *encoder,
 	u32 pclk;
 	DRM_DEBUG_KMS("\n");
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DSI);
+
 	if (IS_GEN9_LP(dev_priv))
 		bxt_dsi_get_pipe_config(encoder, pipe_config);
 
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 53c9b763f4ce..754baa00bea9 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -159,6 +159,8 @@ static void intel_dvo_get_config(struct intel_encoder *encoder,
 	struct intel_dvo *intel_dvo = enc_to_dvo(encoder);
 	u32 tmp, flags = 0;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_DVO);
+
 	tmp = I915_READ(intel_dvo->dev.dvo_reg);
 	if (tmp & DVO_HSYNC_ACTIVE_HIGH)
 		flags |= DRM_MODE_FLAG_PHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index aa486b8925cf..97d08cda3f8e 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -957,6 +957,8 @@ static void intel_hdmi_get_config(struct intel_encoder *encoder,
 	u32 tmp, flags = 0;
 	int dotclock;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_HDMI);
+
 	tmp = I915_READ(intel_hdmi->hdmi_reg);
 
 	if (tmp & SDVO_HSYNC_ACTIVE_HIGH)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 38572d65e46e..ef80499113ee 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -125,6 +125,8 @@ static void intel_lvds_get_config(struct intel_encoder *encoder,
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
 	u32 tmp, flags = 0;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_LVDS);
+
 	tmp = I915_READ(lvds_encoder->reg);
 	if (tmp & LVDS_HSYNC_POLARITY)
 		flags |= DRM_MODE_FLAG_NHSYNC;
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 7437944b388f..42ec2d1f7a61 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -1429,6 +1429,8 @@ static void intel_sdvo_get_config(struct intel_encoder *encoder,
 	u8 val;
 	bool ret;
 
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_SDVO);
+
 	sdvox = I915_READ(intel_sdvo->sdvo_reg);
 
 	ret = intel_sdvo_get_input_timing(intel_sdvo, &dtd);
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index a79a7591b2cf..b18609cebe03 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -868,6 +868,8 @@ static void
 intel_tv_get_config(struct intel_encoder *encoder,
 		    struct intel_crtc_state *pipe_config)
 {
+	pipe_config->output_types |= BIT(INTEL_OUTPUT_TVOUT);
+
 	pipe_config->base.adjusted_mode.crtc_clock = pipe_config->port_clock;
 }
 
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
  2017-10-27 19:31 ` [PATCH v2 1/6] drm/i915: Populate output_types from .get_config() Ville Syrjala
@ 2017-10-27 19:31 ` Ville Syrjala
  2017-10-30  8:59   ` Maarten Lankhorst
  2017-10-27 19:31 ` [PATCH 3/6] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
                   ` (6 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Currently the DDI encoder->type will change at runtime depending on
what kind of hotplugs we've processed. That's quite bad since we can't
really trust that that current value of encoder->type actually matches
the type of signal we're trying to drive through it.

Let's eliminate that problem by declaring that non-eDP DDI port will
always have the encoder type as INTEL_OUTPUT_DDI. This means the code
can no longer try to distinguish DP vs. HDMI based on encoder->type.
We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
there's a bunch of code that relies on that value to identify eDP
encoders.

We'll introduce a new encoder .compute_output_type() hook. This allows
us to compute the full output_types before any encoder .compute_config()
hooks get called, thus those hooks can rely on output_types being
correct, which is useful for cloning on oldr platforms. For now we'll
just look at the connector type and pick the correct mode based on that.
In the future the new hook could be used to implement dynamic switching
between LS and PCON modes for LSPCON.

v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
v4: Rebase
v5: Populate output_types in .get_config() rather than in the caller
v5: Split out populating output_types in .get_config() (Maarten)

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
 drivers/gpu/drm/i915/intel_ddi.c      | 32 ++++++++++++++++++++++++--------
 drivers/gpu/drm/i915/intel_display.c  | 11 ++++++++---
 drivers/gpu/drm/i915/intel_dp.c       | 19 ++++++-------------
 drivers/gpu/drm/i915/intel_drv.h      |  7 +++++--
 drivers/gpu/drm/i915/intel_hdmi.c     | 10 ++--------
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 7 files changed, 47 insertions(+), 36 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index c1e5bba91bff..39883cd915db 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3051,7 +3051,7 @@ static void intel_connector_info(struct seq_file *m,
 		break;
 	case DRM_MODE_CONNECTOR_HDMIA:
 		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
-		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
+		    intel_encoder->type == INTEL_OUTPUT_DDI)
 			intel_hdmi_info(m, intel_connector);
 		break;
 	default:
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 7e0b1a02912a..e5dd281781fe 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
 	switch (encoder->type) {
 	case INTEL_OUTPUT_DP_MST:
 		return enc_to_mst(&encoder->base)->primary->port;
-	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
-	case INTEL_OUTPUT_HDMI:
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 		return enc_to_dig_port(&encoder->base)->port;
 	case INTEL_OUTPUT_ANALOG:
 		return PORT_E;
@@ -1534,6 +1532,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 	uint32_t temp;
+
 	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
 	if (state == true)
 		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
@@ -2652,21 +2651,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
 }
 
+static enum intel_output_type
+intel_ddi_compute_output_type(struct intel_encoder *encoder,
+			      struct intel_crtc_state *crtc_state,
+			      struct drm_connector_state *conn_state)
+{
+	switch (conn_state->connector->connector_type) {
+	case DRM_MODE_CONNECTOR_HDMIA:
+		return INTEL_OUTPUT_HDMI;
+	case DRM_MODE_CONNECTOR_eDP:
+		return INTEL_OUTPUT_EDP;
+	case DRM_MODE_CONNECTOR_DisplayPort:
+		return INTEL_OUTPUT_DP;
+	default:
+		MISSING_CASE(conn_state->connector->connector_type);
+		return INTEL_OUTPUT_UNUSED;
+	}
+}
+
 static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 				     struct intel_crtc_state *pipe_config,
 				     struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	int type = encoder->type;
 	int port = intel_ddi_get_encoder_port(encoder);
 	int ret;
 
-	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
-
 	if (port == PORT_A)
 		pipe_config->cpu_transcoder = TRANSCODER_EDP;
 
-	if (type == INTEL_OUTPUT_HDMI)
+	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
 		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
 	else
 		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
@@ -2815,6 +2829,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
 			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
 
+	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
 	intel_encoder->compute_config = intel_ddi_compute_config;
 	intel_encoder->enable = intel_enable_ddi;
 	if (IS_GEN9_LP(dev_priv))
@@ -2868,9 +2883,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 		max_lanes = 4;
 	}
 
+	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
 	intel_dig_port->max_lanes = max_lanes;
 
-	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
+	intel_encoder->type = INTEL_OUTPUT_DDI;
 	intel_encoder->power_domain = intel_port_to_power_domain(port);
 	intel_encoder->port = port;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8f769e9b9342..737de251d0f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10579,7 +10579,7 @@ static const char * const output_type_str[] = {
 	OUTPUT_TYPE(DP),
 	OUTPUT_TYPE(EDP),
 	OUTPUT_TYPE(DSI),
-	OUTPUT_TYPE(UNKNOWN),
+	OUTPUT_TYPE(DDI),
 	OUTPUT_TYPE(DP_MST),
 };
 
@@ -10750,7 +10750,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 
 		switch (encoder->type) {
 			unsigned int port_mask;
-		case INTEL_OUTPUT_UNKNOWN:
+		case INTEL_OUTPUT_DDI:
 			if (WARN_ON(!HAS_DDI(to_i915(dev))))
 				break;
 		case INTEL_OUTPUT_DP:
@@ -10883,7 +10883,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 		 * Determine output_types before calling the .compute_config()
 		 * hooks so that the hooks can use this information safely.
 		 */
-		pipe_config->output_types |= 1 << encoder->type;
+		if (encoder->compute_output_type)
+			pipe_config->output_types |=
+				BIT(encoder->compute_output_type(encoder, pipe_config,
+								 connector_state));
+		else
+			pipe_config->output_types |= BIT(encoder->type);
 	}
 
 encoder_retry:
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 30688a5d680d..f0c49962ffbe 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
 		struct intel_dp *intel_dp;
 
 		if (encoder->type != INTEL_OUTPUT_DP &&
-		    encoder->type != INTEL_OUTPUT_EDP)
+		    encoder->type != INTEL_OUTPUT_EDP &&
+		    encoder->type != INTEL_OUTPUT_DDI)
 			continue;
 
 		intel_dp = enc_to_intel_dp(&encoder->base);
 
+		/* Skip pure DVI/HDMI DDI encoders */
+		if (!i915_mmio_reg_valid(intel_dp->output_reg))
+			continue;
+
 		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
 
 		if (encoder->type != INTEL_OUTPUT_EDP)
@@ -4733,8 +4738,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 {
 	struct drm_connector *connector = &intel_connector->base;
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
-	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
 	u8 sink_irq_vector = 0;
@@ -4767,9 +4770,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 		goto out;
 	}
 
-	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DP;
-
 	if (intel_dp->reset_link_params) {
 		/* Initial max link lane count */
 		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
@@ -4886,9 +4886,6 @@ intel_dp_force(struct drm_connector *connector)
 	intel_dp_set_edid(intel_dp);
 
 	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
-
-	if (intel_encoder->type != INTEL_OUTPUT_EDP)
-		intel_encoder->type = INTEL_OUTPUT_DP;
 }
 
 static int intel_dp_get_modes(struct drm_connector *connector)
@@ -5107,10 +5104,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	enum irqreturn ret = IRQ_NONE;
 
-	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
-	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
-		intel_dig_port->base.type = INTEL_OUTPUT_DP;
-
 	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
 		/*
 		 * vdd off can generate a long pulse on eDP which
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index df966427232c..629ebc73bb09 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -173,7 +173,7 @@ enum intel_output_type {
 	INTEL_OUTPUT_DP = 7,
 	INTEL_OUTPUT_EDP = 8,
 	INTEL_OUTPUT_DSI = 9,
-	INTEL_OUTPUT_UNKNOWN = 10,
+	INTEL_OUTPUT_DDI = 10,
 	INTEL_OUTPUT_DP_MST = 11,
 };
 
@@ -216,6 +216,9 @@ struct intel_encoder {
 	enum port port;
 	unsigned int cloneable;
 	void (*hot_plug)(struct intel_encoder *);
+	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
+						      struct intel_crtc_state *,
+						      struct drm_connector_state *);
 	bool (*compute_config)(struct intel_encoder *,
 			       struct intel_crtc_state *,
 			       struct drm_connector_state *);
@@ -1152,7 +1155,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
 	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
 
 	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
 	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_EDP:
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 97d08cda3f8e..dede2898cb8a 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1617,12 +1617,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 
 	intel_hdmi_unset_edid(connector);
 
-	if (intel_hdmi_set_edid(connector)) {
-		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
-		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
+	if (intel_hdmi_set_edid(connector))
 		status = connector_status_connected;
-	} else
+	else
 		status = connector_status_disconnected;
 
 	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
@@ -1633,8 +1630,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
 static void
 intel_hdmi_force(struct drm_connector *connector)
 {
-	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
 
@@ -1644,7 +1639,6 @@ intel_hdmi_force(struct drm_connector *connector)
 		return;
 
 	intel_hdmi_set_edid(connector);
-	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
 }
 
 static int intel_hdmi_get_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 1d946240e55f..39714be3eb6b 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 	case INTEL_OUTPUT_ANALOG:
 		type = DISPLAY_TYPE_CRT;
 		break;
-	case INTEL_OUTPUT_UNKNOWN:
+	case INTEL_OUTPUT_DDI:
 	case INTEL_OUTPUT_DP:
 	case INTEL_OUTPUT_HDMI:
 	case INTEL_OUTPUT_DP_MST:
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/6] drm/i915: Nuke intel_ddi_get_encoder_port()
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
  2017-10-27 19:31 ` [PATCH v2 1/6] drm/i915: Populate output_types from .get_config() Ville Syrjala
  2017-10-27 19:31 ` [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
@ 2017-10-27 19:31 ` Ville Syrjala
  2017-10-27 19:31 ` [PATCH v2 4/6] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

encoder->port works for FDI, and it also works for MST (regardless of
whether we're dealing with the "fake" MST encoder, or mst->primary).
So let's eliminate intel_ddi_get_encoder_port().

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c      | 50 ++++++++++++-----------------------
 drivers/gpu/drm/i915/intel_drv.h      |  1 -
 drivers/gpu/drm/i915/intel_opregion.c |  2 +-
 3 files changed, 18 insertions(+), 35 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index e5dd281781fe..aaaca906c97f 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -492,22 +492,6 @@ static const struct cnl_ddi_buf_trans cnl_ddi_translations_edp_1_05V[] = {
 	{ 0x2, 0x7F, 0x3F, 0x00, 0x00 },	/* 400   400      0.0   */
 };
 
-enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
-{
-	switch (encoder->type) {
-	case INTEL_OUTPUT_DP_MST:
-		return enc_to_mst(&encoder->base)->primary->port;
-	case INTEL_OUTPUT_EDP:
-	case INTEL_OUTPUT_DDI:
-		return enc_to_dig_port(&encoder->base)->port;
-	case INTEL_OUTPUT_ANALOG:
-		return PORT_E;
-	default:
-		MISSING_CASE(encoder->type);
-		return PORT_A;
-	}
-}
-
 static const struct ddi_buf_trans *
 bdw_get_buf_trans_edp(struct drm_i915_private *dev_priv, int *n_entries)
 {
@@ -815,7 +799,7 @@ static void intel_prepare_dp_ddi_buffers(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
 	int i, n_entries;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	const struct ddi_buf_trans *ddi_translations;
 
 	if (intel_crtc_has_type(crtc_state, INTEL_OUTPUT_ANALOG))
@@ -852,7 +836,7 @@ static void intel_prepare_hdmi_ddi_buffers(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	u32 iboost_bit = 0;
 	int n_entries;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	const struct ddi_buf_trans *ddi_translations;
 
 	ddi_translations = intel_ddi_get_buf_trans_hdmi(dev_priv, &n_entries);
@@ -1468,7 +1452,7 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
 				struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	enum intel_dpll_id pll_id = port;
 
 	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, pll_id);
@@ -1548,7 +1532,7 @@ void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state)
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	uint32_t temp;
 
 	/* Enable TRANS_DDI_FUNC_CTL for the pipe to work in HDMI mode */
@@ -1644,7 +1628,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_encoder *encoder = intel_connector->encoder;
 	int type = intel_connector->base.connector_type;
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	enum pipe pipe = 0;
 	enum transcoder cpu_transcoder;
 	uint32_t tmp;
@@ -1703,7 +1687,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 {
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	u32 tmp;
 	int i;
 	bool ret;
@@ -1788,7 +1772,7 @@ void intel_ddi_enable_pipe_clock(const struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	struct intel_encoder *encoder = intel_ddi_get_crtc_encoder(crtc);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
 
 	if (cpu_transcoder != TRANSCODER_EDP)
@@ -1927,8 +1911,8 @@ static void cnl_ddi_vswing_program(struct intel_encoder *encoder,
 				   int level, enum intel_output_type type)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
 	const struct cnl_ddi_buf_trans *ddi_translations;
+	enum port port = encoder->port;
 	int n_entries, ln;
 	u32 val;
 
@@ -1991,7 +1975,7 @@ static void cnl_ddi_vswing_sequence(struct intel_encoder *encoder,
 				    int level, enum intel_output_type type)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	int width, rate, ln;
 	u32 val;
 
@@ -2110,7 +2094,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 				 const struct intel_shared_dpll *pll)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	uint32_t val;
 
 	if (WARN_ON(!pll))
@@ -2149,7 +2133,7 @@ static void intel_ddi_clk_select(struct intel_encoder *encoder,
 static void intel_ddi_clk_disable(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 
 	if (IS_CANNONLAKE(dev_priv))
 		I915_WRITE(DPCLKA_CFGCR0, I915_READ(DPCLKA_CFGCR0) |
@@ -2167,7 +2151,7 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 {
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	bool is_mst = intel_crtc_has_type(crtc_state, INTEL_OUTPUT_DP_MST);
 	int level = intel_ddi_dp_level(intel_dp);
@@ -2205,7 +2189,7 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	struct intel_digital_port *intel_dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_hdmi *intel_hdmi = &intel_dig_port->hdmi;
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	int level = intel_ddi_hdmi_level(dev_priv, port);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
@@ -2250,7 +2234,7 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 static void intel_disable_ddi_buf(struct intel_encoder *encoder)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	bool wait = false;
 	u32 val;
 
@@ -2379,7 +2363,7 @@ static void intel_enable_ddi_dp(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dp *intel_dp = enc_to_intel_dp(&encoder->base);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 
 	if (port == PORT_A && INTEL_GEN(dev_priv) < 9)
 		intel_dp_stop_link_train(intel_dp);
@@ -2398,7 +2382,7 @@ static void intel_enable_ddi_hdmi(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
-	enum port port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 
 	intel_hdmi_handle_sink_scrambling(encoder,
 					  conn_state->connector,
@@ -2674,7 +2658,7 @@ static bool intel_ddi_compute_config(struct intel_encoder *encoder,
 				     struct drm_connector_state *conn_state)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	int port = intel_ddi_get_encoder_port(encoder);
+	enum port port = encoder->port;
 	int ret;
 
 	if (port == PORT_A)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 629ebc73bb09..3828d9bee7f5 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1279,7 +1279,6 @@ void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
 void hsw_fdi_link_train(struct intel_crtc *crtc,
 			const struct intel_crtc_state *crtc_state);
 void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port);
-enum port intel_ddi_get_encoder_port(struct intel_encoder *intel_encoder);
 bool intel_ddi_get_hw_state(struct intel_encoder *encoder, enum pipe *pipe);
 void intel_ddi_enable_transcoder_func(const struct intel_crtc_state *crtc_state);
 void intel_ddi_disable_transcoder_func(struct drm_i915_private *dev_priv,
diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
index 39714be3eb6b..fc65f5e451dd 100644
--- a/drivers/gpu/drm/i915/intel_opregion.c
+++ b/drivers/gpu/drm/i915/intel_opregion.c
@@ -367,7 +367,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
 	if (intel_encoder->type == INTEL_OUTPUT_DSI)
 		port = 0;
 	else
-		port = intel_ddi_get_encoder_port(intel_encoder);
+		port = intel_encoder->port;
 
 	if (port == PORT_E)  {
 		port = 0;
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 4/6] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (2 preceding siblings ...)
  2017-10-27 19:31 ` [PATCH 3/6] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
@ 2017-10-27 19:31 ` Ville Syrjala
  2017-10-27 19:31 ` [PATCH v2 5/6] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

We should be using the DPLL hw state we got from the current crtc state
to determine the corresponding port clock frequency rather than getting
it via the current state programmed into the DPLL.

v2: Rebase due to intel_dpll_id changes

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c | 17 +++++------------
 1 file changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index aaaca906c97f..8183304c7d34 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1423,19 +1423,16 @@ static void hsw_ddi_clock_get(struct intel_encoder *encoder,
 	ddi_dotclock_get(pipe_config);
 }
 
-static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
-			     enum intel_dpll_id pll_id)
+static int bxt_calc_pll_link(struct intel_crtc_state *crtc_state)
 {
-	struct intel_shared_dpll *pll;
 	struct intel_dpll_hw_state *state;
 	struct dpll clock;
 
 	/* For DDI ports we always use a shared PLL. */
-	if (WARN_ON(pll_id == DPLL_ID_PRIVATE))
+	if (WARN_ON(!crtc_state->shared_dpll))
 		return 0;
 
-	pll = &dev_priv->shared_dplls[pll_id];
-	state = &pll->state.hw_state;
+	state = &crtc_state->dpll_hw_state;
 
 	clock.m1 = 2;
 	clock.m2 = (state->pll0 & PORT_PLL_M2_MASK) << 22;
@@ -1449,13 +1446,9 @@ static int bxt_calc_pll_link(struct drm_i915_private *dev_priv,
 }
 
 static void bxt_ddi_clock_get(struct intel_encoder *encoder,
-				struct intel_crtc_state *pipe_config)
+			      struct intel_crtc_state *pipe_config)
 {
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum port port = encoder->port;
-	enum intel_dpll_id pll_id = port;
-
-	pipe_config->port_clock = bxt_calc_pll_link(dev_priv, pll_id);
+	pipe_config->port_clock = bxt_calc_pll_link(pipe_config);
 
 	ddi_dotclock_get(pipe_config);
 }
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 5/6] drm/i915: Pass a crtc state to ddi post_disable from MST code
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (3 preceding siblings ...)
  2017-10-27 19:31 ` [PATCH v2 4/6] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
@ 2017-10-27 19:31 ` Ville Syrjala
  2017-10-27 19:31 ` [PATCH v2 6/6] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Pass an old crtc state to intel_ddi_post_disable() from the MST code.

Note that this crtc state won't necessaitly match the one that was
passed to intel_ddi_pre_enable() if the first stream to be enabled isn't
the last stream to be disabled. But this is fine since the states should
be identical in every important way. This does mean people frobbing
the DDI pre_enable/post_disable hooks have to pay attention in what
parts of the state they consult.

The alternative would be to inline the relevant code into the MST code.
That is actually what we used to do for pre_enable before
commit e081c8463ac9 ("drm/i915: Remove duplicate DDI enabling logic
from MST path"). For post_disable we've always called the DDI hook.

v2: Pimp up the comments explaining the MST issues

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    | 37 ++++++++++++++++++++++++++-----------
 drivers/gpu/drm/i915/intel_dp_mst.c |  6 +++---
 2 files changed, 29 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 8183304c7d34..0f603a157dba 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -2214,6 +2214,19 @@ static void intel_ddi_pre_enable(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
 	enum pipe pipe = crtc->pipe;
 
+	/*
+	 * When called from DP MST code:
+	 * - conn_state will be NULL
+	 * - encoder will be the main encoder (ie. mst->primary)
+	 * - the main connector associated with this port
+	 *   won't be active or linked to a crtc
+	 * - crtc_state will be the state of the first stream to
+	 *   be activated on this port, and it may not be the same
+	 *   stream that will be deactivated last, but each stream
+	 *   should have a state that is identical when it comes to
+	 *   DP link parameteres
+	 */
+
 	WARN_ON(crtc_state->has_pch_encoder);
 
 	intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
@@ -2254,12 +2267,7 @@ static void intel_ddi_post_disable_dp(struct intel_encoder *encoder,
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 	struct intel_dp *intel_dp = &dig_port->dp;
-	/*
-	 * old_crtc_state and old_conn_state are NULL when called from
-	 * DP_MST. The main connector associated with this port is never
-	 * bound to a crtc for MST.
-	 */
-	bool is_mst = !old_crtc_state;
+	bool is_mst = intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_DP_MST);
 
 	/*
 	 * Power down sink before disabling the port, otherwise we end
@@ -2303,12 +2311,19 @@ static void intel_ddi_post_disable(struct intel_encoder *encoder,
 				   const struct drm_connector_state *old_conn_state)
 {
 	/*
-	 * old_crtc_state and old_conn_state are NULL when called from
-	 * DP_MST. The main connector associated with this port is never
-	 * bound to a crtc for MST.
+	 * When called from DP MST code:
+	 * - old_conn_state will be NULL
+	 * - encoder will be the main encoder (ie. mst->primary)
+	 * - the main connector associated with this port
+	 *   won't be active or linked to a crtc
+	 * - old_crtc_state will be the state of the last stream to
+	 *   be deactivated on this port, and it may not be the same
+	 *   stream that was activated last, but each stream
+	 *   should have a state that is identical when it comes to
+	 *   the DP link parameteres
 	 */
-	if (old_crtc_state &&
-	    intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
+
+	if (intel_crtc_has_type(old_crtc_state, INTEL_OUTPUT_HDMI))
 		intel_ddi_post_disable_hdmi(encoder,
 					    old_crtc_state, old_conn_state);
 	else
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 210ad0580a66..d523302e5081 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -179,10 +179,10 @@ static void intel_mst_post_disable_dp(struct intel_encoder *encoder,
 	intel_dp->active_mst_links--;
 
 	intel_mst->connector = NULL;
-	if (intel_dp->active_mst_links == 0) {
+	if (intel_dp->active_mst_links == 0)
 		intel_dig_port->base.post_disable(&intel_dig_port->base,
-						  NULL, NULL);
-	}
+						  old_crtc_state, NULL);
+
 	DRM_DEBUG_KMS("active links %d\n", intel_dp->active_mst_links);
 }
 
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH v2 6/6] drm/i915: Use intel_ddi_get_config() for MST
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (4 preceding siblings ...)
  2017-10-27 19:31 ` [PATCH v2 5/6] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
@ 2017-10-27 19:31 ` Ville Syrjala
  2017-10-30  9:00   ` Maarten Lankhorst
  2017-10-27 20:47 ` ✗ Fi.CI.BAT: failure for drm/i915: Last part of DDI encoder->type cleanup (rev2) Patchwork
                   ` (2 subsequent siblings)
  8 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjala @ 2017-10-27 19:31 UTC (permalink / raw)
  To: intel-gfx

From: Ville Syrjälä <ville.syrjala@linux.intel.com>

Eliminate the partially duplicated DDI readout code from MST, and
instead just call intel_ddi_get_config(). As a nice bonus we get
more cross checking as intel_ddi_get_config() will populate
output_types based on the actual mode of the DDI port.

Additonally intel_ddi_get_config() must be changed to get the crtc
from the passed in crtc state rather than from the encoder->crtc link.
encoder->crtc really shouldn't be used anyway.

v2: Rebased on BXT MST latency_optim fix
    Make intel_ddi_clock_get() static

Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c    |  6 ++---
 drivers/gpu/drm/i915/intel_dp_mst.c | 50 +------------------------------------
 drivers/gpu/drm/i915/intel_drv.h    |  2 --
 3 files changed, 4 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 0f603a157dba..cc3d55b5cea0 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1453,8 +1453,8 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
 	ddi_dotclock_get(pipe_config);
 }
 
-void intel_ddi_clock_get(struct intel_encoder *encoder,
-			 struct intel_crtc_state *pipe_config)
+static void intel_ddi_clock_get(struct intel_encoder *encoder,
+				struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 
@@ -2533,7 +2533,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config)
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
+	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
 	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
 	struct intel_digital_port *intel_dig_port;
 	u32 temp, flags = 0;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index d523302e5081..6f11bb35f66f 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -286,56 +286,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
 {
 	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
 	struct intel_digital_port *intel_dig_port = intel_mst->primary;
-	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
-	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
-	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
-	u32 temp, flags = 0;
 
-	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
-
-	pipe_config->has_audio =
-		intel_ddi_is_audio_enabled(dev_priv, crtc);
-
-	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
-	if (temp & TRANS_DDI_PHSYNC)
-		flags |= DRM_MODE_FLAG_PHSYNC;
-	else
-		flags |= DRM_MODE_FLAG_NHSYNC;
-	if (temp & TRANS_DDI_PVSYNC)
-		flags |= DRM_MODE_FLAG_PVSYNC;
-	else
-		flags |= DRM_MODE_FLAG_NVSYNC;
-
-	switch (temp & TRANS_DDI_BPC_MASK) {
-	case TRANS_DDI_BPC_6:
-		pipe_config->pipe_bpp = 18;
-		break;
-	case TRANS_DDI_BPC_8:
-		pipe_config->pipe_bpp = 24;
-		break;
-	case TRANS_DDI_BPC_10:
-		pipe_config->pipe_bpp = 30;
-		break;
-	case TRANS_DDI_BPC_12:
-		pipe_config->pipe_bpp = 36;
-		break;
-	default:
-		break;
-	}
-	pipe_config->base.adjusted_mode.flags |= flags;
-
-	pipe_config->lane_count =
-		((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
-
-	intel_dp_get_m_n(crtc, pipe_config);
-
-	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
-
-	if (IS_GEN9_LP(dev_priv))
-		pipe_config->lane_lat_optim_mask =
-			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
-
-	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
+	intel_ddi_get_config(&intel_dig_port->base, pipe_config);
 }
 
 static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 3828d9bee7f5..4498d743cebc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1295,8 +1295,6 @@ bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
 void intel_ddi_get_config(struct intel_encoder *encoder,
 			  struct intel_crtc_state *pipe_config);
 
-void intel_ddi_clock_get(struct intel_encoder *encoder,
-			 struct intel_crtc_state *pipe_config);
 void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
 				    bool state);
 void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,
-- 
2.13.6

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

^ permalink raw reply related	[flat|nested] 15+ messages in thread

* ✗ Fi.CI.BAT: failure for drm/i915: Last part of DDI encoder->type cleanup (rev2)
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (5 preceding siblings ...)
  2017-10-27 19:31 ` [PATCH v2 6/6] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
@ 2017-10-27 20:47 ` Patchwork
  2017-10-30 15:58 ` ✓ Fi.CI.BAT: success " Patchwork
  2017-10-30 17:20 ` ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-10-27 20:47 UTC (permalink / raw)
  To: Ville Syrjala; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Last part of DDI encoder->type cleanup (rev2)
URL   : https://patchwork.freedesktop.org/series/32298/
State : failure

== Summary ==

Series 32298v2 drm/i915: Last part of DDI encoder->type cleanup
https://patchwork.freedesktop.org/api/1.0/series/32298/revisions/2/mbox/

Test gem_exec_reloc:
        Subgroup basic-cpu-active:
                pass       -> FAIL       (fi-gdg-551) fdo#102582
        Subgroup basic-write-cpu-active:
                pass       -> FAIL       (fi-gdg-551)
Test kms_pipe_crc_basic:
        Subgroup nonblocking-crc-pipe-a:
                pass       -> DMESG-WARN (fi-skl-6700k)
        Subgroup nonblocking-crc-pipe-a-frame-sequence:
                pass       -> INCOMPLETE (fi-skl-6700k)

fdo#102582 https://bugs.freedesktop.org/show_bug.cgi?id=102582

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:437s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:454s
fi-blb-e6850     total:289  pass:223  dwarn:1   dfail:0   fail:0   skip:65  time:378s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:536s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:264s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:497s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:496s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:491s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:479s
fi-cnl-y         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:607s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:425s
fi-gdg-551       total:289  pass:176  dwarn:1   dfail:0   fail:3   skip:109 time:249s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:577s
fi-glk-dsi       total:289  pass:258  dwarn:0   dfail:0   fail:1   skip:30  time:489s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:432s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:428s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:421s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:486s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:461s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:487s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:567s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:481s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:584s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:542s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:454s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:594s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:649s
fi-skl-6700k     total:234  pass:212  dwarn:1   dfail:0   fail:0   skip:20 
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:505s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:465s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:563s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:415s

d0582552491e17f5e386747f82147c1f2d9158c9 drm-tip: 2017y-10m-27d-19h-16m-21s UTC integration manifest
508737d83ea7 drm/i915: Use intel_ddi_get_config() for MST
d40e880967de drm/i915: Pass a crtc state to ddi post_disable from MST code
0b5f468e957b drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
39264e0a9b70 drm/i915: Nuke intel_ddi_get_encoder_port()
36f9b4f25287 drm/i915: Stop frobbing with DDI encoder->type
e24ccd93d7a4 drm/i915: Populate output_types from .get_config()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6248/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-27 19:31 ` [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
@ 2017-10-30  8:59   ` Maarten Lankhorst
  2017-10-30 16:07     ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-10-30  8:59 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 27-10-17 om 21:31 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Currently the DDI encoder->type will change at runtime depending on
> what kind of hotplugs we've processed. That's quite bad since we can't
> really trust that that current value of encoder->type actually matches
> the type of signal we're trying to drive through it.
>
> Let's eliminate that problem by declaring that non-eDP DDI port will
> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> can no longer try to distinguish DP vs. HDMI based on encoder->type.
> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> there's a bunch of code that relies on that value to identify eDP
> encoders.
>
> We'll introduce a new encoder .compute_output_type() hook. This allows
> us to compute the full output_types before any encoder .compute_config()
> hooks get called, thus those hooks can rely on output_types being
> correct, which is useful for cloning on oldr platforms. For now we'll
> just look at the connector type and pick the correct mode based on that.
> In the future the new hook could be used to implement dynamic switching
> between LS and PCON modes for LSPCON.
>
> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> v4: Rebase
> v5: Populate output_types in .get_config() rather than in the caller
> v5: Split out populating output_types in .get_config() (Maarten)
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
>  drivers/gpu/drm/i915/intel_ddi.c      | 32 ++++++++++++++++++++++++--------
>  drivers/gpu/drm/i915/intel_display.c  | 11 ++++++++---
>  drivers/gpu/drm/i915/intel_dp.c       | 19 ++++++-------------
>  drivers/gpu/drm/i915/intel_drv.h      |  7 +++++--
>  drivers/gpu/drm/i915/intel_hdmi.c     | 10 ++--------
>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>  7 files changed, 47 insertions(+), 36 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index c1e5bba91bff..39883cd915db 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -3051,7 +3051,7 @@ static void intel_connector_info(struct seq_file *m,
>  		break;
>  	case DRM_MODE_CONNECTOR_HDMIA:
>  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> +		    intel_encoder->type == INTEL_OUTPUT_DDI)
>  			intel_hdmi_info(m, intel_connector);
>  		break;
>  	default:
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 7e0b1a02912a..e5dd281781fe 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
>  	switch (encoder->type) {
>  	case INTEL_OUTPUT_DP_MST:
>  		return enc_to_mst(&encoder->base)->primary->port;
> -	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
> -	case INTEL_OUTPUT_HDMI:
> -	case INTEL_OUTPUT_UNKNOWN:
> +	case INTEL_OUTPUT_DDI:
>  		return enc_to_dig_port(&encoder->base)->port;
>  	case INTEL_OUTPUT_ANALOG:
>  		return PORT_E;
> @@ -1534,6 +1532,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>  	uint32_t temp;
> +
>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>  	if (state == true)
>  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> @@ -2652,21 +2651,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>  }
>  
> +static enum intel_output_type
> +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> +			      struct intel_crtc_state *crtc_state,
> +			      struct drm_connector_state *conn_state)
> +{
> +	switch (conn_state->connector->connector_type) {
> +	case DRM_MODE_CONNECTOR_HDMIA:
> +		return INTEL_OUTPUT_HDMI;
> +	case DRM_MODE_CONNECTOR_eDP:
> +		return INTEL_OUTPUT_EDP;
> +	case DRM_MODE_CONNECTOR_DisplayPort:
> +		return INTEL_OUTPUT_DP;
> +	default:
> +		MISSING_CASE(conn_state->connector->connector_type);
> +		return INTEL_OUTPUT_UNUSED;
> +	}
> +}
> +
>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>  				     struct intel_crtc_state *pipe_config,
>  				     struct drm_connector_state *conn_state)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	int type = encoder->type;
>  	int port = intel_ddi_get_encoder_port(encoder);
>  	int ret;
>  
> -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> -
>  	if (port == PORT_A)
>  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
>  
> -	if (type == INTEL_OUTPUT_HDMI)
> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
>  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
>  	else
>  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);

Would it be an option to put it in compute_config for DDI only? There is no risk of cloning there, saves another hook. :)

Either way is fine though.
> @@ -2815,6 +2829,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
>  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
>  
> +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
>  	intel_encoder->compute_config = intel_ddi_compute_config;
>  	intel_encoder->enable = intel_enable_ddi;
>  	if (IS_GEN9_LP(dev_priv))
> @@ -2868,9 +2883,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  		max_lanes = 4;
>  	}
>  
> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>  	intel_dig_port->max_lanes = max_lanes;
>  
> -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> +	intel_encoder->type = INTEL_OUTPUT_DDI;
>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
>  	intel_encoder->port = port;
>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 8f769e9b9342..737de251d0f8 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -10579,7 +10579,7 @@ static const char * const output_type_str[] = {
>  	OUTPUT_TYPE(DP),
>  	OUTPUT_TYPE(EDP),
>  	OUTPUT_TYPE(DSI),
> -	OUTPUT_TYPE(UNKNOWN),
> +	OUTPUT_TYPE(DDI),
>  	OUTPUT_TYPE(DP_MST),
>  };
>  
> @@ -10750,7 +10750,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>  
>  		switch (encoder->type) {
>  			unsigned int port_mask;
> -		case INTEL_OUTPUT_UNKNOWN:
> +		case INTEL_OUTPUT_DDI:
>  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
>  				break;
>  		case INTEL_OUTPUT_DP:
> @@ -10883,7 +10883,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>  		 * Determine output_types before calling the .compute_config()
>  		 * hooks so that the hooks can use this information safely.
>  		 */
> -		pipe_config->output_types |= 1 << encoder->type;
> +		if (encoder->compute_output_type)
> +			pipe_config->output_types |=
> +				BIT(encoder->compute_output_type(encoder, pipe_config,
> +								 connector_state));
> +		else
> +			pipe_config->output_types |= BIT(encoder->type);
>  	}
>  
>  encoder_retry:
> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> index 30688a5d680d..f0c49962ffbe 100644
> --- a/drivers/gpu/drm/i915/intel_dp.c
> +++ b/drivers/gpu/drm/i915/intel_dp.c
> @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
>  		struct intel_dp *intel_dp;
>  
>  		if (encoder->type != INTEL_OUTPUT_DP &&
> -		    encoder->type != INTEL_OUTPUT_EDP)
> +		    encoder->type != INTEL_OUTPUT_EDP &&
> +		    encoder->type != INTEL_OUTPUT_DDI)
>  			continue;
>  
>  		intel_dp = enc_to_intel_dp(&encoder->base);
>  
> +		/* Skip pure DVI/HDMI DDI encoders */
> +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> +			continue;
> +
>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>  
>  		if (encoder->type != INTEL_OUTPUT_EDP)
> @@ -4733,8 +4738,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  {
>  	struct drm_connector *connector = &intel_connector->base;
>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>  	struct drm_device *dev = connector->dev;
>  	enum drm_connector_status status;
>  	u8 sink_irq_vector = 0;
> @@ -4767,9 +4770,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>  		goto out;
>  	}
>  
> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -		intel_encoder->type = INTEL_OUTPUT_DP;
> -
>  	if (intel_dp->reset_link_params) {
>  		/* Initial max link lane count */
>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> @@ -4886,9 +4886,6 @@ intel_dp_force(struct drm_connector *connector)
>  	intel_dp_set_edid(intel_dp);
>  
>  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> -
> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> -		intel_encoder->type = INTEL_OUTPUT_DP;
>  }
>  
>  static int intel_dp_get_modes(struct drm_connector *connector)
> @@ -5107,10 +5104,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>  	struct drm_i915_private *dev_priv = to_i915(dev);
>  	enum irqreturn ret = IRQ_NONE;
>  
> -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> -
>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
>  		/*
>  		 * vdd off can generate a long pulse on eDP which
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index df966427232c..629ebc73bb09 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -173,7 +173,7 @@ enum intel_output_type {
>  	INTEL_OUTPUT_DP = 7,
>  	INTEL_OUTPUT_EDP = 8,
>  	INTEL_OUTPUT_DSI = 9,
> -	INTEL_OUTPUT_UNKNOWN = 10,
> +	INTEL_OUTPUT_DDI = 10,
>  	INTEL_OUTPUT_DP_MST = 11,
>  };
>  
> @@ -216,6 +216,9 @@ struct intel_encoder {
>  	enum port port;
>  	unsigned int cloneable;
>  	void (*hot_plug)(struct intel_encoder *);
> +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> +						      struct intel_crtc_state *,
> +						      struct drm_connector_state *);
>  	bool (*compute_config)(struct intel_encoder *,
>  			       struct intel_crtc_state *,
>  			       struct drm_connector_state *);
> @@ -1152,7 +1155,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>  
>  	switch (intel_encoder->type) {
> -	case INTEL_OUTPUT_UNKNOWN:
> +	case INTEL_OUTPUT_DDI:
>  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
>  	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_EDP:
> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> index 97d08cda3f8e..dede2898cb8a 100644
> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> @@ -1617,12 +1617,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  
>  	intel_hdmi_unset_edid(connector);
>  
> -	if (intel_hdmi_set_edid(connector)) {
> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> -
> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> +	if (intel_hdmi_set_edid(connector))
>  		status = connector_status_connected;
> -	} else
> +	else
>  		status = connector_status_disconnected;
>  
>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> @@ -1633,8 +1630,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>  static void
>  intel_hdmi_force(struct drm_connector *connector)
>  {
> -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> -
>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>  		      connector->base.id, connector->name);
>  
> @@ -1644,7 +1639,6 @@ intel_hdmi_force(struct drm_connector *connector)
>  		return;
>  
>  	intel_hdmi_set_edid(connector);
> -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>  }
>  
>  static int intel_hdmi_get_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> index 1d946240e55f..39714be3eb6b 100644
> --- a/drivers/gpu/drm/i915/intel_opregion.c
> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>  	case INTEL_OUTPUT_ANALOG:
>  		type = DISPLAY_TYPE_CRT;
>  		break;
> -	case INTEL_OUTPUT_UNKNOWN:
> +	case INTEL_OUTPUT_DDI:
>  	case INTEL_OUTPUT_DP:
>  	case INTEL_OUTPUT_HDMI:
>  	case INTEL_OUTPUT_DP_MST:


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v2 6/6] drm/i915: Use intel_ddi_get_config() for MST
  2017-10-27 19:31 ` [PATCH v2 6/6] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
@ 2017-10-30  9:00   ` Maarten Lankhorst
  0 siblings, 0 replies; 15+ messages in thread
From: Maarten Lankhorst @ 2017-10-30  9:00 UTC (permalink / raw)
  To: Ville Syrjala, intel-gfx

Op 27-10-17 om 21:31 schreef Ville Syrjala:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Eliminate the partially duplicated DDI readout code from MST, and
> instead just call intel_ddi_get_config(). As a nice bonus we get
> more cross checking as intel_ddi_get_config() will populate
> output_types based on the actual mode of the DDI port.
>
> Additonally intel_ddi_get_config() must be changed to get the crtc
> from the passed in crtc state rather than from the encoder->crtc link.
> encoder->crtc really shouldn't be used anyway.
>
> v2: Rebased on BXT MST latency_optim fix
>     Make intel_ddi_clock_get() static
>
> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_ddi.c    |  6 ++---
>  drivers/gpu/drm/i915/intel_dp_mst.c | 50 +------------------------------------
>  drivers/gpu/drm/i915/intel_drv.h    |  2 --
>  3 files changed, 4 insertions(+), 54 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index 0f603a157dba..cc3d55b5cea0 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1453,8 +1453,8 @@ static void bxt_ddi_clock_get(struct intel_encoder *encoder,
>  	ddi_dotclock_get(pipe_config);
>  }
>  
> -void intel_ddi_clock_get(struct intel_encoder *encoder,
> -			 struct intel_crtc_state *pipe_config)
> +static void intel_ddi_clock_get(struct intel_encoder *encoder,
> +				struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>  
> @@ -2533,7 +2533,7 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>  			  struct intel_crtc_state *pipe_config)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	struct intel_crtc *intel_crtc = to_intel_crtc(encoder->base.crtc);
> +	struct intel_crtc *intel_crtc = to_intel_crtc(pipe_config->base.crtc);
>  	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
>  	struct intel_digital_port *intel_dig_port;
>  	u32 temp, flags = 0;
> diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
> index d523302e5081..6f11bb35f66f 100644
> --- a/drivers/gpu/drm/i915/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/intel_dp_mst.c
> @@ -286,56 +286,8 @@ static void intel_dp_mst_enc_get_config(struct intel_encoder *encoder,
>  {
>  	struct intel_dp_mst_encoder *intel_mst = enc_to_mst(&encoder->base);
>  	struct intel_digital_port *intel_dig_port = intel_mst->primary;
> -	struct intel_crtc *crtc = to_intel_crtc(pipe_config->base.crtc);
> -	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> -	enum transcoder cpu_transcoder = pipe_config->cpu_transcoder;
> -	u32 temp, flags = 0;
>  
> -	pipe_config->output_types |= BIT(INTEL_OUTPUT_DP_MST);
> -
> -	pipe_config->has_audio =
> -		intel_ddi_is_audio_enabled(dev_priv, crtc);
> -
> -	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> -	if (temp & TRANS_DDI_PHSYNC)
> -		flags |= DRM_MODE_FLAG_PHSYNC;
> -	else
> -		flags |= DRM_MODE_FLAG_NHSYNC;
> -	if (temp & TRANS_DDI_PVSYNC)
> -		flags |= DRM_MODE_FLAG_PVSYNC;
> -	else
> -		flags |= DRM_MODE_FLAG_NVSYNC;
> -
> -	switch (temp & TRANS_DDI_BPC_MASK) {
> -	case TRANS_DDI_BPC_6:
> -		pipe_config->pipe_bpp = 18;
> -		break;
> -	case TRANS_DDI_BPC_8:
> -		pipe_config->pipe_bpp = 24;
> -		break;
> -	case TRANS_DDI_BPC_10:
> -		pipe_config->pipe_bpp = 30;
> -		break;
> -	case TRANS_DDI_BPC_12:
> -		pipe_config->pipe_bpp = 36;
> -		break;
> -	default:
> -		break;
> -	}
> -	pipe_config->base.adjusted_mode.flags |= flags;
> -
> -	pipe_config->lane_count =
> -		((temp & DDI_PORT_WIDTH_MASK) >> DDI_PORT_WIDTH_SHIFT) + 1;
> -
> -	intel_dp_get_m_n(crtc, pipe_config);
> -
> -	intel_ddi_clock_get(&intel_dig_port->base, pipe_config);
> -
> -	if (IS_GEN9_LP(dev_priv))
> -		pipe_config->lane_lat_optim_mask =
> -			bxt_ddi_phy_get_lane_lat_optim_mask(encoder);
> -
> -	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
> +	intel_ddi_get_config(&intel_dig_port->base, pipe_config);
>  }
>  
>  static int intel_dp_mst_get_ddc_modes(struct drm_connector *connector)
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 3828d9bee7f5..4498d743cebc 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1295,8 +1295,6 @@ bool intel_ddi_is_audio_enabled(struct drm_i915_private *dev_priv,
>  void intel_ddi_get_config(struct intel_encoder *encoder,
>  			  struct intel_crtc_state *pipe_config);
>  
> -void intel_ddi_clock_get(struct intel_encoder *encoder,
> -			 struct intel_crtc_state *pipe_config);
>  void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>  				    bool state);
>  void intel_ddi_compute_min_voltage_level(struct drm_i915_private *dev_priv,

I still don't like patch 5/6, but lets just see if it breaks too often in the future, if so we could always revert.. for the whole series:

Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* ✓ Fi.CI.BAT: success for drm/i915: Last part of DDI encoder->type cleanup (rev2)
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (6 preceding siblings ...)
  2017-10-27 20:47 ` ✗ Fi.CI.BAT: failure for drm/i915: Last part of DDI encoder->type cleanup (rev2) Patchwork
@ 2017-10-30 15:58 ` Patchwork
  2017-10-30 17:20 ` ✓ Fi.CI.IGT: " Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-10-30 15:58 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Last part of DDI encoder->type cleanup (rev2)
URL   : https://patchwork.freedesktop.org/series/32298/
State : success

== Summary ==

Series 32298v2 drm/i915: Last part of DDI encoder->type cleanup
https://patchwork.freedesktop.org/api/1.0/series/32298/revisions/2/mbox/

Test debugfs_test:
        Subgroup read_all_entries:
                dmesg-warn -> PASS       (fi-cfl-s)
Test drv_getparams_basic:
        Subgroup basic-eu-total:
                dmesg-warn -> PASS       (fi-cfl-s)
        Subgroup basic-subslice-total:
                dmesg-warn -> PASS       (fi-cfl-s)
Test drv_hangman:
        Subgroup error-state-basic:
                dmesg-warn -> PASS       (fi-cfl-s)
Test kms_frontbuffer_tracking:
        Subgroup basic:
                fail       -> PASS       (fi-glk-dsi) fdo#103167
Test kms_pipe_crc_basic:
        Subgroup suspend-read-crc-pipe-b:
                pass       -> DMESG-WARN (fi-byt-n2820) fdo#101705

fdo#103167 https://bugs.freedesktop.org/show_bug.cgi?id=103167
fdo#101705 https://bugs.freedesktop.org/show_bug.cgi?id=101705

fi-bdw-5557u     total:289  pass:268  dwarn:0   dfail:0   fail:0   skip:21  time:445s
fi-bdw-gvtdvm    total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:445s
fi-bsw-n3050     total:289  pass:243  dwarn:0   dfail:0   fail:0   skip:46  time:530s
fi-bwr-2160      total:289  pass:183  dwarn:0   dfail:0   fail:0   skip:106 time:265s
fi-bxt-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:503s
fi-bxt-j4205     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:496s
fi-byt-j1900     total:289  pass:253  dwarn:1   dfail:0   fail:0   skip:35  time:491s
fi-byt-n2820     total:289  pass:249  dwarn:1   dfail:0   fail:0   skip:39  time:477s
fi-cfl-s         total:289  pass:253  dwarn:4   dfail:0   fail:0   skip:32  time:552s
fi-elk-e7500     total:289  pass:229  dwarn:0   dfail:0   fail:0   skip:60  time:424s
fi-gdg-551       total:289  pass:178  dwarn:1   dfail:0   fail:1   skip:109 time:251s
fi-glk-1         total:289  pass:261  dwarn:0   dfail:0   fail:0   skip:28  time:575s
fi-glk-dsi       total:289  pass:259  dwarn:0   dfail:0   fail:0   skip:30  time:489s
fi-hsw-4770      total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:430s
fi-hsw-4770r     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:427s
fi-ilk-650       total:289  pass:228  dwarn:0   dfail:0   fail:0   skip:61  time:423s
fi-ivb-3520m     total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:497s
fi-ivb-3770      total:289  pass:260  dwarn:0   dfail:0   fail:0   skip:29  time:458s
fi-kbl-7500u     total:289  pass:264  dwarn:1   dfail:0   fail:0   skip:24  time:487s
fi-kbl-7560u     total:289  pass:270  dwarn:0   dfail:0   fail:0   skip:19  time:592s
fi-kbl-7567u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:478s
fi-kbl-r         total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:585s
fi-pnv-d510      total:289  pass:222  dwarn:1   dfail:0   fail:0   skip:66  time:543s
fi-skl-6260u     total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:451s
fi-skl-6600u     total:289  pass:262  dwarn:0   dfail:0   fail:0   skip:27  time:592s
fi-skl-6700hq    total:289  pass:263  dwarn:0   dfail:0   fail:0   skip:26  time:654s
fi-skl-6700k     total:289  pass:265  dwarn:0   dfail:0   fail:0   skip:24  time:522s
fi-skl-6770hq    total:289  pass:269  dwarn:0   dfail:0   fail:0   skip:20  time:502s
fi-skl-gvtdvm    total:289  pass:266  dwarn:0   dfail:0   fail:0   skip:23  time:455s
fi-snb-2520m     total:289  pass:250  dwarn:0   dfail:0   fail:0   skip:39  time:558s
fi-snb-2600      total:289  pass:249  dwarn:0   dfail:0   fail:0   skip:40  time:416s
fi-blb-e6850 failed to connect after reboot
fi-cnl-y failed to connect after reboot

2dd14a4ad87f449830d9fbb7b4a33f5d369dcde2 drm-tip: 2017y-10m-30d-13h-40m-22s UTC integration manifest
593de641a275 drm/i915: Use intel_ddi_get_config() for MST
ed3de605d8f3 drm/i915: Pass a crtc state to ddi post_disable from MST code
c9b55039476f drm/i915: Eliminate pll->state usage from bxt_calc_pll_link()
e6039598e34e drm/i915: Nuke intel_ddi_get_encoder_port()
c9f456f3b7ab drm/i915: Stop frobbing with DDI encoder->type
67f15a3517b6 drm/i915: Populate output_types from .get_config()

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6263/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-30  8:59   ` Maarten Lankhorst
@ 2017-10-30 16:07     ` Ville Syrjälä
  2017-10-30 16:29       ` Maarten Lankhorst
  0 siblings, 1 reply; 15+ messages in thread
From: Ville Syrjälä @ 2017-10-30 16:07 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Mon, Oct 30, 2017 at 09:59:29AM +0100, Maarten Lankhorst wrote:
> Op 27-10-17 om 21:31 schreef Ville Syrjala:
> > From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >
> > Currently the DDI encoder->type will change at runtime depending on
> > what kind of hotplugs we've processed. That's quite bad since we can't
> > really trust that that current value of encoder->type actually matches
> > the type of signal we're trying to drive through it.
> >
> > Let's eliminate that problem by declaring that non-eDP DDI port will
> > always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> > can no longer try to distinguish DP vs. HDMI based on encoder->type.
> > We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> > there's a bunch of code that relies on that value to identify eDP
> > encoders.
> >
> > We'll introduce a new encoder .compute_output_type() hook. This allows
> > us to compute the full output_types before any encoder .compute_config()
> > hooks get called, thus those hooks can rely on output_types being
> > correct, which is useful for cloning on oldr platforms. For now we'll
> > just look at the connector type and pick the correct mode based on that.
> > In the future the new hook could be used to implement dynamic switching
> > between LS and PCON modes for LSPCON.
> >
> > v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> > v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> > v4: Rebase
> > v5: Populate output_types in .get_config() rather than in the caller
> > v5: Split out populating output_types in .get_config() (Maarten)
> >
> > Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> > Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
> >  drivers/gpu/drm/i915/intel_ddi.c      | 32 ++++++++++++++++++++++++--------
> >  drivers/gpu/drm/i915/intel_display.c  | 11 ++++++++---
> >  drivers/gpu/drm/i915/intel_dp.c       | 19 ++++++-------------
> >  drivers/gpu/drm/i915/intel_drv.h      |  7 +++++--
> >  drivers/gpu/drm/i915/intel_hdmi.c     | 10 ++--------
> >  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >  7 files changed, 47 insertions(+), 36 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> > index c1e5bba91bff..39883cd915db 100644
> > --- a/drivers/gpu/drm/i915/i915_debugfs.c
> > +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> > @@ -3051,7 +3051,7 @@ static void intel_connector_info(struct seq_file *m,
> >  		break;
> >  	case DRM_MODE_CONNECTOR_HDMIA:
> >  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> > -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> > +		    intel_encoder->type == INTEL_OUTPUT_DDI)
> >  			intel_hdmi_info(m, intel_connector);
> >  		break;
> >  	default:
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index 7e0b1a02912a..e5dd281781fe 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> >  	switch (encoder->type) {
> >  	case INTEL_OUTPUT_DP_MST:
> >  		return enc_to_mst(&encoder->base)->primary->port;
> > -	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_EDP:
> > -	case INTEL_OUTPUT_HDMI:
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  		return enc_to_dig_port(&encoder->base)->port;
> >  	case INTEL_OUTPUT_ANALOG:
> >  		return PORT_E;
> > @@ -1534,6 +1532,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
> >  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >  	uint32_t temp;
> > +
> >  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >  	if (state == true)
> >  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> > @@ -2652,21 +2651,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
> >  }
> >  
> > +static enum intel_output_type
> > +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> > +			      struct intel_crtc_state *crtc_state,
> > +			      struct drm_connector_state *conn_state)
> > +{
> > +	switch (conn_state->connector->connector_type) {
> > +	case DRM_MODE_CONNECTOR_HDMIA:
> > +		return INTEL_OUTPUT_HDMI;
> > +	case DRM_MODE_CONNECTOR_eDP:
> > +		return INTEL_OUTPUT_EDP;
> > +	case DRM_MODE_CONNECTOR_DisplayPort:
> > +		return INTEL_OUTPUT_DP;
> > +	default:
> > +		MISSING_CASE(conn_state->connector->connector_type);
> > +		return INTEL_OUTPUT_UNUSED;
> > +	}
> > +}
> > +
> >  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >  				     struct intel_crtc_state *pipe_config,
> >  				     struct drm_connector_state *conn_state)
> >  {
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> > -	int type = encoder->type;
> >  	int port = intel_ddi_get_encoder_port(encoder);
> >  	int ret;
> >  
> > -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> > -
> >  	if (port == PORT_A)
> >  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >  
> > -	if (type == INTEL_OUTPUT_HDMI)
> > +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> >  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
> >  	else
> >  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
> 
> Would it be an option to put it in compute_config for DDI only? There is no risk of cloning there, saves another hook. :)

That would mean the caller would set 'output_types|=DDI' which we'd have
to undo here, or we would need to special case DDI in the caller and not
set output_types there at all. Neither would look very pretty I think.

> 
> Either way is fine though.
> > @@ -2815,6 +2829,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
> >  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
> >  
> > +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
> >  	intel_encoder->compute_config = intel_ddi_compute_config;
> >  	intel_encoder->enable = intel_enable_ddi;
> >  	if (IS_GEN9_LP(dev_priv))
> > @@ -2868,9 +2883,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  		max_lanes = 4;
> >  	}
> >  
> > +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> >  	intel_dig_port->max_lanes = max_lanes;
> >  
> > -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> > +	intel_encoder->type = INTEL_OUTPUT_DDI;
> >  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> >  	intel_encoder->port = port;
> >  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 8f769e9b9342..737de251d0f8 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -10579,7 +10579,7 @@ static const char * const output_type_str[] = {
> >  	OUTPUT_TYPE(DP),
> >  	OUTPUT_TYPE(EDP),
> >  	OUTPUT_TYPE(DSI),
> > -	OUTPUT_TYPE(UNKNOWN),
> > +	OUTPUT_TYPE(DDI),
> >  	OUTPUT_TYPE(DP_MST),
> >  };
> >  
> > @@ -10750,7 +10750,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >  
> >  		switch (encoder->type) {
> >  			unsigned int port_mask;
> > -		case INTEL_OUTPUT_UNKNOWN:
> > +		case INTEL_OUTPUT_DDI:
> >  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
> >  				break;
> >  		case INTEL_OUTPUT_DP:
> > @@ -10883,7 +10883,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >  		 * Determine output_types before calling the .compute_config()
> >  		 * hooks so that the hooks can use this information safely.
> >  		 */
> > -		pipe_config->output_types |= 1 << encoder->type;
> > +		if (encoder->compute_output_type)
> > +			pipe_config->output_types |=
> > +				BIT(encoder->compute_output_type(encoder, pipe_config,
> > +								 connector_state));
> > +		else
> > +			pipe_config->output_types |= BIT(encoder->type);
> >  	}
> >  
> >  encoder_retry:
> > diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> > index 30688a5d680d..f0c49962ffbe 100644
> > --- a/drivers/gpu/drm/i915/intel_dp.c
> > +++ b/drivers/gpu/drm/i915/intel_dp.c
> > @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> >  		struct intel_dp *intel_dp;
> >  
> >  		if (encoder->type != INTEL_OUTPUT_DP &&
> > -		    encoder->type != INTEL_OUTPUT_EDP)
> > +		    encoder->type != INTEL_OUTPUT_EDP &&
> > +		    encoder->type != INTEL_OUTPUT_DDI)
> >  			continue;
> >  
> >  		intel_dp = enc_to_intel_dp(&encoder->base);
> >  
> > +		/* Skip pure DVI/HDMI DDI encoders */
> > +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> > +			continue;
> > +
> >  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >  
> >  		if (encoder->type != INTEL_OUTPUT_EDP)
> > @@ -4733,8 +4738,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  {
> >  	struct drm_connector *connector = &intel_connector->base;
> >  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> > -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> > -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >  	struct drm_device *dev = connector->dev;
> >  	enum drm_connector_status status;
> >  	u8 sink_irq_vector = 0;
> > @@ -4767,9 +4770,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >  		goto out;
> >  	}
> >  
> > -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > -		intel_encoder->type = INTEL_OUTPUT_DP;
> > -
> >  	if (intel_dp->reset_link_params) {
> >  		/* Initial max link lane count */
> >  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> > @@ -4886,9 +4886,6 @@ intel_dp_force(struct drm_connector *connector)
> >  	intel_dp_set_edid(intel_dp);
> >  
> >  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> > -
> > -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> > -		intel_encoder->type = INTEL_OUTPUT_DP;
> >  }
> >  
> >  static int intel_dp_get_modes(struct drm_connector *connector)
> > @@ -5107,10 +5104,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >  	struct drm_i915_private *dev_priv = to_i915(dev);
> >  	enum irqreturn ret = IRQ_NONE;
> >  
> > -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> > -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> > -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> > -
> >  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> >  		/*
> >  		 * vdd off can generate a long pulse on eDP which
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index df966427232c..629ebc73bb09 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -173,7 +173,7 @@ enum intel_output_type {
> >  	INTEL_OUTPUT_DP = 7,
> >  	INTEL_OUTPUT_EDP = 8,
> >  	INTEL_OUTPUT_DSI = 9,
> > -	INTEL_OUTPUT_UNKNOWN = 10,
> > +	INTEL_OUTPUT_DDI = 10,
> >  	INTEL_OUTPUT_DP_MST = 11,
> >  };
> >  
> > @@ -216,6 +216,9 @@ struct intel_encoder {
> >  	enum port port;
> >  	unsigned int cloneable;
> >  	void (*hot_plug)(struct intel_encoder *);
> > +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> > +						      struct intel_crtc_state *,
> > +						      struct drm_connector_state *);
> >  	bool (*compute_config)(struct intel_encoder *,
> >  			       struct intel_crtc_state *,
> >  			       struct drm_connector_state *);
> > @@ -1152,7 +1155,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
> >  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >  
> >  	switch (intel_encoder->type) {
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> >  	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_EDP:
> > diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> > index 97d08cda3f8e..dede2898cb8a 100644
> > --- a/drivers/gpu/drm/i915/intel_hdmi.c
> > +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> > @@ -1617,12 +1617,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  
> >  	intel_hdmi_unset_edid(connector);
> >  
> > -	if (intel_hdmi_set_edid(connector)) {
> > -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> > -
> > -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> > +	if (intel_hdmi_set_edid(connector))
> >  		status = connector_status_connected;
> > -	} else
> > +	else
> >  		status = connector_status_disconnected;
> >  
> >  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> > @@ -1633,8 +1630,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >  static void
> >  intel_hdmi_force(struct drm_connector *connector)
> >  {
> > -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> > -
> >  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> >  		      connector->base.id, connector->name);
> >  
> > @@ -1644,7 +1639,6 @@ intel_hdmi_force(struct drm_connector *connector)
> >  		return;
> >  
> >  	intel_hdmi_set_edid(connector);
> > -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >  }
> >  
> >  static int intel_hdmi_get_modes(struct drm_connector *connector)
> > diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> > index 1d946240e55f..39714be3eb6b 100644
> > --- a/drivers/gpu/drm/i915/intel_opregion.c
> > +++ b/drivers/gpu/drm/i915/intel_opregion.c
> > @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >  	case INTEL_OUTPUT_ANALOG:
> >  		type = DISPLAY_TYPE_CRT;
> >  		break;
> > -	case INTEL_OUTPUT_UNKNOWN:
> > +	case INTEL_OUTPUT_DDI:
> >  	case INTEL_OUTPUT_DP:
> >  	case INTEL_OUTPUT_HDMI:
> >  	case INTEL_OUTPUT_DP_MST:
> 

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-30 16:07     ` Ville Syrjälä
@ 2017-10-30 16:29       ` Maarten Lankhorst
  2017-10-30 18:28         ` Ville Syrjälä
  0 siblings, 1 reply; 15+ messages in thread
From: Maarten Lankhorst @ 2017-10-30 16:29 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

Op 30-10-17 om 17:07 schreef Ville Syrjälä:
> On Mon, Oct 30, 2017 at 09:59:29AM +0100, Maarten Lankhorst wrote:
>> Op 27-10-17 om 21:31 schreef Ville Syrjala:
>>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>>
>>> Currently the DDI encoder->type will change at runtime depending on
>>> what kind of hotplugs we've processed. That's quite bad since we can't
>>> really trust that that current value of encoder->type actually matches
>>> the type of signal we're trying to drive through it.
>>>
>>> Let's eliminate that problem by declaring that non-eDP DDI port will
>>> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
>>> can no longer try to distinguish DP vs. HDMI based on encoder->type.
>>> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
>>> there's a bunch of code that relies on that value to identify eDP
>>> encoders.
>>>
>>> We'll introduce a new encoder .compute_output_type() hook. This allows
>>> us to compute the full output_types before any encoder .compute_config()
>>> hooks get called, thus those hooks can rely on output_types being
>>> correct, which is useful for cloning on oldr platforms. For now we'll
>>> just look at the connector type and pick the correct mode based on that.
>>> In the future the new hook could be used to implement dynamic switching
>>> between LS and PCON modes for LSPCON.
>>>
>>> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
>>> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
>>> v4: Rebase
>>> v5: Populate output_types in .get_config() rather than in the caller
>>> v5: Split out populating output_types in .get_config() (Maarten)
>>>
>>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
>>> ---
>>>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
>>>  drivers/gpu/drm/i915/intel_ddi.c      | 32 ++++++++++++++++++++++++--------
>>>  drivers/gpu/drm/i915/intel_display.c  | 11 ++++++++---
>>>  drivers/gpu/drm/i915/intel_dp.c       | 19 ++++++-------------
>>>  drivers/gpu/drm/i915/intel_drv.h      |  7 +++++--
>>>  drivers/gpu/drm/i915/intel_hdmi.c     | 10 ++--------
>>>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
>>>  7 files changed, 47 insertions(+), 36 deletions(-)
>>>
>>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>>> index c1e5bba91bff..39883cd915db 100644
>>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>>> @@ -3051,7 +3051,7 @@ static void intel_connector_info(struct seq_file *m,
>>>  		break;
>>>  	case DRM_MODE_CONNECTOR_HDMIA:
>>>  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
>>> -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
>>> +		    intel_encoder->type == INTEL_OUTPUT_DDI)
>>>  			intel_hdmi_info(m, intel_connector);
>>>  		break;
>>>  	default:
>>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
>>> index 7e0b1a02912a..e5dd281781fe 100644
>>> --- a/drivers/gpu/drm/i915/intel_ddi.c
>>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
>>> @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
>>>  	switch (encoder->type) {
>>>  	case INTEL_OUTPUT_DP_MST:
>>>  		return enc_to_mst(&encoder->base)->primary->port;
>>> -	case INTEL_OUTPUT_DP:
>>>  	case INTEL_OUTPUT_EDP:
>>> -	case INTEL_OUTPUT_HDMI:
>>> -	case INTEL_OUTPUT_UNKNOWN:
>>> +	case INTEL_OUTPUT_DDI:
>>>  		return enc_to_dig_port(&encoder->base)->port;
>>>  	case INTEL_OUTPUT_ANALOG:
>>>  		return PORT_E;
>>> @@ -1534,6 +1532,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
>>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
>>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
>>>  	uint32_t temp;
>>> +
>>>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
>>>  	if (state == true)
>>>  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
>>> @@ -2652,21 +2651,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
>>>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
>>>  }
>>>  
>>> +static enum intel_output_type
>>> +intel_ddi_compute_output_type(struct intel_encoder *encoder,
>>> +			      struct intel_crtc_state *crtc_state,
>>> +			      struct drm_connector_state *conn_state)
>>> +{
>>> +	switch (conn_state->connector->connector_type) {
>>> +	case DRM_MODE_CONNECTOR_HDMIA:
>>> +		return INTEL_OUTPUT_HDMI;
>>> +	case DRM_MODE_CONNECTOR_eDP:
>>> +		return INTEL_OUTPUT_EDP;
>>> +	case DRM_MODE_CONNECTOR_DisplayPort:
>>> +		return INTEL_OUTPUT_DP;
>>> +	default:
>>> +		MISSING_CASE(conn_state->connector->connector_type);
>>> +		return INTEL_OUTPUT_UNUSED;
>>> +	}
>>> +}
>>> +
>>>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
>>>  				     struct intel_crtc_state *pipe_config,
>>>  				     struct drm_connector_state *conn_state)
>>>  {
>>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
>>> -	int type = encoder->type;
>>>  	int port = intel_ddi_get_encoder_port(encoder);
>>>  	int ret;
>>>  
>>> -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
>>> -
>>>  	if (port == PORT_A)
>>>  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
>>>  
>>> -	if (type == INTEL_OUTPUT_HDMI)
>>> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
>>>  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
>>>  	else
>>>  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
>> Would it be an option to put it in compute_config for DDI only? There is no risk of cloning there, saves another hook. :)
> That would mean the caller would set 'output_types|=DDI' which we'd have
> to undo here, or we would need to special case DDI in the caller and not
> set output_types there at all. Neither would look very pretty I think.
Something like:

/* There is no 1 to 1 mapping for DDI, it's set in compute_config()
if (type != DDI)
output_types |= BIT(type)

would work for me. :)

>> Either way is fine though.
>>> @@ -2815,6 +2829,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
>>>  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
>>>  
>>> +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
>>>  	intel_encoder->compute_config = intel_ddi_compute_config;
>>>  	intel_encoder->enable = intel_enable_ddi;
>>>  	if (IS_GEN9_LP(dev_priv))
>>> @@ -2868,9 +2883,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>>>  		max_lanes = 4;
>>>  	}
>>>  
>>> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
>>>  	intel_dig_port->max_lanes = max_lanes;
>>>  
>>> -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
>>> +	intel_encoder->type = INTEL_OUTPUT_DDI;
>>>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
>>>  	intel_encoder->port = port;
>>>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
>>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>>> index 8f769e9b9342..737de251d0f8 100644
>>> --- a/drivers/gpu/drm/i915/intel_display.c
>>> +++ b/drivers/gpu/drm/i915/intel_display.c
>>> @@ -10579,7 +10579,7 @@ static const char * const output_type_str[] = {
>>>  	OUTPUT_TYPE(DP),
>>>  	OUTPUT_TYPE(EDP),
>>>  	OUTPUT_TYPE(DSI),
>>> -	OUTPUT_TYPE(UNKNOWN),
>>> +	OUTPUT_TYPE(DDI),
>>>  	OUTPUT_TYPE(DP_MST),
>>>  };
>>>  
>>> @@ -10750,7 +10750,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
>>>  
>>>  		switch (encoder->type) {
>>>  			unsigned int port_mask;
>>> -		case INTEL_OUTPUT_UNKNOWN:
>>> +		case INTEL_OUTPUT_DDI:
>>>  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
>>>  				break;
>>>  		case INTEL_OUTPUT_DP:
>>> @@ -10883,7 +10883,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
>>>  		 * Determine output_types before calling the .compute_config()
>>>  		 * hooks so that the hooks can use this information safely.
>>>  		 */
>>> -		pipe_config->output_types |= 1 << encoder->type;
>>> +		if (encoder->compute_output_type)
>>> +			pipe_config->output_types |=
>>> +				BIT(encoder->compute_output_type(encoder, pipe_config,
>>> +								 connector_state));
>>> +		else
>>> +			pipe_config->output_types |= BIT(encoder->type);
>>>  	}
>>>  
>>>  encoder_retry:
>>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
>>> index 30688a5d680d..f0c49962ffbe 100644
>>> --- a/drivers/gpu/drm/i915/intel_dp.c
>>> +++ b/drivers/gpu/drm/i915/intel_dp.c
>>> @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
>>>  		struct intel_dp *intel_dp;
>>>  
>>>  		if (encoder->type != INTEL_OUTPUT_DP &&
>>> -		    encoder->type != INTEL_OUTPUT_EDP)
>>> +		    encoder->type != INTEL_OUTPUT_EDP &&
>>> +		    encoder->type != INTEL_OUTPUT_DDI)
>>>  			continue;
>>>  
>>>  		intel_dp = enc_to_intel_dp(&encoder->base);
>>>  
>>> +		/* Skip pure DVI/HDMI DDI encoders */
>>> +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
>>> +			continue;
>>> +
>>>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
>>>  
>>>  		if (encoder->type != INTEL_OUTPUT_EDP)
>>> @@ -4733,8 +4738,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>>  {
>>>  	struct drm_connector *connector = &intel_connector->base;
>>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
>>> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
>>> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
>>>  	struct drm_device *dev = connector->dev;
>>>  	enum drm_connector_status status;
>>>  	u8 sink_irq_vector = 0;
>>> @@ -4767,9 +4770,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
>>>  		goto out;
>>>  	}
>>>  
>>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> -		intel_encoder->type = INTEL_OUTPUT_DP;
>>> -
>>>  	if (intel_dp->reset_link_params) {
>>>  		/* Initial max link lane count */
>>>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
>>> @@ -4886,9 +4886,6 @@ intel_dp_force(struct drm_connector *connector)
>>>  	intel_dp_set_edid(intel_dp);
>>>  
>>>  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
>>> -
>>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
>>> -		intel_encoder->type = INTEL_OUTPUT_DP;
>>>  }
>>>  
>>>  static int intel_dp_get_modes(struct drm_connector *connector)
>>> @@ -5107,10 +5104,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
>>>  	struct drm_i915_private *dev_priv = to_i915(dev);
>>>  	enum irqreturn ret = IRQ_NONE;
>>>  
>>> -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
>>> -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
>>> -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
>>> -
>>>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
>>>  		/*
>>>  		 * vdd off can generate a long pulse on eDP which
>>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>>> index df966427232c..629ebc73bb09 100644
>>> --- a/drivers/gpu/drm/i915/intel_drv.h
>>> +++ b/drivers/gpu/drm/i915/intel_drv.h
>>> @@ -173,7 +173,7 @@ enum intel_output_type {
>>>  	INTEL_OUTPUT_DP = 7,
>>>  	INTEL_OUTPUT_EDP = 8,
>>>  	INTEL_OUTPUT_DSI = 9,
>>> -	INTEL_OUTPUT_UNKNOWN = 10,
>>> +	INTEL_OUTPUT_DDI = 10,
>>>  	INTEL_OUTPUT_DP_MST = 11,
>>>  };
>>>  
>>> @@ -216,6 +216,9 @@ struct intel_encoder {
>>>  	enum port port;
>>>  	unsigned int cloneable;
>>>  	void (*hot_plug)(struct intel_encoder *);
>>> +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
>>> +						      struct intel_crtc_state *,
>>> +						      struct drm_connector_state *);
>>>  	bool (*compute_config)(struct intel_encoder *,
>>>  			       struct intel_crtc_state *,
>>>  			       struct drm_connector_state *);
>>> @@ -1152,7 +1155,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
>>>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
>>>  
>>>  	switch (intel_encoder->type) {
>>> -	case INTEL_OUTPUT_UNKNOWN:
>>> +	case INTEL_OUTPUT_DDI:
>>>  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
>>>  	case INTEL_OUTPUT_DP:
>>>  	case INTEL_OUTPUT_EDP:
>>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
>>> index 97d08cda3f8e..dede2898cb8a 100644
>>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
>>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
>>> @@ -1617,12 +1617,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>>>  
>>>  	intel_hdmi_unset_edid(connector);
>>>  
>>> -	if (intel_hdmi_set_edid(connector)) {
>>> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>>> -
>>> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>>> +	if (intel_hdmi_set_edid(connector))
>>>  		status = connector_status_connected;
>>> -	} else
>>> +	else
>>>  		status = connector_status_disconnected;
>>>  
>>>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
>>> @@ -1633,8 +1630,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
>>>  static void
>>>  intel_hdmi_force(struct drm_connector *connector)
>>>  {
>>> -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
>>> -
>>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
>>>  		      connector->base.id, connector->name);
>>>  
>>> @@ -1644,7 +1639,6 @@ intel_hdmi_force(struct drm_connector *connector)
>>>  		return;
>>>  
>>>  	intel_hdmi_set_edid(connector);
>>> -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
>>>  }
>>>  
>>>  static int intel_hdmi_get_modes(struct drm_connector *connector)
>>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
>>> index 1d946240e55f..39714be3eb6b 100644
>>> --- a/drivers/gpu/drm/i915/intel_opregion.c
>>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
>>> @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
>>>  	case INTEL_OUTPUT_ANALOG:
>>>  		type = DISPLAY_TYPE_CRT;
>>>  		break;
>>> -	case INTEL_OUTPUT_UNKNOWN:
>>> +	case INTEL_OUTPUT_DDI:
>>>  	case INTEL_OUTPUT_DP:
>>>  	case INTEL_OUTPUT_HDMI:
>>>  	case INTEL_OUTPUT_DP_MST:


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

^ permalink raw reply	[flat|nested] 15+ messages in thread

* ✓ Fi.CI.IGT: success for drm/i915: Last part of DDI encoder->type cleanup (rev2)
  2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
                   ` (7 preceding siblings ...)
  2017-10-30 15:58 ` ✓ Fi.CI.BAT: success " Patchwork
@ 2017-10-30 17:20 ` Patchwork
  8 siblings, 0 replies; 15+ messages in thread
From: Patchwork @ 2017-10-30 17:20 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

== Series Details ==

Series: drm/i915: Last part of DDI encoder->type cleanup (rev2)
URL   : https://patchwork.freedesktop.org/series/32298/
State : success

== Summary ==

Test gem_tiled_swapping:
        Subgroup non-threaded:
                incomplete -> PASS       (shard-hsw)
Test kms_flip:
        Subgroup wf_vblank-vs-dpms:
                dmesg-warn -> PASS       (shard-hsw) fdo#102614
Test kms_busy:
        Subgroup extended-modeset-hang-oldfb-with-reset-render-C:
                pass       -> DMESG-WARN (shard-hsw) fdo#102249 +1

fdo#102614 https://bugs.freedesktop.org/show_bug.cgi?id=102614
fdo#102249 https://bugs.freedesktop.org/show_bug.cgi?id=102249

shard-hsw        total:2539 pass:1432 dwarn:2   dfail:0   fail:8   skip:1097 time:9196s

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_6263/shards.html
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type
  2017-10-30 16:29       ` Maarten Lankhorst
@ 2017-10-30 18:28         ` Ville Syrjälä
  0 siblings, 0 replies; 15+ messages in thread
From: Ville Syrjälä @ 2017-10-30 18:28 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Mon, Oct 30, 2017 at 05:29:13PM +0100, Maarten Lankhorst wrote:
> Op 30-10-17 om 17:07 schreef Ville Syrjälä:
> > On Mon, Oct 30, 2017 at 09:59:29AM +0100, Maarten Lankhorst wrote:
> >> Op 27-10-17 om 21:31 schreef Ville Syrjala:
> >>> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>>
> >>> Currently the DDI encoder->type will change at runtime depending on
> >>> what kind of hotplugs we've processed. That's quite bad since we can't
> >>> really trust that that current value of encoder->type actually matches
> >>> the type of signal we're trying to drive through it.
> >>>
> >>> Let's eliminate that problem by declaring that non-eDP DDI port will
> >>> always have the encoder type as INTEL_OUTPUT_DDI. This means the code
> >>> can no longer try to distinguish DP vs. HDMI based on encoder->type.
> >>> We'll leave eDP as INTEL_OUTPUT_EDP, since it'll never change and
> >>> there's a bunch of code that relies on that value to identify eDP
> >>> encoders.
> >>>
> >>> We'll introduce a new encoder .compute_output_type() hook. This allows
> >>> us to compute the full output_types before any encoder .compute_config()
> >>> hooks get called, thus those hooks can rely on output_types being
> >>> correct, which is useful for cloning on oldr platforms. For now we'll
> >>> just look at the connector type and pick the correct mode based on that.
> >>> In the future the new hook could be used to implement dynamic switching
> >>> between LS and PCON modes for LSPCON.
> >>>
> >>> v2: Fix BXT/GLK PPS explosion with DSI/MST encoders
> >>> v3: Avoid the PPS warn on pure HDMI/DVI DDI encoders by checking dp.output_reg
> >>> v4: Rebase
> >>> v5: Populate output_types in .get_config() rather than in the caller
> >>> v5: Split out populating output_types in .get_config() (Maarten)
> >>>
> >>> Cc: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> >>> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> >>> ---
> >>>  drivers/gpu/drm/i915/i915_debugfs.c   |  2 +-
> >>>  drivers/gpu/drm/i915/intel_ddi.c      | 32 ++++++++++++++++++++++++--------
> >>>  drivers/gpu/drm/i915/intel_display.c  | 11 ++++++++---
> >>>  drivers/gpu/drm/i915/intel_dp.c       | 19 ++++++-------------
> >>>  drivers/gpu/drm/i915/intel_drv.h      |  7 +++++--
> >>>  drivers/gpu/drm/i915/intel_hdmi.c     | 10 ++--------
> >>>  drivers/gpu/drm/i915/intel_opregion.c |  2 +-
> >>>  7 files changed, 47 insertions(+), 36 deletions(-)
> >>>
> >>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> >>> index c1e5bba91bff..39883cd915db 100644
> >>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> >>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> >>> @@ -3051,7 +3051,7 @@ static void intel_connector_info(struct seq_file *m,
> >>>  		break;
> >>>  	case DRM_MODE_CONNECTOR_HDMIA:
> >>>  		if (intel_encoder->type == INTEL_OUTPUT_HDMI ||
> >>> -		    intel_encoder->type == INTEL_OUTPUT_UNKNOWN)
> >>> +		    intel_encoder->type == INTEL_OUTPUT_DDI)
> >>>  			intel_hdmi_info(m, intel_connector);
> >>>  		break;
> >>>  	default:
> >>> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> >>> index 7e0b1a02912a..e5dd281781fe 100644
> >>> --- a/drivers/gpu/drm/i915/intel_ddi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> >>> @@ -497,10 +497,8 @@ enum port intel_ddi_get_encoder_port(struct intel_encoder *encoder)
> >>>  	switch (encoder->type) {
> >>>  	case INTEL_OUTPUT_DP_MST:
> >>>  		return enc_to_mst(&encoder->base)->primary->port;
> >>> -	case INTEL_OUTPUT_DP:
> >>>  	case INTEL_OUTPUT_EDP:
> >>> -	case INTEL_OUTPUT_HDMI:
> >>> -	case INTEL_OUTPUT_UNKNOWN:
> >>> +	case INTEL_OUTPUT_DDI:
> >>>  		return enc_to_dig_port(&encoder->base)->port;
> >>>  	case INTEL_OUTPUT_ANALOG:
> >>>  		return PORT_E;
> >>> @@ -1534,6 +1532,7 @@ void intel_ddi_set_vc_payload_alloc(const struct intel_crtc_state *crtc_state,
> >>>  	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
> >>>  	enum transcoder cpu_transcoder = crtc_state->cpu_transcoder;
> >>>  	uint32_t temp;
> >>> +
> >>>  	temp = I915_READ(TRANS_DDI_FUNC_CTL(cpu_transcoder));
> >>>  	if (state == true)
> >>>  		temp |= TRANS_DDI_DP_VC_PAYLOAD_ALLOC;
> >>> @@ -2652,21 +2651,36 @@ void intel_ddi_get_config(struct intel_encoder *encoder,
> >>>  	intel_ddi_compute_min_voltage_level(dev_priv, pipe_config);
> >>>  }
> >>>  
> >>> +static enum intel_output_type
> >>> +intel_ddi_compute_output_type(struct intel_encoder *encoder,
> >>> +			      struct intel_crtc_state *crtc_state,
> >>> +			      struct drm_connector_state *conn_state)
> >>> +{
> >>> +	switch (conn_state->connector->connector_type) {
> >>> +	case DRM_MODE_CONNECTOR_HDMIA:
> >>> +		return INTEL_OUTPUT_HDMI;
> >>> +	case DRM_MODE_CONNECTOR_eDP:
> >>> +		return INTEL_OUTPUT_EDP;
> >>> +	case DRM_MODE_CONNECTOR_DisplayPort:
> >>> +		return INTEL_OUTPUT_DP;
> >>> +	default:
> >>> +		MISSING_CASE(conn_state->connector->connector_type);
> >>> +		return INTEL_OUTPUT_UNUSED;
> >>> +	}
> >>> +}
> >>> +
> >>>  static bool intel_ddi_compute_config(struct intel_encoder *encoder,
> >>>  				     struct intel_crtc_state *pipe_config,
> >>>  				     struct drm_connector_state *conn_state)
> >>>  {
> >>>  	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
> >>> -	int type = encoder->type;
> >>>  	int port = intel_ddi_get_encoder_port(encoder);
> >>>  	int ret;
> >>>  
> >>> -	WARN(type == INTEL_OUTPUT_UNKNOWN, "compute_config() on unknown output!\n");
> >>> -
> >>>  	if (port == PORT_A)
> >>>  		pipe_config->cpu_transcoder = TRANSCODER_EDP;
> >>>  
> >>> -	if (type == INTEL_OUTPUT_HDMI)
> >>> +	if (intel_crtc_has_type(pipe_config, INTEL_OUTPUT_HDMI))
> >>>  		ret = intel_hdmi_compute_config(encoder, pipe_config, conn_state);
> >>>  	else
> >>>  		ret = intel_dp_compute_config(encoder, pipe_config, conn_state);
> >> Would it be an option to put it in compute_config for DDI only? There is no risk of cloning there, saves another hook. :)
> > That would mean the caller would set 'output_types|=DDI' which we'd have
> > to undo here, or we would need to special case DDI in the caller and not
> > set output_types there at all. Neither would look very pretty I think.
> Something like:
> 
> /* There is no 1 to 1 mapping for DDI, it's set in compute_config()
> if (type != DDI)
> output_types |= BIT(type)
> 
> would work for me. :)

Hmm. I guess that wouldn't be too bad. Though I don't particularly
enjoy encoder specific information leaking out.

But I've just gone and pushed the entire series as is, just to avoid
dragging this series along any longer. If someone feels strongly that
the hook shouldn't be there they can send a patch to kill it.

Thanks for the review.

> 
> >> Either way is fine though.
> >>> @@ -2815,6 +2829,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >>>  	drm_encoder_init(&dev_priv->drm, encoder, &intel_ddi_funcs,
> >>>  			 DRM_MODE_ENCODER_TMDS, "DDI %c", port_name(port));
> >>>  
> >>> +	intel_encoder->compute_output_type = intel_ddi_compute_output_type;
> >>>  	intel_encoder->compute_config = intel_ddi_compute_config;
> >>>  	intel_encoder->enable = intel_enable_ddi;
> >>>  	if (IS_GEN9_LP(dev_priv))
> >>> @@ -2868,9 +2883,10 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >>>  		max_lanes = 4;
> >>>  	}
> >>>  
> >>> +	intel_dig_port->dp.output_reg = INVALID_MMIO_REG;
> >>>  	intel_dig_port->max_lanes = max_lanes;
> >>>  
> >>> -	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
> >>> +	intel_encoder->type = INTEL_OUTPUT_DDI;
> >>>  	intel_encoder->power_domain = intel_port_to_power_domain(port);
> >>>  	intel_encoder->port = port;
> >>>  	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
> >>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> >>> index 8f769e9b9342..737de251d0f8 100644
> >>> --- a/drivers/gpu/drm/i915/intel_display.c
> >>> +++ b/drivers/gpu/drm/i915/intel_display.c
> >>> @@ -10579,7 +10579,7 @@ static const char * const output_type_str[] = {
> >>>  	OUTPUT_TYPE(DP),
> >>>  	OUTPUT_TYPE(EDP),
> >>>  	OUTPUT_TYPE(DSI),
> >>> -	OUTPUT_TYPE(UNKNOWN),
> >>> +	OUTPUT_TYPE(DDI),
> >>>  	OUTPUT_TYPE(DP_MST),
> >>>  };
> >>>  
> >>> @@ -10750,7 +10750,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
> >>>  
> >>>  		switch (encoder->type) {
> >>>  			unsigned int port_mask;
> >>> -		case INTEL_OUTPUT_UNKNOWN:
> >>> +		case INTEL_OUTPUT_DDI:
> >>>  			if (WARN_ON(!HAS_DDI(to_i915(dev))))
> >>>  				break;
> >>>  		case INTEL_OUTPUT_DP:
> >>> @@ -10883,7 +10883,12 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
> >>>  		 * Determine output_types before calling the .compute_config()
> >>>  		 * hooks so that the hooks can use this information safely.
> >>>  		 */
> >>> -		pipe_config->output_types |= 1 << encoder->type;
> >>> +		if (encoder->compute_output_type)
> >>> +			pipe_config->output_types |=
> >>> +				BIT(encoder->compute_output_type(encoder, pipe_config,
> >>> +								 connector_state));
> >>> +		else
> >>> +			pipe_config->output_types |= BIT(encoder->type);
> >>>  	}
> >>>  
> >>>  encoder_retry:
> >>> diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
> >>> index 30688a5d680d..f0c49962ffbe 100644
> >>> --- a/drivers/gpu/drm/i915/intel_dp.c
> >>> +++ b/drivers/gpu/drm/i915/intel_dp.c
> >>> @@ -758,11 +758,16 @@ void intel_power_sequencer_reset(struct drm_i915_private *dev_priv)
> >>>  		struct intel_dp *intel_dp;
> >>>  
> >>>  		if (encoder->type != INTEL_OUTPUT_DP &&
> >>> -		    encoder->type != INTEL_OUTPUT_EDP)
> >>> +		    encoder->type != INTEL_OUTPUT_EDP &&
> >>> +		    encoder->type != INTEL_OUTPUT_DDI)
> >>>  			continue;
> >>>  
> >>>  		intel_dp = enc_to_intel_dp(&encoder->base);
> >>>  
> >>> +		/* Skip pure DVI/HDMI DDI encoders */
> >>> +		if (!i915_mmio_reg_valid(intel_dp->output_reg))
> >>> +			continue;
> >>> +
> >>>  		WARN_ON(intel_dp->active_pipe != INVALID_PIPE);
> >>>  
> >>>  		if (encoder->type != INTEL_OUTPUT_EDP)
> >>> @@ -4733,8 +4738,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >>>  {
> >>>  	struct drm_connector *connector = &intel_connector->base;
> >>>  	struct intel_dp *intel_dp = intel_attached_dp(connector);
> >>> -	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
> >>> -	struct intel_encoder *intel_encoder = &intel_dig_port->base;
> >>>  	struct drm_device *dev = connector->dev;
> >>>  	enum drm_connector_status status;
> >>>  	u8 sink_irq_vector = 0;
> >>> @@ -4767,9 +4770,6 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
> >>>  		goto out;
> >>>  	}
> >>>  
> >>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> >>> -		intel_encoder->type = INTEL_OUTPUT_DP;
> >>> -
> >>>  	if (intel_dp->reset_link_params) {
> >>>  		/* Initial max link lane count */
> >>>  		intel_dp->max_link_lane_count = intel_dp_max_common_lane_count(intel_dp);
> >>> @@ -4886,9 +4886,6 @@ intel_dp_force(struct drm_connector *connector)
> >>>  	intel_dp_set_edid(intel_dp);
> >>>  
> >>>  	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
> >>> -
> >>> -	if (intel_encoder->type != INTEL_OUTPUT_EDP)
> >>> -		intel_encoder->type = INTEL_OUTPUT_DP;
> >>>  }
> >>>  
> >>>  static int intel_dp_get_modes(struct drm_connector *connector)
> >>> @@ -5107,10 +5104,6 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
> >>>  	struct drm_i915_private *dev_priv = to_i915(dev);
> >>>  	enum irqreturn ret = IRQ_NONE;
> >>>  
> >>> -	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
> >>> -	    intel_dig_port->base.type != INTEL_OUTPUT_HDMI)
> >>> -		intel_dig_port->base.type = INTEL_OUTPUT_DP;
> >>> -
> >>>  	if (long_hpd && intel_dig_port->base.type == INTEL_OUTPUT_EDP) {
> >>>  		/*
> >>>  		 * vdd off can generate a long pulse on eDP which
> >>> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> >>> index df966427232c..629ebc73bb09 100644
> >>> --- a/drivers/gpu/drm/i915/intel_drv.h
> >>> +++ b/drivers/gpu/drm/i915/intel_drv.h
> >>> @@ -173,7 +173,7 @@ enum intel_output_type {
> >>>  	INTEL_OUTPUT_DP = 7,
> >>>  	INTEL_OUTPUT_EDP = 8,
> >>>  	INTEL_OUTPUT_DSI = 9,
> >>> -	INTEL_OUTPUT_UNKNOWN = 10,
> >>> +	INTEL_OUTPUT_DDI = 10,
> >>>  	INTEL_OUTPUT_DP_MST = 11,
> >>>  };
> >>>  
> >>> @@ -216,6 +216,9 @@ struct intel_encoder {
> >>>  	enum port port;
> >>>  	unsigned int cloneable;
> >>>  	void (*hot_plug)(struct intel_encoder *);
> >>> +	enum intel_output_type (*compute_output_type)(struct intel_encoder *,
> >>> +						      struct intel_crtc_state *,
> >>> +						      struct drm_connector_state *);
> >>>  	bool (*compute_config)(struct intel_encoder *,
> >>>  			       struct intel_crtc_state *,
> >>>  			       struct drm_connector_state *);
> >>> @@ -1152,7 +1155,7 @@ enc_to_dig_port(struct drm_encoder *encoder)
> >>>  	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> >>>  
> >>>  	switch (intel_encoder->type) {
> >>> -	case INTEL_OUTPUT_UNKNOWN:
> >>> +	case INTEL_OUTPUT_DDI:
> >>>  		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> >>>  	case INTEL_OUTPUT_DP:
> >>>  	case INTEL_OUTPUT_EDP:
> >>> diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> index 97d08cda3f8e..dede2898cb8a 100644
> >>> --- a/drivers/gpu/drm/i915/intel_hdmi.c
> >>> +++ b/drivers/gpu/drm/i915/intel_hdmi.c
> >>> @@ -1617,12 +1617,9 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >>>  
> >>>  	intel_hdmi_unset_edid(connector);
> >>>  
> >>> -	if (intel_hdmi_set_edid(connector)) {
> >>> -		struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> >>> -
> >>> -		hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >>> +	if (intel_hdmi_set_edid(connector))
> >>>  		status = connector_status_connected;
> >>> -	} else
> >>> +	else
> >>>  		status = connector_status_disconnected;
> >>>  
> >>>  	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
> >>> @@ -1633,8 +1630,6 @@ intel_hdmi_detect(struct drm_connector *connector, bool force)
> >>>  static void
> >>>  intel_hdmi_force(struct drm_connector *connector)
> >>>  {
> >>> -	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
> >>> -
> >>>  	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
> >>>  		      connector->base.id, connector->name);
> >>>  
> >>> @@ -1644,7 +1639,6 @@ intel_hdmi_force(struct drm_connector *connector)
> >>>  		return;
> >>>  
> >>>  	intel_hdmi_set_edid(connector);
> >>> -	hdmi_to_dig_port(intel_hdmi)->base.type = INTEL_OUTPUT_HDMI;
> >>>  }
> >>>  
> >>>  static int intel_hdmi_get_modes(struct drm_connector *connector)
> >>> diff --git a/drivers/gpu/drm/i915/intel_opregion.c b/drivers/gpu/drm/i915/intel_opregion.c
> >>> index 1d946240e55f..39714be3eb6b 100644
> >>> --- a/drivers/gpu/drm/i915/intel_opregion.c
> >>> +++ b/drivers/gpu/drm/i915/intel_opregion.c
> >>> @@ -383,7 +383,7 @@ int intel_opregion_notify_encoder(struct intel_encoder *intel_encoder,
> >>>  	case INTEL_OUTPUT_ANALOG:
> >>>  		type = DISPLAY_TYPE_CRT;
> >>>  		break;
> >>> -	case INTEL_OUTPUT_UNKNOWN:
> >>> +	case INTEL_OUTPUT_DDI:
> >>>  	case INTEL_OUTPUT_DP:
> >>>  	case INTEL_OUTPUT_HDMI:
> >>>  	case INTEL_OUTPUT_DP_MST:
> 

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

^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2017-10-30 18:28 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-10-27 19:31 [PATCH v2 0/6] drm/i915: Last part of DDI encoder->type cleanup Ville Syrjala
2017-10-27 19:31 ` [PATCH v2 1/6] drm/i915: Populate output_types from .get_config() Ville Syrjala
2017-10-27 19:31 ` [PATCH v5 2/6] drm/i915: Stop frobbing with DDI encoder->type Ville Syrjala
2017-10-30  8:59   ` Maarten Lankhorst
2017-10-30 16:07     ` Ville Syrjälä
2017-10-30 16:29       ` Maarten Lankhorst
2017-10-30 18:28         ` Ville Syrjälä
2017-10-27 19:31 ` [PATCH 3/6] drm/i915: Nuke intel_ddi_get_encoder_port() Ville Syrjala
2017-10-27 19:31 ` [PATCH v2 4/6] drm/i915: Eliminate pll->state usage from bxt_calc_pll_link() Ville Syrjala
2017-10-27 19:31 ` [PATCH v2 5/6] drm/i915: Pass a crtc state to ddi post_disable from MST code Ville Syrjala
2017-10-27 19:31 ` [PATCH v2 6/6] drm/i915: Use intel_ddi_get_config() for MST Ville Syrjala
2017-10-30  9:00   ` Maarten Lankhorst
2017-10-27 20:47 ` ✗ Fi.CI.BAT: failure for drm/i915: Last part of DDI encoder->type cleanup (rev2) Patchwork
2017-10-30 15:58 ` ✓ Fi.CI.BAT: success " Patchwork
2017-10-30 17:20 ` ✓ Fi.CI.IGT: " Patchwork

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.