Re: [Intel-gfx] [PATCH 2/6] drm/i915: factor out is_always_on_domain

2013-10-18 Thread Jesse Barnes 
On Wed, 16 Oct 2013 17:25:49 +0300
Imre Deak imre.d...@intel.com wrote:

 It is just cleaner this way and makes it easier to add support for
 other HW generations with always-on power wells powering a different
 set of domains.
 
 Signed-off-by: Imre Deak imre.d...@intel.com
 ---
  drivers/gpu/drm/i915/i915_drv.h |  8 
  drivers/gpu/drm/i915/intel_pm.c | 84 
 +++--
  2 files changed, 38 insertions(+), 54 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
 index 9b04d05..ca05f3a 100644
 --- a/drivers/gpu/drm/i915/i915_drv.h
 +++ b/drivers/gpu/drm/i915/i915_drv.h
 @@ -100,8 +100,12 @@ enum intel_display_power_domain {
   POWER_DOMAIN_TRANSCODER_C,
   POWER_DOMAIN_TRANSCODER_EDP,
   POWER_DOMAIN_VGA,
 +
 + POWER_DOMAIN_NUM,
  };
  
 +#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
 +
  #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
  #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \
   ((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER)
 @@ -109,6 +113,10 @@ enum intel_display_power_domain {
   ((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \
(tran) + POWER_DOMAIN_TRANSCODER_A)
  
 +#define HSW_ALWAYS_ON_POWER_DOMAINS (\
 + BIT(POWER_DOMAIN_PIPE_A) |  \
 + BIT(POWER_DOMAIN_TRANSCODER_EDP))
 +
  enum hpd_pin {
   HPD_NONE = 0,
   HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
 diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
 index 3d3658c..57d08a2 100644
 --- a/drivers/gpu/drm/i915/intel_pm.c
 +++ b/drivers/gpu/drm/i915/intel_pm.c
 @@ -5367,6 +5367,23 @@ void intel_suspend_hw(struct drm_device *dev)
   lpt_suspend_hw(dev);
  }
  
 +static bool is_always_on_power_domain(struct drm_device *dev,
 +   enum intel_display_power_domain domain)
 +{
 + unsigned long always_on_domains;
 +
 + BUG_ON(BIT(domain)  ~POWER_DOMAIN_MASK);
 +
 + if (IS_HASWELL(dev)) {
 + always_on_domains = HSW_ALWAYS_ON_POWER_DOMAINS;
 + } else {
 + WARN_ON(1);
 + return true;
 + }
 +
 + return BIT(domain)  always_on_domains;
 +}
 +
  /**
   * We should only use the power well if we explicitly asked the hardware to
   * enable it, so check if it's enabled and also check if we've requested it 
 to
 @@ -5380,24 +5397,11 @@ bool intel_display_power_enabled(struct drm_device 
 *dev,
   if (!HAS_POWER_WELL(dev))
   return true;
  
 - switch (domain) {
 - case POWER_DOMAIN_PIPE_A:
 - case POWER_DOMAIN_TRANSCODER_EDP:
 + if (is_always_on_power_domain(dev, domain))
   return true;
 - case POWER_DOMAIN_VGA:
 - case POWER_DOMAIN_PIPE_B:
 - case POWER_DOMAIN_PIPE_C:
 - case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
 - case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
 - case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
 - case POWER_DOMAIN_TRANSCODER_A:
 - case POWER_DOMAIN_TRANSCODER_B:
 - case POWER_DOMAIN_TRANSCODER_C:
 - return I915_READ(HSW_PWR_WELL_DRIVER) ==
 +
 + return I915_READ(HSW_PWR_WELL_DRIVER) ==
(HSW_PWR_WELL_ENABLE_REQUEST | HSW_PWR_WELL_STATE_ENABLED);
 - default:
 - BUG();
 - }
  }
  
  static void __intel_set_power_well(struct drm_device *dev, bool enable)
 @@ -5469,26 +5473,12 @@ void intel_display_power_get(struct drm_device *dev,
   if (!HAS_POWER_WELL(dev))
   return;
  
 - switch (domain) {
 - case POWER_DOMAIN_PIPE_A:
 - case POWER_DOMAIN_TRANSCODER_EDP:
 + if (is_always_on_power_domain(dev, domain))
   return;
 - case POWER_DOMAIN_VGA:
 - case POWER_DOMAIN_PIPE_B:
 - case POWER_DOMAIN_PIPE_C:
 - case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
 - case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
 - case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
 - case POWER_DOMAIN_TRANSCODER_A:
 - case POWER_DOMAIN_TRANSCODER_B:
 - case POWER_DOMAIN_TRANSCODER_C:
 - spin_lock_irq(power_well-lock);
 - __intel_power_well_get(power_well);
 - spin_unlock_irq(power_well-lock);
 - return;
 - default:
 - BUG();
 - }
 +
 + spin_lock_irq(power_well-lock);
 + __intel_power_well_get(power_well);
 + spin_unlock_irq(power_well-lock);
  }
  
  void intel_display_power_put(struct drm_device *dev,
 @@ -5500,26 +5490,12 @@ void intel_display_power_put(struct drm_device *dev,
   if (!HAS_POWER_WELL(dev))
   return;
  
 - switch (domain) {
 - case POWER_DOMAIN_PIPE_A:
 - case POWER_DOMAIN_TRANSCODER_EDP:
 + if (is_always_on_power_domain(dev, domain))
   return;
 - case POWER_DOMAIN_VGA:
 - case POWER_DOMAIN_PIPE_B:
 - case POWER_DOMAIN_PIPE_C:
 - case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
 - case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
 - case

[Intel-gfx] [PATCH 2/6] drm/i915: factor out is_always_on_domain

2013-10-16 Thread Imre Deak 
It is just cleaner this way and makes it easier to add support for
other HW generations with always-on power wells powering a different
set of domains.

Signed-off-by: Imre Deak imre.d...@intel.com
---
 drivers/gpu/drm/i915/i915_drv.h |  8 
 drivers/gpu/drm/i915/intel_pm.c | 84 +++--
 2 files changed, 38 insertions(+), 54 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 9b04d05..ca05f3a 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -100,8 +100,12 @@ enum intel_display_power_domain {
POWER_DOMAIN_TRANSCODER_C,
POWER_DOMAIN_TRANSCODER_EDP,
POWER_DOMAIN_VGA,
+
+   POWER_DOMAIN_NUM,
 };
 
+#define POWER_DOMAIN_MASK (BIT(POWER_DOMAIN_NUM) - 1)
+
 #define POWER_DOMAIN_PIPE(pipe) ((pipe) + POWER_DOMAIN_PIPE_A)
 #define POWER_DOMAIN_PIPE_PANEL_FITTER(pipe) \
((pipe) + POWER_DOMAIN_PIPE_A_PANEL_FITTER)
@@ -109,6 +113,10 @@ enum intel_display_power_domain {
((tran) == TRANSCODER_EDP ? POWER_DOMAIN_TRANSCODER_EDP : \
 (tran) + POWER_DOMAIN_TRANSCODER_A)
 
+#define HSW_ALWAYS_ON_POWER_DOMAINS (  \
+   BIT(POWER_DOMAIN_PIPE_A) |  \
+   BIT(POWER_DOMAIN_TRANSCODER_EDP))
+
 enum hpd_pin {
HPD_NONE = 0,
HPD_PORT_A = HPD_NONE, /* PORT_A is internal */
diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
index 3d3658c..57d08a2 100644
--- a/drivers/gpu/drm/i915/intel_pm.c
+++ b/drivers/gpu/drm/i915/intel_pm.c
@@ -5367,6 +5367,23 @@ void intel_suspend_hw(struct drm_device *dev)
lpt_suspend_hw(dev);
 }
 
+static bool is_always_on_power_domain(struct drm_device *dev,
+ enum intel_display_power_domain domain)
+{
+   unsigned long always_on_domains;
+
+   BUG_ON(BIT(domain)  ~POWER_DOMAIN_MASK);
+
+   if (IS_HASWELL(dev)) {
+   always_on_domains = HSW_ALWAYS_ON_POWER_DOMAINS;
+   } else {
+   WARN_ON(1);
+   return true;
+   }
+
+   return BIT(domain)  always_on_domains;
+}
+
 /**
  * We should only use the power well if we explicitly asked the hardware to
  * enable it, so check if it's enabled and also check if we've requested it to
@@ -5380,24 +5397,11 @@ bool intel_display_power_enabled(struct drm_device *dev,
if (!HAS_POWER_WELL(dev))
return true;
 
-   switch (domain) {
-   case POWER_DOMAIN_PIPE_A:
-   case POWER_DOMAIN_TRANSCODER_EDP:
+   if (is_always_on_power_domain(dev, domain))
return true;
-   case POWER_DOMAIN_VGA:
-   case POWER_DOMAIN_PIPE_B:
-   case POWER_DOMAIN_PIPE_C:
-   case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
-   case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
-   case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
-   case POWER_DOMAIN_TRANSCODER_A:
-   case POWER_DOMAIN_TRANSCODER_B:
-   case POWER_DOMAIN_TRANSCODER_C:
-   return I915_READ(HSW_PWR_WELL_DRIVER) ==
+
+   return I915_READ(HSW_PWR_WELL_DRIVER) ==
 (HSW_PWR_WELL_ENABLE_REQUEST | HSW_PWR_WELL_STATE_ENABLED);
-   default:
-   BUG();
-   }
 }
 
 static void __intel_set_power_well(struct drm_device *dev, bool enable)
@@ -5469,26 +5473,12 @@ void intel_display_power_get(struct drm_device *dev,
if (!HAS_POWER_WELL(dev))
return;
 
-   switch (domain) {
-   case POWER_DOMAIN_PIPE_A:
-   case POWER_DOMAIN_TRANSCODER_EDP:
+   if (is_always_on_power_domain(dev, domain))
return;
-   case POWER_DOMAIN_VGA:
-   case POWER_DOMAIN_PIPE_B:
-   case POWER_DOMAIN_PIPE_C:
-   case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
-   case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
-   case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
-   case POWER_DOMAIN_TRANSCODER_A:
-   case POWER_DOMAIN_TRANSCODER_B:
-   case POWER_DOMAIN_TRANSCODER_C:
-   spin_lock_irq(power_well-lock);
-   __intel_power_well_get(power_well);
-   spin_unlock_irq(power_well-lock);
-   return;
-   default:
-   BUG();
-   }
+
+   spin_lock_irq(power_well-lock);
+   __intel_power_well_get(power_well);
+   spin_unlock_irq(power_well-lock);
 }
 
 void intel_display_power_put(struct drm_device *dev,
@@ -5500,26 +5490,12 @@ void intel_display_power_put(struct drm_device *dev,
if (!HAS_POWER_WELL(dev))
return;
 
-   switch (domain) {
-   case POWER_DOMAIN_PIPE_A:
-   case POWER_DOMAIN_TRANSCODER_EDP:
+   if (is_always_on_power_domain(dev, domain))
return;
-   case POWER_DOMAIN_VGA:
-   case POWER_DOMAIN_PIPE_B:
-   case POWER_DOMAIN_PIPE_C:
-   case POWER_DOMAIN_PIPE_A_PANEL_FITTER:
-   case POWER_DOMAIN_PIPE_B_PANEL_FITTER:
-   case POWER_DOMAIN_PIPE_C_PANEL_FITTER:
-   case