[PATCH 0/8] Skylake DMC/DC-state fixes and redesign

[PATCH 0/8] Skylake DMC/DC-state fixes and redesign

[Patrik Jakobsson]

These patches should sit on top of the DMC redesign patches from
Animesh/Imre [1] which in turn depends on Mika's FW debug patches [2].

First two patches are from Ville and is included since they otherwise
might be forgotten. The third from Ville helps with handling DC off when
doing Aux A communication.

[1]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/079041.html

[2]
http://lists.freedesktop.org/archives/intel-gfx/2015-October/078898.html

Patrik Jakobsson (5):
  drm/i915: Add a modeset power domain
  drm/i915: Do not warn on PG2 enabled in gen9_disable_dc5()
  drm/i915/skl: Turn DC handling into a power well
  drm/i915/skl: Add boot parameter for disabling DC6
  drm/i915: Force loading of csr program at boot

Ville Syrjälä (3):
  drm/i915: Clean up AUX power domain handling
  drm/i915: Introduce a gmbus power domain
  drm/i915: Remove DDI power domain exclusion
    SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS

 drivers/gpu/drm/i915/i915_debugfs.c     |   4 +
 drivers/gpu/drm/i915/i915_drv.c         |   8 +-
 drivers/gpu/drm/i915/i915_drv.h         |   5 +-
 drivers/gpu/drm/i915/i915_params.c      |   6 ++
 drivers/gpu/drm/i915/intel_csr.c        |   6 +-
 drivers/gpu/drm/i915/intel_display.c    |  46 ++++++++++
 drivers/gpu/drm/i915/intel_dp.c         |  48 +++--------
 drivers/gpu/drm/i915/intel_drv.h        |   6 +-
 drivers/gpu/drm/i915/intel_hdmi.c       |   8 +-
 drivers/gpu/drm/i915/intel_i2c.c        |   6 +-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 148 ++++++++++++++++----------------
 11 files changed, 163 insertions(+), 128 deletions(-)

-- 
2.1.4

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

[PATCH 1/8] drm/i915: Clean up AUX power domain handling

[Patrik Jakobsson]

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

Introduce intel_display_port_aux_power_domain() which simply returns
the appropriate AUX power domain for a specific port, and then replace
the intel_display_port_power_domain() with calls to the new function
in the DP code. As long as we're not actually enabling the port we don't
need the lane power domains, and those are handled now purely from
modeset_update_crtc_power_domains().

My initial motivation for this was to see if I could keep the DPIO power
wells powered down while doing AUX on CHV, but turns out I can't so this
doesn't change anything for CHV at least. But I think it's still a
worthwile change.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 40 ++++++++++++++++++++++++++++++
 drivers/gpu/drm/i915/intel_dp.c      | 48 +++++++++++-------------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 3 files changed, 56 insertions(+), 34 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3f1b545..c6d60b8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5170,6 +5170,23 @@ 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;
+	default:
+		WARN_ON_ONCE(1);
+		return POWER_DOMAIN_AUX_A;
+	}
+}
+
 #define for_each_power_domain(domain, mask)				\
 	for ((domain) = 0; (domain) < POWER_DOMAIN_NUM; (domain)++)	\
 		if ((1 << (domain)) & (mask))
@@ -5201,6 +5218,29 @@ 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_device *dev = 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));
+	case INTEL_OUTPUT_DISPLAYPORT:
+	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:
+		WARN_ON_ONCE(1);
+		return POWER_DOMAIN_AUX_A;
+	}
+}
+
 static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
diff --git a/drivers/gpu/drm/i915/intel_dp.c b/drivers/gpu/drm/i915/intel_dp.c
index 1cb1f3f..258d8b1 100644
--- a/drivers/gpu/drm/i915/intel_dp.c
+++ b/drivers/gpu/drm/i915/intel_dp.c
@@ -277,7 +277,7 @@ static void pps_lock(struct intel_dp *intel_dp)
 	 * See vlv_power_sequencer_reset() why we need
 	 * a power domain reference here.
 	 */
-	power_domain = intel_display_port_power_domain(encoder);
+	power_domain = intel_display_port_aux_power_domain(encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
 	mutex_lock(&dev_priv->pps_mutex);
@@ -293,7 +293,7 @@ static void pps_unlock(struct intel_dp *intel_dp)
 
 	mutex_unlock(&dev_priv->pps_mutex);
 
-	power_domain = intel_display_port_power_domain(encoder);
+	power_domain = intel_display_port_aux_power_domain(encoder);
 	intel_display_power_put(dev_priv, power_domain);
 }
 
@@ -816,8 +816,6 @@ intel_dp_aux_ch(struct intel_dp *intel_dp,
 
 	intel_dp_check_edp(intel_dp);
 
-	intel_aux_display_runtime_get(dev_priv);
-
 	/* Try to wait for any previous AUX channel activity */
 	for (try = 0; try < 3; try++) {
 		status = I915_READ_NOTRACE(ch_ctl);
@@ -926,7 +924,6 @@ done:
 	ret = recv_bytes;
 out:
 	pm_qos_update_request(&dev_priv->pm_qos, PM_QOS_DEFAULT_VALUE);
-	intel_aux_display_runtime_put(dev_priv);
 
 	if (vdd)
 		edp_panel_vdd_off(intel_dp, false);
@@ -1784,7 +1781,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_power_domain(intel_encoder);
+	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
 	DRM_DEBUG_KMS("Turning eDP port %c VDD on\n",
@@ -1874,7 +1871,7 @@ static void edp_panel_vdd_off_sync(struct intel_dp *intel_dp)
 	if ((pp & POWER_TARGET_ON) == 0)
 		intel_dp->last_power_cycle = jiffies;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
+	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
 }
 
@@ -2025,7 +2022,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_power_domain(intel_encoder);
+	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_put(dev_priv, power_domain);
 }
 
@@ -4765,26 +4762,6 @@ intel_dp_unset_edid(struct intel_dp *intel_dp)
 	intel_dp->has_audio = false;
 }
 
-static enum intel_display_power_domain
-intel_dp_power_get(struct intel_dp *dp)
-{
-	struct intel_encoder *encoder = &dp_to_dig_port(dp)->base;
-	enum intel_display_power_domain power_domain;
-
-	power_domain = intel_display_port_power_domain(encoder);
-	intel_display_power_get(to_i915(encoder->base.dev), power_domain);
-
-	return power_domain;
-}
-
-static void
-intel_dp_power_put(struct intel_dp *dp,
-		   enum intel_display_power_domain power_domain)
-{
-	struct intel_encoder *encoder = &dp_to_dig_port(dp)->base;
-	intel_display_power_put(to_i915(encoder->base.dev), power_domain);
-}
-
 static enum drm_connector_status
 intel_dp_detect(struct drm_connector *connector, bool force)
 {
@@ -4808,7 +4785,8 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 		return connector_status_disconnected;
 	}
 
-	power_domain = intel_dp_power_get(intel_dp);
+	power_domain = intel_display_port_aux_power_domain(intel_encoder);
+	intel_display_power_get(to_i915(dev), power_domain);
 
 	/* Can't disconnect eDP, but you can close the lid... */
 	if (is_edp(intel_dp))
@@ -4853,7 +4831,7 @@ intel_dp_detect(struct drm_connector *connector, bool force)
 	}
 
 out:
-	intel_dp_power_put(intel_dp, power_domain);
+	intel_display_power_put(to_i915(dev), power_domain);
 	return status;
 }
 
@@ -4862,6 +4840,7 @@ 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",
@@ -4871,11 +4850,12 @@ intel_dp_force(struct drm_connector *connector)
 	if (connector->status != connector_status_connected)
 		return;
 
-	power_domain = intel_dp_power_get(intel_dp);
+	power_domain = intel_display_port_aux_power_domain(intel_encoder);
+	intel_display_power_get(dev_priv, power_domain);
 
 	intel_dp_set_edid(intel_dp);
 
-	intel_dp_power_put(intel_dp, power_domain);
+	intel_display_power_put(dev_priv, power_domain);
 
 	if (intel_encoder->type != INTEL_OUTPUT_EDP)
 		intel_encoder->type = INTEL_OUTPUT_DISPLAYPORT;
@@ -5091,7 +5071,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_power_domain(&intel_dig_port->base);
+	power_domain = intel_display_port_aux_power_domain(&intel_dig_port->base);
 	intel_display_power_get(dev_priv, power_domain);
 
 	edp_panel_vdd_schedule_off(intel_dp);
@@ -5172,7 +5152,7 @@ intel_dp_hpd_pulse(struct intel_digital_port *intel_dig_port, bool long_hpd)
 		      port_name(intel_dig_port->port),
 		      long_hpd ? "long" : "short");
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
+	power_domain = intel_display_port_aux_power_domain(intel_encoder);
 	intel_display_power_get(dev_priv, power_domain);
 
 	if (long_hpd) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index a2006b7..933f1e8 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1188,6 +1188,8 @@ 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);
 void intel_crtc_wait_for_pending_flips(struct drm_crtc *crtc);
-- 
2.1.4

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

[PATCH 2/8] drm/i915: Introduce a gmbus power domain

[Patrik Jakobsson]

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

Currently the gmbus code uses intel_aux_display_runtime_get/put in an
effort to make sure the hardware is powered up sufficiently for gmbus.
That function only takes the runtime PM reference which on VLV/CHV/BXT
is not enough. We need the disp2d/pipe-a well on VLV/CHV and power well
2 on BXT. So add a new power domnain for gmbus and kill off the now
unused intel_aux_display_runtime_get/put. And change
intel_hdmi_set_edid() to use the gmbus power domain too since that's all
we need there.

Also toss in a BUILD_BUG_ON() to catch problems if we run out of
bits for power domains. We're already really close to the limit...

[Patrik: Add gmbus string to debugfs output]

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     |  2 ++
 drivers/gpu/drm/i915/i915_drv.h         |  1 +
 drivers/gpu/drm/i915/intel_drv.h        |  2 --
 drivers/gpu/drm/i915/intel_hdmi.c       |  8 ++------
 drivers/gpu/drm/i915/intel_i2c.c        |  6 ++++--
 drivers/gpu/drm/i915/intel_runtime_pm.c | 34 ++++-----------------------------
 6 files changed, 13 insertions(+), 40 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f727887..f7b85fe 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2744,6 +2744,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
 		return "AUX_C";
 	case POWER_DOMAIN_AUX_D:
 		return "AUX_D";
+	case POWER_DOMAIN_GMBUS:
+		return "GMBUS";
 	case POWER_DOMAIN_INIT:
 		return "INIT";
 	default:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 05aeaee..c3d3b2a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -199,6 +199,7 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_AUX_B,
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_AUX_D,
+	POWER_DOMAIN_GMBUS,
 	POWER_DOMAIN_INIT,
 
 	POWER_DOMAIN_NUM,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 933f1e8..d110555 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1394,8 +1394,6 @@ void intel_display_power_get(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
 void intel_display_power_put(struct drm_i915_private *dev_priv,
 			     enum intel_display_power_domain domain);
-void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv);
-void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_get_noresume(struct drm_i915_private *dev_priv);
 void intel_runtime_pm_put(struct drm_i915_private *dev_priv);
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index 013bd7d..cea05b8 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -1335,21 +1335,17 @@ intel_hdmi_set_edid(struct drm_connector *connector, bool force)
 {
 	struct drm_i915_private *dev_priv = to_i915(connector->dev);
 	struct intel_hdmi *intel_hdmi = intel_attached_hdmi(connector);
-	struct intel_encoder *intel_encoder =
-		&hdmi_to_dig_port(intel_hdmi)->base;
-	enum intel_display_power_domain power_domain;
 	struct edid *edid = NULL;
 	bool connected = false;
 
-	power_domain = intel_display_port_power_domain(intel_encoder);
-	intel_display_power_get(dev_priv, power_domain);
+	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
 
 	if (force)
 		edid = drm_get_edid(connector,
 				    intel_gmbus_get_adapter(dev_priv,
 				    intel_hdmi->ddc_bus));
 
-	intel_display_power_put(dev_priv, power_domain);
+	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
 
 	to_intel_connector(connector)->detect_edid = edid;
 	if (edid && edid->input & DRM_EDID_INPUT_DIGITAL) {
diff --git a/drivers/gpu/drm/i915/intel_i2c.c b/drivers/gpu/drm/i915/intel_i2c.c
index bd58da0..fe69623 100644
--- a/drivers/gpu/drm/i915/intel_i2c.c
+++ b/drivers/gpu/drm/i915/intel_i2c.c
@@ -483,7 +483,7 @@ gmbus_xfer(struct i2c_adapter *adapter,
 	int i = 0, inc, try = 0;
 	int ret = 0;
 
-	intel_aux_display_runtime_get(dev_priv);
+	intel_display_power_get(dev_priv, POWER_DOMAIN_GMBUS);
 	mutex_lock(&dev_priv->gmbus_mutex);
 
 	if (bus->force_bit) {
@@ -595,7 +595,9 @@ timeout:
 
 out:
 	mutex_unlock(&dev_priv->gmbus_mutex);
-	intel_aux_display_runtime_put(dev_priv);
+
+	intel_display_power_put(dev_priv, POWER_DOMAIN_GMBUS);
+
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index d150f93..7d309d5 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -362,6 +362,7 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_AUX_C) |			\
 	BIT(POWER_DOMAIN_AUDIO) |			\
 	BIT(POWER_DOMAIN_VGA) |				\
+	BIT(POWER_DOMAIN_GMBUS) |			\
 	BIT(POWER_DOMAIN_INIT))
 #define BXT_DISPLAY_POWERWELL_1_POWER_DOMAINS (		\
 	BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
@@ -1473,6 +1474,7 @@ void intel_display_power_put(struct drm_i915_private *dev_priv,
 	BIT(POWER_DOMAIN_AUX_B) |			\
 	BIT(POWER_DOMAIN_AUX_C) |			\
 	BIT(POWER_DOMAIN_AUX_D) |			\
+	BIT(POWER_DOMAIN_GMBUS) |			\
 	BIT(POWER_DOMAIN_INIT))
 #define HSW_DISPLAY_POWER_DOMAINS (				\
 	(POWER_DOMAIN_MASK & ~HSW_ALWAYS_ON_POWER_DOMAINS) |	\
@@ -1817,6 +1819,8 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 
+	BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
+
 	mutex_init(&power_domains->lock);
 
 	/*
@@ -2036,36 +2040,6 @@ void intel_power_domains_init_hw(struct drm_i915_private *dev_priv)
 }
 
 /**
- * intel_aux_display_runtime_get - grab an auxiliary power domain reference
- * @dev_priv: i915 device instance
- *
- * This function grabs a power domain reference for the auxiliary power domain
- * (for access to the GMBUS and DP AUX blocks) and ensures that it and all its
- * parents are powered up. Therefore users should only grab a reference to the
- * innermost power domain they need.
- *
- * Any power domain reference obtained by this function must have a symmetric
- * call to intel_aux_display_runtime_put() to release the reference again.
- */
-void intel_aux_display_runtime_get(struct drm_i915_private *dev_priv)
-{
-	intel_runtime_pm_get(dev_priv);
-}
-
-/**
- * intel_aux_display_runtime_put - release an auxiliary power domain reference
- * @dev_priv: i915 device instance
- *
- * This function drops the auxiliary power domain reference obtained by
- * intel_aux_display_runtime_get() and might power down the corresponding
- * hardware block right away if this is the last reference.
- */
-void intel_aux_display_runtime_put(struct drm_i915_private *dev_priv)
-{
-	intel_runtime_pm_put(dev_priv);
-}
-
-/**
  * intel_runtime_pm_get - grab a runtime pm reference
  * @dev_priv: i915 device instance
  *
-- 
2.1.4

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

[PATCH 3/8] drm/i915: Remove DDI power domain exclusion SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS

[Patrik Jakobsson]

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

All the DDI power domains are already excluded from
SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS on account of
excluding SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS and
SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS, no need to spell them out again.

Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
Reviewed-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 7d309d5..b6ce77f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -339,10 +339,6 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
 	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
 	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
-	SKL_DISPLAY_DDI_A_E_POWER_DOMAINS |		\
-	SKL_DISPLAY_DDI_B_POWER_DOMAINS |		\
-	SKL_DISPLAY_DDI_C_POWER_DOMAINS |		\
-	SKL_DISPLAY_DDI_D_POWER_DOMAINS |		\
 	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
 	BIT(POWER_DOMAIN_INIT))
 
-- 
2.1.4

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

[PATCH 4/8] drm/i915: Add a modeset power domain

[Patrik Jakobsson]

We need DC5/DC6 to be disabled around modesets to prevent confusing the
DMC. Also, we've run out of bits in the 32 bit power domain mask so now
it's a 64 bit mask.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c     | 2 ++
 drivers/gpu/drm/i915/i915_drv.h         | 3 ++-
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 3 files changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index f7b85fe..ae9a2ed 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -2746,6 +2746,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
 		return "AUX_D";
 	case POWER_DOMAIN_GMBUS:
 		return "GMBUS";
+	case POWER_DOMAIN_MODESET:
+		return "MODESET";
 	case POWER_DOMAIN_INIT:
 		return "INIT";
 	default:
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index c3d3b2a..efb6a00 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -200,6 +200,7 @@ enum intel_display_power_domain {
 	POWER_DOMAIN_AUX_C,
 	POWER_DOMAIN_AUX_D,
 	POWER_DOMAIN_GMBUS,
+	POWER_DOMAIN_MODESET,
 	POWER_DOMAIN_INIT,
 
 	POWER_DOMAIN_NUM,
@@ -1226,7 +1227,7 @@ struct i915_power_well {
 	int count;
 	/* cached hw enabled state */
 	bool hw_enabled;
-	unsigned long domains;
+	unsigned long long domains;
 	unsigned long data;
 	const struct i915_power_well_ops *ops;
 };
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index b6ce77f..d0ed38b 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -1815,7 +1815,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
 {
 	struct i915_power_domains *power_domains = &dev_priv->power_domains;
 
-	BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
+	BUILD_BUG_ON(POWER_DOMAIN_NUM > 63);
 
 	mutex_init(&power_domains->lock);
 
-- 
2.1.4

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

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Ville Syrjälä]

On Tue, Nov 03, 2015 at 01:31:10PM +0100, Patrik Jakobsson wrote:
> We need DC5/DC6 to be disabled around modesets to prevent confusing the
> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
> it's a 64 bit mask.

We could get back 4 bits by squashing each 2 and 4 lane DDI power doamin
into just one power domain. I don't think we use the 2 vs. 4 lane
distinciton anywhere. It was basically added for VLV but turns it it
can't be used there, so I think we could just get rid of it.

> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c     | 2 ++
>  drivers/gpu/drm/i915/i915_drv.h         | 3 ++-
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  3 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
> index f7b85fe..ae9a2ed 100644
> --- a/drivers/gpu/drm/i915/i915_debugfs.c
> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
> @@ -2746,6 +2746,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
>  		return "AUX_D";
>  	case POWER_DOMAIN_GMBUS:
>  		return "GMBUS";
> +	case POWER_DOMAIN_MODESET:
> +		return "MODESET";
>  	case POWER_DOMAIN_INIT:
>  		return "INIT";
>  	default:
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c3d3b2a..efb6a00 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -200,6 +200,7 @@ enum intel_display_power_domain {
>  	POWER_DOMAIN_AUX_C,
>  	POWER_DOMAIN_AUX_D,
>  	POWER_DOMAIN_GMBUS,
> +	POWER_DOMAIN_MODESET,
>  	POWER_DOMAIN_INIT,
>  
>  	POWER_DOMAIN_NUM,
> @@ -1226,7 +1227,7 @@ struct i915_power_well {
>  	int count;
>  	/* cached hw enabled state */
>  	bool hw_enabled;
> -	unsigned long domains;
> +	unsigned long long domains;
>  	unsigned long data;
>  	const struct i915_power_well_ops *ops;
>  };
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index b6ce77f..d0ed38b 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -1815,7 +1815,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>  {
>  	struct i915_power_domains *power_domains = &dev_priv->power_domains;
>  
> -	BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
> +	BUILD_BUG_ON(POWER_DOMAIN_NUM > 63);
>  
>  	mutex_init(&power_domains->lock);
>  
> -- 
> 2.1.4

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

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Patrik Jakobsson]

On Wed, Nov 4, 2015 at 6:29 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Nov 03, 2015 at 01:31:10PM +0100, Patrik Jakobsson wrote:
>> We need DC5/DC6 to be disabled around modesets to prevent confusing the
>> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
>> it's a 64 bit mask.
>
> We could get back 4 bits by squashing each 2 and 4 lane DDI power doamin
> into just one power domain. I don't think we use the 2 vs. 4 lane
> distinciton anywhere. It was basically added for VLV but turns it it
> can't be used there, so I think we could just get rid of it.

Good idea. I can't see that we're making a distinction anywhere either.

>
>>
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c     | 2 ++
>>  drivers/gpu/drm/i915/i915_drv.h         | 3 ++-
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>>  3 files changed, 5 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
>> index f7b85fe..ae9a2ed 100644
>> --- a/drivers/gpu/drm/i915/i915_debugfs.c
>> +++ b/drivers/gpu/drm/i915/i915_debugfs.c
>> @@ -2746,6 +2746,8 @@ static const char *power_domain_str(enum intel_display_power_domain domain)
>>               return "AUX_D";
>>       case POWER_DOMAIN_GMBUS:
>>               return "GMBUS";
>> +     case POWER_DOMAIN_MODESET:
>> +             return "MODESET";
>>       case POWER_DOMAIN_INIT:
>>               return "INIT";
>>       default:
>> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> index c3d3b2a..efb6a00 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.h
>> +++ b/drivers/gpu/drm/i915/i915_drv.h
>> @@ -200,6 +200,7 @@ enum intel_display_power_domain {
>>       POWER_DOMAIN_AUX_C,
>>       POWER_DOMAIN_AUX_D,
>>       POWER_DOMAIN_GMBUS,
>> +     POWER_DOMAIN_MODESET,
>>       POWER_DOMAIN_INIT,
>>
>>       POWER_DOMAIN_NUM,
>> @@ -1226,7 +1227,7 @@ struct i915_power_well {
>>       int count;
>>       /* cached hw enabled state */
>>       bool hw_enabled;
>> -     unsigned long domains;
>> +     unsigned long long domains;
>>       unsigned long data;
>>       const struct i915_power_well_ops *ops;
>>  };
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index b6ce77f..d0ed38b 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -1815,7 +1815,7 @@ int intel_power_domains_init(struct drm_i915_private *dev_priv)
>>  {
>>       struct i915_power_domains *power_domains = &dev_priv->power_domains;
>>
>> -     BUILD_BUG_ON(POWER_DOMAIN_NUM > 31);
>> +     BUILD_BUG_ON(POWER_DOMAIN_NUM > 63);
>>
>>       mutex_init(&power_domains->lock);
>>
>> --
>> 2.1.4
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Daniel Stone]

Hi,

On 3 November 2015 at 12:31, Patrik Jakobsson
<patrik.jakobsson@linux.intel.com> wrote:
> We need DC5/DC6 to be disabled around modesets to prevent confusing the
> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
> it's a 64 bit mask.

There are quite a lot of users in intel_display.c (search for
put_domains, display_power_put, put_power_domains) which need updating
for the unsigned long long change.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Use extended power domain bitmask

[Daniel Stone]

c4111f0ac6 extended the size of the power-domain enum to 64 bits wide,
but there are still a few 'unsigned long' users inside intel_display.c.

Make these unsigned long long so we can also capture DRIVER_MODESET,
and use this to simplify the modeset power domain handling a little.

[daniels: New; only required when working against Patrik/Imre's tree.]

Signed-off-by: Daniel Stone <daniels@collabora.com>
---
 drivers/gpu/drm/i915/intel_display.c | 27 +++++++++++++--------------
 1 file changed, 13 insertions(+), 14 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc31f33..9c3aa68 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5240,12 +5240,12 @@ static unsigned long get_crtc_power_domains(struct drm_crtc *crtc)
 	return mask;
 }
 
-static unsigned long modeset_get_crtc_power_domains(struct drm_crtc *crtc)
+static unsigned long long modeset_get_crtc_power_domains(struct drm_crtc *crtc)
 {
 	struct drm_i915_private *dev_priv = crtc->dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	enum intel_display_power_domain domain;
-	unsigned long domains, new_domains, old_domains;
+	unsigned long long domains, new_domains, old_domains;
 
 	old_domains = intel_crtc->enabled_power_domains;
 	intel_crtc->enabled_power_domains = new_domains = get_crtc_power_domains(crtc);
@@ -5259,7 +5259,7 @@ static unsigned long modeset_get_crtc_power_domains(struct drm_crtc *crtc)
 }
 
 static void modeset_put_power_domains(struct drm_i915_private *dev_priv,
-				      unsigned long domains)
+				      unsigned long long domains)
 {
 	enum intel_display_power_domain domain;
 
@@ -5271,7 +5271,7 @@ static void modeset_update_crtc_power_domains(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	unsigned long put_domains[I915_MAX_PIPES] = {};
+	unsigned long long put_domains[I915_MAX_PIPES] = {};
 	struct drm_crtc_state *crtc_state;
 	struct drm_crtc *crtc;
 	int i;
@@ -6250,7 +6250,7 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_i915_private *dev_priv = to_i915(crtc->dev);
 	enum intel_display_power_domain domain;
-	unsigned long domains;
+	unsigned long long domains;
 
 	if (!intel_crtc->active)
 		return;
@@ -13318,10 +13318,12 @@ static int intel_atomic_commit(struct drm_device *dev,
 		bool modeset = needs_modeset(crtc->state);
 		bool update_pipe = !modeset &&
 			to_intel_crtc_state(crtc->state)->update_pipe;
-		unsigned long put_domains = 0;
+		unsigned long long put_domains = 0;
 
-		if (modeset)
+		if (modeset) {
 			intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
+			put_domains |= POWER_DOMAIN_MODESET;
+		}
 
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
@@ -13329,7 +13331,7 @@ static int intel_atomic_commit(struct drm_device *dev,
 		}
 
 		if (update_pipe) {
-			put_domains = modeset_get_crtc_power_domains(crtc);
+			put_domains |= modeset_get_crtc_power_domains(crtc);
 
 			/* make sure intel_modeset_check_state runs */
 			any_ms = true;
@@ -13342,13 +13344,10 @@ static int intel_atomic_commit(struct drm_device *dev,
 		    (crtc->state->planes_changed || update_pipe))
 			drm_atomic_helper_commit_planes_on_crtc(crtc_state);
 
-		if (put_domains)
-			modeset_put_power_domains(dev_priv, put_domains);
-
 		intel_post_plane_update(intel_crtc);
 
-		if (modeset)
-			intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
+		if (put_domains)
+			modeset_put_power_domains(dev_priv, put_domains);
 	}
 
 	/* FIXME: add subpixel order */
@@ -15514,7 +15513,7 @@ intel_modeset_setup_hw_state(struct drm_device *dev)
 		ilk_wm_get_hw_state(dev);
 
 	for_each_intel_crtc(dev, crtc) {
-		unsigned long put_domains;
+		unsigned long long put_domains;
 
 		put_domains = modeset_get_crtc_power_domains(&crtc->base);
 		if (WARN_ON(put_domains))
-- 
2.5.0

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

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Patrik Jakobsson]

On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> wrote:
> Hi,
>
> On 3 November 2015 at 12:31, Patrik Jakobsson
> <patrik.jakobsson@linux.intel.com> wrote:
>> We need DC5/DC6 to be disabled around modesets to prevent confusing the
>> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
>> it's a 64 bit mask.
>
> There are quite a lot of users in intel_display.c (search for
> put_domains, display_power_put, put_power_domains) which need updating
> for the unsigned long long change.

Ah yes, we carry the mask around there as well. Thanks for catching
that. I like the move of POWER_DOMAIN_MODESET into put_domain as well.
I will resend the whole series again rebased on Imre's latest series.
Ok if I incorporate your changes directly?

Thanks
Patrik

>
> Cheers,
> Daniel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Daniel Stone]

Hi,

On 5 November 2015 at 17:12, Patrik Jakobsson
<patrik.r.jakobsson@gmail.com> wrote:
> On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> wrote:
>> On 3 November 2015 at 12:31, Patrik Jakobsson
>> <patrik.jakobsson@linux.intel.com> wrote:
>>> We need DC5/DC6 to be disabled around modesets to prevent confusing the
>>> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
>>> it's a 64 bit mask.
>>
>> There are quite a lot of users in intel_display.c (search for
>> put_domains, display_power_put, put_power_domains) which need updating
>> for the unsigned long long change.
>
> Ah yes, we carry the mask around there as well. Thanks for catching
> that. I like the move of POWER_DOMAIN_MODESET into put_domain as well.
> I will resend the whole series again rebased on Imre's latest series.
> Ok if I incorporate your changes directly?

Of course! Sending it like that wasn't fishing for attribution, just
that as I rebased and shifted around a bunch of other changes,
git-send-email was the most laziness-friendly option. :) Thanks for
pulling it in. If you're an admin on intel-gfx Patchwork, would you
mind marking my patch as superseded or something so it doesn't show
up?

I assume you've co-ordinated with Imre, but he does have a combined
tree on github.com/ideak/linux#dmc-fixes, which may save you some
time.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Imre Deak]

On pe, 2015-11-06 at 13:53 +0000, Daniel Stone wrote:
> Hi,
> 
> On 5 November 2015 at 17:12, Patrik Jakobsson
> <patrik.r.jakobsson@gmail.com> wrote:
> > On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org>
> > wrote:
> > > On 3 November 2015 at 12:31, Patrik Jakobsson
> > > <patrik.jakobsson@linux.intel.com> wrote:
> > > > We need DC5/DC6 to be disabled around modesets to prevent
> > > > confusing the
> > > > DMC. Also, we've run out of bits in the 32 bit power domain
> > > > mask so now
> > > > it's a 64 bit mask.
> > > 
> > > There are quite a lot of users in intel_display.c (search for
> > > put_domains, display_power_put, put_power_domains) which need
> > > updating
> > > for the unsigned long long change.
> > 
> > Ah yes, we carry the mask around there as well. Thanks for catching
> > that. I like the move of POWER_DOMAIN_MODESET into put_domain as
> > well.
> > I will resend the whole series again rebased on Imre's latest
> > series.
> > Ok if I incorporate your changes directly?
> 
> Of course! Sending it like that wasn't fishing for attribution, just
> that as I rebased and shifted around a bunch of other changes,
> git-send-email was the most laziness-friendly option. :) Thanks for
> pulling it in. If you're an admin on intel-gfx Patchwork, would you
> mind marking my patch as superseded or something so it doesn't show
> up?
> 
> I assume you've co-ordinated with Imre, but he does have a combined
> tree on github.com/ideak/linux#dmc-fixes, which may save you some
> time.

Well, that one has my patches on top while we agreed with Patrik to
have the opposite order in the end, since I think that makes more sense
for bisectability: Patrik's change to move DC6 enabling earlier might
reveal some issues that are supposed to be fixed in my patchset. Anyway
the end result should be the same so hopefully that branch was useful
for testing.

> 
> Cheers,
> Daniel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Daniel Stone]

Hi,

On 6 November 2015 at 14:17, Imre Deak <imre.deak@intel.com> wrote:
> On pe, 2015-11-06 at 13:53 +0000, Daniel Stone wrote:
>> On 5 November 2015 at 17:12, Patrik Jakobsson
>> <patrik.r.jakobsson@gmail.com> wrote:
>> > Ah yes, we carry the mask around there as well. Thanks for catching
>> > that. I like the move of POWER_DOMAIN_MODESET into put_domain as
>> > well.
>> > I will resend the whole series again rebased on Imre's latest
>> > series.
>> > Ok if I incorporate your changes directly?
>>
>> Of course! Sending it like that wasn't fishing for attribution, just
>> that as I rebased and shifted around a bunch of other changes,
>> git-send-email was the most laziness-friendly option. :) Thanks for
>> pulling it in. If you're an admin on intel-gfx Patchwork, would you
>> mind marking my patch as superseded or something so it doesn't show
>> up?
>>
>> I assume you've co-ordinated with Imre, but he does have a combined
>> tree on github.com/ideak/linux#dmc-fixes, which may save you some
>> time.
>
> Well, that one has my patches on top while we agreed with Patrik to
> have the opposite order in the end, since I think that makes more sense
> for bisectability: Patrik's change to move DC6 enabling earlier might
> reveal some issues that are supposed to be fixed in my patchset. Anyway
> the end result should be the same so hopefully that branch was useful
> for testing.

Yeah, having the combined branch in that order was very useful - thanks a lot!

I have no useful opinion on which order is better, so whatever you
guys decide will be best I'm sure.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Daniel Stone]

Hi,

On 6 November 2015 at 13:53, Daniel Stone <daniel@fooishbar.org> wrote:
> On 5 November 2015 at 17:12, Patrik Jakobsson
> <patrik.r.jakobsson@gmail.com> wrote:
>> On Thu, Nov 5, 2015 at 4:02 PM, Daniel Stone <daniel@fooishbar.org> wrote:
>>> On 3 November 2015 at 12:31, Patrik Jakobsson
>>> <patrik.jakobsson@linux.intel.com> wrote:
>>>> We need DC5/DC6 to be disabled around modesets to prevent confusing the
>>>> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
>>>> it's a 64 bit mask.
>>>
>>> There are quite a lot of users in intel_display.c (search for
>>> put_domains, display_power_put, put_power_domains) which need updating
>>> for the unsigned long long change.
>>
>> Ah yes, we carry the mask around there as well. Thanks for catching
>> that. I like the move of POWER_DOMAIN_MODESET into put_domain as well.
>> I will resend the whole series again rebased on Imre's latest series.
>> Ok if I incorporate your changes directly?
>
> Of course! Sending it like that wasn't fishing for attribution, just
> that as I rebased and shifted around a bunch of other changes,
> git-send-email was the most laziness-friendly option. :) Thanks for
> pulling it in. If you're an admin on intel-gfx Patchwork, would you
> mind marking my patch as superseded or something so it doesn't show
> up?

Safe to say though that I'd recommend work->put_power_domains |= (1 <<
POWER_MODESET), rather than |= POWER_DOMAIN_MODESET ...

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Dave Airlie]

On 3 November 2015 at 22:31, Patrik Jakobsson
<patrik.jakobsson@linux.intel.com> wrote:
> We need DC5/DC6 to be disabled around modesets to prevent confusing the
> DMC. Also, we've run out of bits in the 32 bit power domain mask so now
> it's a 64 bit mask.
>

It was bad enough the original code used unsigned long so was
different on 32/64 bit,

but don't keep going with it. uint64_t already please.

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

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Daniel Vetter]

On Sat, Nov 07, 2015 at 08:29:49AM +1000, Dave Airlie wrote:
> On 3 November 2015 at 22:31, Patrik Jakobsson
> <patrik.jakobsson@linux.intel.com> wrote:
> > We need DC5/DC6 to be disabled around modesets to prevent confusing the
> > DMC. Also, we've run out of bits in the 32 bit power domain mask so now
> > it's a 64 bit mask.
> >
> 
> It was bad enough the original code used unsigned long so was
> different on 32/64 bit,
> 
> but don't keep going with it. uint64_t already please.

Concurred. Also I don't like typing ;-)
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 4/8] drm/i915: Add a modeset power domain

[Patrik Jakobsson]

On Wed, Nov 18, 2015 at 10:02 AM, Daniel Vetter <daniel@ffwll.ch> wrote:
> On Sat, Nov 07, 2015 at 08:29:49AM +1000, Dave Airlie wrote:
>> On 3 November 2015 at 22:31, Patrik Jakobsson
>> <patrik.jakobsson@linux.intel.com> wrote:
>> > We need DC5/DC6 to be disabled around modesets to prevent confusing the
>> > DMC. Also, we've run out of bits in the 32 bit power domain mask so now
>> > it's a 64 bit mask.
>> >
>>
>> It was bad enough the original code used unsigned long so was
>> different on 32/64 bit,
>>
>> but don't keep going with it. uint64_t already please.
>
> Concurred. Also I don't like typing ;-)

We removed a few unused power domains so didn't need to bump it to 64
bits. Can do that anyway or just change it to an uint32_t in the next
round of PW/DMC cleanups.

-Patrik

> -Daniel
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 5/8] drm/i915: Do not warn on PG2 enabled in gen9_disable_dc5()

[Patrik Jakobsson]

PG2 enabled is not a requirement for disabling DC5. It's just one
of the reasons why we wouldn't enter DC5. During modeset we don't care
about PG2 from a DC perspective, only the fact that DC5/DC6 is not
allowed.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index d0ed38b..c901b19 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -483,8 +483,6 @@ static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
 
 static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
 {
-	bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
-					SKL_DISP_PW_2);
 	/*
 	 * During initialization, the firmware may not be loaded yet.
 	 * We still want to make sure that the DC enabling flag is cleared.
@@ -492,7 +490,6 @@ static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
 	if (dev_priv->power_domains.initializing)
 		return;
 
-	WARN_ONCE(!pg2_enabled, "PG2 not enabled to disable DC5.\n");
 	WARN_ONCE(dev_priv->pm.suspended,
 		"Disabling of DC5 while platform is runtime-suspended should never happen.\n");
 }
-- 
2.1.4

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

Re: [PATCH 5/8] drm/i915: Do not warn on PG2 enabled in gen9_disable_dc5()

[Ville Syrjälä]

On Tue, Nov 03, 2015 at 01:31:11PM +0100, Patrik Jakobsson wrote:
> PG2 enabled is not a requirement for disabling DC5. It's just one
> of the reasons why we wouldn't enter DC5. During modeset we don't care
> about PG2 from a DC perspective, only the fact that DC5/DC6 is not
> allowed.
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>

PW2 must be disabled before enabling DC5, and conversely DC5
must be disabled before enabling PW2. So yeah, asserting that
PW2 must be enabled before disabling DC5 doesn't make sense.

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

> ---
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index d0ed38b..c901b19 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -483,8 +483,6 @@ static void assert_can_enable_dc5(struct drm_i915_private *dev_priv)
>  
>  static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
>  {
> -	bool pg2_enabled = intel_display_power_well_is_enabled(dev_priv,
> -					SKL_DISP_PW_2);
>  	/*
>  	 * During initialization, the firmware may not be loaded yet.
>  	 * We still want to make sure that the DC enabling flag is cleared.
> @@ -492,7 +490,6 @@ static void assert_can_disable_dc5(struct drm_i915_private *dev_priv)
>  	if (dev_priv->power_domains.initializing)
>  		return;
>  
> -	WARN_ONCE(!pg2_enabled, "PG2 not enabled to disable DC5.\n");
>  	WARN_ONCE(dev_priv->pm.suspended,
>  		"Disabling of DC5 while platform is runtime-suspended should never happen.\n");
>  }
> -- 
> 2.1.4

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

[PATCH 6/8] drm/i915/skl: Turn DC handling into a power well

[Patrik Jakobsson]

Handle DC off as a power well where enabling the power well will prevent
the DMC to enter selected DC states (required around modesets and Aux
A). Disabling the power well will allow DC states again. For now the
highest DC state is DC6 but will be configurable in a later patch.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c         |   6 --
 drivers/gpu/drm/i915/intel_display.c    |   6 ++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 107 +++++++++++++++++++++-----------
 3 files changed, 78 insertions(+), 41 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 37319b0..e190237 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1069,9 +1069,6 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
 {
 	skl_uninit_cdclk(dev_priv);
 
-	if (dev_priv->csr.dmc_payload)
-		skl_enable_dc6(dev_priv);
-
 	return 0;
 }
 
@@ -1116,9 +1113,6 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
 
 static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
-	if (dev_priv->csr.dmc_payload)
-		skl_disable_dc6(dev_priv);
-
 	skl_init_cdclk(dev_priv);
 	intel_csr_load_program(dev_priv);
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c6d60b8..e401871 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -13296,6 +13296,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 			to_intel_crtc_state(crtc->state)->update_pipe;
 		unsigned long put_domains = 0;
 
+		if (modeset)
+			intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
+
 		if (modeset && crtc->state->active) {
 			update_scanline_offset(to_intel_crtc(crtc));
 			dev_priv->display.crtc_enable(crtc);
@@ -13319,6 +13322,9 @@ static int intel_atomic_commit(struct drm_device *dev,
 			modeset_put_power_domains(dev_priv, put_domains);
 
 		intel_post_plane_update(intel_crtc);
+
+		if (modeset)
+			intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
 	}
 
 	/* FIXME: add subpixel order */
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index c901b19..042d92f 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -49,9 +49,6 @@
  * present for a given platform.
  */
 
-#define GEN9_ENABLE_DC5(dev) 0
-#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev)
-
 #define for_each_power_well(i, power_well, domain_mask, power_domains)	\
 	for (i = 0;							\
 	     i < (power_domains)->power_well_count &&			\
@@ -336,10 +333,15 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
 	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
 	BIT(POWER_DOMAIN_PLLS) |			\
 	BIT(POWER_DOMAIN_INIT))
+#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
+	BIT(POWER_DOMAIN_MODESET) |			\
+	BIT(POWER_DOMAIN_AUX_A) |			\
+	BIT(POWER_DOMAIN_INIT))
 #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
 	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
 	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
-	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
+	SKL_DISPLAY_MISC_IO_POWER_DOMAINS |		\
+	SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) |		\
 	BIT(POWER_DOMAIN_INIT))
 
 #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
@@ -511,20 +513,6 @@ static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
 	POSTING_READ(DC_STATE_EN);
 }
 
-static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
-{
-	uint32_t val;
-
-	assert_can_disable_dc5(dev_priv);
-
-	DRM_DEBUG_KMS("Disabling DC5\n");
-
-	val = I915_READ(DC_STATE_EN);
-	val &= ~DC_STATE_EN_UPTO_DC5;
-	I915_WRITE(DC_STATE_EN, val);
-	POSTING_READ(DC_STATE_EN);
-}
-
 static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
 {
 	struct drm_device *dev = dev_priv->dev;
@@ -553,6 +541,21 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
 		  "DC6 already programmed to be disabled.\n");
 }
 
+static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
+{
+	uint32_t val;
+
+	assert_can_disable_dc5(dev_priv);
+	assert_can_disable_dc6(dev_priv);
+
+	DRM_DEBUG_KMS("Disabling DC5 and DC6\n");
+
+	val = I915_READ(DC_STATE_EN);
+	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
+	I915_WRITE(DC_STATE_EN, val);
+	POSTING_READ(DC_STATE_EN);
+}
+
 void skl_enable_dc6(struct drm_i915_private *dev_priv)
 {
 	uint32_t val;
@@ -632,17 +635,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 				"Invalid for power well status to be enabled, unless done by the BIOS, \
 				when request is to disable!\n");
 			if (power_well->data == SKL_DISP_PW_2) {
-				if (GEN9_ENABLE_DC5(dev))
-					gen9_disable_dc5(dev_priv);
-				if (SKL_ENABLE_DC6(dev)) {
-					/*
-					 * DDI buffer programming unnecessary during driver-load/resume
-					 * as it's already done during modeset initialization then.
-					 * It's also invalid here as encoder list is still uninitialized.
-					 */
-					if (!dev_priv->power_domains.initializing)
-						intel_prepare_ddi(dev);
-				}
+				/*
+				 * DDI buffer programming unnecessary during driver-load/resume
+				 * as it's already done during modeset initialization then.
+				 * It's also invalid here as encoder list is still uninitialized.
+				 */
+				if (!dev_priv->power_domains.initializing)
+					intel_prepare_ddi(dev);
 			}
 			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
 		}
@@ -658,17 +657,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
 	} else {
 		if (enable_requested) {
 			if (IS_SKYLAKE(dev) &&
-				(power_well->data == SKL_DISP_PW_1))
+				(power_well->data == SKL_DISP_PW_1)) {
 				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
-			else {
+			} else {
 				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
 				POSTING_READ(HSW_PWR_WELL_DRIVER);
 				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
 			}
-
-			if (GEN9_ENABLE_DC5(dev) &&
-				power_well->data == SKL_DISP_PW_2)
-					gen9_enable_dc5(dev_priv);
 		}
 	}
 
@@ -743,6 +738,36 @@ static void skl_power_well_disable(struct drm_i915_private *dev_priv,
 	skl_set_power_well(dev_priv, power_well, false);
 }
 
+static bool skl_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
+					  struct i915_power_well *power_well)
+{
+	return !(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6);
+}
+
+static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
+					 struct i915_power_well *power_well)
+{
+	gen9_disable_dc5_dc6(dev_priv);
+}
+
+static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
+					  struct i915_power_well *power_well)
+{
+	if (IS_SKYLAKE(dev_priv))
+		skl_enable_dc6(dev_priv);
+	else
+		gen9_enable_dc5(dev_priv);
+}
+
+static void skl_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
+					  struct i915_power_well *power_well)
+{
+	if (power_well->count > 0)
+		skl_dc_off_power_well_enable(dev_priv, power_well);
+	else
+		skl_dc_off_power_well_disable(dev_priv, power_well);
+}
+
 static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
 					   struct i915_power_well *power_well)
 {
@@ -1574,6 +1599,13 @@ static const struct i915_power_well_ops skl_power_well_ops = {
 	.is_enabled = skl_power_well_enabled,
 };
 
+static const struct i915_power_well_ops skl_dc_off_power_well_ops = {
+	.sync_hw = skl_dc_off_power_well_sync_hw,
+	.enable = skl_dc_off_power_well_enable,
+	.disable = skl_dc_off_power_well_disable,
+	.is_enabled = skl_dc_off_power_well_enabled,
+};
+
 static struct i915_power_well hsw_power_wells[] = {
 	{
 		.name = "always-on",
@@ -1738,6 +1770,11 @@ static struct i915_power_well skl_power_wells[] = {
 		.data = SKL_DISP_PW_1,
 	},
 	{
+		.name = "DC off",
+		.domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
+		.ops = &skl_dc_off_power_well_ops,
+	},
+	{
 		.name = "MISC IO power well",
 		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
 		.ops = &skl_power_well_ops,
-- 
2.1.4

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

Re: [PATCH 6/8] drm/i915/skl: Turn DC handling into a power well

[Ville Syrjälä]

On Tue, Nov 03, 2015 at 01:31:12PM +0100, Patrik Jakobsson wrote:
> Handle DC off as a power well where enabling the power well will prevent
> the DMC to enter selected DC states (required around modesets and Aux
> A). Disabling the power well will allow DC states again. For now the
> highest DC state is DC6 but will be configurable in a later patch.
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |   6 --
>  drivers/gpu/drm/i915/intel_display.c    |   6 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 107 +++++++++++++++++++++-----------
>  3 files changed, 78 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 37319b0..e190237 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1069,9 +1069,6 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  {
>  	skl_uninit_cdclk(dev_priv);
>  
> -	if (dev_priv->csr.dmc_payload)
> -		skl_enable_dc6(dev_priv);
> -
>  	return 0;
>  }
>  
> @@ -1116,9 +1113,6 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
>  
>  static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
> -	if (dev_priv->csr.dmc_payload)
> -		skl_disable_dc6(dev_priv);
> -
>  	skl_init_cdclk(dev_priv);
>  	intel_csr_load_program(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c6d60b8..e401871 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13296,6 +13296,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			to_intel_crtc_state(crtc->state)->update_pipe;
>  		unsigned long put_domains = 0;
>  
> +		if (modeset)
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> +
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
>  			dev_priv->display.crtc_enable(crtc);
> @@ -13319,6 +13322,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			modeset_put_power_domains(dev_priv, put_domains);
>  
>  		intel_post_plane_update(intel_crtc);
> +
> +		if (modeset)
> +			intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>  	}
>  
>  	/* FIXME: add subpixel order */
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index c901b19..042d92f 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -49,9 +49,6 @@
>   * present for a given platform.
>   */
>  
> -#define GEN9_ENABLE_DC5(dev) 0
> -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev)
> -
>  #define for_each_power_well(i, power_well, domain_mask, power_domains)	\
>  	for (i = 0;							\
>  	     i < (power_domains)->power_well_count &&			\
> @@ -336,10 +333,15 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
>  	BIT(POWER_DOMAIN_PLLS) |			\
>  	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_MODESET) |			\
> +	BIT(POWER_DOMAIN_AUX_A) |			\
> +	BIT(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
>  	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
>  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> -	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
> +	SKL_DISPLAY_MISC_IO_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) |		\
>  	BIT(POWER_DOMAIN_INIT))
>  
>  #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
> @@ -511,20 +513,6 @@ static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
>  	POSTING_READ(DC_STATE_EN);
>  }
>  
> -static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
> -{
> -	uint32_t val;
> -
> -	assert_can_disable_dc5(dev_priv);
> -
> -	DRM_DEBUG_KMS("Disabling DC5\n");
> -
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_UPTO_DC5;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> -}
> -
>  static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -553,6 +541,21 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
>  		  "DC6 already programmed to be disabled.\n");
>  }
>  
> +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
> +{
> +	uint32_t val;
> +
> +	assert_can_disable_dc5(dev_priv);
> +	assert_can_disable_dc6(dev_priv);
> +
> +	DRM_DEBUG_KMS("Disabling DC5 and DC6\n");
> +
> +	val = I915_READ(DC_STATE_EN);
> +	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
> +	I915_WRITE(DC_STATE_EN, val);
> +	POSTING_READ(DC_STATE_EN);
> +}
> +
>  void skl_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>  	uint32_t val;
> @@ -632,17 +635,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  				"Invalid for power well status to be enabled, unless done by the BIOS, \
>  				when request is to disable!\n");
>  			if (power_well->data == SKL_DISP_PW_2) {
> -				if (GEN9_ENABLE_DC5(dev))
> -					gen9_disable_dc5(dev_priv);
> -				if (SKL_ENABLE_DC6(dev)) {
> -					/*
> -					 * DDI buffer programming unnecessary during driver-load/resume
> -					 * as it's already done during modeset initialization then.
> -					 * It's also invalid here as encoder list is still uninitialized.
> -					 */
> -					if (!dev_priv->power_domains.initializing)
> -						intel_prepare_ddi(dev);
> -				}
> +				/*
> +				 * DDI buffer programming unnecessary during driver-load/resume
> +				 * as it's already done during modeset initialization then.
> +				 * It's also invalid here as encoder list is still uninitialized.
> +				 */
> +				if (!dev_priv->power_domains.initializing)
> +					intel_prepare_ddi(dev);
>  			}
>  			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
>  		}
> @@ -658,17 +657,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  	} else {
>  		if (enable_requested) {
>  			if (IS_SKYLAKE(dev) &&
> -				(power_well->data == SKL_DISP_PW_1))
> +				(power_well->data == SKL_DISP_PW_1)) {
>  				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
> -			else {
> +			} else {
>  				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
>  				POSTING_READ(HSW_PWR_WELL_DRIVER);
>  				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>  			}
> -
> -			if (GEN9_ENABLE_DC5(dev) &&
> -				power_well->data == SKL_DISP_PW_2)
> -					gen9_enable_dc5(dev_priv);
>  		}
>  	}
>  
> @@ -743,6 +738,36 @@ static void skl_power_well_disable(struct drm_i915_private *dev_priv,
>  	skl_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static bool skl_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	return !(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6);

This doesn't handle DC5...

> +}
> +
> +static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> +					 struct i915_power_well *power_well)
> +{
> +	gen9_disable_dc5_dc6(dev_priv);
> +}
> +
> +static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	if (IS_SKYLAKE(dev_priv))
> +		skl_enable_dc6(dev_priv);
> +	else
> +		gen9_enable_dc5(dev_priv);

...whereas this one tries to handle DC5.

Assuming we want to handle DC5 and DC6 with a single power well (to make
it easier to change between DC5 and DC6 using a module parameter on SKL,
skl_dc_off_power_well_enabled() should probably just be:

{
	return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
}

Otherwise the DC off power well bits look all right to me. The rest
presumably needs some coordination with Imre's work since the PW1 and
MISC IO stuff should be going away from here.

> +}
> +
> +static void skl_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	if (power_well->count > 0)
> +		skl_dc_off_power_well_enable(dev_priv, power_well);
> +	else
> +		skl_dc_off_power_well_disable(dev_priv, power_well);
> +}
> +
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> @@ -1574,6 +1599,13 @@ static const struct i915_power_well_ops skl_power_well_ops = {
>  	.is_enabled = skl_power_well_enabled,
>  };
>  
> +static const struct i915_power_well_ops skl_dc_off_power_well_ops = {
> +	.sync_hw = skl_dc_off_power_well_sync_hw,
> +	.enable = skl_dc_off_power_well_enable,
> +	.disable = skl_dc_off_power_well_disable,
> +	.is_enabled = skl_dc_off_power_well_enabled,
> +};
> +
>  static struct i915_power_well hsw_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -1738,6 +1770,11 @@ static struct i915_power_well skl_power_wells[] = {
>  		.data = SKL_DISP_PW_1,
>  	},
>  	{
> +		.name = "DC off",
> +		.domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
> +		.ops = &skl_dc_off_power_well_ops,
> +	},
> +	{
>  		.name = "MISC IO power well",
>  		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,
> -- 
> 2.1.4

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

Re: [PATCH 6/8] drm/i915/skl: Turn DC handling into a power well

[Patrik Jakobsson]

On Wed, Nov 4, 2015 at 6:53 PM, Ville Syrjälä
<ville.syrjala@linux.intel.com> wrote:
> On Tue, Nov 03, 2015 at 01:31:12PM +0100, Patrik Jakobsson wrote:
>> Handle DC off as a power well where enabling the power well will prevent
>> the DMC to enter selected DC states (required around modesets and Aux
>> A). Disabling the power well will allow DC states again. For now the
>> highest DC state is DC6 but will be configurable in a later patch.
>>
>> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_drv.c         |   6 --
>>  drivers/gpu/drm/i915/intel_display.c    |   6 ++
>>  drivers/gpu/drm/i915/intel_runtime_pm.c | 107 +++++++++++++++++++++-----------
>>  3 files changed, 78 insertions(+), 41 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
>> index 37319b0..e190237 100644
>> --- a/drivers/gpu/drm/i915/i915_drv.c
>> +++ b/drivers/gpu/drm/i915/i915_drv.c
>> @@ -1069,9 +1069,6 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>>  {
>>       skl_uninit_cdclk(dev_priv);
>>
>> -     if (dev_priv->csr.dmc_payload)
>> -             skl_enable_dc6(dev_priv);
>> -
>>       return 0;
>>  }
>>
>> @@ -1116,9 +1113,6 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
>>
>>  static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>>  {
>> -     if (dev_priv->csr.dmc_payload)
>> -             skl_disable_dc6(dev_priv);
>> -
>>       skl_init_cdclk(dev_priv);
>>       intel_csr_load_program(dev_priv);
>>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index c6d60b8..e401871 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -13296,6 +13296,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>>                       to_intel_crtc_state(crtc->state)->update_pipe;
>>               unsigned long put_domains = 0;
>>
>> +             if (modeset)
>> +                     intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
>> +
>>               if (modeset && crtc->state->active) {
>>                       update_scanline_offset(to_intel_crtc(crtc));
>>                       dev_priv->display.crtc_enable(crtc);
>> @@ -13319,6 +13322,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>>                       modeset_put_power_domains(dev_priv, put_domains);
>>
>>               intel_post_plane_update(intel_crtc);
>> +
>> +             if (modeset)
>> +                     intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>>       }
>>
>>       /* FIXME: add subpixel order */
>> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> index c901b19..042d92f 100644
>> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> @@ -49,9 +49,6 @@
>>   * present for a given platform.
>>   */
>>
>> -#define GEN9_ENABLE_DC5(dev) 0
>> -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev)
>> -
>>  #define for_each_power_well(i, power_well, domain_mask, power_domains)       \
>>       for (i = 0;                                                     \
>>            i < (power_domains)->power_well_count &&                   \
>> @@ -336,10 +333,15 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>>       SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |         \
>>       BIT(POWER_DOMAIN_PLLS) |                        \
>>       BIT(POWER_DOMAIN_INIT))
>> +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (           \
>> +     BIT(POWER_DOMAIN_MODESET) |                     \
>> +     BIT(POWER_DOMAIN_AUX_A) |                       \
>> +     BIT(POWER_DOMAIN_INIT))
>>  #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (                \
>>       (POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |  \
>>       SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |         \
>> -     SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |           \
>> +     SKL_DISPLAY_MISC_IO_POWER_DOMAINS |             \
>> +     SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) |            \
>>       BIT(POWER_DOMAIN_INIT))
>>
>>  #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (              \
>> @@ -511,20 +513,6 @@ static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
>>       POSTING_READ(DC_STATE_EN);
>>  }
>>
>> -static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
>> -{
>> -     uint32_t val;
>> -
>> -     assert_can_disable_dc5(dev_priv);
>> -
>> -     DRM_DEBUG_KMS("Disabling DC5\n");
>> -
>> -     val = I915_READ(DC_STATE_EN);
>> -     val &= ~DC_STATE_EN_UPTO_DC5;
>> -     I915_WRITE(DC_STATE_EN, val);
>> -     POSTING_READ(DC_STATE_EN);
>> -}
>> -
>>  static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
>>  {
>>       struct drm_device *dev = dev_priv->dev;
>> @@ -553,6 +541,21 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
>>                 "DC6 already programmed to be disabled.\n");
>>  }
>>
>> +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
>> +{
>> +     uint32_t val;
>> +
>> +     assert_can_disable_dc5(dev_priv);
>> +     assert_can_disable_dc6(dev_priv);
>> +
>> +     DRM_DEBUG_KMS("Disabling DC5 and DC6\n");
>> +
>> +     val = I915_READ(DC_STATE_EN);
>> +     val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
>> +     I915_WRITE(DC_STATE_EN, val);
>> +     POSTING_READ(DC_STATE_EN);
>> +}
>> +
>>  void skl_enable_dc6(struct drm_i915_private *dev_priv)
>>  {
>>       uint32_t val;
>> @@ -632,17 +635,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>>                               "Invalid for power well status to be enabled, unless done by the BIOS, \
>>                               when request is to disable!\n");
>>                       if (power_well->data == SKL_DISP_PW_2) {
>> -                             if (GEN9_ENABLE_DC5(dev))
>> -                                     gen9_disable_dc5(dev_priv);
>> -                             if (SKL_ENABLE_DC6(dev)) {
>> -                                     /*
>> -                                      * DDI buffer programming unnecessary during driver-load/resume
>> -                                      * as it's already done during modeset initialization then.
>> -                                      * It's also invalid here as encoder list is still uninitialized.
>> -                                      */
>> -                                     if (!dev_priv->power_domains.initializing)
>> -                                             intel_prepare_ddi(dev);
>> -                             }
>> +                             /*
>> +                              * DDI buffer programming unnecessary during driver-load/resume
>> +                              * as it's already done during modeset initialization then.
>> +                              * It's also invalid here as encoder list is still uninitialized.
>> +                              */
>> +                             if (!dev_priv->power_domains.initializing)
>> +                                     intel_prepare_ddi(dev);
>>                       }
>>                       I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
>>               }
>> @@ -658,17 +657,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>>       } else {
>>               if (enable_requested) {
>>                       if (IS_SKYLAKE(dev) &&
>> -                             (power_well->data == SKL_DISP_PW_1))
>> +                             (power_well->data == SKL_DISP_PW_1)) {
>>                               DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
>> -                     else {
>> +                     } else {
>>                               I915_WRITE(HSW_PWR_WELL_DRIVER, tmp & ~req_mask);
>>                               POSTING_READ(HSW_PWR_WELL_DRIVER);
>>                               DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>>                       }
>> -
>> -                     if (GEN9_ENABLE_DC5(dev) &&
>> -                             power_well->data == SKL_DISP_PW_2)
>> -                                     gen9_enable_dc5(dev_priv);
>>               }
>>       }
>>
>> @@ -743,6 +738,36 @@ static void skl_power_well_disable(struct drm_i915_private *dev_priv,
>>       skl_set_power_well(dev_priv, power_well, false);
>>  }
>>
>> +static bool skl_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
>> +                                       struct i915_power_well *power_well)
>> +{
>> +     return !(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6);
>
> This doesn't handle DC5...
>
>> +}
>> +
>> +static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>> +                                      struct i915_power_well *power_well)
>> +{
>> +     gen9_disable_dc5_dc6(dev_priv);
>> +}
>> +
>> +static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>> +                                       struct i915_power_well *power_well)
>> +{
>> +     if (IS_SKYLAKE(dev_priv))
>> +             skl_enable_dc6(dev_priv);
>> +     else
>> +             gen9_enable_dc5(dev_priv);
>
> ...whereas this one tries to handle DC5.
>
> Assuming we want to handle DC5 and DC6 with a single power well (to make
> it easier to change between DC5 and DC6 using a module parameter on SKL,
> skl_dc_off_power_well_enabled() should probably just be:
>
> {
>         return (I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC5_DC6_MASK) == 0;
> }

Thanks, that slipped me by. Checking DC5 and DC6 like you do above is
what we want.

>
> Otherwise the DC off power well bits look all right to me. The rest
> presumably needs some coordination with Imre's work since the PW1 and
> MISC IO stuff should be going away from here.

Yes, I'm syncing this with Imre.

>
>> +}
>> +
>> +static void skl_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
>> +                                       struct i915_power_well *power_well)
>> +{
>> +     if (power_well->count > 0)
>> +             skl_dc_off_power_well_enable(dev_priv, power_well);
>> +     else
>> +             skl_dc_off_power_well_disable(dev_priv, power_well);
>> +}
>> +
>>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
>>                                          struct i915_power_well *power_well)
>>  {
>> @@ -1574,6 +1599,13 @@ static const struct i915_power_well_ops skl_power_well_ops = {
>>       .is_enabled = skl_power_well_enabled,
>>  };
>>
>> +static const struct i915_power_well_ops skl_dc_off_power_well_ops = {
>> +     .sync_hw = skl_dc_off_power_well_sync_hw,
>> +     .enable = skl_dc_off_power_well_enable,
>> +     .disable = skl_dc_off_power_well_disable,
>> +     .is_enabled = skl_dc_off_power_well_enabled,
>> +};
>> +
>>  static struct i915_power_well hsw_power_wells[] = {
>>       {
>>               .name = "always-on",
>> @@ -1738,6 +1770,11 @@ static struct i915_power_well skl_power_wells[] = {
>>               .data = SKL_DISP_PW_1,
>>       },
>>       {
>> +             .name = "DC off",
>> +             .domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
>> +             .ops = &skl_dc_off_power_well_ops,
>> +     },
>> +     {
>>               .name = "MISC IO power well",
>>               .domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
>>               .ops = &skl_power_well_ops,
>> --
>> 2.1.4
>
> --
> Ville Syrjälä
> Intel OTC
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 6/8] drm/i915/skl: Turn DC handling into a power well

[Imre Deak]

On ti, 2015-11-03 at 13:31 +0100, Patrik Jakobsson wrote:
> Handle DC off as a power well where enabling the power well will prevent
> the DMC to enter selected DC states (required around modesets and Aux
> A). Disabling the power well will allow DC states again. For now the
> highest DC state is DC6 but will be configurable in a later patch.
> 
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.c         |   6 --
>  drivers/gpu/drm/i915/intel_display.c    |   6 ++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 107 +++++++++++++++++++++-----------
>  3 files changed, 78 insertions(+), 41 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
> index 37319b0..e190237 100644
> --- a/drivers/gpu/drm/i915/i915_drv.c
> +++ b/drivers/gpu/drm/i915/i915_drv.c
> @@ -1069,9 +1069,6 @@ static int skl_suspend_complete(struct drm_i915_private *dev_priv)
>  {
>  	skl_uninit_cdclk(dev_priv);
>  
> -	if (dev_priv->csr.dmc_payload)
> -		skl_enable_dc6(dev_priv);
> -
>  	return 0;
>  }
>  
> @@ -1116,9 +1113,6 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
>  
>  static int skl_resume_prepare(struct drm_i915_private *dev_priv)
>  {
> -	if (dev_priv->csr.dmc_payload)
> -		skl_disable_dc6(dev_priv);
> -
>  	skl_init_cdclk(dev_priv);
>  	intel_csr_load_program(dev_priv);
>  
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c6d60b8..e401871 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13296,6 +13296,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			to_intel_crtc_state(crtc->state)->update_pipe;
>  		unsigned long put_domains = 0;
>  
> +		if (modeset)
> +			intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> +
>  		if (modeset && crtc->state->active) {
>  			update_scanline_offset(to_intel_crtc(crtc));
>  			dev_priv->display.crtc_enable(crtc);
> @@ -13319,6 +13322,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>  			modeset_put_power_domains(dev_priv, put_domains);
>  
>  		intel_post_plane_update(intel_crtc);
> +
> +		if (modeset)
> +			intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>  	}
>  
>  	/* FIXME: add subpixel order */
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index c901b19..042d92f 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -49,9 +49,6 @@
>   * present for a given platform.
>   */
>  
> -#define GEN9_ENABLE_DC5(dev) 0
> -#define SKL_ENABLE_DC6(dev) IS_SKYLAKE(dev)
> -
>  #define for_each_power_well(i, power_well, domain_mask, power_domains)	\
>  	for (i = 0;							\
>  	     i < (power_domains)->power_well_count &&			\
> @@ -336,10 +333,15 @@ static void hsw_set_power_well(struct drm_i915_private *dev_priv,
>  	SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |		\
>  	BIT(POWER_DOMAIN_PLLS) |			\
>  	BIT(POWER_DOMAIN_INIT))
> +#define SKL_DISPLAY_DC_OFF_POWER_DOMAINS (		\
> +	BIT(POWER_DOMAIN_MODESET) |			\
> +	BIT(POWER_DOMAIN_AUX_A) |			\
> +	BIT(POWER_DOMAIN_INIT))
>  #define SKL_DISPLAY_ALWAYS_ON_POWER_DOMAINS (		\
>  	(POWER_DOMAIN_MASK & ~(SKL_DISPLAY_POWERWELL_1_POWER_DOMAINS |	\
>  	SKL_DISPLAY_POWERWELL_2_POWER_DOMAINS |		\
> -	SKL_DISPLAY_MISC_IO_POWER_DOMAINS)) |		\
> +	SKL_DISPLAY_MISC_IO_POWER_DOMAINS |		\
> +	SKL_DISPLAY_DC_OFF_POWER_DOMAINS)) |		\
>  	BIT(POWER_DOMAIN_INIT))
>  
>  #define BXT_DISPLAY_POWERWELL_2_POWER_DOMAINS (		\
> @@ -511,20 +513,6 @@ static void gen9_enable_dc5(struct drm_i915_private *dev_priv)
>  	POSTING_READ(DC_STATE_EN);
>  }
>  
> -static void gen9_disable_dc5(struct drm_i915_private *dev_priv)
> -{
> -	uint32_t val;
> -
> -	assert_can_disable_dc5(dev_priv);
> -
> -	DRM_DEBUG_KMS("Disabling DC5\n");
> -
> -	val = I915_READ(DC_STATE_EN);
> -	val &= ~DC_STATE_EN_UPTO_DC5;
> -	I915_WRITE(DC_STATE_EN, val);
> -	POSTING_READ(DC_STATE_EN);
> -}
> -
>  static void assert_can_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>  	struct drm_device *dev = dev_priv->dev;
> @@ -553,6 +541,21 @@ static void assert_can_disable_dc6(struct drm_i915_private *dev_priv)
>  		  "DC6 already programmed to be disabled.\n");
>  }
>  
> +static void gen9_disable_dc5_dc6(struct drm_i915_private *dev_priv)
> +{
> +	uint32_t val;
> +
> +	assert_can_disable_dc5(dev_priv);
> +	assert_can_disable_dc6(dev_priv);
> +
> +	DRM_DEBUG_KMS("Disabling DC5 and DC6\n");
> +
> +	val = I915_READ(DC_STATE_EN);
> +	val &= ~DC_STATE_EN_UPTO_DC5_DC6_MASK;
> +	I915_WRITE(DC_STATE_EN, val);
> +	POSTING_READ(DC_STATE_EN);
> +}
> +
>  void skl_enable_dc6(struct drm_i915_private *dev_priv)
>  {
>  	uint32_t val;
> @@ -632,17 +635,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  				"Invalid for power well status to be enabled, unless done by the BIOS, \
>  				when request is to disable!\n");
>  			if (power_well->data == SKL_DISP_PW_2) {
> -				if (GEN9_ENABLE_DC5(dev))
> -					gen9_disable_dc5(dev_priv);
> -				if (SKL_ENABLE_DC6(dev)) {
> -					/*
> -					 * DDI buffer programming unnecessary during driver-load/resume
> -					 * as it's already done during modeset initialization then.
> -					 * It's also invalid here as encoder list is still uninitialized.
> -					 */
> -					if (!dev_priv->power_domains.initializing)
> -						intel_prepare_ddi(dev);
> -				}
> +				/*
> +				 * DDI buffer programming unnecessary during driver-load/resume
> +				 * as it's already done during modeset initialization then.
> +				 * It's also invalid here as encoder list is still uninitialized.
> +				 */
> +				if (!dev_priv->power_domains.initializing)
> +					intel_prepare_ddi(dev);
>  			}
>  			I915_WRITE(HSW_PWR_WELL_DRIVER, tmp | req_mask);
>  		}
> @@ -658,17 +657,13 @@ static void skl_set_power_well(struct drm_i915_private *dev_priv,
>  	} else {
>  		if (enable_requested) {
>  			if (IS_SKYLAKE(dev) &&
> -				(power_well->data == SKL_DISP_PW_1))
> +				(power_well->data == SKL_DISP_PW_1)) {
>  				DRM_DEBUG_KMS("Not Disabling PW1, dmc will handle\n");
> -			else {
> +			} else {
>  				I915_WRITE(HSW_PWR_WELL_DRIVER,	tmp & ~req_mask);
>  				POSTING_READ(HSW_PWR_WELL_DRIVER);
>  				DRM_DEBUG_KMS("Disabling %s\n", power_well->name);
>  			}
> -
> -			if (GEN9_ENABLE_DC5(dev) &&
> -				power_well->data == SKL_DISP_PW_2)
> -					gen9_enable_dc5(dev_priv);
>  		}
>  	}
>  
> @@ -743,6 +738,36 @@ static void skl_power_well_disable(struct drm_i915_private *dev_priv,
>  	skl_set_power_well(dev_priv, power_well, false);
>  }
>  
> +static bool skl_dc_off_power_well_enabled(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	return !(I915_READ(DC_STATE_EN) & DC_STATE_EN_UPTO_DC6);
> +}
> +
> +static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> +					 struct i915_power_well *power_well)
> +{
> +	gen9_disable_dc5_dc6(dev_priv);
> +}
> +
> +static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	if (IS_SKYLAKE(dev_priv))
> +		skl_enable_dc6(dev_priv);
> +	else
> +		gen9_enable_dc5(dev_priv);
> +}
> +
> +static void skl_dc_off_power_well_sync_hw(struct drm_i915_private *dev_priv,
> +					  struct i915_power_well *power_well)
> +{
> +	if (power_well->count > 0)
> +		skl_dc_off_power_well_enable(dev_priv, power_well);
> +	else
> +		skl_dc_off_power_well_disable(dev_priv, power_well);
> +}
> +
>  static void i9xx_always_on_power_well_noop(struct drm_i915_private *dev_priv,
>  					   struct i915_power_well *power_well)
>  {
> @@ -1574,6 +1599,13 @@ static const struct i915_power_well_ops skl_power_well_ops = {
>  	.is_enabled = skl_power_well_enabled,
>  };
>  
> +static const struct i915_power_well_ops skl_dc_off_power_well_ops = {
> +	.sync_hw = skl_dc_off_power_well_sync_hw,
> +	.enable = skl_dc_off_power_well_enable,
> +	.disable = skl_dc_off_power_well_disable,
> +	.is_enabled = skl_dc_off_power_well_enabled,
> +};
> +
>  static struct i915_power_well hsw_power_wells[] = {
>  	{
>  		.name = "always-on",
> @@ -1738,6 +1770,11 @@ static struct i915_power_well skl_power_wells[] = {
>  		.data = SKL_DISP_PW_1,
>  	},
>  	{
> +		.name = "DC off",
> +		.domains = SKL_DISPLAY_DC_OFF_POWER_DOMAINS,
> +		.ops = &skl_dc_off_power_well_ops,

We need to set a unique ID in .data, since we use it to look up power
wells. I have a patch to fix up the other places missing an ID, but you
could fix this one up already now.

> +	},
> +	{
>  		.name = "MISC IO power well",
>  		.domains = SKL_DISPLAY_MISC_IO_POWER_DOMAINS,
>  		.ops = &skl_power_well_ops,


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

Re: [PATCH 6/8] drm/i915/skl: Turn DC handling into a power well

[Daniel Stone]

Hi,

On 3 November 2015 at 12:31, Patrik Jakobsson
<patrik.jakobsson@linux.intel.com> wrote:
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c6d60b8..e401871 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -13296,6 +13296,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>                         to_intel_crtc_state(crtc->state)->update_pipe;
>                 unsigned long put_domains = 0;
>
> +               if (modeset)
> +                       intel_display_power_get(dev_priv, POWER_DOMAIN_MODESET);
> +
>                 if (modeset && crtc->state->active) {
>                         update_scanline_offset(to_intel_crtc(crtc));
>                         dev_priv->display.crtc_enable(crtc);
> @@ -13319,6 +13322,9 @@ static int intel_atomic_commit(struct drm_device *dev,
>                         modeset_put_power_domains(dev_priv, put_domains);
>
>                 intel_post_plane_update(intel_crtc);
> +
> +               if (modeset)
> +                       intel_display_power_put(dev_priv, POWER_DOMAIN_MODESET);
>         }

If it's safe to shift the modeset_put_power_domains call to after
post_plane_update, you might as well just put POWER_DOMAIN_MODESET in
there, saving a call. (But see the comment on the other patch ...)

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 7/8] drm/i915/skl: Add boot parameter for disabling DC6

[Patrik Jakobsson]

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h         | 1 +
 drivers/gpu/drm/i915/i915_params.c      | 6 ++++++
 drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index efb6a00..9c8cb43 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -2660,6 +2660,7 @@ struct i915_params {
 	int panel_use_ssc;
 	int vbt_sdvo_panel_type;
 	int enable_rc6;
+	int enable_dc6;
 	int enable_fbc;
 	int enable_ppgtt;
 	int enable_execlists;
diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
index 368df67..1a7dedf 100644
--- a/drivers/gpu/drm/i915/i915_params.c
+++ b/drivers/gpu/drm/i915/i915_params.c
@@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
 	.panel_use_ssc = -1,
 	.vbt_sdvo_panel_type = -1,
 	.enable_rc6 = -1,
+	.enable_dc6 = 1,
 	.enable_fbc = -1,
 	.enable_execlists = -1,
 	.enable_hangcheck = true,
@@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
 	"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
 	"default: -1 (use per-chip default)");
 
+module_param_named(enable_dc6, i915.enable_dc6, int, 0400);
+MODULE_PARM_DESC(enable_dc6,
+	"Enable power-saving display C-state 6. "
+	"(0 = disable; 1 = enable [default])");
+
 module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
 MODULE_PARM_DESC(enable_fbc,
 	"Enable frame buffer compression for power savings "
diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
index 042d92f..4843c5c 100644
--- a/drivers/gpu/drm/i915/intel_runtime_pm.c
+++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
@@ -753,7 +753,7 @@ static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
 static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
 					  struct i915_power_well *power_well)
 {
-	if (IS_SKYLAKE(dev_priv))
+	if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
 		skl_enable_dc6(dev_priv);
 	else
 		gen9_enable_dc5(dev_priv);
-- 
2.1.4

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

Re: [PATCH 7/8] drm/i915/skl: Add boot parameter for disabling DC6

[Jani Nikula]

On Tue, 03 Nov 2015, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_drv.h         | 1 +
>  drivers/gpu/drm/i915/i915_params.c      | 6 ++++++
>  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>  3 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index efb6a00..9c8cb43 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -2660,6 +2660,7 @@ struct i915_params {
>  	int panel_use_ssc;
>  	int vbt_sdvo_panel_type;
>  	int enable_rc6;
> +	int enable_dc6;
>  	int enable_fbc;
>  	int enable_ppgtt;
>  	int enable_execlists;
> diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> index 368df67..1a7dedf 100644
> --- a/drivers/gpu/drm/i915/i915_params.c
> +++ b/drivers/gpu/drm/i915/i915_params.c
> @@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
>  	.panel_use_ssc = -1,
>  	.vbt_sdvo_panel_type = -1,
>  	.enable_rc6 = -1,
> +	.enable_dc6 = 1,
>  	.enable_fbc = -1,
>  	.enable_execlists = -1,
>  	.enable_hangcheck = true,
> @@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
>  	"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
>  	"default: -1 (use per-chip default)");
>  
> +module_param_named(enable_dc6, i915.enable_dc6, int, 0400);

_unsafe please.

BR,
Jani.

> +MODULE_PARM_DESC(enable_dc6,
> +	"Enable power-saving display C-state 6. "
> +	"(0 = disable; 1 = enable [default])");
> +
>  module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
>  MODULE_PARM_DESC(enable_fbc,
>  	"Enable frame buffer compression for power savings "
> diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> index 042d92f..4843c5c 100644
> --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> @@ -753,7 +753,7 @@ static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>  static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>  					  struct i915_power_well *power_well)
>  {
> -	if (IS_SKYLAKE(dev_priv))
> +	if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
>  		skl_enable_dc6(dev_priv);
>  	else
>  		gen9_enable_dc5(dev_priv);

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

Re: [PATCH 7/8] drm/i915/skl: Add boot parameter for disabling DC6

[Patrik Jakobsson]

On Tue, Nov 03, 2015 at 03:08:41PM +0200, Jani Nikula wrote:
> On Tue, 03 Nov 2015, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
> > ---
> >  drivers/gpu/drm/i915/i915_drv.h         | 1 +
> >  drivers/gpu/drm/i915/i915_params.c      | 6 ++++++
> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
> >  3 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> > index efb6a00..9c8cb43 100644
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -2660,6 +2660,7 @@ struct i915_params {
> >  	int panel_use_ssc;
> >  	int vbt_sdvo_panel_type;
> >  	int enable_rc6;
> > +	int enable_dc6;
> >  	int enable_fbc;
> >  	int enable_ppgtt;
> >  	int enable_execlists;
> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
> > index 368df67..1a7dedf 100644
> > --- a/drivers/gpu/drm/i915/i915_params.c
> > +++ b/drivers/gpu/drm/i915/i915_params.c
> > @@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
> >  	.panel_use_ssc = -1,
> >  	.vbt_sdvo_panel_type = -1,
> >  	.enable_rc6 = -1,
> > +	.enable_dc6 = 1,
> >  	.enable_fbc = -1,
> >  	.enable_execlists = -1,
> >  	.enable_hangcheck = true,
> > @@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
> >  	"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
> >  	"default: -1 (use per-chip default)");
> >  
> > +module_param_named(enable_dc6, i915.enable_dc6, int, 0400);
> 
> _unsafe please.

Is the convention to taint for all non-default options in i915?

-Patrik

> 
> BR,
> Jani.
> 
> > +MODULE_PARM_DESC(enable_dc6,
> > +	"Enable power-saving display C-state 6. "
> > +	"(0 = disable; 1 = enable [default])");
> > +
> >  module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
> >  MODULE_PARM_DESC(enable_fbc,
> >  	"Enable frame buffer compression for power savings "
> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > index 042d92f..4843c5c 100644
> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
> > @@ -753,7 +753,7 @@ static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
> >  static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
> >  					  struct i915_power_well *power_well)
> >  {
> > -	if (IS_SKYLAKE(dev_priv))
> > +	if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
> >  		skl_enable_dc6(dev_priv);
> >  	else
> >  		gen9_enable_dc5(dev_priv);
> 
> -- 
> Jani Nikula, Intel Open Source Technology Center
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 7/8] drm/i915/skl: Add boot parameter for disabling DC6

[Jani Nikula]

On Tue, 03 Nov 2015, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
> On Tue, Nov 03, 2015 at 03:08:41PM +0200, Jani Nikula wrote:
>> On Tue, 03 Nov 2015, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
>> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>> > ---
>> >  drivers/gpu/drm/i915/i915_drv.h         | 1 +
>> >  drivers/gpu/drm/i915/i915_params.c      | 6 ++++++
>> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>> >  3 files changed, 8 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>> > index efb6a00..9c8cb43 100644
>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>> > @@ -2660,6 +2660,7 @@ struct i915_params {
>> >  	int panel_use_ssc;
>> >  	int vbt_sdvo_panel_type;
>> >  	int enable_rc6;
>> > +	int enable_dc6;
>> >  	int enable_fbc;
>> >  	int enable_ppgtt;
>> >  	int enable_execlists;
>> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>> > index 368df67..1a7dedf 100644
>> > --- a/drivers/gpu/drm/i915/i915_params.c
>> > +++ b/drivers/gpu/drm/i915/i915_params.c
>> > @@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
>> >  	.panel_use_ssc = -1,
>> >  	.vbt_sdvo_panel_type = -1,
>> >  	.enable_rc6 = -1,
>> > +	.enable_dc6 = 1,
>> >  	.enable_fbc = -1,
>> >  	.enable_execlists = -1,
>> >  	.enable_hangcheck = true,
>> > @@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
>> >  	"For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
>> >  	"default: -1 (use per-chip default)");
>> >  
>> > +module_param_named(enable_dc6, i915.enable_dc6, int, 0400);
>> 
>> _unsafe please.
>
> Is the convention to taint for all non-default options in i915?

The intention is to taint all i915 options that people have no clue
about and blindly set just because they read on a forum they should do
it, and then report bugs because it fails.

BR,
Jani.



>
> -Patrik
>
>> 
>> BR,
>> Jani.
>> 
>> > +MODULE_PARM_DESC(enable_dc6,
>> > +	"Enable power-saving display C-state 6. "
>> > +	"(0 = disable; 1 = enable [default])");
>> > +
>> >  module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
>> >  MODULE_PARM_DESC(enable_fbc,
>> >  	"Enable frame buffer compression for power savings "
>> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > index 042d92f..4843c5c 100644
>> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>> > @@ -753,7 +753,7 @@ static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>> >  static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>> >  					  struct i915_power_well *power_well)
>> >  {
>> > -	if (IS_SKYLAKE(dev_priv))
>> > +	if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
>> >  		skl_enable_dc6(dev_priv);
>> >  	else
>> >  		gen9_enable_dc5(dev_priv);
>> 
>> -- 
>> Jani Nikula, Intel Open Source Technology Center

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

Re: [PATCH 7/8] drm/i915/skl: Add boot parameter for disabling DC6

[Patrik Jakobsson]

On Tue, Nov 3, 2015 at 3:19 PM, Jani Nikula <jani.nikula@linux.intel.com> wrote:
> On Tue, 03 Nov 2015, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
>> On Tue, Nov 03, 2015 at 03:08:41PM +0200, Jani Nikula wrote:
>>> On Tue, 03 Nov 2015, Patrik Jakobsson <patrik.jakobsson@linux.intel.com> wrote:
>>> > Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
>>> > ---
>>> >  drivers/gpu/drm/i915/i915_drv.h         | 1 +
>>> >  drivers/gpu/drm/i915/i915_params.c      | 6 ++++++
>>> >  drivers/gpu/drm/i915/intel_runtime_pm.c | 2 +-
>>> >  3 files changed, 8 insertions(+), 1 deletion(-)
>>> >
>>> > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
>>> > index efb6a00..9c8cb43 100644
>>> > --- a/drivers/gpu/drm/i915/i915_drv.h
>>> > +++ b/drivers/gpu/drm/i915/i915_drv.h
>>> > @@ -2660,6 +2660,7 @@ struct i915_params {
>>> >    int panel_use_ssc;
>>> >    int vbt_sdvo_panel_type;
>>> >    int enable_rc6;
>>> > +  int enable_dc6;
>>> >    int enable_fbc;
>>> >    int enable_ppgtt;
>>> >    int enable_execlists;
>>> > diff --git a/drivers/gpu/drm/i915/i915_params.c b/drivers/gpu/drm/i915/i915_params.c
>>> > index 368df67..1a7dedf 100644
>>> > --- a/drivers/gpu/drm/i915/i915_params.c
>>> > +++ b/drivers/gpu/drm/i915/i915_params.c
>>> > @@ -32,6 +32,7 @@ struct i915_params i915 __read_mostly = {
>>> >    .panel_use_ssc = -1,
>>> >    .vbt_sdvo_panel_type = -1,
>>> >    .enable_rc6 = -1,
>>> > +  .enable_dc6 = 1,
>>> >    .enable_fbc = -1,
>>> >    .enable_execlists = -1,
>>> >    .enable_hangcheck = true,
>>> > @@ -79,6 +80,11 @@ MODULE_PARM_DESC(enable_rc6,
>>> >    "For example, 3 would enable rc6 and deep rc6, and 7 would enable everything. "
>>> >    "default: -1 (use per-chip default)");
>>> >
>>> > +module_param_named(enable_dc6, i915.enable_dc6, int, 0400);
>>>
>>> _unsafe please.
>>
>> Is the convention to taint for all non-default options in i915?
>
> The intention is to taint all i915 options that people have no clue
> about and blindly set just because they read on a forum they should do
> it, and then report bugs because it fails.

This one certainly fits that description. Thanks for the explanation.

-Patrik

>
> BR,
> Jani.
>
>
>
>>
>> -Patrik
>>
>>>
>>> BR,
>>> Jani.
>>>
>>> > +MODULE_PARM_DESC(enable_dc6,
>>> > +  "Enable power-saving display C-state 6. "
>>> > +  "(0 = disable; 1 = enable [default])");
>>> > +
>>> >  module_param_named_unsafe(enable_fbc, i915.enable_fbc, int, 0600);
>>> >  MODULE_PARM_DESC(enable_fbc,
>>> >    "Enable frame buffer compression for power savings "
>>> > diff --git a/drivers/gpu/drm/i915/intel_runtime_pm.c b/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> > index 042d92f..4843c5c 100644
>>> > --- a/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> > +++ b/drivers/gpu/drm/i915/intel_runtime_pm.c
>>> > @@ -753,7 +753,7 @@ static void skl_dc_off_power_well_enable(struct drm_i915_private *dev_priv,
>>> >  static void skl_dc_off_power_well_disable(struct drm_i915_private *dev_priv,
>>> >                                      struct i915_power_well *power_well)
>>> >  {
>>> > -  if (IS_SKYLAKE(dev_priv))
>>> > +  if (IS_SKYLAKE(dev_priv) && i915.enable_dc6)
>>> >            skl_enable_dc6(dev_priv);
>>> >    else
>>> >            gen9_enable_dc5(dev_priv);
>>>
>>> --
>>> Jani Nikula, Intel Open Source Technology Center
>
> --
> Jani Nikula, Intel Open Source Technology Center
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH 8/8] drm/i915: Force loading of csr program at boot

[Patrik Jakobsson]

When booting (warm or cold) we must always program the csr. Previously
we checked if the CSR program memory matched with the firmware data but
this turned out to fail on warm boots.

Signed-off-by: Patrik Jakobsson <patrik.jakobsson@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_drv.c  | 2 +-
 drivers/gpu/drm/i915/intel_csr.c | 6 +++---
 drivers/gpu/drm/i915/intel_drv.h | 2 +-
 3 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index e190237..7e08f3f 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -1114,7 +1114,7 @@ static int bxt_resume_prepare(struct drm_i915_private *dev_priv)
 static int skl_resume_prepare(struct drm_i915_private *dev_priv)
 {
 	skl_init_cdclk(dev_priv);
-	intel_csr_load_program(dev_priv);
+	intel_csr_load_program(dev_priv, false);
 
 	return 0;
 }
diff --git a/drivers/gpu/drm/i915/intel_csr.c b/drivers/gpu/drm/i915/intel_csr.c
index ecb7c70..586ea55 100644
--- a/drivers/gpu/drm/i915/intel_csr.c
+++ b/drivers/gpu/drm/i915/intel_csr.c
@@ -210,7 +210,7 @@ static char intel_get_substepping(struct drm_device *dev)
  * Everytime display comes back from low power state this function is called to
  * copy the firmware from internal memory to registers.
  */
-void intel_csr_load_program(struct drm_i915_private *dev_priv)
+void intel_csr_load_program(struct drm_i915_private *dev_priv, bool force)
 {
 	u32 *payload = dev_priv->csr.dmc_payload;
 	uint32_t i, fw_size;
@@ -226,7 +226,7 @@ void intel_csr_load_program(struct drm_i915_private *dev_priv)
 	 * Unfortunately the ACPI subsystem doesn't yet give us a way to
 	 * differentiate this, hence figure it out with this hack.
 	 */
-	if ((!dev_priv->csr.dmc_payload) || I915_READ(CSR_PROGRAM(0)))
+	if (!force && (!dev_priv->csr.dmc_payload || I915_READ(CSR_PROGRAM(0))))
 		return;
 
 	fw_size = dev_priv->csr.dmc_fw_size;
@@ -384,7 +384,7 @@ static void csr_load_work_fn(struct work_struct *work)
 		goto out;
 
 	/* load csr program during system boot, as needed for DC states */
-	intel_csr_load_program(dev_priv);
+	intel_csr_load_program(dev_priv, true);
 
 out:
 	if (dev_priv->csr.dmc_payload) {
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index d110555..58cd63a3 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1208,7 +1208,7 @@ u32 skl_plane_ctl_rotation(unsigned int rotation);
 
 /* intel_csr.c */
 void intel_csr_ucode_init(struct drm_i915_private *);
-void intel_csr_load_program(struct drm_i915_private *);
+void intel_csr_load_program(struct drm_i915_private *, bool);
 void intel_csr_ucode_fini(struct drm_i915_private *);
 
 /* intel_dp.c */
-- 
2.1.4

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

Re: [PATCH 0/8] Skylake DMC/DC-state fixes and redesign

[Daniel Stone]

Hi,

On 3 November 2015 at 12:31, Patrik Jakobsson
<patrik.jakobsson@linux.intel.com> wrote:
> These patches should sit on top of the DMC redesign patches from
> Animesh/Imre [1] which in turn depends on Mika's FW debug patches [2].
>
> First two patches are from Ville and is included since they otherwise
> might be forgotten. The third from Ville helps with handling DC off when
> doing Aux A communication.

Tested-by: Daniel Stone <daniels@collabora.com> # SKL

The intel_atomic_commit changes obviously conflict with Maarten's
async-commit-completion work, but when trivially rebased on top it
works just fine.

Cheers,
Daniel
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [PATCH 0/8] Skylake DMC/DC-state fixes and redesign

[Patrik Jakobsson]

On Wed, Nov 4, 2015 at 6:17 PM, Daniel Stone <daniel@fooishbar.org> wrote:
> Hi,
>
> On 3 November 2015 at 12:31, Patrik Jakobsson
> <patrik.jakobsson@linux.intel.com> wrote:
>> These patches should sit on top of the DMC redesign patches from
>> Animesh/Imre [1] which in turn depends on Mika's FW debug patches [2].
>>
>> First two patches are from Ville and is included since they otherwise
>> might be forgotten. The third from Ville helps with handling DC off when
>> doing Aux A communication.
>
> Tested-by: Daniel Stone <daniels@collabora.com> # SKL
>
> The intel_atomic_commit changes obviously conflict with Maarten's
> async-commit-completion work, but when trivially rebased on top it
> works just fine.

I'll sync this with Maarten. Thanks for testing!

-Patrik

>
> Cheers,
> Daniel
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx