All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts
@ 2017-02-22  6:34 Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
                   ` (7 more replies)
  0 siblings, 8 replies; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Hi,

Here's an updated version of the series, with an extra patch to prevent
us from converting a analog encoder to a struct intel_digital_port. With
this we can aboid the wrong conversion in intel_ddi_post_disable() which
leads to a put to a reference to the wrong power domain.

Thanks,
Ander


Ander Conselvan de Oliveira (6):
  drm/i915: Store aux power domain in intel_dp
  drm/i915: Store encoder power domain in struct intel_encoder
  drm/i915: Check encoder type in enc_to_dig_port()
  drm/i915/glk: Implement WaDDIIOTimeout
  drm/i915/glk: Don't enable DDI IO power domains during init
  drm/i915: Only enable DDI IO power domains after enabling DPLL

 drivers/gpu/drm/i915/i915_drv.h         |  11 ++++
 drivers/gpu/drm/i915/i915_reg.h         |   5 ++
 drivers/gpu/drm/i915/intel_crt.c        |  21 +++----
 drivers/gpu/drm/i915/intel_ddi.c        |  65 +++++++++++++++++---
 drivers/gpu/drm/i915/intel_display.c    | 101 +++++++-------------------------
 drivers/gpu/drm/i915/intel_dp.c         |  69 +++++++++-------------
 drivers/gpu/drm/i915/intel_dp_mst.c     |   1 +
 drivers/gpu/drm/i915/intel_drv.h        |  24 ++++++--
 drivers/gpu/drm/i915/intel_dsi.c        |   9 +--
 drivers/gpu/drm/i915/intel_dvo.c        |   1 +
 drivers/gpu/drm/i915/intel_hdmi.c       |   8 +--
 drivers/gpu/drm/i915/intel_lvds.c       |   8 +--
 drivers/gpu/drm/i915/intel_pm.c         |  10 ++++
 drivers/gpu/drm/i915/intel_runtime_pm.c |  71 ++++++++++++----------
 drivers/gpu/drm/i915/intel_sdvo.c       |   1 +
 drivers/gpu/drm/i915/intel_tv.c         |   1 +
 16 files changed, 216 insertions(+), 190 deletions(-)

-- 
2.9.3

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

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

* [PATCH v3 1/6] drm/i915: Store aux power domain in intel_dp
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
@ 2017-02-22  6:34 ` Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 2/6] drm/i915: Store encoder power domain in struct intel_encoder Ander Conselvan de Oliveira
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

The aux power domain only makes sense in the DP code. Storing it in
struct intel_dp avoids some indirection.

v2: Rebase
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 50 -----------------------------
 drivers/gpu/drm/i915/intel_dp.c      | 61 ++++++++++++++----------------------
 drivers/gpu/drm/i915/intel_drv.h     |  3 +-
 3 files changed, 24 insertions(+), 90 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f6feb93..80cfb67 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5587,26 +5587,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
 	}
 }
 
-static enum intel_display_power_domain port_to_aux_power_domain(enum port port)
-{
-	switch (port) {
-	case PORT_A:
-		return POWER_DOMAIN_AUX_A;
-	case PORT_B:
-		return POWER_DOMAIN_AUX_B;
-	case PORT_C:
-		return POWER_DOMAIN_AUX_C;
-	case PORT_D:
-		return POWER_DOMAIN_AUX_D;
-	case PORT_E:
-		/* FIXME: Check VBT for actual wiring of PORT E */
-		return POWER_DOMAIN_AUX_D;
-	default:
-		MISSING_CASE(port);
-		return POWER_DOMAIN_AUX_A;
-	}
-}
-
 enum intel_display_power_domain
 intel_display_port_power_domain(struct intel_encoder *intel_encoder)
 {
@@ -5634,36 +5614,6 @@ intel_display_port_power_domain(struct intel_encoder *intel_encoder)
 	}
 }
 
-enum intel_display_power_domain
-intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder)
-{
-	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
-	struct intel_digital_port *intel_dig_port;
-
-	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_UNKNOWN:
-	case INTEL_OUTPUT_HDMI:
-		/*
-		 * Only DDI platforms should ever use these output types.
-		 * We can get here after the HDMI detect code has already set
-		 * the type of the shared encoder. Since we can't be sure
-		 * what's the status of the given connectors, play safe and
-		 * run the DP detection too.
-		 */
-		WARN_ON_ONCE(!HAS_DDI(dev_priv));
-	case INTEL_OUTPUT_DP:
-	case INTEL_OUTPUT_EDP:
-		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
-		return port_to_aux_power_domain(intel_dig_port->port);
-	case INTEL_OUTPUT_DP_MST:
-		intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
-		return port_to_aux_power_domain(intel_dig_port->port);
-	default:
-		MISSING_CASE(intel_encoder->type);
-		return POWER_DOMAIN_AUX_A;
-	}
-}
-
 static u64 get_crtc_power_domains(struct drm_crtc *crtc,
 				  struct intel_crtc_state *crtc_state)
 {
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 024798a..258a30e 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -396,14 +396,12 @@ static void pps_lock(struct intel_dp *intel_dp)
 	struct intel_encoder *encoder = &intel_dig_port->base;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 
 	/*
 	 * See vlv_power_sequencer_reset() why we need
 	 * a power domain reference here.
 	 */
-	power_domain = intel_display_port_aux_power_domain(encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
 	mutex_lock(&dev_priv->pps_mutex);
 }
@@ -414,12 +412,10 @@ static void pps_unlock(struct intel_dp *intel_dp)
 	struct intel_encoder *encoder = &intel_dig_port->base;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 
 	mutex_unlock(&dev_priv->pps_mutex);
 
-	power_domain = intel_display_port_aux_power_domain(encoder);
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
 }
 
 static void
@@ -2005,9 +2001,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
 {
 	struct drm_device *dev = intel_dp_to_dev(intel_dp);
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 	u32 pp;
 	i915_reg_t pp_stat_reg, pp_ctrl_reg;
 	bool need_to_disable = !intel_dp->want_panel_vdd;
@@ -2023,8 +2017,7 @@ static bool edp_panel_vdd_on(struct intel_dp *intel_dp)
 	if (edp_have_panel_vdd(intel_dp))
 		return need_to_disable;
 
-	power_domain = intel_display_port_aux_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
 	DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
 		      port_name(intel_dig_port->port));
@@ -2082,8 +2075,6 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_digital_port *intel_dig_port =
 		dp_to_dig_port(intel_dp);
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
-	enum intel_display_power_domain power_domain;
 	u32 pp;
 	i915_reg_t pp_stat_reg, pp_ctrl_reg;
 
@@ -2113,8 +2104,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	if ((pp & PANEL_POWER_ON) == 0)
 		intel_dp->panel_power_off_time = ktime_get_boottime();
 
-	power_domain = intel_display_port_aux_power_domain(intel_encoder);
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
 }
 
 static void edp_panel_vdd_work(struct work_struct *__work)
@@ -2227,11 +2217,8 @@ void intel_edp_panel_on(struct intel_dp *intel_dp)
 
 static void edp_panel_off(struct intel_dp *intel_dp)
 {
-	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 = intel_dp_to_dev(intel_dp);
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 	u32 pp;
 	i915_reg_t pp_ctrl_reg;
 
@@ -2263,8 +2250,7 @@ static void edp_panel_off(struct intel_dp *intel_dp)
 	wait_panel_off(intel_dp);
 
 	/* We got a reference when we enabled the VDD. */
-	power_domain = intel_display_port_aux_power_domain(intel_encoder);
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
 }
 
 void intel_edp_panel_off(struct intel_dp *intel_dp)
@@ -4587,11 +4573,9 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = connector->dev;
 	enum drm_connector_status status;
-	enum intel_display_power_domain power_domain;
 	u8 sink_irq_vector = 0;
 
-	power_domain = intel_display_port_aux_power_domain(intel_encoder);
-	intel_display_power_get(to_i915(dev), power_domain);
+	intel_display_power_get(to_i915(dev), intel_dp->aux_power_domain);
 
 	/* Can't disconnect eDP, but you can close the lid... */
 	if (is_edp(intel_dp))
@@ -4692,7 +4676,7 @@ intel_dp_long_pulse(struct intel_connector *intel_connector)
 	if (status != connector_status_connected && !intel_dp->is_mst)
 		intel_dp_unset_edid(intel_dp);
 
-	intel_display_power_put(to_i915(dev), power_domain);
+	intel_display_power_put(to_i915(dev), intel_dp->aux_power_domain);
 	return status;
 }
 
@@ -4720,7 +4704,6 @@ intel_dp_force(struct drm_connector *connector)
 	struct intel_dp *intel_dp = intel_attached_dp(connector);
 	struct intel_encoder *intel_encoder = &dp_to_dig_port(intel_dp)->base;
 	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
-	enum intel_display_power_domain power_domain;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s]\n",
 		      connector->base.id, connector->name);
@@ -4729,12 +4712,11 @@ intel_dp_force(struct drm_connector *connector)
 	if (connector->status != connector_status_connected)
 		return;
 
-	power_domain = intel_display_port_aux_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
 	intel_dp_set_edid(intel_dp);
 
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
 
 	if (intel_encoder->type != INTEL_OUTPUT_EDP)
 		intel_encoder->type = INTEL_OUTPUT_DP;
@@ -4969,7 +4951,6 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 	struct intel_digital_port *intel_dig_port = dp_to_dig_port(intel_dp);
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 
 	lockdep_assert_held(&dev_priv->pps_mutex);
 
@@ -4983,8 +4964,7 @@ static void intel_edp_panel_vdd_sanitize(struct intel_dp *intel_dp)
 	 * indefinitely.
 	 */
 	DRM_DEBUG_KMS("VDD left on by BIOS, adjusting state tracking\n");
-	power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
 	edp_panel_vdd_schedule_off(intel_dp);
 }
@@ -5058,10 +5038,8 @@ enum irqreturn
 intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 {
 	struct intel_dp *intel_dp = &intel_dig_port->dp;
-	struct intel_encoder *intel_encoder = &intel_dig_port->base;
 	struct drm_device *dev = intel_dig_port->base.base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 	enum irqreturn ret = IRQ_NONE;
 
 	if (intel_dig_port->base.type != INTEL_OUTPUT_EDP &&
@@ -5090,8 +5068,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		return IRQ_NONE;
 	}
 
-	power_domain = intel_display_port_aux_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_dp->aux_power_domain);
 
 	if (intel_dp->is_mst) {
 		if (intel_dp_check_mst_status(intel_dp) == -EINVAL) {
@@ -5119,7 +5096,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 	ret = IRQ_HANDLED;
 
 put_power:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_dp->aux_power_domain);
 
 	return ret;
 }
@@ -5910,27 +5887,35 @@ static bool intel_edp_init_connector(struct intel_dp *intel_dp,
 	return false;
 }
 
+/* Set up the hotplug pin and aux power domain. */
 static void
 intel_dp_init_connector_port_info(struct intel_digital_port *intel_dig_port)
 {
 	struct intel_encoder *encoder = &intel_dig_port->base;
+	struct intel_dp *intel_dp = &intel_dig_port->dp;
 
-	/* Set up the hotplug pin. */
 	switch (intel_dig_port->port) {
 	case PORT_A:
 		encoder->hpd_pin = HPD_PORT_A;
+		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_A;
 		break;
 	case PORT_B:
 		encoder->hpd_pin = HPD_PORT_B;
+		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_B;
 		break;
 	case PORT_C:
 		encoder->hpd_pin = HPD_PORT_C;
+		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_C;
 		break;
 	case PORT_D:
 		encoder->hpd_pin = HPD_PORT_D;
+		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
 		break;
 	case PORT_E:
 		encoder->hpd_pin = HPD_PORT_E;
+
+		/* FIXME: Check VBT for actual wiring of PORT E */
+		intel_dp->aux_power_domain = POWER_DOMAIN_AUX_D;
 		break;
 	default:
 		MISSING_CASE(intel_dig_port->port);
@@ -6011,6 +5996,8 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	connector->interlace_allowed = true;
 	connector->doublescan_allowed = 0;
 
+	intel_dp_init_connector_port_info(intel_dig_port);
+
 	intel_dp_aux_init(intel_dp);
 
 	INIT_DELAYED_WORK(&intel_dp->panel_vdd_work,
@@ -6023,8 +6010,6 @@ intel_dp_init_connector(struct intel_digital_port *intel_dig_port,
 	else
 		intel_connector->get_hw_state = intel_connector_get_hw_state;
 
-	intel_dp_init_connector_port_info(intel_dig_port);
-
 	/* init MST on ports that can support it */
 	if (HAS_DP_MST(dev_priv) && !is_edp(intel_dp) &&
 	    (port == PORT_B || port == PORT_C || port == PORT_D))
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 821c57c..096d77b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -951,6 +951,7 @@ struct intel_dp {
 	/* sink or branch descriptor */
 	struct intel_dp_desc desc;
 	struct drm_dp_aux aux;
+	enum intel_display_power_domain aux_power_domain;
 	uint8_t train_set[4];
 	int panel_power_up_delay;
 	int panel_power_down_delay;
@@ -1421,8 +1422,6 @@ void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
 enum intel_display_power_domain
 intel_display_port_power_domain(struct intel_encoder *intel_encoder);
-enum intel_display_power_domain
-intel_display_port_aux_power_domain(struct intel_encoder *intel_encoder);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 
-- 
2.9.3

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

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

* [PATCH v3 2/6] drm/i915: Store encoder power domain in struct intel_encoder
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
@ 2017-02-22  6:34 ` Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 3/6] drm/i915: Check encoder type in enc_to_dig_port() Ander Conselvan de Oliveira
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

The encoder power domain is obviously tied to the encoder, so store it
in struct intel_encoder. This avoids some indirection.

v2: Rebase
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/intel_crt.c     | 21 +++++++++------------
 drivers/gpu/drm/i915/intel_ddi.c     | 15 +++++++--------
 drivers/gpu/drm/i915/intel_display.c | 31 ++-----------------------------
 drivers/gpu/drm/i915/intel_dp.c      |  8 ++++----
 drivers/gpu/drm/i915/intel_dp_mst.c  |  1 +
 drivers/gpu/drm/i915/intel_drv.h     |  4 ++--
 drivers/gpu/drm/i915/intel_dsi.c     |  9 +++++----
 drivers/gpu/drm/i915/intel_dvo.c     |  1 +
 drivers/gpu/drm/i915/intel_hdmi.c    |  8 ++++----
 drivers/gpu/drm/i915/intel_lvds.c    |  8 ++++----
 drivers/gpu/drm/i915/intel_sdvo.c    |  1 +
 drivers/gpu/drm/i915/intel_tv.c      |  1 +
 12 files changed, 41 insertions(+), 67 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_crt.c b/drivers/gpu/drm/i915/intel_crt.c
index 2bf5aca..8c82607 100644
--- a/drivers/gpu/drm/i915/intel_crt.c
+++ b/drivers/gpu/drm/i915/intel_crt.c
@@ -69,12 +69,11 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crt *crt = intel_encoder_to_crt(encoder);
-	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	bool ret;
 
-	power_domain = intel_display_port_power_domain(encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
 		return false;
 
 	ret = false;
@@ -91,7 +90,7 @@ static bool intel_crt_get_hw_state(struct intel_encoder *encoder,
 
 	ret = true;
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
 }
@@ -676,7 +675,6 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_crt *crt = intel_attached_crt(connector);
 	struct intel_encoder *intel_encoder = &crt->base;
-	enum intel_display_power_domain power_domain;
 	enum drm_connector_status status;
 	struct intel_load_detect_pipe tmp;
 	struct drm_modeset_acquire_ctx ctx;
@@ -689,8 +687,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	if (dmi_check_system(intel_spurious_crt_detect))
 		return connector_status_disconnected;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_encoder->power_domain);
 
 	if (I915_HAS_HOTPLUG(dev_priv)) {
 		/* We can not rely on the HPD pin always being correctly wired
@@ -745,7 +742,7 @@ intel_crt_detect(struct drm_connector *connector, bool force)
 	drm_modeset_acquire_fini(&ctx);
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_encoder->power_domain);
 	return status;
 }
 
@@ -761,12 +758,10 @@ static int intel_crt_get_modes(struct drm_connector *connector)
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_crt *crt = intel_attached_crt(connector);
 	struct intel_encoder *intel_encoder = &crt->base;
-	enum intel_display_power_domain power_domain;
 	int ret;
 	struct i2c_adapter *i2c;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, intel_encoder->power_domain);
 
 	i2c = intel_gmbus_get_adapter(dev_priv, dev_priv->vbt.crt_ddc_pin);
 	ret = intel_crt_ddc_get_modes(connector, i2c);
@@ -778,7 +773,7 @@ static int intel_crt_get_modes(struct drm_connector *connector)
 	ret = intel_crt_ddc_get_modes(connector, i2c);
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_encoder->power_domain);
 
 	return ret;
 }
@@ -904,6 +899,8 @@ void intel_crt_init(struct drm_i915_private *dev_priv)
 
 	crt->adpa_reg = adpa_reg;
 
+	crt->base.power_domain = POWER_DOMAIN_PORT_CRT;
+
 	crt->base.compute_config = intel_crt_compute_config;
 	if (HAS_PCH_SPLIT(dev_priv)) {
 		crt->base.disable = pch_disable_crt;
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index cd6fedd..b0c4d23 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1316,12 +1316,11 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
 	enum pipe pipe = 0;
 	enum transcoder cpu_transcoder;
-	enum intel_display_power_domain power_domain;
 	uint32_t tmp;
 	bool ret;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						intel_encoder->power_domain))
 		return false;
 
 	if (!intel_encoder->get_hw_state(intel_encoder, &pipe)) {
@@ -1363,7 +1362,7 @@ bool intel_ddi_connector_get_hw_state(struct intel_connector *intel_connector)
 	}
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, intel_encoder->power_domain);
 
 	return ret;
 }
@@ -1374,13 +1373,12 @@ 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 intel_display_power_domain power_domain;
 	u32 tmp;
 	int i;
 	bool ret;
 
-	power_domain = intel_display_port_power_domain(encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
 		return false;
 
 	ret = false;
@@ -1437,7 +1435,7 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 				  "(PHY_CTL %08x)\n", port_name(port), tmp);
 	}
 
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
 }
@@ -2216,6 +2214,7 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_dig_port->max_lanes = max_lanes;
 
 	intel_encoder->type = INTEL_OUTPUT_UNKNOWN;
+	intel_encoder->power_domain = intel_port_to_power_domain(port);
 	intel_encoder->port = port;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1) | (1 << 2);
 	intel_encoder->cloneable = 0;
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 80cfb67..1ffec8f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5568,7 +5568,7 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
 	I915_WRITE(BCLRPAT(crtc->pipe), 0);
 }
 
-static enum intel_display_power_domain port_to_power_domain(enum port port)
+enum intel_display_power_domain intel_port_to_power_domain(enum port port)
 {
 	switch (port) {
 	case PORT_A:
@@ -5587,33 +5587,6 @@ static enum intel_display_power_domain port_to_power_domain(enum port port)
 	}
 }
 
-enum intel_display_power_domain
-intel_display_port_power_domain(struct intel_encoder *intel_encoder)
-{
-	struct drm_i915_private *dev_priv = to_i915(intel_encoder->base.dev);
-	struct intel_digital_port *intel_dig_port;
-
-	switch (intel_encoder->type) {
-	case INTEL_OUTPUT_UNKNOWN:
-		/* Only DDI platforms should ever use this output type */
-		WARN_ON_ONCE(!HAS_DDI(dev_priv));
-	case INTEL_OUTPUT_DP:
-	case INTEL_OUTPUT_HDMI:
-	case INTEL_OUTPUT_EDP:
-		intel_dig_port = enc_to_dig_port(&intel_encoder->base);
-		return port_to_power_domain(intel_dig_port->port);
-	case INTEL_OUTPUT_DP_MST:
-		intel_dig_port = enc_to_mst(&intel_encoder->base)->primary;
-		return port_to_power_domain(intel_dig_port->port);
-	case INTEL_OUTPUT_ANALOG:
-		return POWER_DOMAIN_PORT_CRT;
-	case INTEL_OUTPUT_DSI:
-		return POWER_DOMAIN_PORT_DSI;
-	default:
-		return POWER_DOMAIN_PORT_OTHER;
-	}
-}
-
 static u64 get_crtc_power_domains(struct drm_crtc *crtc,
 				  struct intel_crtc_state *crtc_state)
 {
@@ -5637,7 +5610,7 @@ static u64 get_crtc_power_domains(struct drm_crtc *crtc,
 	drm_for_each_encoder_mask(encoder, dev, crtc_state->base.encoder_mask) {
 		struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
 
-		mask |= BIT_ULL(intel_display_port_power_domain(intel_encoder));
+		mask |= BIT_ULL(intel_encoder->power_domain);
 	}
 
 	if (HAS_DDI(dev_priv) && crtc_state->has_audio)
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 258a30e..6633890 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -2496,12 +2496,11 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	enum port port = dp_to_dig_port(intel_dp)->port;
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	bool ret;
 
-	power_domain = intel_display_port_power_domain(encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
 		return false;
 
 	ret = false;
@@ -2537,7 +2536,7 @@ static bool intel_dp_get_hw_state(struct intel_encoder *encoder,
 	ret = true;
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
 }
@@ -6094,6 +6093,7 @@ bool intel_dp_init(struct drm_i915_private *dev_priv,
 	intel_dig_port->max_lanes = 4;
 
 	intel_encoder->type = INTEL_OUTPUT_DP;
+	intel_encoder->power_domain = intel_port_to_power_domain(port);
 	if (IS_CHERRYVIEW(dev_priv)) {
 		if (port == PORT_D)
 			intel_encoder->crtc_mask = 1 << 2;
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 6a85d38..d94fd4b 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -548,6 +548,7 @@ intel_dp_create_fake_mst_encoder(struct intel_digital_port *intel_dig_port, enum
 			 DRM_MODE_ENCODER_DPMST, "DP-MST %c", pipe_name(pipe));
 
 	intel_encoder->type = INTEL_OUTPUT_DP_MST;
+	intel_encoder->power_domain = intel_dig_port->base.power_domain;
 	intel_encoder->port = intel_dig_port->port;
 	intel_encoder->crtc_mask = 0x7;
 	intel_encoder->cloneable = 0;
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 096d77b..a3cf866 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -249,6 +249,7 @@ struct intel_encoder {
 	void (*suspend)(struct intel_encoder *);
 	int crtc_mask;
 	enum hpd_pin hpd_pin;
+	enum intel_display_power_domain power_domain;
 	/* for communication with audio component; protected by av_mutex */
 	const struct drm_connector *audio_connector;
 };
@@ -1420,8 +1421,7 @@ int chv_calc_dpll_params(int refclk, struct dpll *pll_clock);
 bool intel_crtc_active(struct intel_crtc *crtc);
 void hsw_enable_ips(struct intel_crtc *crtc);
 void hsw_disable_ips(struct intel_crtc *crtc);
-enum intel_display_power_domain
-intel_display_port_power_domain(struct intel_encoder *intel_encoder);
+enum intel_display_power_domain intel_port_to_power_domain(enum port port);
 void intel_mode_from_pipe_config(struct drm_display_mode *mode,
 				 struct intel_crtc_state *pipe_config);
 
diff --git a/drivers/gpu/drm/i915/intel_dsi.c b/drivers/gpu/drm/i915/intel_dsi.c
index c26fe4f..d7ffb9a 100644
--- a/drivers/gpu/drm/i915/intel_dsi.c
+++ b/drivers/gpu/drm/i915/intel_dsi.c
@@ -775,14 +775,13 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 {
 	struct drm_i915_private *dev_priv = to_i915(encoder->base.dev);
 	struct intel_dsi *intel_dsi = enc_to_intel_dsi(&encoder->base);
-	enum intel_display_power_domain power_domain;
 	enum port port;
 	bool active = false;
 
 	DRM_DEBUG_KMS("\n");
 
-	power_domain = intel_display_port_power_domain(encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
 		return false;
 
 	/*
@@ -838,7 +837,7 @@ static bool intel_dsi_get_hw_state(struct intel_encoder *encoder,
 	}
 
 out_put_power:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return active;
 }
@@ -1516,6 +1515,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	intel_connector->get_hw_state = intel_connector_get_hw_state;
 
 	intel_encoder->port = port;
+
 	/*
 	 * On BYT/CHV, pipe A maps to MIPI DSI port A, pipe B maps to MIPI DSI
 	 * port C. BXT isn't limited like this.
@@ -1603,6 +1603,7 @@ void intel_dsi_init(struct drm_i915_private *dev_priv)
 	}
 
 	intel_encoder->type = INTEL_OUTPUT_DSI;
+	intel_encoder->power_domain = POWER_DOMAIN_PORT_DSI;
 	intel_encoder->cloneable = 0;
 	drm_connector_init(dev, connector, &intel_dsi_connector_funcs,
 			   DRM_MODE_CONNECTOR_DSI);
diff --git a/drivers/gpu/drm/i915/intel_dvo.c b/drivers/gpu/drm/i915/intel_dvo.c
index 50da89d..6025839 100644
--- a/drivers/gpu/drm/i915/intel_dvo.c
+++ b/drivers/gpu/drm/i915/intel_dvo.c
@@ -515,6 +515,7 @@ void intel_dvo_init(struct drm_i915_private *dev_priv)
 				 "DVO %c", port_name(port));
 
 		intel_encoder->type = INTEL_OUTPUT_DVO;
+		intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
 		intel_encoder->port = port;
 		intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index a580de8..c2184f7 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -902,12 +902,11 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_hdmi *intel_hdmi = enc_to_intel_hdmi(&encoder->base);
-	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	bool ret;
 
-	power_domain = intel_display_port_power_domain(encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
 		return false;
 
 	ret = false;
@@ -927,7 +926,7 @@ static bool intel_hdmi_get_hw_state(struct intel_encoder *encoder,
 	ret = true;
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
 }
@@ -1981,6 +1980,7 @@ void intel_hdmi_init(struct drm_i915_private *dev_priv,
 	}
 
 	intel_encoder->type = INTEL_OUTPUT_HDMI;
+	intel_encoder->power_domain = intel_port_to_power_domain(port);
 	intel_encoder->port = port;
 	if (IS_CHERRYVIEW(dev_priv)) {
 		if (port == PORT_D)
diff --git a/drivers/gpu/drm/i915/intel_lvds.c b/drivers/gpu/drm/i915/intel_lvds.c
index 9ca4dc4..8b942ef 100644
--- a/drivers/gpu/drm/i915/intel_lvds.c
+++ b/drivers/gpu/drm/i915/intel_lvds.c
@@ -91,12 +91,11 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 	struct drm_device *dev = encoder->base.dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
 	struct intel_lvds_encoder *lvds_encoder = to_lvds_encoder(&encoder->base);
-	enum intel_display_power_domain power_domain;
 	u32 tmp;
 	bool ret;
 
-	power_domain = intel_display_port_power_domain(encoder);
-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain))
+	if (!intel_display_power_get_if_enabled(dev_priv,
+						encoder->power_domain))
 		return false;
 
 	ret = false;
@@ -114,7 +113,7 @@ static bool intel_lvds_get_hw_state(struct intel_encoder *encoder,
 	ret = true;
 
 out:
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, encoder->power_domain);
 
 	return ret;
 }
@@ -1066,6 +1065,7 @@ void intel_lvds_init(struct drm_i915_private *dev_priv)
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 
 	intel_encoder->type = INTEL_OUTPUT_LVDS;
+	intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
 	intel_encoder->port = PORT_NONE;
 	intel_encoder->cloneable = 0;
 	if (HAS_PCH_SPLIT(dev_priv))
diff --git a/drivers/gpu/drm/i915/intel_sdvo.c b/drivers/gpu/drm/i915/intel_sdvo.c
index 2ad1390..816a6f5 100644
--- a/drivers/gpu/drm/i915/intel_sdvo.c
+++ b/drivers/gpu/drm/i915/intel_sdvo.c
@@ -2981,6 +2981,7 @@ bool intel_sdvo_init(struct drm_i915_private *dev_priv,
 	/* encoder type will be decided later */
 	intel_encoder = &intel_sdvo->base;
 	intel_encoder->type = INTEL_OUTPUT_SDVO;
+	intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
 	intel_encoder->port = port;
 	drm_encoder_init(&dev_priv->drm, &intel_encoder->base,
 			 &intel_sdvo_enc_funcs, 0,
diff --git a/drivers/gpu/drm/i915/intel_tv.c b/drivers/gpu/drm/i915/intel_tv.c
index eb692e4..6ed1a3c 100644
--- a/drivers/gpu/drm/i915/intel_tv.c
+++ b/drivers/gpu/drm/i915/intel_tv.c
@@ -1621,6 +1621,7 @@ intel_tv_init(struct drm_i915_private *dev_priv)
 	intel_connector_attach_encoder(intel_connector, intel_encoder);
 
 	intel_encoder->type = INTEL_OUTPUT_TVOUT;
+	intel_encoder->power_domain = POWER_DOMAIN_PORT_OTHER;
 	intel_encoder->port = PORT_NONE;
 	intel_encoder->crtc_mask = (1 << 0) | (1 << 1);
 	intel_encoder->cloneable = 0;
-- 
2.9.3

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

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

* [PATCH v3 3/6] drm/i915: Check encoder type in enc_to_dig_port()
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 2/6] drm/i915: Store encoder power domain in struct intel_encoder Ander Conselvan de Oliveira
@ 2017-02-22  6:34 ` Ander Conselvan de Oliveira
  2017-02-24 13:38   ` Imre Deak
  2017-02-22  6:34 ` [PATCH v3 4/6] drm/i915/glk: Implement WaDDIIOTimeout Ander Conselvan de Oliveira
                   ` (4 subsequent siblings)
  7 siblings, 1 reply; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Don't allow conversion from arbitraty encoder types to a digital port.
Calling enc_to_dig_port() with the wrong encoder may seem far fetched,
but certain paths of the ddi code may be called with hasell's analog
encoder and the conversion is wrong for DP mst encoders too, so safe
guard against it.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a3cf866..2011651 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1121,7 +1121,18 @@ intel_attached_encoder(struct drm_connector *connector)
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
-	return container_of(encoder, struct intel_digital_port, base.base);
+	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
+	switch (intel_encoder->type) {
+	case INTEL_OUTPUT_DP:
+	case INTEL_OUTPUT_EDP:
+	case INTEL_OUTPUT_HDMI:
+	case INTEL_OUTPUT_UNKNOWN:
+		return container_of(encoder, struct intel_digital_port,
+				    base.base);
+	default:
+		return NULL;
+	}
 }
 
 static inline struct intel_dp_mst_encoder *
-- 
2.9.3

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

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

* [PATCH v3 4/6] drm/i915/glk: Implement WaDDIIOTimeout
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
                   ` (2 preceding siblings ...)
  2017-02-22  6:34 ` [PATCH v3 3/6] drm/i915: Check encoder type in enc_to_dig_port() Ander Conselvan de Oliveira
@ 2017-02-22  6:34 ` Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 5/6] drm/i915/glk: Don't enable DDI IO power domains during init Ander Conselvan de Oliveira
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Implement WaDDIIOTimeout to avoid a timeout when enabling the DDI IO
power domains.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h |  6 ++++++
 drivers/gpu/drm/i915/i915_reg.h |  5 +++++
 drivers/gpu/drm/i915/intel_pm.c | 10 ++++++++++
 3 files changed, 21 insertions(+)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4e7046d..d23069b 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2794,6 +2794,12 @@ intel_info(const struct drm_i915_private *dev_priv)
 #define IS_KBL_REVID(dev_priv, since, until) \
 	(IS_KABYLAKE(dev_priv) && IS_REVID(dev_priv, since, until))
 
+#define GLK_REVID_A0		0x0
+#define GLK_REVID_A1		0x1
+
+#define IS_GLK_REVID(dev_priv, since, until) \
+	(IS_GEMINILAKE(dev_priv) && IS_REVID(dev_priv, since, until))
+
 /*
  * The genX designation typically refers to the render engine, so render
  * capability related checks should use IS_GEN, while display and other checks
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 141a5c1..ce6f096 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -6495,6 +6495,11 @@ enum {
 #define CHICKEN_PAR2_1		_MMIO(0x42090)
 #define  KVM_CONFIG_CHANGE_NOTIFICATION_SELECT	(1 << 14)
 
+#define CHICKEN_MISC_2		_MMIO(0x42084)
+#define  GLK_CL0_PWR_DOWN	(1 << 10)
+#define  GLK_CL1_PWR_DOWN	(1 << 11)
+#define  GLK_CL2_PWR_DOWN	(1 << 12)
+
 #define _CHICKEN_PIPESL_1_A	0x420b0
 #define _CHICKEN_PIPESL_1_B	0x420b4
 #define  HSW_FBCQ_DIS			(1 << 22)
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 169c490..b38707e 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -114,6 +114,16 @@ static void glk_init_clock_gating(struct drm_i915_private *dev_priv)
 	 */
 	I915_WRITE(GEN9_CLKGATE_DIS_0, I915_READ(GEN9_CLKGATE_DIS_0) |
 		   PWM1_GATING_DIS | PWM2_GATING_DIS);
+
+	/* WaDDIIOTimeout:glk */
+	if (IS_GLK_REVID(dev_priv, 0, GLK_REVID_A1)) {
+		u32 val = I915_READ(CHICKEN_MISC_2);
+		val &= ~(GLK_CL0_PWR_DOWN |
+			 GLK_CL1_PWR_DOWN |
+			 GLK_CL2_PWR_DOWN);
+		I915_WRITE(CHICKEN_MISC_2, val);
+	}
+
 }
 
 static void i915_pineview_get_mem_freq(struct drm_i915_private *dev_priv)
-- 
2.9.3

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

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

* [PATCH v3 5/6] drm/i915/glk: Don't enable DDI IO power domains during init
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
                   ` (3 preceding siblings ...)
  2017-02-22  6:34 ` [PATCH v3 4/6] drm/i915/glk: Implement WaDDIIOTimeout Ander Conselvan de Oliveira
@ 2017-02-22  6:34 ` Ander Conselvan de Oliveira
  2017-02-22  6:34 ` [PATCH v3 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

In Geminilake, the DDI IO power domains can't be enabled before a DPLL
is running and mapped to the appropriate DDI. At least on Geminilake,
attempting to enable those during init will lead to a timeout.

The failure to enable the power domain also causes issues with the state
verifier during resume from suspend. After all the init power domains
are enabled, the call to intel_power_domains_sync_hw() from the resume
path will cause the hw_enabled field on the respective power wells to be
false while the usage count remains above zero. Further attempts to
enable the power domain caused by a modeset will simply update the usage
count without doing anything else. When the state verifier attempts to
read the state of a DDI encoder, intel_display_power_get_if_enabled()
returns false, leading to the following WARN:

WARNING: CPU: 3 PID: 1743 at drivers/gpu/drm/i915/intel_display.c:7001 verify_connector_state.isra.80+0x26c/0x2b0 [i915]
attached crtc is active, but connector isn't
Modules linked in: i915(E) tun ip6t_rpfilter ip6t_REJECT nf_reject_ipv6 xt_conntrack ebtable_broute bridge stp llc ebtable_nat ip6table_mangle ip6table_security ip6table_nat nf_conntrack_ipv6 nf_defrag_ipv6 nf_nat_ipv6 ip6table_raw iptable_mangle iptable_security iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 nf_nat nf_conntrack iptable_raw ebtable_filter ebtables ip6table_filter ip6_tables x86_pkg_temp_thermal coretemp kvm_intel kvm i2c_algo_bit drm_kms_helper irqbypass crct10dif_pclmul crc32_pclmul ghash_clmulni_intel drm shpchp tpm_tis tpm_tis_core tpm nfsd auth_rpcgss nfs_acl lockd grace sunrpc crc32c_intel serio_raw [last unloaded: i915]
CPU: 3 PID: 1743 Comm: kworker/u8:22 Tainted: G        W   E   4.10.0-rc3ander+ #300
Hardware name: Intel Corp. Geminilake/GLK RVP1 DDR4 (05), BIOS GELKRVPA.X64.0023.B40.1611302145 11/30/2016
Workqueue: events_unbound async_run_entry_fn
Call Trace:
 dump_stack+0x86/0xc3
 __warn+0xcb/0xf0
 warn_slowpath_fmt+0x5f/0x80
 verify_connector_state.isra.80+0x26c/0x2b0 [i915]
 intel_atomic_commit_tail+0x520/0x1000 [i915]
 ? remove_wait_queue+0x70/0x70
 intel_atomic_commit+0x3f8/0x520 [i915]
 ? intel_runtime_pm_put+0x6e/0xa0 [i915]
 drm_atomic_commit+0x4b/0x50 [drm]
 __intel_display_resume+0x72/0xc0 [i915]
 intel_display_resume+0x107/0x150 [i915]
 i915_drm_resume+0xe0/0x180 [i915]
 i915_pm_restore+0x1e/0x30 [i915]
 i915_pm_resume+0xe/0x10 [i915]
 pci_pm_resume+0x64/0xa0
 dpm_run_callback+0xa1/0x2a0
 ? pci_pm_thaw+0x90/0x90
 device_resume+0xe3/0x200
 async_resume+0x1d/0x50
 async_run_entry_fn+0x39/0x170
 process_one_work+0x212/0x670
 ? process_one_work+0x197/0x670
 worker_thread+0x4e/0x490
 kthread+0x101/0x140
 ? process_one_work+0x670/0x670
 ? kthread_create_on_node+0x60/0x60
 ret_from_fork+0x2a/0x40

Cc: David Weinehall <david.weinehall@linux.intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 6b52258..514ef56 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -452,14 +452,11 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_VGA) |				\
 	BIT_ULL(POWER_DOMAIN_INIT))
 #define GLK_DISPLAY_DDI_A_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
-	BIT_ULL(POWER_DOMAIN_INIT))
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
 #define GLK_DISPLAY_DDI_B_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
-	BIT_ULL(POWER_DOMAIN_INIT))
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
 #define GLK_DISPLAY_DDI_C_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
-	BIT_ULL(POWER_DOMAIN_INIT))
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
 #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
 	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
 	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
-- 
2.9.3

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

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

* [PATCH v3 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
                   ` (4 preceding siblings ...)
  2017-02-22  6:34 ` [PATCH v3 5/6] drm/i915/glk: Don't enable DDI IO power domains during init Ander Conselvan de Oliveira
@ 2017-02-22  6:34 ` Ander Conselvan de Oliveira
  2017-02-24 13:50   ` Imre Deak
  2017-02-22  7:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev3) Patchwork
  2017-02-24 15:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev6) Patchwork
  7 siblings, 1 reply; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-22  6:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

According to bspec, the DDI IO power domains should be enabled after
enabling the DPLL and mapping it to the DDI. The current order doesn't
seem to create problems with Skylake and Kabylake, but causes enable
timeouts in Geminilake.

v2: Rebase.
  - Take power domain references before sanitizing encoders. (Imre)
  - Add comment to get_encoder_power_domains() defition. (Ander)

v3: Don't put the domain if called with HSW/BDW's analog encoder. (CI)

Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> # v1
---
 drivers/gpu/drm/i915/i915_drv.h         |  5 +++
 drivers/gpu/drm/i915/intel_ddi.c        | 50 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c    | 20 ++++++++++
 drivers/gpu/drm/i915/intel_drv.h        |  4 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 68 +++++++++++++++++++--------------
 5 files changed, 118 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index d23069b..e346b2d 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -343,6 +343,11 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_PORT_DDI_C_LANES,
 	POWER_DOMAIN_PORT_DDI_D_LANES,
 	POWER_DOMAIN_PORT_DDI_E_LANES,
+	POWER_DOMAIN_PORT_DDI_A_IO,
+	POWER_DOMAIN_PORT_DDI_B_IO,
+	POWER_DOMAIN_PORT_DDI_C_IO,
+	POWER_DOMAIN_PORT_DDI_D_IO,
+	POWER_DOMAIN_PORT_DDI_E_IO,
 	POWER_DOMAIN_PORT_DSI,
 	POWER_DOMAIN_PORT_CRT,
 	POWER_DOMAIN_PORT_OTHER,
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b0c4d23..f50b485 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1440,6 +1440,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	return ret;
 }
 
+static unsigned long long
+intel_ddi_get_power_domains(struct intel_encoder *encoder)
+{
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	enum pipe pipe;
+
+	if (intel_ddi_get_hw_state(encoder, &pipe))
+		return BIT_ULL(dig_port->ddi_io_power_domain);
+
+	return 0;
+}
+
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
@@ -1682,6 +1694,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);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
 	intel_dp_set_link_params(intel_dp, link_rate, lane_count,
 				 link_mst);
@@ -1689,6 +1702,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 		intel_edp_panel_on(intel_dp);
 
 	intel_ddi_clk_select(encoder, pll);
+
+	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
+
 	intel_prepare_dp_ddi_buffers(encoder);
 	intel_ddi_init_dp_buf_reg(encoder);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
@@ -1708,9 +1724,13 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	struct drm_encoder *drm_encoder = &encoder->base;
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	int level = intel_ddi_hdmi_level(dev_priv, port);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
 	intel_ddi_clk_select(encoder, pll);
+
+	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
+
 	intel_prepare_hdmi_ddi_buffers(encoder);
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level);
@@ -1754,6 +1774,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 	int type = intel_encoder->type;
 	uint32_t val;
 	bool wait = false;
@@ -1793,6 +1814,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 
 		intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
 	}
+
+	if (dig_port)
+		intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
 }
 
 void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
@@ -2190,12 +2214,38 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
 	intel_encoder->get_config = intel_ddi_get_config;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
+	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
 
 	intel_dig_port->port = port;
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
 
+	switch (port) {
+	case PORT_A:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_A_IO;
+		break;
+	case PORT_B:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_B_IO;
+		break;
+	case PORT_C:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_C_IO;
+		break;
+	case PORT_D:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_D_IO;
+		break;
+	case PORT_E:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_E_IO;
+		break;
+	default:
+		MISSING_CASE(port);
+	}
+
 	/*
 	 * Bspec says that DDI_A_4_LANES is the only supported configuration
 	 * for Broxton.  Yet some BIOS fail to set this bit on port A if eDP
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1ffec8f..957c62d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15407,6 +15407,24 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	}
 }
 
+static void
+get_encoder_power_domains(struct drm_i915_private *dev_priv)
+{
+	struct intel_encoder *encoder;
+
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		u64 get_domains;
+		enum intel_display_power_domain domain;
+
+		if (!encoder->get_power_domains)
+			continue;
+
+		get_domains = encoder->get_power_domains(encoder);
+		for_each_power_domain(domain, get_domains)
+			intel_display_power_get(dev_priv, domain);
+	}
+}
+
 /* Scan out the current hw modeset state,
  * and sanitizes it to the current state
  */
@@ -15422,6 +15440,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
 	intel_modeset_readout_hw_state(dev);
 
 	/* HW state is read out, now we need to sanitize this mess. */
+	get_encoder_power_domains(dev_priv);
+
 	for_each_intel_encoder(dev, encoder) {
 		intel_sanitize_encoder(encoder);
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 2011651..0edc499 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -241,6 +241,9 @@ struct intel_encoder {
 	 * be set correctly before calling this function. */
 	void (*get_config)(struct intel_encoder *,
 			   struct intel_crtc_state *pipe_config);
+	/* Returns a mask of power domains that need to be referenced as part
+	 * of the hardware state readout code. */
+	u64 (*get_power_domains)(struct intel_encoder *encoder);
 	/*
 	 * Called during system suspend after all pending requests for the
 	 * encoder are flushed (for example for DP AUX transactions) and
@@ -1027,6 +1030,7 @@ struct intel_digital_port {
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;
+	enum intel_display_power_domain ddi_io_power_domain;
 };
 
 struct intel_dp_mst_encoder {
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 514ef56..012bc35 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -93,6 +93,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "PORT_DDI_D_LANES";
 	case POWER_DOMAIN_PORT_DDI_E_LANES:
 		return "PORT_DDI_E_LANES";
+	case POWER_DOMAIN_PORT_DDI_A_IO:
+		return "PORT_DDI_A_IO";
+	case POWER_DOMAIN_PORT_DDI_B_IO:
+		return "PORT_DDI_B_IO";
+	case POWER_DOMAIN_PORT_DDI_C_IO:
+		return "PORT_DDI_C_IO";
+	case POWER_DOMAIN_PORT_DDI_D_IO:
+		return "PORT_DDI_D_IO";
+	case POWER_DOMAIN_PORT_DDI_E_IO:
+		return "PORT_DDI_E_IO";
 	case POWER_DOMAIN_PORT_DSI:
 		return "PORT_DSI";
 	case POWER_DOMAIN_PORT_CRT:
@@ -385,18 +395,18 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
 	BIT_ULL(POWER_DOMAIN_VGA) |				\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO) |		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
 	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
@@ -451,12 +461,12 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
 	BIT_ULL(POWER_DOMAIN_VGA) |				\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define GLK_DISPLAY_DDI_A_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
-#define GLK_DISPLAY_DDI_B_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
-#define GLK_DISPLAY_DDI_C_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
+#define GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO))
+#define GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO))
+#define GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO))
 #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
 	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
 	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
@@ -2114,26 +2124,26 @@ static struct i915_power_well skl_power_wells[] = {
 		.id = SKL_DISP_PW_2,
 	},
 	{
-		.name = "DDI A/E power well",
-		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
+		.name = "DDI A/E IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_A_E,
 	},
 	{
-		.name = "DDI B power well",
-		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
+		.name = "DDI B IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_B,
 	},
 	{
-		.name = "DDI C power well",
-		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
+		.name = "DDI C IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_C,
 	},
 	{
-		.name = "DDI D power well",
-		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
+		.name = "DDI D IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_D,
 	},
@@ -2246,20 +2256,20 @@ static struct i915_power_well glk_power_wells[] = {
 		.id = GLK_DISP_PW_AUX_C,
 	},
 	{
-		.name = "DDI A power well",
-		.domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
+		.name = "DDI A IO power well",
+		.domains = GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = GLK_DISP_PW_DDI_A,
 	},
 	{
-		.name = "DDI B power well",
-		.domains = GLK_DISPLAY_DDI_B_POWER_DOMAINS,
+		.name = "DDI B IO power well",
+		.domains = GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_B,
 	},
 	{
-		.name = "DDI C power well",
-		.domains = GLK_DISPLAY_DDI_C_POWER_DOMAINS,
+		.name = "DDI C IO power well",
+		.domains = GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_C,
 	},
-- 
2.9.3

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

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

* ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev3)
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
                   ` (5 preceding siblings ...)
  2017-02-22  6:34 ` [PATCH v3 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
@ 2017-02-22  7:22 ` Patchwork
  2017-02-24 15:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev6) Patchwork
  7 siblings, 0 replies; 18+ messages in thread
From: Patchwork @ 2017-02-22  7:22 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: Fix Geminilake DDI power well enable timeouts (rev3)
URL   : https://patchwork.freedesktop.org/series/19451/
State : success

== Summary ==

Series 19451v3 Fix Geminilake DDI power well enable timeouts
https://patchwork.freedesktop.org/api/1.0/series/19451/revisions/3/mbox/

fi-bdw-5557u     total:253  pass:242  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:253  pass:214  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:253  pass:234  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:83   pass:70   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:253  pass:226  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:253  pass:222  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:253  pass:237  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:253  pass:237  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:253  pass:203  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:253  pass:235  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:253  pass:235  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:253  pass:235  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:253  pass:243  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:253  pass:236  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:253  pass:231  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:253  pass:243  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:253  pass:225  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:253  pass:224  dwarn:0   dfail:0   fail:0   skip:29 

e922d8924920be31fc76f5813bc1fde5d6cbf950 drm-tip: 2017y-02m-21d-16h-18m-14s UTC integration manifest
e25ac0a drm/i915: Only enable DDI IO power domains after enabling DPLL
4e4f2e5 drm/i915/glk: Don't enable DDI IO power domains during init
2625299 drm/i915/glk: Implement WaDDIIOTimeout
9c94283 drm/i915: Check encoder type in enc_to_dig_port()
9a69080 drm/i915: Store encoder power domain in struct intel_encoder
c6c067e drm/i915: Store aux power domain in intel_dp

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3922/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 3/6] drm/i915: Check encoder type in enc_to_dig_port()
  2017-02-22  6:34 ` [PATCH v3 3/6] drm/i915: Check encoder type in enc_to_dig_port() Ander Conselvan de Oliveira
@ 2017-02-24 13:38   ` Imre Deak
  2017-02-24 14:11     ` [PATCH] " Ander Conselvan de Oliveira
  0 siblings, 1 reply; 18+ messages in thread
From: Imre Deak @ 2017-02-24 13:38 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Wed, Feb 22, 2017 at 08:34:28AM +0200, Ander Conselvan de Oliveira wrote:
> Don't allow conversion from arbitraty encoder types to a digital port.
> Calling enc_to_dig_port() with the wrong encoder may seem far fetched,
> but certain paths of the ddi code may be called with hasell's analog
> encoder and the conversion is wrong for DP mst encoders too, so safe
> guard against it.
> 
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_drv.h | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index a3cf866..2011651 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1121,7 +1121,18 @@ intel_attached_encoder(struct drm_connector *connector)
>  static inline struct intel_digital_port *
>  enc_to_dig_port(struct drm_encoder *encoder)
>  {
> -	return container_of(encoder, struct intel_digital_port, base.base);
> +	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> +
> +	switch (intel_encoder->type) {
> +	case INTEL_OUTPUT_DP:
> +	case INTEL_OUTPUT_EDP:
> +	case INTEL_OUTPUT_HDMI:
> +	case INTEL_OUTPUT_UNKNOWN:

Nitpick: for UNKNOWN could've added a WARN_ON(!DDI) check. Either way:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +		return container_of(encoder, struct intel_digital_port,
> +				    base.base);
> +	default:
> +		return NULL;
> +	}
>  }
>  
>  static inline struct intel_dp_mst_encoder *
> -- 
> 2.9.3
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH v3 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL
  2017-02-22  6:34 ` [PATCH v3 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
@ 2017-02-24 13:50   ` Imre Deak
  2017-02-24 14:19     ` [PATCH] " Ander Conselvan de Oliveira
  0 siblings, 1 reply; 18+ messages in thread
From: Imre Deak @ 2017-02-24 13:50 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Wed, Feb 22, 2017 at 08:34:31AM +0200, Ander Conselvan de Oliveira wrote:
> According to bspec, the DDI IO power domains should be enabled after
> enabling the DPLL and mapping it to the DDI. The current order doesn't
> seem to create problems with Skylake and Kabylake, but causes enable
> timeouts in Geminilake.
> 
> v2: Rebase.
>   - Take power domain references before sanitizing encoders. (Imre)
>   - Add comment to get_encoder_power_domains() defition. (Ander)
> 
> v3: Don't put the domain if called with HSW/BDW's analog encoder. (CI)
> 
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> # v1
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  5 +++
>  drivers/gpu/drm/i915/intel_ddi.c        | 50 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c    | 20 ++++++++++
>  drivers/gpu/drm/i915/intel_drv.h        |  4 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 68 +++++++++++++++++++--------------
>  5 files changed, 118 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index d23069b..e346b2d 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -343,6 +343,11 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_PORT_DDI_C_LANES,
>  	POWER_DOMAIN_PORT_DDI_D_LANES,
>  	POWER_DOMAIN_PORT_DDI_E_LANES,
> +	POWER_DOMAIN_PORT_DDI_A_IO,
> +	POWER_DOMAIN_PORT_DDI_B_IO,
> +	POWER_DOMAIN_PORT_DDI_C_IO,
> +	POWER_DOMAIN_PORT_DDI_D_IO,
> +	POWER_DOMAIN_PORT_DDI_E_IO,
>  	POWER_DOMAIN_PORT_DSI,
>  	POWER_DOMAIN_PORT_CRT,
>  	POWER_DOMAIN_PORT_OTHER,
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b0c4d23..f50b485 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1440,6 +1440,18 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  	return ret;
>  }
>  
> +static unsigned long long

Nitpick: u64

> +intel_ddi_get_power_domains(struct intel_encoder *encoder)
> +{
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	enum pipe pipe;
> +
> +	if (intel_ddi_get_hw_state(encoder, &pipe))
> +		return BIT_ULL(dig_port->ddi_io_power_domain);
> +
> +	return 0;
> +}
> +
>  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_crtc *crtc = &intel_crtc->base;
> @@ -1682,6 +1694,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);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  
>  	intel_dp_set_link_params(intel_dp, link_rate, lane_count,
>  				 link_mst);
> @@ -1689,6 +1702,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  		intel_edp_panel_on(intel_dp);
>  
>  	intel_ddi_clk_select(encoder, pll);
> +
> +	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> +
>  	intel_prepare_dp_ddi_buffers(encoder);
>  	intel_ddi_init_dp_buf_reg(encoder);
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> @@ -1708,9 +1724,13 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	struct drm_encoder *drm_encoder = &encoder->base;
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	int level = intel_ddi_hdmi_level(dev_priv, port);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  
>  	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
>  	intel_ddi_clk_select(encoder, pll);
> +
> +	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> +
>  	intel_prepare_hdmi_ddi_buffers(encoder);
>  	if (IS_GEN9_BC(dev_priv))
>  		skl_ddi_set_iboost(encoder, level);
> @@ -1754,6 +1774,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	int type = intel_encoder->type;
>  	uint32_t val;
>  	bool wait = false;
> @@ -1793,6 +1814,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
>  
>  		intel_dp_dual_mode_set_tmds_output(intel_hdmi, false);
>  	}
> +
> +	if (dig_port)
> +		intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);

This matters only for GEN9_BC but there this comes before the PLL to
port mapping is reset according to bspec.

>  }
>  
>  void intel_ddi_fdi_post_disable(struct intel_encoder *intel_encoder,
> @@ -2190,12 +2214,38 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>  	intel_encoder->get_config = intel_ddi_get_config;
>  	intel_encoder->suspend = intel_dp_encoder_suspend;
> +	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
>  
>  	intel_dig_port->port = port;
>  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>  					  (DDI_BUF_PORT_REVERSAL |
>  					   DDI_A_4_LANES);
>  
> +	switch (port) {
> +	case PORT_A:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_A_IO;
> +		break;
> +	case PORT_B:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_B_IO;
> +		break;
> +	case PORT_C:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_C_IO;
> +		break;
> +	case PORT_D:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_D_IO;
> +		break;
> +	case PORT_E:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_E_IO;
> +		break;
> +	default:
> +		MISSING_CASE(port);
> +	}
> +
>  	/*
>  	 * Bspec says that DDI_A_4_LANES is the only supported configuration
>  	 * for Broxton.  Yet some BIOS fail to set this bit on port A if eDP
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 1ffec8f..957c62d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15407,6 +15407,24 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	}
>  }
>  
> +static void
> +get_encoder_power_domains(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_encoder *encoder;
> +
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		u64 get_domains;
> +		enum intel_display_power_domain domain;
> +
> +		if (!encoder->get_power_domains)
> +			continue;
> +
> +		get_domains = encoder->get_power_domains(encoder);
> +		for_each_power_domain(domain, get_domains)
> +			intel_display_power_get(dev_priv, domain);
> +	}
> +}
> +
>  /* Scan out the current hw modeset state,
>   * and sanitizes it to the current state
>   */
> @@ -15422,6 +15440,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>  	intel_modeset_readout_hw_state(dev);
>  
>  	/* HW state is read out, now we need to sanitize this mess. */
> +	get_encoder_power_domains(dev_priv);
> +
>  	for_each_intel_encoder(dev, encoder) {
>  		intel_sanitize_encoder(encoder);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 2011651..0edc499 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -241,6 +241,9 @@ struct intel_encoder {
>  	 * be set correctly before calling this function. */
>  	void (*get_config)(struct intel_encoder *,
>  			   struct intel_crtc_state *pipe_config);
> +	/* Returns a mask of power domains that need to be referenced as part
> +	 * of the hardware state readout code. */
> +	u64 (*get_power_domains)(struct intel_encoder *encoder);
>  	/*
>  	 * Called during system suspend after all pending requests for the
>  	 * encoder are flushed (for example for DP AUX transactions) and
> @@ -1027,6 +1030,7 @@ struct intel_digital_port {
>  	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
>  	bool release_cl2_override;
>  	uint8_t max_lanes;
> +	enum intel_display_power_domain ddi_io_power_domain;
>  };
>  
>  struct intel_dp_mst_encoder {
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 514ef56..012bc35 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -93,6 +93,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>  		return "PORT_DDI_D_LANES";
>  	case POWER_DOMAIN_PORT_DDI_E_LANES:
>  		return "PORT_DDI_E_LANES";
> +	case POWER_DOMAIN_PORT_DDI_A_IO:
> +		return "PORT_DDI_A_IO";
> +	case POWER_DOMAIN_PORT_DDI_B_IO:
> +		return "PORT_DDI_B_IO";
> +	case POWER_DOMAIN_PORT_DDI_C_IO:
> +		return "PORT_DDI_C_IO";
> +	case POWER_DOMAIN_PORT_DDI_D_IO:
> +		return "PORT_DDI_D_IO";
> +	case POWER_DOMAIN_PORT_DDI_E_IO:
> +		return "PORT_DDI_E_IO";
>  	case POWER_DOMAIN_PORT_DSI:
>  		return "PORT_DSI";
>  	case POWER_DOMAIN_PORT_CRT:
> @@ -385,18 +395,18 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
>  	BIT_ULL(POWER_DOMAIN_VGA) |				\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO) |		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))

I guess these power wells can be detached from the INIT power domain
on SKL as well. But it's a follow-up stuff in any case.

With the domain-put moved earlier in intel_ddi_post_disable():
Reviewed-by: Imre Deak <imre.deak@intel.com>

> -#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
>  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> @@ -451,12 +461,12 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
>  	BIT_ULL(POWER_DOMAIN_VGA) |				\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define GLK_DISPLAY_DDI_A_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
> -#define GLK_DISPLAY_DDI_B_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
> -#define GLK_DISPLAY_DDI_C_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
> +#define GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO))
> +#define GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO))
> +#define GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO))
>  #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
>  	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
>  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> @@ -2114,26 +2124,26 @@ static struct i915_power_well skl_power_wells[] = {
>  		.id = SKL_DISP_PW_2,
>  	},
>  	{
> -		.name = "DDI A/E power well",
> -		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
> +		.name = "DDI A/E IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_A_E,
>  	},
>  	{
> -		.name = "DDI B power well",
> -		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
> +		.name = "DDI B IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_B,
>  	},
>  	{
> -		.name = "DDI C power well",
> -		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
> +		.name = "DDI C IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_C,
>  	},
>  	{
> -		.name = "DDI D power well",
> -		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
> +		.name = "DDI D IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_D,
>  	},
> @@ -2246,20 +2256,20 @@ static struct i915_power_well glk_power_wells[] = {
>  		.id = GLK_DISP_PW_AUX_C,
>  	},
>  	{
> -		.name = "DDI A power well",
> -		.domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
> +		.name = "DDI A IO power well",
> +		.domains = GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = GLK_DISP_PW_DDI_A,
>  	},
>  	{
> -		.name = "DDI B power well",
> -		.domains = GLK_DISPLAY_DDI_B_POWER_DOMAINS,
> +		.name = "DDI B IO power well",
> +		.domains = GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_B,
>  	},
>  	{
> -		.name = "DDI C power well",
> -		.domains = GLK_DISPLAY_DDI_C_POWER_DOMAINS,
> +		.name = "DDI C IO power well",
> +		.domains = GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_C,
>  	},
> -- 
> 2.9.3
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* [PATCH] drm/i915: Check encoder type in enc_to_dig_port()
  2017-02-24 13:38   ` Imre Deak
@ 2017-02-24 14:11     ` Ander Conselvan de Oliveira
  2017-02-24 14:18       ` Ander Conselvan de Oliveira
  0 siblings, 1 reply; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-24 14:11 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Don't allow conversion from arbitraty encoder types to a digital port.
Calling enc_to_dig_port() with the wrong encoder may seem far fetched,
but certain paths of the ddi code may be called with hasell's analog
encoder and the conversion is wrong for DP mst encoders too, so safe
guard against it.

v2: Warn if encoder type is unknown and device is not DDI. (Imre)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c |  3 +++
 drivers/gpu/drm/i915/intel_drv.h | 14 +++++++++++++-
 2 files changed, 16 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b0c4d23..4ba44db 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1782,6 +1782,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 		intel_edp_panel_off(intel_dp);
 	}
 
+	if (dig_port)
+		intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
+
 	if (IS_GEN9_BC(dev_priv))
 		I915_WRITE(DPLL_CTRL2, (I915_READ(DPLL_CTRL2) |
 					DPLL_CTRL2_DDI_CLK_OFF(port)));
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c916338..97621a1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1121,7 +1121,19 @@ intel_attached_encoder(struct drm_connector *connector)
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
-	return container_of(encoder, struct intel_digital_port, base.base);
+	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
+	switch (intel_encoder->type) {
+	case INTEL_OUTPUT_UNKNOWN:
+		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
+	case INTEL_OUTPUT_DP:
+	case INTEL_OUTPUT_EDP:
+	case INTEL_OUTPUT_HDMI:
+		return container_of(encoder, struct intel_digital_port,
+				    base.base);
+	default:
+		return NULL;
+	}
 }
 
 static inline struct intel_dp_mst_encoder *
-- 
2.9.3

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

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

* [PATCH] drm/i915: Check encoder type in enc_to_dig_port()
  2017-02-24 14:11     ` [PATCH] " Ander Conselvan de Oliveira
@ 2017-02-24 14:18       ` Ander Conselvan de Oliveira
  2017-02-24 15:34         ` Imre Deak
  0 siblings, 1 reply; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-24 14:18 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Don't allow conversion from arbitraty encoder types to a digital port.
Calling enc_to_dig_port() with the wrong encoder may seem far fetched,
but certain paths of the ddi code may be called with hasell's analog
encoder and the conversion is wrong for DP mst encoders too, so safe
guard against it.

v2: Warn if encoder type is unknown and device is not DDI. (Imre)
v3: Remove stray hunk from rebase error. (Ander)
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index c916338..97621a1 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1121,7 +1121,19 @@ intel_attached_encoder(struct drm_connector *connector)
 static inline struct intel_digital_port *
 enc_to_dig_port(struct drm_encoder *encoder)
 {
-	return container_of(encoder, struct intel_digital_port, base.base);
+	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
+
+	switch (intel_encoder->type) {
+	case INTEL_OUTPUT_UNKNOWN:
+		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
+	case INTEL_OUTPUT_DP:
+	case INTEL_OUTPUT_EDP:
+	case INTEL_OUTPUT_HDMI:
+		return container_of(encoder, struct intel_digital_port,
+				    base.base);
+	default:
+		return NULL;
+	}
 }
 
 static inline struct intel_dp_mst_encoder *
-- 
2.9.3

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

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

* [PATCH] drm/i915: Only enable DDI IO power domains after enabling DPLL
  2017-02-24 13:50   ` Imre Deak
@ 2017-02-24 14:19     ` Ander Conselvan de Oliveira
  2017-02-27 20:38       ` Ville Syrjälä
  0 siblings, 1 reply; 18+ messages in thread
From: Ander Conselvan de Oliveira @ 2017-02-24 14:19 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

According to bspec, the DDI IO power domains should be enabled after
enabling the DPLL and mapping it to the DDI. The current order doesn't
seem to create problems with Skylake and Kabylake, but causes enable
timeouts in Geminilake.

v2: Rebase.
  - Take power domain references before sanitizing encoders. (Imre)
  - Add comment to get_encoder_power_domains() defition. (Ander)

v3: Don't put the domain if called with HSW/BDW's analog encoder. (CI)

v4: Put IO power domain before unmapping DPLL. (Imre)
  - Change return type of intel_ddi_get_power_domains() to u64. (Imre)

Cc: David Weinehall <david.weinehall@linux.intel.com>
Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> # v1
Reviewed-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         |  5 +++
 drivers/gpu/drm/i915/intel_ddi.c        | 49 ++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_display.c    | 20 ++++++++++
 drivers/gpu/drm/i915/intel_drv.h        |  4 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 68 +++++++++++++++++++--------------
 5 files changed, 117 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 4673912..5a032ba 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -344,6 +344,11 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_PORT_DDI_C_LANES,
 	POWER_DOMAIN_PORT_DDI_D_LANES,
 	POWER_DOMAIN_PORT_DDI_E_LANES,
+	POWER_DOMAIN_PORT_DDI_A_IO,
+	POWER_DOMAIN_PORT_DDI_B_IO,
+	POWER_DOMAIN_PORT_DDI_C_IO,
+	POWER_DOMAIN_PORT_DDI_D_IO,
+	POWER_DOMAIN_PORT_DDI_E_IO,
 	POWER_DOMAIN_PORT_DSI,
 	POWER_DOMAIN_PORT_CRT,
 	POWER_DOMAIN_PORT_OTHER,
diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index b0c4d23..e9013f1 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -1440,6 +1440,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
 	return ret;
 }
 
+static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder)
+{
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
+	enum pipe pipe;
+
+	if (intel_ddi_get_hw_state(encoder, &pipe))
+		return BIT_ULL(dig_port->ddi_io_power_domain);
+
+	return 0;
+}
+
 void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
 {
 	struct drm_crtc *crtc = &intel_crtc->base;
@@ -1682,6 +1693,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);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
 	intel_dp_set_link_params(intel_dp, link_rate, lane_count,
 				 link_mst);
@@ -1689,6 +1701,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
 		intel_edp_panel_on(intel_dp);
 
 	intel_ddi_clk_select(encoder, pll);
+
+	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
+
 	intel_prepare_dp_ddi_buffers(encoder);
 	intel_ddi_init_dp_buf_reg(encoder);
 	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
@@ -1708,9 +1723,13 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
 	struct drm_encoder *drm_encoder = &encoder->base;
 	enum port port = intel_ddi_get_encoder_port(encoder);
 	int level = intel_ddi_hdmi_level(dev_priv, port);
+	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
 
 	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
 	intel_ddi_clk_select(encoder, pll);
+
+	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
+
 	intel_prepare_hdmi_ddi_buffers(encoder);
 	if (IS_GEN9_BC(dev_priv))
 		skl_ddi_set_iboost(encoder, level);
@@ -1754,6 +1773,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 	struct drm_encoder *encoder = &intel_encoder->base;
 	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
 	enum port port = intel_ddi_get_encoder_port(intel_encoder);
+	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
 	int type = intel_encoder->type;
 	uint32_t val;
 	bool wait = false;
@@ -1782,6 +1802,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
 		intel_edp_panel_off(intel_dp);
 	}
 
+	if (dig_port)
+		intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
+
 	if (IS_GEN9_BC(dev_priv))
 		I915_WRITE(DPLL_CTRL2, (I915_READ(DPLL_CTRL2) |
 					DPLL_CTRL2_DDI_CLK_OFF(port)));
@@ -2190,12 +2213,38 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
 	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
 	intel_encoder->get_config = intel_ddi_get_config;
 	intel_encoder->suspend = intel_dp_encoder_suspend;
+	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
 
 	intel_dig_port->port = port;
 	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
 					  (DDI_BUF_PORT_REVERSAL |
 					   DDI_A_4_LANES);
 
+	switch (port) {
+	case PORT_A:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_A_IO;
+		break;
+	case PORT_B:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_B_IO;
+		break;
+	case PORT_C:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_C_IO;
+		break;
+	case PORT_D:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_D_IO;
+		break;
+	case PORT_E:
+		intel_dig_port->ddi_io_power_domain =
+			POWER_DOMAIN_PORT_DDI_E_IO;
+		break;
+	default:
+		MISSING_CASE(port);
+	}
+
 	/*
 	 * Bspec says that DDI_A_4_LANES is the only supported configuration
 	 * for Broxton.  Yet some BIOS fail to set this bit on port A if eDP
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 010086c..7673d5d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -15413,6 +15413,24 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	}
 }
 
+static void
+get_encoder_power_domains(struct drm_i915_private *dev_priv)
+{
+	struct intel_encoder *encoder;
+
+	for_each_intel_encoder(&dev_priv->drm, encoder) {
+		u64 get_domains;
+		enum intel_display_power_domain domain;
+
+		if (!encoder->get_power_domains)
+			continue;
+
+		get_domains = encoder->get_power_domains(encoder);
+		for_each_power_domain(domain, get_domains)
+			intel_display_power_get(dev_priv, domain);
+	}
+}
+
 /* Scan out the current hw modeset state,
  * and sanitizes it to the current state
  */
@@ -15428,6 +15446,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
 	intel_modeset_readout_hw_state(dev);
 
 	/* HW state is read out, now we need to sanitize this mess. */
+	get_encoder_power_domains(dev_priv);
+
 	for_each_intel_encoder(dev, encoder) {
 		intel_sanitize_encoder(encoder);
 	}
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 97621a1..0425dbe 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -241,6 +241,9 @@ struct intel_encoder {
 	 * be set correctly before calling this function. */
 	void (*get_config)(struct intel_encoder *,
 			   struct intel_crtc_state *pipe_config);
+	/* Returns a mask of power domains that need to be referenced as part
+	 * of the hardware state readout code. */
+	u64 (*get_power_domains)(struct intel_encoder *encoder);
 	/*
 	 * Called during system suspend after all pending requests for the
 	 * encoder are flushed (for example for DP AUX transactions) and
@@ -1027,6 +1030,7 @@ struct intel_digital_port {
 	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
 	bool release_cl2_override;
 	uint8_t max_lanes;
+	enum intel_display_power_domain ddi_io_power_domain;
 };
 
 struct intel_dp_mst_encoder {
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 514ef56..012bc35 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -93,6 +93,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
 		return "PORT_DDI_D_LANES";
 	case POWER_DOMAIN_PORT_DDI_E_LANES:
 		return "PORT_DDI_E_LANES";
+	case POWER_DOMAIN_PORT_DDI_A_IO:
+		return "PORT_DDI_A_IO";
+	case POWER_DOMAIN_PORT_DDI_B_IO:
+		return "PORT_DDI_B_IO";
+	case POWER_DOMAIN_PORT_DDI_C_IO:
+		return "PORT_DDI_C_IO";
+	case POWER_DOMAIN_PORT_DDI_D_IO:
+		return "PORT_DDI_D_IO";
+	case POWER_DOMAIN_PORT_DDI_E_IO:
+		return "PORT_DDI_E_IO";
 	case POWER_DOMAIN_PORT_DSI:
 		return "PORT_DSI";
 	case POWER_DOMAIN_PORT_CRT:
@@ -385,18 +395,18 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
 	BIT_ULL(POWER_DOMAIN_VGA) |				\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO) |		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
+#define SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_IO) |		\
 	BIT_ULL(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
 	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
@@ -451,12 +461,12 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
 	BIT_ULL(POWER_DOMAIN_VGA) |				\
 	BIT_ULL(POWER_DOMAIN_INIT))
-#define GLK_DISPLAY_DDI_A_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
-#define GLK_DISPLAY_DDI_B_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
-#define GLK_DISPLAY_DDI_C_POWER_DOMAINS (		\
-	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
+#define GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO))
+#define GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO))
+#define GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
+	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO))
 #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
 	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
 	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
@@ -2114,26 +2124,26 @@ static struct i915_power_well skl_power_wells[] = {
 		.id = SKL_DISP_PW_2,
 	},
 	{
-		.name = "DDI A/E power well",
-		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
+		.name = "DDI A/E IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_A_E,
 	},
 	{
-		.name = "DDI B power well",
-		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
+		.name = "DDI B IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_B,
 	},
 	{
-		.name = "DDI C power well",
-		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
+		.name = "DDI C IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_C,
 	},
 	{
-		.name = "DDI D power well",
-		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
+		.name = "DDI D IO power well",
+		.domains = SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_D,
 	},
@@ -2246,20 +2256,20 @@ static struct i915_power_well glk_power_wells[] = {
 		.id = GLK_DISP_PW_AUX_C,
 	},
 	{
-		.name = "DDI A power well",
-		.domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
+		.name = "DDI A IO power well",
+		.domains = GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = GLK_DISP_PW_DDI_A,
 	},
 	{
-		.name = "DDI B power well",
-		.domains = GLK_DISPLAY_DDI_B_POWER_DOMAINS,
+		.name = "DDI B IO power well",
+		.domains = GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_B,
 	},
 	{
-		.name = "DDI C power well",
-		.domains = GLK_DISPLAY_DDI_C_POWER_DOMAINS,
+		.name = "DDI C IO power well",
+		.domains = GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
 		.id = SKL_DISP_PW_DDI_C,
 	},
-- 
2.9.3

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

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

* ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev6)
  2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
                   ` (6 preceding siblings ...)
  2017-02-22  7:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev3) Patchwork
@ 2017-02-24 15:22 ` Patchwork
  2017-02-27  7:15   ` Ander Conselvan De Oliveira
  7 siblings, 1 reply; 18+ messages in thread
From: Patchwork @ 2017-02-24 15:22 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

== Series Details ==

Series: Fix Geminilake DDI power well enable timeouts (rev6)
URL   : https://patchwork.freedesktop.org/series/19451/
State : success

== Summary ==

Series 19451v6 Fix Geminilake DDI power well enable timeouts
https://patchwork.freedesktop.org/api/1.0/series/19451/revisions/6/mbox/

fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
fi-bxt-t5700     total:108  pass:95   dwarn:0   dfail:0   fail:0   skip:12 
fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 

69fbd8896f5f272550747b763068883326962d76 drm-tip: 2017y-02m-24d-14h-04m-04s UTC integration manifest
fa6caa8 drm/i915: Only enable DDI IO power domains after enabling DPLL
8740cdf drm/i915/glk: Don't enable DDI IO power domains during init
1fb52e2 drm/i915/glk: Implement WaDDIIOTimeout
bb5cd45 drm/i915: Check encoder type in enc_to_dig_port()
5229434 drm/i915: Store encoder power domain in struct intel_encoder
27958a0 drm/i915: Store aux power domain in intel_dp

== Logs ==

For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3960/
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Check encoder type in enc_to_dig_port()
  2017-02-24 14:18       ` Ander Conselvan de Oliveira
@ 2017-02-24 15:34         ` Imre Deak
  0 siblings, 0 replies; 18+ messages in thread
From: Imre Deak @ 2017-02-24 15:34 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Fri, Feb 24, 2017 at 04:18:45PM +0200, Ander Conselvan de Oliveira wrote:
> Don't allow conversion from arbitraty encoder types to a digital port.
> Calling enc_to_dig_port() with the wrong encoder may seem far fetched,
> but certain paths of the ddi code may be called with hasell's analog
> encoder and the conversion is wrong for DP mst encoders too, so safe
> guard against it.
> 
> v2: Warn if encoder type is unknown and device is not DDI. (Imre)
> v3: Remove stray hunk from rebase error. (Ander)
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  drivers/gpu/drm/i915/intel_drv.h | 14 +++++++++++++-
>  1 file changed, 13 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index c916338..97621a1 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -1121,7 +1121,19 @@ intel_attached_encoder(struct drm_connector *connector)
>  static inline struct intel_digital_port *
>  enc_to_dig_port(struct drm_encoder *encoder)
>  {
> -	return container_of(encoder, struct intel_digital_port, base.base);
> +	struct intel_encoder *intel_encoder = to_intel_encoder(encoder);
> +
> +	switch (intel_encoder->type) {
> +	case INTEL_OUTPUT_UNKNOWN:
> +		WARN_ON(!HAS_DDI(to_i915(encoder->dev)));
> +	case INTEL_OUTPUT_DP:
> +	case INTEL_OUTPUT_EDP:
> +	case INTEL_OUTPUT_HDMI:
> +		return container_of(encoder, struct intel_digital_port,
> +				    base.base);
> +	default:
> +		return NULL;
> +	}
>  }
>  
>  static inline struct intel_dp_mst_encoder *
> -- 
> 2.9.3
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev6)
  2017-02-24 15:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev6) Patchwork
@ 2017-02-27  7:15   ` Ander Conselvan De Oliveira
  0 siblings, 0 replies; 18+ messages in thread
From: Ander Conselvan De Oliveira @ 2017-02-27  7:15 UTC (permalink / raw)
  To: intel-gfx, Deak, Imre

On Fri, 2017-02-24 at 15:22 +0000, Patchwork wrote:
> == Series Details ==
> 
> Series: Fix Geminilake DDI power well enable timeouts (rev6)
> URL   : https://patchwork.freedesktop.org/series/19451/
> State : success

Patches pushed. Thanks for the reviews.

Ander

> 
> == Summary ==
> 
> Series 19451v6 Fix Geminilake DDI power well enable timeouts
> https://patchwork.freedesktop.org/api/1.0/series/19451/revisions/6/mbox/
> 
> fi-bdw-5557u     total:278  pass:267  dwarn:0   dfail:0   fail:0   skip:11 
> fi-bsw-n3050     total:278  pass:239  dwarn:0   dfail:0   fail:0   skip:39 
> fi-bxt-j4205     total:278  pass:259  dwarn:0   dfail:0   fail:0   skip:19 
> fi-bxt-t5700     total:108  pass:95   dwarn:0   dfail:0   fail:0   skip:12 
> fi-byt-j1900     total:278  pass:251  dwarn:0   dfail:0   fail:0   skip:27 
> fi-byt-n2820     total:278  pass:247  dwarn:0   dfail:0   fail:0   skip:31 
> fi-hsw-4770      total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
> fi-hsw-4770r     total:278  pass:262  dwarn:0   dfail:0   fail:0   skip:16 
> fi-ilk-650       total:278  pass:228  dwarn:0   dfail:0   fail:0   skip:50 
> fi-ivb-3520m     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-ivb-3770      total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-kbl-7500u     total:278  pass:260  dwarn:0   dfail:0   fail:0   skip:18 
> fi-skl-6260u     total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
> fi-skl-6700hq    total:278  pass:261  dwarn:0   dfail:0   fail:0   skip:17 
> fi-skl-6700k     total:278  pass:256  dwarn:4   dfail:0   fail:0   skip:18 
> fi-skl-6770hq    total:278  pass:268  dwarn:0   dfail:0   fail:0   skip:10 
> fi-snb-2520m     total:278  pass:250  dwarn:0   dfail:0   fail:0   skip:28 
> fi-snb-2600      total:278  pass:249  dwarn:0   dfail:0   fail:0   skip:29 
> 
> 69fbd8896f5f272550747b763068883326962d76 drm-tip: 2017y-02m-24d-14h-04m-04s UTC integration manifest
> fa6caa8 drm/i915: Only enable DDI IO power domains after enabling DPLL
> 8740cdf drm/i915/glk: Don't enable DDI IO power domains during init
> 1fb52e2 drm/i915/glk: Implement WaDDIIOTimeout
> bb5cd45 drm/i915: Check encoder type in enc_to_dig_port()
> 5229434 drm/i915: Store encoder power domain in struct intel_encoder
> 27958a0 drm/i915: Store aux power domain in intel_dp
> 
> == Logs ==
> 
> For more details see: https://intel-gfx-ci.01.org/CI/Patchwork_3960/
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

* Re: [PATCH] drm/i915: Only enable DDI IO power domains after enabling DPLL
  2017-02-24 14:19     ` [PATCH] " Ander Conselvan de Oliveira
@ 2017-02-27 20:38       ` Ville Syrjälä
  2017-02-28  7:28         ` Ander Conselvan De Oliveira
  0 siblings, 1 reply; 18+ messages in thread
From: Ville Syrjälä @ 2017-02-27 20:38 UTC (permalink / raw)
  To: Ander Conselvan de Oliveira; +Cc: intel-gfx

On Fri, Feb 24, 2017 at 04:19:59PM +0200, Ander Conselvan de Oliveira wrote:
> According to bspec, the DDI IO power domains should be enabled after
> enabling the DPLL and mapping it to the DDI. The current order doesn't
> seem to create problems with Skylake and Kabylake, but causes enable
> timeouts in Geminilake.

This one seems to kill SKL+MST.

Just load the driver with an MST display plugged in and we get:
[   79.389527] Kernel panic - not syncing: Timeout: Not all CPUs entered broadcast exception handler

So presumably we're trying to access something that's not powered/clocked
and the CPU just gets totally stuck.

> 
> v2: Rebase.
>   - Take power domain references before sanitizing encoders. (Imre)
>   - Add comment to get_encoder_power_domains() defition. (Ander)
> 
> v3: Don't put the domain if called with HSW/BDW's analog encoder. (CI)
> 
> v4: Put IO power domain before unmapping DPLL. (Imre)
>   - Change return type of intel_ddi_get_power_domains() to u64. (Imre)
> 
> Cc: David Weinehall <david.weinehall@linux.intel.com>
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> # v1
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         |  5 +++
>  drivers/gpu/drm/i915/intel_ddi.c        | 49 ++++++++++++++++++++++++
>  drivers/gpu/drm/i915/intel_display.c    | 20 ++++++++++
>  drivers/gpu/drm/i915/intel_drv.h        |  4 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 68 +++++++++++++++++++--------------
>  5 files changed, 117 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 4673912..5a032ba 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -344,6 +344,11 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_PORT_DDI_C_LANES,
>  	POWER_DOMAIN_PORT_DDI_D_LANES,
>  	POWER_DOMAIN_PORT_DDI_E_LANES,
> +	POWER_DOMAIN_PORT_DDI_A_IO,
> +	POWER_DOMAIN_PORT_DDI_B_IO,
> +	POWER_DOMAIN_PORT_DDI_C_IO,
> +	POWER_DOMAIN_PORT_DDI_D_IO,
> +	POWER_DOMAIN_PORT_DDI_E_IO,
>  	POWER_DOMAIN_PORT_DSI,
>  	POWER_DOMAIN_PORT_CRT,
>  	POWER_DOMAIN_PORT_OTHER,
> diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> index b0c4d23..e9013f1 100644
> --- a/drivers/gpu/drm/i915/intel_ddi.c
> +++ b/drivers/gpu/drm/i915/intel_ddi.c
> @@ -1440,6 +1440,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
>  	return ret;
>  }
>  
> +static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder)
> +{
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> +	enum pipe pipe;
> +
> +	if (intel_ddi_get_hw_state(encoder, &pipe))
> +		return BIT_ULL(dig_port->ddi_io_power_domain);
> +
> +	return 0;
> +}
> +
>  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
>  {
>  	struct drm_crtc *crtc = &intel_crtc->base;
> @@ -1682,6 +1693,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);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  
>  	intel_dp_set_link_params(intel_dp, link_rate, lane_count,
>  				 link_mst);
> @@ -1689,6 +1701,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
>  		intel_edp_panel_on(intel_dp);
>  
>  	intel_ddi_clk_select(encoder, pll);
> +
> +	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> +
>  	intel_prepare_dp_ddi_buffers(encoder);
>  	intel_ddi_init_dp_buf_reg(encoder);
>  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> @@ -1708,9 +1723,13 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
>  	struct drm_encoder *drm_encoder = &encoder->base;
>  	enum port port = intel_ddi_get_encoder_port(encoder);
>  	int level = intel_ddi_hdmi_level(dev_priv, port);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
>  
>  	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
>  	intel_ddi_clk_select(encoder, pll);
> +
> +	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> +
>  	intel_prepare_hdmi_ddi_buffers(encoder);
>  	if (IS_GEN9_BC(dev_priv))
>  		skl_ddi_set_iboost(encoder, level);
> @@ -1754,6 +1773,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
>  	struct drm_encoder *encoder = &intel_encoder->base;
>  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
>  	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
>  	int type = intel_encoder->type;
>  	uint32_t val;
>  	bool wait = false;
> @@ -1782,6 +1802,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
>  		intel_edp_panel_off(intel_dp);
>  	}
>  
> +	if (dig_port)
> +		intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
> +
>  	if (IS_GEN9_BC(dev_priv))
>  		I915_WRITE(DPLL_CTRL2, (I915_READ(DPLL_CTRL2) |
>  					DPLL_CTRL2_DDI_CLK_OFF(port)));
> @@ -2190,12 +2213,38 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
>  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
>  	intel_encoder->get_config = intel_ddi_get_config;
>  	intel_encoder->suspend = intel_dp_encoder_suspend;
> +	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
>  
>  	intel_dig_port->port = port;
>  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
>  					  (DDI_BUF_PORT_REVERSAL |
>  					   DDI_A_4_LANES);
>  
> +	switch (port) {
> +	case PORT_A:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_A_IO;
> +		break;
> +	case PORT_B:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_B_IO;
> +		break;
> +	case PORT_C:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_C_IO;
> +		break;
> +	case PORT_D:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_D_IO;
> +		break;
> +	case PORT_E:
> +		intel_dig_port->ddi_io_power_domain =
> +			POWER_DOMAIN_PORT_DDI_E_IO;
> +		break;
> +	default:
> +		MISSING_CASE(port);
> +	}
> +
>  	/*
>  	 * Bspec says that DDI_A_4_LANES is the only supported configuration
>  	 * for Broxton.  Yet some BIOS fail to set this bit on port A if eDP
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 010086c..7673d5d 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15413,6 +15413,24 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	}
>  }
>  
> +static void
> +get_encoder_power_domains(struct drm_i915_private *dev_priv)
> +{
> +	struct intel_encoder *encoder;
> +
> +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> +		u64 get_domains;
> +		enum intel_display_power_domain domain;
> +
> +		if (!encoder->get_power_domains)
> +			continue;
> +
> +		get_domains = encoder->get_power_domains(encoder);
> +		for_each_power_domain(domain, get_domains)
> +			intel_display_power_get(dev_priv, domain);
> +	}
> +}
> +
>  /* Scan out the current hw modeset state,
>   * and sanitizes it to the current state
>   */
> @@ -15428,6 +15446,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
>  	intel_modeset_readout_hw_state(dev);
>  
>  	/* HW state is read out, now we need to sanitize this mess. */
> +	get_encoder_power_domains(dev_priv);
> +
>  	for_each_intel_encoder(dev, encoder) {
>  		intel_sanitize_encoder(encoder);
>  	}
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 97621a1..0425dbe 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -241,6 +241,9 @@ struct intel_encoder {
>  	 * be set correctly before calling this function. */
>  	void (*get_config)(struct intel_encoder *,
>  			   struct intel_crtc_state *pipe_config);
> +	/* Returns a mask of power domains that need to be referenced as part
> +	 * of the hardware state readout code. */
> +	u64 (*get_power_domains)(struct intel_encoder *encoder);
>  	/*
>  	 * Called during system suspend after all pending requests for the
>  	 * encoder are flushed (for example for DP AUX transactions) and
> @@ -1027,6 +1030,7 @@ struct intel_digital_port {
>  	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
>  	bool release_cl2_override;
>  	uint8_t max_lanes;
> +	enum intel_display_power_domain ddi_io_power_domain;
>  };
>  
>  struct intel_dp_mst_encoder {
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 514ef56..012bc35 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -93,6 +93,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
>  		return "PORT_DDI_D_LANES";
>  	case POWER_DOMAIN_PORT_DDI_E_LANES:
>  		return "PORT_DDI_E_LANES";
> +	case POWER_DOMAIN_PORT_DDI_A_IO:
> +		return "PORT_DDI_A_IO";
> +	case POWER_DOMAIN_PORT_DDI_B_IO:
> +		return "PORT_DDI_B_IO";
> +	case POWER_DOMAIN_PORT_DDI_C_IO:
> +		return "PORT_DDI_C_IO";
> +	case POWER_DOMAIN_PORT_DDI_D_IO:
> +		return "PORT_DDI_D_IO";
> +	case POWER_DOMAIN_PORT_DDI_E_IO:
> +		return "PORT_DDI_E_IO";
>  	case POWER_DOMAIN_PORT_DSI:
>  		return "PORT_DSI";
>  	case POWER_DOMAIN_PORT_CRT:
> @@ -385,18 +395,18 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
>  	BIT_ULL(POWER_DOMAIN_VGA) |				\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO) |		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> +#define SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_IO) |		\
>  	BIT_ULL(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
>  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> @@ -451,12 +461,12 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
>  	BIT_ULL(POWER_DOMAIN_VGA) |				\
>  	BIT_ULL(POWER_DOMAIN_INIT))
> -#define GLK_DISPLAY_DDI_A_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
> -#define GLK_DISPLAY_DDI_B_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
> -#define GLK_DISPLAY_DDI_C_POWER_DOMAINS (		\
> -	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
> +#define GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO))
> +#define GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO))
> +#define GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
> +	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO))
>  #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
>  	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
>  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> @@ -2114,26 +2124,26 @@ static struct i915_power_well skl_power_wells[] = {
>  		.id = SKL_DISP_PW_2,
>  	},
>  	{
> -		.name = "DDI A/E power well",
> -		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
> +		.name = "DDI A/E IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_A_E,
>  	},
>  	{
> -		.name = "DDI B power well",
> -		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
> +		.name = "DDI B IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_B,
>  	},
>  	{
> -		.name = "DDI C power well",
> -		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
> +		.name = "DDI C IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_C,
>  	},
>  	{
> -		.name = "DDI D power well",
> -		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
> +		.name = "DDI D IO power well",
> +		.domains = SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_D,
>  	},
> @@ -2246,20 +2256,20 @@ static struct i915_power_well glk_power_wells[] = {
>  		.id = GLK_DISP_PW_AUX_C,
>  	},
>  	{
> -		.name = "DDI A power well",
> -		.domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
> +		.name = "DDI A IO power well",
> +		.domains = GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = GLK_DISP_PW_DDI_A,
>  	},
>  	{
> -		.name = "DDI B power well",
> -		.domains = GLK_DISPLAY_DDI_B_POWER_DOMAINS,
> +		.name = "DDI B IO power well",
> +		.domains = GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_B,
>  	},
>  	{
> -		.name = "DDI C power well",
> -		.domains = GLK_DISPLAY_DDI_C_POWER_DOMAINS,
> +		.name = "DDI C IO power well",
> +		.domains = GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
>  		.id = SKL_DISP_PW_DDI_C,
>  	},
> -- 
> 2.9.3
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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] 18+ messages in thread

* Re: [PATCH] drm/i915: Only enable DDI IO power domains after enabling DPLL
  2017-02-27 20:38       ` Ville Syrjälä
@ 2017-02-28  7:28         ` Ander Conselvan De Oliveira
  0 siblings, 0 replies; 18+ messages in thread
From: Ander Conselvan De Oliveira @ 2017-02-28  7:28 UTC (permalink / raw)
  To: Ville Syrjälä; +Cc: intel-gfx

On Mon, 2017-02-27 at 22:38 +0200, Ville Syrjälä wrote:
> On Fri, Feb 24, 2017 at 04:19:59PM +0200, Ander Conselvan de Oliveira wrote:
> > According to bspec, the DDI IO power domains should be enabled after
> > enabling the DPLL and mapping it to the DDI. The current order doesn't
> > seem to create problems with Skylake and Kabylake, but causes enable
> > timeouts in Geminilake.
> 
> This one seems to kill SKL+MST.
> 
> Just load the driver with an MST display plugged in and we get:
> [   79.389527] Kernel panic - not syncing: Timeout: Not all CPUs entered broadcast exception handler
> 
> So presumably we're trying to access something that's not powered/clocked
> and the CPU just gets totally stuck.

Hmm, we need to enable the DDI IO power domain in the MST path too. /o\

I sent the fix as a separate series so CI doesn't get confused, although it
doesn't seem to test MST+SKL.

https://patchwork.freedesktop.org/series/20345/

Ander


> 
> > 
> > v2: Rebase.
> >   - Take power domain references before sanitizing encoders. (Imre)
> >   - Add comment to get_encoder_power_domains() defition. (Ander)
> > 
> > v3: Don't put the domain if called with HSW/BDW's analog encoder. (CI)
> > 
> > v4: Put IO power domain before unmapping DPLL. (Imre)
> >   - Change return type of intel_ddi_get_power_domains() to u64. (Imre)
> > 
> > Cc: David Weinehall <david.weinehall@linux.intel.com>
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> > Reviewed-by: David Weinehall <david.weinehall@linux.intel.com> # v1
> > Reviewed-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         |  5 +++
> >  drivers/gpu/drm/i915/intel_ddi.c        | 49 ++++++++++++++++++++++++
> >  drivers/gpu/drm/i915/intel_display.c    | 20 ++++++++++
> >  drivers/gpu/drm/i915/intel_drv.h        |  4 ++
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 68 +++++++++++++++++++--------------
> >  5 files changed, 117 insertions(+), 29 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index 4673912..5a032ba 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -344,6 +344,11 @@ enum intel_display_power_domain {
> >  	POWER_DOMAIN_PORT_DDI_C_LANES,
> >  	POWER_DOMAIN_PORT_DDI_D_LANES,
> >  	POWER_DOMAIN_PORT_DDI_E_LANES,
> > +	POWER_DOMAIN_PORT_DDI_A_IO,
> > +	POWER_DOMAIN_PORT_DDI_B_IO,
> > +	POWER_DOMAIN_PORT_DDI_C_IO,
> > +	POWER_DOMAIN_PORT_DDI_D_IO,
> > +	POWER_DOMAIN_PORT_DDI_E_IO,
> >  	POWER_DOMAIN_PORT_DSI,
> >  	POWER_DOMAIN_PORT_CRT,
> >  	POWER_DOMAIN_PORT_OTHER,
> > diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
> > index b0c4d23..e9013f1 100644
> > --- a/drivers/gpu/drm/i915/intel_ddi.c
> > +++ b/drivers/gpu/drm/i915/intel_ddi.c
> > @@ -1440,6 +1440,17 @@ bool intel_ddi_get_hw_state(struct intel_encoder *encoder,
> >  	return ret;
> >  }
> >  
> > +static u64 intel_ddi_get_power_domains(struct intel_encoder *encoder)
> > +{
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> > +	enum pipe pipe;
> > +
> > +	if (intel_ddi_get_hw_state(encoder, &pipe))
> > +		return BIT_ULL(dig_port->ddi_io_power_domain);
> > +
> > +	return 0;
> > +}
> > +
> >  void intel_ddi_enable_pipe_clock(struct intel_crtc *intel_crtc)
> >  {
> >  	struct drm_crtc *crtc = &intel_crtc->base;
> > @@ -1682,6 +1693,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);
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  
> >  	intel_dp_set_link_params(intel_dp, link_rate, lane_count,
> >  				 link_mst);
> > @@ -1689,6 +1701,9 @@ static void intel_ddi_pre_enable_dp(struct intel_encoder *encoder,
> >  		intel_edp_panel_on(intel_dp);
> >  
> >  	intel_ddi_clk_select(encoder, pll);
> > +
> > +	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> > +
> >  	intel_prepare_dp_ddi_buffers(encoder);
> >  	intel_ddi_init_dp_buf_reg(encoder);
> >  	intel_dp_sink_dpms(intel_dp, DRM_MODE_DPMS_ON);
> > @@ -1708,9 +1723,13 @@ static void intel_ddi_pre_enable_hdmi(struct intel_encoder *encoder,
> >  	struct drm_encoder *drm_encoder = &encoder->base;
> >  	enum port port = intel_ddi_get_encoder_port(encoder);
> >  	int level = intel_ddi_hdmi_level(dev_priv, port);
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(&encoder->base);
> >  
> >  	intel_dp_dual_mode_set_tmds_output(intel_hdmi, true);
> >  	intel_ddi_clk_select(encoder, pll);
> > +
> > +	intel_display_power_get(dev_priv, dig_port->ddi_io_power_domain);
> > +
> >  	intel_prepare_hdmi_ddi_buffers(encoder);
> >  	if (IS_GEN9_BC(dev_priv))
> >  		skl_ddi_set_iboost(encoder, level);
> > @@ -1754,6 +1773,7 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
> >  	struct drm_encoder *encoder = &intel_encoder->base;
> >  	struct drm_i915_private *dev_priv = to_i915(encoder->dev);
> >  	enum port port = intel_ddi_get_encoder_port(intel_encoder);
> > +	struct intel_digital_port *dig_port = enc_to_dig_port(encoder);
> >  	int type = intel_encoder->type;
> >  	uint32_t val;
> >  	bool wait = false;
> > @@ -1782,6 +1802,9 @@ static void intel_ddi_post_disable(struct intel_encoder *intel_encoder,
> >  		intel_edp_panel_off(intel_dp);
> >  	}
> >  
> > +	if (dig_port)
> > +		intel_display_power_put(dev_priv, dig_port->ddi_io_power_domain);
> > +
> >  	if (IS_GEN9_BC(dev_priv))
> >  		I915_WRITE(DPLL_CTRL2, (I915_READ(DPLL_CTRL2) |
> >  					DPLL_CTRL2_DDI_CLK_OFF(port)));
> > @@ -2190,12 +2213,38 @@ void intel_ddi_init(struct drm_i915_private *dev_priv, enum port port)
> >  	intel_encoder->get_hw_state = intel_ddi_get_hw_state;
> >  	intel_encoder->get_config = intel_ddi_get_config;
> >  	intel_encoder->suspend = intel_dp_encoder_suspend;
> > +	intel_encoder->get_power_domains = intel_ddi_get_power_domains;
> >  
> >  	intel_dig_port->port = port;
> >  	intel_dig_port->saved_port_bits = I915_READ(DDI_BUF_CTL(port)) &
> >  					  (DDI_BUF_PORT_REVERSAL |
> >  					   DDI_A_4_LANES);
> >  
> > +	switch (port) {
> > +	case PORT_A:
> > +		intel_dig_port->ddi_io_power_domain =
> > +			POWER_DOMAIN_PORT_DDI_A_IO;
> > +		break;
> > +	case PORT_B:
> > +		intel_dig_port->ddi_io_power_domain =
> > +			POWER_DOMAIN_PORT_DDI_B_IO;
> > +		break;
> > +	case PORT_C:
> > +		intel_dig_port->ddi_io_power_domain =
> > +			POWER_DOMAIN_PORT_DDI_C_IO;
> > +		break;
> > +	case PORT_D:
> > +		intel_dig_port->ddi_io_power_domain =
> > +			POWER_DOMAIN_PORT_DDI_D_IO;
> > +		break;
> > +	case PORT_E:
> > +		intel_dig_port->ddi_io_power_domain =
> > +			POWER_DOMAIN_PORT_DDI_E_IO;
> > +		break;
> > +	default:
> > +		MISSING_CASE(port);
> > +	}
> > +
> >  	/*
> >  	 * Bspec says that DDI_A_4_LANES is the only supported configuration
> >  	 * for Broxton.  Yet some BIOS fail to set this bit on port A if eDP
> > diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> > index 010086c..7673d5d 100644
> > --- a/drivers/gpu/drm/i915/intel_display.c
> > +++ b/drivers/gpu/drm/i915/intel_display.c
> > @@ -15413,6 +15413,24 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
> >  	}
> >  }
> >  
> > +static void
> > +get_encoder_power_domains(struct drm_i915_private *dev_priv)
> > +{
> > +	struct intel_encoder *encoder;
> > +
> > +	for_each_intel_encoder(&dev_priv->drm, encoder) {
> > +		u64 get_domains;
> > +		enum intel_display_power_domain domain;
> > +
> > +		if (!encoder->get_power_domains)
> > +			continue;
> > +
> > +		get_domains = encoder->get_power_domains(encoder);
> > +		for_each_power_domain(domain, get_domains)
> > +			intel_display_power_get(dev_priv, domain);
> > +	}
> > +}
> > +
> >  /* Scan out the current hw modeset state,
> >   * and sanitizes it to the current state
> >   */
> > @@ -15428,6 +15446,8 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
> >  	intel_modeset_readout_hw_state(dev);
> >  
> >  	/* HW state is read out, now we need to sanitize this mess. */
> > +	get_encoder_power_domains(dev_priv);
> > +
> >  	for_each_intel_encoder(dev, encoder) {
> >  		intel_sanitize_encoder(encoder);
> >  	}
> > diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> > index 97621a1..0425dbe 100644
> > --- a/drivers/gpu/drm/i915/intel_drv.h
> > +++ b/drivers/gpu/drm/i915/intel_drv.h
> > @@ -241,6 +241,9 @@ struct intel_encoder {
> >  	 * be set correctly before calling this function. */
> >  	void (*get_config)(struct intel_encoder *,
> >  			   struct intel_crtc_state *pipe_config);
> > +	/* Returns a mask of power domains that need to be referenced as part
> > +	 * of the hardware state readout code. */
> > +	u64 (*get_power_domains)(struct intel_encoder *encoder);
> >  	/*
> >  	 * Called during system suspend after all pending requests for the
> >  	 * encoder are flushed (for example for DP AUX transactions) and
> > @@ -1027,6 +1030,7 @@ struct intel_digital_port {
> >  	enum irqreturn (*hpd_pulse)(struct intel_digital_port *, bool);
> >  	bool release_cl2_override;
> >  	uint8_t max_lanes;
> > +	enum intel_display_power_domain ddi_io_power_domain;
> >  };
> >  
> >  struct intel_dp_mst_encoder {
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 514ef56..012bc35 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -93,6 +93,16 @@ intel_display_power_domain_str(enum intel_display_power_domain domain)
> >  		return "PORT_DDI_D_LANES";
> >  	case POWER_DOMAIN_PORT_DDI_E_LANES:
> >  		return "PORT_DDI_E_LANES";
> > +	case POWER_DOMAIN_PORT_DDI_A_IO:
> > +		return "PORT_DDI_A_IO";
> > +	case POWER_DOMAIN_PORT_DDI_B_IO:
> > +		return "PORT_DDI_B_IO";
> > +	case POWER_DOMAIN_PORT_DDI_C_IO:
> > +		return "PORT_DDI_C_IO";
> > +	case POWER_DOMAIN_PORT_DDI_D_IO:
> > +		return "PORT_DDI_D_IO";
> > +	case POWER_DOMAIN_PORT_DDI_E_IO:
> > +		return "PORT_DDI_E_IO";
> >  	case POWER_DOMAIN_PORT_DSI:
> >  		return "PORT_DSI";
> >  	case POWER_DOMAIN_PORT_CRT:
> > @@ -385,18 +395,18 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> >  	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
> >  	BIT_ULL(POWER_DOMAIN_VGA) |				\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> > -#define SKL_DISPLAY_DDI_A_E_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_LANES) |		\
> > +#define SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO) |		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_E_IO) |		\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> > -#define SKL_DISPLAY_DDI_B_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES) |		\
> > +#define SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO) |		\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> > -#define SKL_DISPLAY_DDI_C_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES) |		\
> > +#define SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO) |		\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> > -#define SKL_DISPLAY_DDI_D_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_LANES) |		\
> > +#define SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_D_IO) |		\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> >  #define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
> >  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> > @@ -451,12 +461,12 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
> >  	BIT_ULL(POWER_DOMAIN_AUDIO) |			\
> >  	BIT_ULL(POWER_DOMAIN_VGA) |				\
> >  	BIT_ULL(POWER_DOMAIN_INIT))
> > -#define GLK_DISPLAY_DDI_A_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES))
> > -#define GLK_DISPLAY_DDI_B_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_LANES))
> > -#define GLK_DISPLAY_DDI_C_POWER_DOMAINS (		\
> > -	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_LANES))
> > +#define GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_IO))
> > +#define GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_B_IO))
> > +#define GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS (		\
> > +	BIT_ULL(POWER_DOMAIN_PORT_DDI_C_IO))
> >  #define GLK_DPIO_CMN_A_POWER_DOMAINS (			\
> >  	BIT_ULL(POWER_DOMAIN_PORT_DDI_A_LANES) |		\
> >  	BIT_ULL(POWER_DOMAIN_AUX_A) |			\
> > @@ -2114,26 +2124,26 @@ static struct i915_power_well skl_power_wells[] = {
> >  		.id = SKL_DISP_PW_2,
> >  	},
> >  	{
> > -		.name = "DDI A/E power well",
> > -		.domains = SKL_DISPLAY_DDI_A_E_POWER_DOMAINS,
> > +		.name = "DDI A/E IO power well",
> > +		.domains = SKL_DISPLAY_DDI_IO_A_E_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = SKL_DISP_PW_DDI_A_E,
> >  	},
> >  	{
> > -		.name = "DDI B power well",
> > -		.domains = SKL_DISPLAY_DDI_B_POWER_DOMAINS,
> > +		.name = "DDI B IO power well",
> > +		.domains = SKL_DISPLAY_DDI_IO_B_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = SKL_DISP_PW_DDI_B,
> >  	},
> >  	{
> > -		.name = "DDI C power well",
> > -		.domains = SKL_DISPLAY_DDI_C_POWER_DOMAINS,
> > +		.name = "DDI C IO power well",
> > +		.domains = SKL_DISPLAY_DDI_IO_C_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = SKL_DISP_PW_DDI_C,
> >  	},
> >  	{
> > -		.name = "DDI D power well",
> > -		.domains = SKL_DISPLAY_DDI_D_POWER_DOMAINS,
> > +		.name = "DDI D IO power well",
> > +		.domains = SKL_DISPLAY_DDI_IO_D_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = SKL_DISP_PW_DDI_D,
> >  	},
> > @@ -2246,20 +2256,20 @@ static struct i915_power_well glk_power_wells[] = {
> >  		.id = GLK_DISP_PW_AUX_C,
> >  	},
> >  	{
> > -		.name = "DDI A power well",
> > -		.domains = GLK_DISPLAY_DDI_A_POWER_DOMAINS,
> > +		.name = "DDI A IO power well",
> > +		.domains = GLK_DISPLAY_DDI_IO_A_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = GLK_DISP_PW_DDI_A,
> >  	},
> >  	{
> > -		.name = "DDI B power well",
> > -		.domains = GLK_DISPLAY_DDI_B_POWER_DOMAINS,
> > +		.name = "DDI B IO power well",
> > +		.domains = GLK_DISPLAY_DDI_IO_B_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = SKL_DISP_PW_DDI_B,
> >  	},
> >  	{
> > -		.name = "DDI C power well",
> > -		.domains = GLK_DISPLAY_DDI_C_POWER_DOMAINS,
> > +		.name = "DDI C IO power well",
> > +		.domains = GLK_DISPLAY_DDI_IO_C_POWER_DOMAINS,
> >  		.ops = &skl_power_well_ops,
> >  		.id = SKL_DISP_PW_DDI_C,
> >  	},
> > -- 
> > 2.9.3
> > 
> > _______________________________________________
> > Intel-gfx mailing list
> > Intel-gfx@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/intel-gfx
> 
> 
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

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

end of thread, other threads:[~2017-02-28  7:28 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-22  6:34 [PATCH v3 0/6] Fix Geminilake DDI power well enable timeouts Ander Conselvan de Oliveira
2017-02-22  6:34 ` [PATCH v3 1/6] drm/i915: Store aux power domain in intel_dp Ander Conselvan de Oliveira
2017-02-22  6:34 ` [PATCH v3 2/6] drm/i915: Store encoder power domain in struct intel_encoder Ander Conselvan de Oliveira
2017-02-22  6:34 ` [PATCH v3 3/6] drm/i915: Check encoder type in enc_to_dig_port() Ander Conselvan de Oliveira
2017-02-24 13:38   ` Imre Deak
2017-02-24 14:11     ` [PATCH] " Ander Conselvan de Oliveira
2017-02-24 14:18       ` Ander Conselvan de Oliveira
2017-02-24 15:34         ` Imre Deak
2017-02-22  6:34 ` [PATCH v3 4/6] drm/i915/glk: Implement WaDDIIOTimeout Ander Conselvan de Oliveira
2017-02-22  6:34 ` [PATCH v3 5/6] drm/i915/glk: Don't enable DDI IO power domains during init Ander Conselvan de Oliveira
2017-02-22  6:34 ` [PATCH v3 6/6] drm/i915: Only enable DDI IO power domains after enabling DPLL Ander Conselvan de Oliveira
2017-02-24 13:50   ` Imre Deak
2017-02-24 14:19     ` [PATCH] " Ander Conselvan de Oliveira
2017-02-27 20:38       ` Ville Syrjälä
2017-02-28  7:28         ` Ander Conselvan De Oliveira
2017-02-22  7:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev3) Patchwork
2017-02-24 15:22 ` ✓ Fi.CI.BAT: success for Fix Geminilake DDI power well enable timeouts (rev6) Patchwork
2017-02-27  7:15   ` Ander Conselvan De Oliveira

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.