[Intel-gfx] [PATCH 08/42] drm/i915: Implement intel_crtc_toggle using atomic state

2015-05-11 Thread Maarten Lankhorst
Assume the function is locked with drm_modeset_lock_all for now.

Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
 drivers/gpu/drm/i915/i915_irq.c  |  2 +-
 drivers/gpu/drm/i915/intel_display.c | 79 ++--
 2 files changed, 50 insertions(+), 31 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
index 9da955e4f355..a6816503a080 100644
--- a/drivers/gpu/drm/i915/i915_irq.c
+++ b/drivers/gpu/drm/i915/i915_irq.c
@@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device 
*dev, int pipe,
return -EINVAL;
}
 
-   if (!crtc-state-enable) {
+   if (!crtc-state-active) {
DRM_DEBUG_KMS(crtc %d is disabled\n, pipe);
return -EBUSY;
}
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 8c2fb951029b..a21b2e51c054 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4656,7 +4656,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
bool reenable_ips = false;
 
/* The clocks have to be on to load the palette. */
-   if (!crtc-state-enable || !intel_crtc-active)
+   if (!crtc-state-active || !intel_crtc-active)
return;
 
if (HAS_GMCH_DISPLAY(dev_priv-dev)) {
@@ -5767,7 +5767,7 @@ static int valleyview_modeset_global_pipes(struct 
drm_atomic_state *state)
 
/* add all active pipes to the state */
for_each_crtc(state-dev, crtc) {
-   if (!crtc-state-enable)
+   if (!crtc-state-active)
continue;
 
crtc_state = drm_atomic_get_crtc_state(state, crtc);
@@ -5865,7 +5865,7 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
int pipe = intel_crtc-pipe;
bool is_dsi;
 
-   WARN_ON(!crtc-state-enable);
+   WARN_ON(!crtc-state-active);
 
if (intel_crtc-active)
return;
@@ -5943,7 +5943,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
struct intel_encoder *encoder;
int pipe = intel_crtc-pipe;
 
-   WARN_ON(!crtc-state-enable);
+   WARN_ON(!crtc-state-active);
 
if (intel_crtc-active)
return;
@@ -6054,10 +6054,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 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 drm_mode_config *config = dev-mode_config;
+   struct drm_modeset_acquire_ctx *ctx = config-acquire_ctx;
struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-   enum intel_display_power_domain domain;
-   unsigned long domains;
+   struct intel_crtc_state *pipe_config;
+   struct drm_plane_state *plane_state;
+   struct drm_atomic_state *state;
+   int ret;
 
if (enable == intel_crtc-active)
return;
@@ -6065,24 +6068,40 @@ void intel_crtc_control(struct drm_crtc *crtc, bool 
enable)
if (enable  !crtc-state-enable)
return;
 
-   crtc-state-active = enable;
-   if (enable) {
-   domains = get_crtc_power_domains(crtc);
-   for_each_power_domain(domain, domains)
-   intel_display_power_get(dev_priv, domain);
-   intel_crtc-enabled_power_domains = domains;
+   /* this function should be called with drm_modeset_lock_all for now */
+   if (WARN_ON(!ctx))
+   return;
+   lockdep_assert_held(ctx-ww_ctx);
 
-   dev_priv-display.crtc_enable(crtc);
-   intel_crtc_enable_planes(crtc);
-   } else {
-   intel_crtc_disable_planes(crtc);
-   dev_priv-display.crtc_disable(crtc);
+   state = drm_atomic_state_alloc(dev);
+   if (WARN_ON(!state))
+   return;
 
-   domains = intel_crtc-enabled_power_domains;
-   for_each_power_domain(domain, domains)
-   intel_display_power_put(dev_priv, domain);
-   intel_crtc-enabled_power_domains = 0;
+   state-acquire_ctx = ctx;
+   state-allow_modeset = true;
+
+   pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
+   if (IS_ERR(pipe_config)) {
+   ret = PTR_ERR(pipe_config);
+   goto err;
}
+   pipe_config-base.active = enable;
+
+   plane_state = drm_atomic_get_plane_state(state, crtc-primary);
+   if (IS_ERR(plane_state)) {
+   ret = PTR_ERR(plane_state);
+   goto err;
+   }
+
+   ret = intel_set_mode(crtc, state);
+   if (!ret)
+   return;
+
+   DRM_ERROR(Failed to toggle crtc!\n);
+
+err:
+   DRM_ERROR(Updating crtc active failed with %i\n, ret);
+   drm_atomic_state_free(state);
 }
 
 /**
@@ -6158,7 +6177,7 @@ static void 

Re: [Intel-gfx] [PATCH 08/42] drm/i915: Implement intel_crtc_toggle using atomic state

2015-05-11 Thread Daniel Vetter
On Mon, May 11, 2015 at 04:24:44PM +0200, Maarten Lankhorst wrote:
 Assume the function is locked with drm_modeset_lock_all for now.

s/toggle/control/ in the commit message.

And a bit of blabla would be good to explain why we need to make a
mass-replacement of state-enable to state-active here now. And looking
at the enable/active replacement in detail I think we should do this in an
upfront patch first (the state should be consistent already). And have the
reworking of intel_crtc_control in a separate split-out patch.

 Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
 ---
  drivers/gpu/drm/i915/i915_irq.c  |  2 +-
  drivers/gpu/drm/i915/intel_display.c | 79 
 ++--
  2 files changed, 50 insertions(+), 31 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
 index 9da955e4f355..a6816503a080 100644
 --- a/drivers/gpu/drm/i915/i915_irq.c
 +++ b/drivers/gpu/drm/i915/i915_irq.c
 @@ -796,7 +796,7 @@ static int i915_get_vblank_timestamp(struct drm_device 
 *dev, int pipe,
   return -EINVAL;
   }
  
 - if (!crtc-state-enable) {
 + if (!crtc-state-active) {
   DRM_DEBUG_KMS(crtc %d is disabled\n, pipe);
   return -EBUSY;
   }
 diff --git a/drivers/gpu/drm/i915/intel_display.c 
 b/drivers/gpu/drm/i915/intel_display.c
 index 8c2fb951029b..a21b2e51c054 100644
 --- a/drivers/gpu/drm/i915/intel_display.c
 +++ b/drivers/gpu/drm/i915/intel_display.c

One s/state-enable/state-active/ in modeset_update_crtc_power_domains
seems to be missing.

 @@ -4656,7 +4656,7 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
   bool reenable_ips = false;
  
   /* The clocks have to be on to load the palette. */
 - if (!crtc-state-enable || !intel_crtc-active)
 + if (!crtc-state-active || !intel_crtc-active)

Replace the check for intel_crtc-active with a WARN_ON?

   return;
  
   if (HAS_GMCH_DISPLAY(dev_priv-dev)) {
 @@ -5767,7 +5767,7 @@ static int valleyview_modeset_global_pipes(struct 
 drm_atomic_state *state)
  
   /* add all active pipes to the state */
   for_each_crtc(state-dev, crtc) {
 - if (!crtc-state-enable)
 + if (!crtc-state-active)
   continue;
  
   crtc_state = drm_atomic_get_crtc_state(state, crtc);

The second state-enable looks a bit funky, imo

if (crtc_state-active)
crtc_state-active_changed

after this patch would make a lot of sense. That way we won't try to frob
pipes which are already off.

 @@ -5865,7 +5865,7 @@ static void valleyview_crtc_enable(struct drm_crtc 
 *crtc)
   int pipe = intel_crtc-pipe;
   bool is_dsi;
  
 - WARN_ON(!crtc-state-enable);
 + WARN_ON(!crtc-state-active);

You don't update haswell/ironlake_crtc_enable, which is inconsistent.

  
   if (intel_crtc-active)
   return;
 @@ -5943,7 +5943,7 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
   struct intel_encoder *encoder;
   int pipe = intel_crtc-pipe;
  
 - WARN_ON(!crtc-state-enable);
 + WARN_ON(!crtc-state-active);
  
   if (intel_crtc-active)
   return;

intel_connector_check_state should gain a

I915_STATE_WARN(!crtc-state-active,
crtc not active\n);

imo.

Similar for check_crtc_state:

I915_STATE_WARN(active != crtc-state-active,
 crtc's computed active state doesn't match sw active 
state 
 (expected %i, found %i)\n, active, crtc-active);


 @@ -6054,10 +6054,13 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
  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 drm_mode_config *config = dev-mode_config;
 + struct drm_modeset_acquire_ctx *ctx = config-acquire_ctx;
   struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 - enum intel_display_power_domain domain;
 - unsigned long domains;
 + struct intel_crtc_state *pipe_config;
 + struct drm_plane_state *plane_state;
 + struct drm_atomic_state *state;
 + int ret;
  
   if (enable == intel_crtc-active)
   return;
 @@ -6065,24 +6068,40 @@ void intel_crtc_control(struct drm_crtc *crtc, bool 
 enable)
   if (enable  !crtc-state-enable)
   return;
  
 - crtc-state-active = enable;
 - if (enable) {
 - domains = get_crtc_power_domains(crtc);
 - for_each_power_domain(domain, domains)
 - intel_display_power_get(dev_priv, domain);
 - intel_crtc-enabled_power_domains = domains;
 + /* this function should be called with drm_modeset_lock_all for now */
 + if (WARN_ON(!ctx))
 + return;
 + lockdep_assert_held(ctx-ww_ctx);
  
 -