Re: [Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

2014-07-25 Thread Daniel Vetter 
On Fri, Jul 25, 2014 at 01:28:48PM +1000, Dave Airlie wrote:
 On 23 July 2014 15:11, Daniel Vetter dan...@ffwll.ch wrote:
  On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kam...@intel.com wrote:
  From: Borun Fu borun...@intel.com
 
  On VLV, after i915_pm_suspend display power wells are staying
  power ungated. So, after initiating mem sleep echo mem  /sys/power/state
  Display is staing D0 State. There might be better way/place to power gate
  these wells. Also, we need to make sure that if wells are power gated due 
  to
  DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.
 
  v2: Extracted helper for intel_crtc_disable and power gating CRTC power 
  wells.
  [Daniel]
 
  Cc: Imre Deak imre.d...@intel.com
  Cc: Paulo Zanoni paulo.r.zan...@intel.com
  Cc: Daniel Vetter daniel.vet...@ffwll.ch
  Cc: Jani Nikula jani.nik...@linux.intel.com
  Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848
 
  s-o-b from the original author (Borun Fu) missing. Added myself since we
  all work for the same company, but please don't forget this. Every person
  including the original author, who handles a patch must add their sob
  line.
  -Daniel
 
 Is this queued or on its way, I was getting a warning on HSW about not
 entering pc8+
 due to display with MST enabled, and I thought it was MSTs fault, but I 
 suspect
 its just this,
 
 mode set turns the global resources power well on, but nothing ever
 turns it off.

mst doesn't factor into this. Might need to double-check but from what
I've seen mst was only wired into the system s/r paths, not the runtime
pm. They're unfortunately not sufficiently shared. So I'd suspect that.

The bug here happens when you move/update the cursor (or sprite/primary
plane) while the display is off. You'll get unclaimed register read/write
errors, no WARNINGs if you hit this one.
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

Re: [Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

2014-07-24 Thread Dave Airlie 
On 23 July 2014 15:11, Daniel Vetter dan...@ffwll.ch wrote:
 On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kam...@intel.com wrote:
 From: Borun Fu borun...@intel.com

 On VLV, after i915_pm_suspend display power wells are staying
 power ungated. So, after initiating mem sleep echo mem  /sys/power/state
 Display is staing D0 State. There might be better way/place to power gate
 these wells. Also, we need to make sure that if wells are power gated due to
 DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.

 v2: Extracted helper for intel_crtc_disable and power gating CRTC power 
 wells.
 [Daniel]

 Cc: Imre Deak imre.d...@intel.com
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jani Nikula jani.nik...@linux.intel.com
 Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848

 s-o-b from the original author (Borun Fu) missing. Added myself since we
 all work for the same company, but please don't forget this. Every person
 including the original author, who handles a patch must add their sob
 line.
 -Daniel

Is this queued or on its way, I was getting a warning on HSW about not
entering pc8+
due to display with MST enabled, and I thought it was MSTs fault, but I suspect
its just this,

mode set turns the global resources power well on, but nothing ever
turns it off.

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

Re: [Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

2014-07-22 Thread Daniel Vetter 
On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kam...@intel.com wrote:
 From: Borun Fu borun...@intel.com
 
 On VLV, after i915_pm_suspend display power wells are staying
 power ungated. So, after initiating mem sleep echo mem  /sys/power/state
 Display is staing D0 State. There might be better way/place to power gate
 these wells. Also, we need to make sure that if wells are power gated due to
 DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.
 
 v2: Extracted helper for intel_crtc_disable and power gating CRTC power wells.
 [Daniel]
 
 Cc: Imre Deak imre.d...@intel.com
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jani Nikula jani.nik...@linux.intel.com
 Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848

s-o-b from the original author (Borun Fu) missing. Added myself since we
all work for the same company, but please don't forget this. Every person
including the original author, who handles a patch must add their sob
line.
-Daniel

 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.c  |  7 +++
  drivers/gpu/drm/i915/i915_drv.h  |  4 
  drivers/gpu/drm/i915/intel_display.c | 30 +-
  drivers/gpu/drm/i915/intel_drv.h |  1 +
  4 files changed, 25 insertions(+), 17 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 83cb43a..5e4fefd 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -524,12 +524,11 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   /*
* Disable CRTCs directly since we want to preserve sw state
 -  * for _thaw.
 +  * for _thaw. Also, power gate the CRTC power wells.
*/
   drm_modeset_lock_all(dev);
 - for_each_crtc(dev, crtc) {
 - dev_priv-display.crtc_disable(crtc);
 - }
 + for_each_crtc(dev, crtc)
 + intel_crtc_control(crtc, false);
   drm_modeset_unlock_all(dev);
  
   intel_modeset_suspend_hw(dev);
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index a89c912..54f98d3 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -179,6 +179,10 @@ enum hpd_pin {
   list_for_each_entry((intel_connector), 
 (dev)-mode_config.connector_list, base.head) \
   if ((intel_connector)-base.encoder == (__encoder))
  
 +#define for_each_power_domain(domain, mask)  \
 + for ((domain) = 0; (domain)  POWER_DOMAIN_NUM; (domain)++) \
 + if ((1  (domain))  (mask))
 +
  struct drm_i915_private;
  struct i915_mmu_object;
  
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index fe6f1db..7a1f14f 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4300,10 +4300,6 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
   I915_WRITE(BCLRPAT(crtc-pipe), 0);
  }
  
 -#define for_each_power_domain(domain, mask)  \
 - for ((domain) = 0; (domain)  POWER_DOMAIN_NUM; (domain)++) \
 - if ((1  (domain))  (mask))
 -
  enum intel_display_power_domain
  intel_display_port_power_domain(struct intel_encoder *intel_encoder)
  {
 @@ -4872,21 +4868,14 @@ static void intel_crtc_update_sarea(struct drm_crtc 
 *crtc,
   }
  }
  
 -/**
 - * Sets the power management mode of the pipe and plane.
 - */
 -void intel_crtc_update_dpms(struct drm_crtc *crtc)
 +/* Master function to enable/disable CRTC and corresponding power wells */
 +void intel_crtc_control(struct drm_crtc *crtc, bool enable)
  {
   struct drm_device *dev = crtc-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 - struct intel_encoder *intel_encoder;
   enum intel_display_power_domain domain;
   unsigned long domains;
 - bool enable = false;
 -
 - for_each_encoder_on_crtc(dev, crtc, intel_encoder)
 - enable |= intel_encoder-connectors_active;
  
   if (enable) {
   if (!intel_crtc-active) {
 @@ -4907,6 +4896,21 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
   intel_crtc-enabled_power_domains = 0;
   }
   }
 +}
 +
 +/**
 + * Sets the power management mode of the pipe and plane.
 + */
 +void intel_crtc_update_dpms(struct drm_crtc *crtc)
 +{
 + struct drm_device *dev = crtc-dev;
 + struct intel_encoder *intel_encoder;
 + bool enable = false;
 +
 + for_each_encoder_on_crtc(dev, crtc, intel_encoder)
 + enable |= intel_encoder-connectors_active;
 +
 + intel_crtc_control(crtc, enable);
  
   intel_crtc_update_sarea(crtc, enable);
  }
 diff --git a/drivers/gpu/drm/i915/intel_drv.h

Re: [Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

2014-07-12 Thread Daniel Vetter 
On Sat, Jul 12, 2014 at 10:02:27AM +0530, sagar.a.kam...@intel.com wrote:
 From: Borun Fu borun...@intel.com
 
 On VLV, after i915_pm_suspend display power wells are staying
 power ungated. So, after initiating mem sleep echo mem  /sys/power/state
 Display is staing D0 State. There might be better way/place to power gate
 these wells. Also, we need to make sure that if wells are power gated due to
 DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.
 
 v2: Extracted helper for intel_crtc_disable and power gating CRTC power wells.
 [Daniel]
 
 Cc: Imre Deak imre.d...@intel.com
 Cc: Paulo Zanoni paulo.r.zan...@intel.com
 Cc: Daniel Vetter daniel.vet...@ffwll.ch
 Cc: Jani Nikula jani.nik...@linux.intel.com
 Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848
 Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com

intel_crtc_control looks like a neat idea - I've started also thinking
about the enable side and noticed that we'll have similar issues there.
But complicated with modeset_global_resources and the differences between
the dpms paths and modeset paths.

But that's work for another day. Queued for -next, thanks for the patch.
-Daniel

 ---
  drivers/gpu/drm/i915/i915_drv.c  |  7 +++
  drivers/gpu/drm/i915/i915_drv.h  |  4 
  drivers/gpu/drm/i915/intel_display.c | 30 +-
  drivers/gpu/drm/i915/intel_drv.h |  1 +
  4 files changed, 25 insertions(+), 17 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
 index 83cb43a..5e4fefd 100644
 --- a/drivers/gpu/drm/i915/i915_drv.c
 +++ b/drivers/gpu/drm/i915/i915_drv.c
 @@ -524,12 +524,11 @@ static int i915_drm_freeze(struct drm_device *dev)
  
   /*
* Disable CRTCs directly since we want to preserve sw state
 -  * for _thaw.
 +  * for _thaw. Also, power gate the CRTC power wells.
*/
   drm_modeset_lock_all(dev);
 - for_each_crtc(dev, crtc) {
 - dev_priv-display.crtc_disable(crtc);
 - }
 + for_each_crtc(dev, crtc)
 + intel_crtc_control(crtc, false);
   drm_modeset_unlock_all(dev);
  
   intel_modeset_suspend_hw(dev);
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index a89c912..54f98d3 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -179,6 +179,10 @@ enum hpd_pin {
   list_for_each_entry((intel_connector), 
 (dev)-mode_config.connector_list, base.head) \
   if ((intel_connector)-base.encoder == (__encoder))
  
 +#define for_each_power_domain(domain, mask)  \
 + for ((domain) = 0; (domain)  POWER_DOMAIN_NUM; (domain)++) \
 + if ((1  (domain))  (mask))
 +
  struct drm_i915_private;
  struct i915_mmu_object;
  
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index fe6f1db..7a1f14f 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c
 @@ -4300,10 +4300,6 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
   I915_WRITE(BCLRPAT(crtc-pipe), 0);
  }
  
 -#define for_each_power_domain(domain, mask)  \
 - for ((domain) = 0; (domain)  POWER_DOMAIN_NUM; (domain)++) \
 - if ((1  (domain))  (mask))
 -
  enum intel_display_power_domain
  intel_display_port_power_domain(struct intel_encoder *intel_encoder)
  {
 @@ -4872,21 +4868,14 @@ static void intel_crtc_update_sarea(struct drm_crtc 
 *crtc,
   }
  }
  
 -/**
 - * Sets the power management mode of the pipe and plane.
 - */
 -void intel_crtc_update_dpms(struct drm_crtc *crtc)
 +/* Master function to enable/disable CRTC and corresponding power wells */
 +void intel_crtc_control(struct drm_crtc *crtc, bool enable)
  {
   struct drm_device *dev = crtc-dev;
   struct drm_i915_private *dev_priv = dev-dev_private;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 - struct intel_encoder *intel_encoder;
   enum intel_display_power_domain domain;
   unsigned long domains;
 - bool enable = false;
 -
 - for_each_encoder_on_crtc(dev, crtc, intel_encoder)
 - enable |= intel_encoder-connectors_active;
  
   if (enable) {
   if (!intel_crtc-active) {
 @@ -4907,6 +4896,21 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
   intel_crtc-enabled_power_domains = 0;
   }
   }
 +}
 +
 +/**
 + * Sets the power management mode of the pipe and plane.
 + */
 +void intel_crtc_update_dpms(struct drm_crtc *crtc)
 +{
 + struct drm_device *dev = crtc-dev;
 + struct intel_encoder *intel_encoder;
 + bool enable = false;
 +
 + for_each_encoder_on_crtc(dev, crtc, intel_encoder)
 + enable |= intel_encoder-connectors_active;
 +
 + intel_crtc_control(crtc, enable);

[Intel-gfx] [RFC v2 1/1] drm/i915: Power gating display wells during i915_pm_suspend

2014-07-11 Thread sagar . a . kamble 
From: Borun Fu borun...@intel.com

On VLV, after i915_pm_suspend display power wells are staying
power ungated. So, after initiating mem sleep echo mem  /sys/power/state
Display is staing D0 State. There might be better way/place to power gate
these wells. Also, we need to make sure that if wells are power gated due to
DPMS OFF sequence, they need not be turned off by i915_pm_suspend again.

v2: Extracted helper for intel_crtc_disable and power gating CRTC power wells.
[Daniel]

Cc: Imre Deak imre.d...@intel.com
Cc: Paulo Zanoni paulo.r.zan...@intel.com
Cc: Daniel Vetter daniel.vet...@ffwll.ch
Cc: Jani Nikula jani.nik...@linux.intel.com
Change-Id: I34c80da66aa24c423a5576c68aa1f3a8d0f43848
Signed-off-by: Sagar Kamble sagar.a.kam...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.c  |  7 +++
 drivers/gpu/drm/i915/i915_drv.h  |  4 
 drivers/gpu/drm/i915/intel_display.c | 30 +-
 drivers/gpu/drm/i915/intel_drv.h |  1 +
 4 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.c b/drivers/gpu/drm/i915/i915_drv.c
index 83cb43a..5e4fefd 100644
--- a/drivers/gpu/drm/i915/i915_drv.c
+++ b/drivers/gpu/drm/i915/i915_drv.c
@@ -524,12 +524,11 @@ static int i915_drm_freeze(struct drm_device *dev)
 
/*
 * Disable CRTCs directly since we want to preserve sw state
-* for _thaw.
+* for _thaw. Also, power gate the CRTC power wells.
 */
drm_modeset_lock_all(dev);
-   for_each_crtc(dev, crtc) {
-   dev_priv-display.crtc_disable(crtc);
-   }
+   for_each_crtc(dev, crtc)
+   intel_crtc_control(crtc, false);
drm_modeset_unlock_all(dev);
 
intel_modeset_suspend_hw(dev);
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index a89c912..54f98d3 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -179,6 +179,10 @@ enum hpd_pin {
list_for_each_entry((intel_connector), 
(dev)-mode_config.connector_list, base.head) \
if ((intel_connector)-base.encoder == (__encoder))
 
+#define for_each_power_domain(domain, mask)\
+   for ((domain) = 0; (domain)  POWER_DOMAIN_NUM; (domain)++) \
+   if ((1  (domain))  (mask))
+
 struct drm_i915_private;
 struct i915_mmu_object;
 
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index fe6f1db..7a1f14f 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4300,10 +4300,6 @@ static void i9xx_pfit_enable(struct intel_crtc *crtc)
I915_WRITE(BCLRPAT(crtc-pipe), 0);
 }
 
-#define for_each_power_domain(domain, mask)\
-   for ((domain) = 0; (domain)  POWER_DOMAIN_NUM; (domain)++) \
-   if ((1  (domain))  (mask))
-
 enum intel_display_power_domain
 intel_display_port_power_domain(struct intel_encoder *intel_encoder)
 {
@@ -4872,21 +4868,14 @@ static void intel_crtc_update_sarea(struct drm_crtc 
*crtc,
}
 }
 
-/**
- * Sets the power management mode of the pipe and plane.
- */
-void intel_crtc_update_dpms(struct drm_crtc *crtc)
+/* Master function to enable/disable CRTC and corresponding power wells */
+void intel_crtc_control(struct drm_crtc *crtc, bool enable)
 {
struct drm_device *dev = crtc-dev;
struct drm_i915_private *dev_priv = dev-dev_private;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   struct intel_encoder *intel_encoder;
enum intel_display_power_domain domain;
unsigned long domains;
-   bool enable = false;
-
-   for_each_encoder_on_crtc(dev, crtc, intel_encoder)
-   enable |= intel_encoder-connectors_active;
 
if (enable) {
if (!intel_crtc-active) {
@@ -4907,6 +4896,21 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
intel_crtc-enabled_power_domains = 0;
}
}
+}
+
+/**
+ * Sets the power management mode of the pipe and plane.
+ */
+void intel_crtc_update_dpms(struct drm_crtc *crtc)
+{
+   struct drm_device *dev = crtc-dev;
+   struct intel_encoder *intel_encoder;
+   bool enable = false;
+
+   for_each_encoder_on_crtc(dev, crtc, intel_encoder)
+   enable |= intel_encoder-connectors_active;
+
+   intel_crtc_control(crtc, enable);
 
intel_crtc_update_sarea(crtc, enable);
 }
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 016d894..4c24f88 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -755,6 +755,7 @@ void intel_frontbuffer_flip(struct drm_device *dev,
 void intel_fb_obj_flush(struct drm_i915_gem_object *obj, bool retire);
 void intel_mark_idle(struct drm_device *dev);
 void