Re: [Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-12 Thread Daniel Vetter
On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
 This kills off most of the transitional helpers and uses atomic plane updates
 in the modeset path to update everything.
 
 Getting rid of the transitional plane helpers meant that planes had to be 
 added
 in the crtc check function. On modeset a connector can be moved to a different
 crtc, and this is not handled correctly otherwise.
 
 Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com

Ok pile of comments on this one. I don't yet fully grasp it all, but at
the very bottom I've jotted down a list of ideas for how to move forward
with this one.

 ---
  drivers/gpu/drm/i915/intel_atomic_plane.c |  59 ++-
  drivers/gpu/drm/i915/intel_display.c  | 655 
 ++
  drivers/gpu/drm/i915/intel_drv.h  |   2 +-
  drivers/gpu/drm/i915/intel_sprite.c   |  80 +---
  4 files changed, 441 insertions(+), 355 deletions(-)
 
 diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
 b/drivers/gpu/drm/i915/intel_atomic_plane.c
 index 86ba4b2c3a65..85b87e4d4b6e 100644
 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
 +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
 @@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane 
 *plane,
   struct drm_plane_state *state)
  {
   struct drm_crtc *crtc = state-crtc;
 - struct intel_crtc *intel_crtc;
 - struct intel_crtc_state *crtc_state;
 + struct drm_crtc_state *crtc_state;
   struct intel_plane *intel_plane = to_intel_plane(plane);
   struct intel_plane_state *intel_state = to_intel_plane_state(state);
  
 - crtc = crtc ? crtc : plane-crtc;
 - intel_crtc = to_intel_crtc(crtc);
 -
 + intel_state-visible = false;
   /*
* Both crtc and plane-crtc could be NULL if we're updating a
* property while the plane is disabled.  We don't actually have
* anything driver-specific we need to test in that case, so
* just return success.
*/
 - if (!crtc)
 + if (!crtc) {
 + DRM_DEBUG_ATOMIC(Invisible: no crtc\n);
   return 0;
 + }
 +
 + crtc_state = state-state-crtc_states[drm_crtc_index(crtc)];

Please reuse drm_atomic_get_crtc_state, this deref magic is hard to read.
Maybe we should have a nofail variant of those to encode the below WARN_ON
even ...

 + if (WARN_ON(!crtc_state))
 + return 0;
 +
 + if (!crtc_state-enable) {
 + DRM_DEBUG_ATOMIC(Invisible: crtc off\n);
  
 - /* FIXME: temporary hack necessary while we still use the plane update
 -  * helper. */
 - if (state-state) {
 - crtc_state =
 - intel_atomic_get_crtc_state(state-state, intel_crtc);
 - if (IS_ERR(crtc_state))
 - return PTR_ERR(crtc_state);
 - } else {
 - crtc_state = intel_crtc-config;
 + /*
 +  * Probably allowed after converting to atomic. Right
 +  * now it probably means we have the state confused.
 +  */
 + I915_STATE_WARN_ON(plane-type == DRM_PLANE_TYPE_PRIMARY);

This is already possible with the primary plane support - you can disable
it (though not change the mode at the same time).

 + return 0;
 + }
 +
 + if (!crtc_state-active) {
 + DRM_DEBUG_ATOMIC(Invisible: dpms off\n);
 + return 0;
   }
  
   /*
 @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane 
 *plane,
   /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
   intel_state-clip.x1 = 0;
   intel_state-clip.y1 = 0;
 - intel_state-clip.x2 =
 - crtc_state-base.active ? crtc_state-pipe_src_w : 0;
 - intel_state-clip.y2 =
 - crtc_state-base.active ? crtc_state-pipe_src_h : 0;
 -
 - /*
 -  * Disabling a plane is always okay; we just need to update
 -  * fb tracking in a special way since cleanup_fb() won't
 -  * get called by the plane helpers.
 -  */
 - if (state-fb == NULL  plane-state-fb != NULL) {
 - /*
 -  * 'prepare' is never called when plane is being disabled, so
 -  * we need to handle frontbuffer tracking as a special case
 -  */
 - intel_crtc-atomic.disabled_planes |=
 - (1  drm_plane_index(plane));
 - }
 + drm_crtc_get_hv_timing(crtc_state-mode,
 +intel_state-clip.x2,
 +intel_state-clip.y2);

Imo this is obfuscating things a bit, why not just unconditionally copy
pipe_src_w/h to clip.x/y2? get_hv_timing is for the pipe size, which on
most platforms must match the primary plane window except for gen2 and
gen9+.

  
   if (state-fb  intel_rotation_90_or_270(state-rotation)) {
   if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 diff --git 

Re: [Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-12 Thread Ville Syrjälä
On Tue, May 12, 2015 at 03:33:12PM +0200, Maarten Lankhorst wrote:
 Op 12-05-15 om 10:18 schreef Daniel Vetter:
  On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
  @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane 
  *plane,
 /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
 intel_state-clip.x1 = 0;
 intel_state-clip.y1 = 0;
  -  intel_state-clip.x2 =
  -  crtc_state-base.active ? crtc_state-pipe_src_w : 0;
  -  intel_state-clip.y2 =
  -  crtc_state-base.active ? crtc_state-pipe_src_h : 0;
  -
  -  /*
  -   * Disabling a plane is always okay; we just need to update
  -   * fb tracking in a special way since cleanup_fb() won't
  -   * get called by the plane helpers.
  -   */
  -  if (state-fb == NULL  plane-state-fb != NULL) {
  -  /*
  -   * 'prepare' is never called when plane is being disabled, so
  -   * we need to handle frontbuffer tracking as a special case
  -   */
  -  intel_crtc-atomic.disabled_planes |=
  -  (1  drm_plane_index(plane));
  -  }
  +  drm_crtc_get_hv_timing(crtc_state-mode,
  + intel_state-clip.x2,
  + intel_state-clip.y2);
  Imo this is obfuscating things a bit, why not just unconditionally copy
  pipe_src_w/h to clip.x/y2? get_hv_timing is for the pipe size, which on
  most platforms must match the primary plane window except for gen2 and
  gen9+.
 pipe_src_* is calculated in the same way,

Except we may end up rounding pipe_src_w down if we have
double-wide/dual link lvds etc., and we definitely need to clip the
planes against the real pipe src size.

 and at the time of plane validation the crtc validation hasn't run yet,
 so pipe_src_* contains outdated values which break in interesting ways.

Just check crtc first. Or actually we probably need pre + post crtc
checks since we want to do wm compute and such after the planes have
been handled.

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


Re: [Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-12 Thread Ville Syrjälä
On Tue, May 12, 2015 at 04:43:09PM +0300, Ville Syrjälä wrote:
 On Tue, May 12, 2015 at 03:33:12PM +0200, Maarten Lankhorst wrote:
  Op 12-05-15 om 10:18 schreef Daniel Vetter:
   On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
   @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct 
   drm_plane *plane,
/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
intel_state-clip.x1 = 0;
intel_state-clip.y1 = 0;
   -intel_state-clip.x2 =
   -crtc_state-base.active ? crtc_state-pipe_src_w : 0;
   -intel_state-clip.y2 =
   -crtc_state-base.active ? crtc_state-pipe_src_h : 0;
   -
   -/*
   - * Disabling a plane is always okay; we just need to update
   - * fb tracking in a special way since cleanup_fb() won't
   - * get called by the plane helpers.
   - */
   -if (state-fb == NULL  plane-state-fb != NULL) {
   -/*
   - * 'prepare' is never called when plane is being 
   disabled, so
   - * we need to handle frontbuffer tracking as a special 
   case
   - */
   -intel_crtc-atomic.disabled_planes |=
   -(1  drm_plane_index(plane));
   -}
   +drm_crtc_get_hv_timing(crtc_state-mode,
   +   intel_state-clip.x2,
   +   intel_state-clip.y2);
   Imo this is obfuscating things a bit, why not just unconditionally copy
   pipe_src_w/h to clip.x/y2? get_hv_timing is for the pipe size, which on
   most platforms must match the primary plane window except for gen2 and
   gen9+.
  pipe_src_* is calculated in the same way,
 
 Except we may end up rounding pipe_src_w down if we have
 double-wide/dual link lvds etc., and we definitely need to clip the
 planes against the real pipe src size.

Oh and we definitely want to keep the 'user mode == pipe src'
assumptions to a minimum so that we might some day finish the expose
panel fitter to userland task.

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


Re: [Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-12 Thread Maarten Lankhorst
Op 12-05-15 om 10:18 schreef Daniel Vetter:
 On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
 This kills off most of the transitional helpers and uses atomic plane updates
 in the modeset path to update everything.

 Getting rid of the transitional plane helpers meant that planes had to be 
 added
 in the crtc check function. On modeset a connector can be moved to a 
 different
 crtc, and this is not handled correctly otherwise.

 Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
 Ok pile of comments on this one. I don't yet fully grasp it all, but at
 the very bottom I've jotted down a list of ideas for how to move forward
 with this one.

 ---
  drivers/gpu/drm/i915/intel_atomic_plane.c |  59 ++-
  drivers/gpu/drm/i915/intel_display.c  | 655 
 ++
  drivers/gpu/drm/i915/intel_drv.h  |   2 +-
  drivers/gpu/drm/i915/intel_sprite.c   |  80 +---
  4 files changed, 441 insertions(+), 355 deletions(-)

 diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
 b/drivers/gpu/drm/i915/intel_atomic_plane.c
 index 86ba4b2c3a65..85b87e4d4b6e 100644
 --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
 +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
 @@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane 
 *plane,
  struct drm_plane_state *state)
  {
  struct drm_crtc *crtc = state-crtc;
 -struct intel_crtc *intel_crtc;
 -struct intel_crtc_state *crtc_state;
 +struct drm_crtc_state *crtc_state;
  struct intel_plane *intel_plane = to_intel_plane(plane);
  struct intel_plane_state *intel_state = to_intel_plane_state(state);
  
 -crtc = crtc ? crtc : plane-crtc;
 -intel_crtc = to_intel_crtc(crtc);
 -
 +intel_state-visible = false;
  /*
   * Both crtc and plane-crtc could be NULL if we're updating a
   * property while the plane is disabled.  We don't actually have
   * anything driver-specific we need to test in that case, so
   * just return success.
   */
 -if (!crtc)
 +if (!crtc) {
 +DRM_DEBUG_ATOMIC(Invisible: no crtc\n);
  return 0;
 +}
 +
 +crtc_state = state-state-crtc_states[drm_crtc_index(crtc)];
 Please reuse drm_atomic_get_crtc_state, this deref magic is hard to read.
 Maybe we should have a nofail variant of those to encode the below WARN_ON
 even ...
Yeah, there are a few places where I use it like this, because in those cases 
the relevant state should already exist.
 +if (WARN_ON(!crtc_state))
 +return 0;
 +
 +if (!crtc_state-enable) {
 +DRM_DEBUG_ATOMIC(Invisible: crtc off\n);
  
 -/* FIXME: temporary hack necessary while we still use the plane update
 - * helper. */
 -if (state-state) {
 -crtc_state =
 -intel_atomic_get_crtc_state(state-state, intel_crtc);
 -if (IS_ERR(crtc_state))
 -return PTR_ERR(crtc_state);
 -} else {
 -crtc_state = intel_crtc-config;
 +/*
 + * Probably allowed after converting to atomic. Right
 + * now it probably means we have the state confused.
 + */
 +I915_STATE_WARN_ON(plane-type == DRM_PLANE_TYPE_PRIMARY);
 This is already possible with the primary plane support - you can disable
 it (though not change the mode at the same time).
That explains a few warnings, thanks. :-)
 +return 0;
 +}
 +
 +if (!crtc_state-active) {
 +DRM_DEBUG_ATOMIC(Invisible: dpms off\n);
 +return 0;
  }
  
  /*
 @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane 
 *plane,
  /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
  intel_state-clip.x1 = 0;
  intel_state-clip.y1 = 0;
 -intel_state-clip.x2 =
 -crtc_state-base.active ? crtc_state-pipe_src_w : 0;
 -intel_state-clip.y2 =
 -crtc_state-base.active ? crtc_state-pipe_src_h : 0;
 -
 -/*
 - * Disabling a plane is always okay; we just need to update
 - * fb tracking in a special way since cleanup_fb() won't
 - * get called by the plane helpers.
 - */
 -if (state-fb == NULL  plane-state-fb != NULL) {
 -/*
 - * 'prepare' is never called when plane is being disabled, so
 - * we need to handle frontbuffer tracking as a special case
 - */
 -intel_crtc-atomic.disabled_planes |=
 -(1  drm_plane_index(plane));
 -}
 +drm_crtc_get_hv_timing(crtc_state-mode,
 +   intel_state-clip.x2,
 +   intel_state-clip.y2);
 Imo this is obfuscating things a bit, why not just unconditionally copy
 pipe_src_w/h to clip.x/y2? get_hv_timing is for the pipe size, which on
 most platforms must match the primary plane window except for gen2 and
 gen9+.
pipe_src_* is calculated in the same way, and 

Re: [Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-12 Thread Daniel Vetter
On Tue, May 12, 2015 at 04:43:09PM +0300, Ville Syrjälä wrote:
 On Tue, May 12, 2015 at 03:33:12PM +0200, Maarten Lankhorst wrote:
  Op 12-05-15 om 10:18 schreef Daniel Vetter:
   On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
   @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct 
   drm_plane *plane,
/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
intel_state-clip.x1 = 0;
intel_state-clip.y1 = 0;
   -intel_state-clip.x2 =
   -crtc_state-base.active ? crtc_state-pipe_src_w : 0;
   -intel_state-clip.y2 =
   -crtc_state-base.active ? crtc_state-pipe_src_h : 0;
   -
   -/*
   - * Disabling a plane is always okay; we just need to update
   - * fb tracking in a special way since cleanup_fb() won't
   - * get called by the plane helpers.
   - */
   -if (state-fb == NULL  plane-state-fb != NULL) {
   -/*
   - * 'prepare' is never called when plane is being 
   disabled, so
   - * we need to handle frontbuffer tracking as a special 
   case
   - */
   -intel_crtc-atomic.disabled_planes |=
   -(1  drm_plane_index(plane));
   -}
   +drm_crtc_get_hv_timing(crtc_state-mode,
   +   intel_state-clip.x2,
   +   intel_state-clip.y2);
   Imo this is obfuscating things a bit, why not just unconditionally copy
   pipe_src_w/h to clip.x/y2? get_hv_timing is for the pipe size, which on
   most platforms must match the primary plane window except for gen2 and
   gen9+.
  pipe_src_* is calculated in the same way,
 
 Except we may end up rounding pipe_src_w down if we have
 double-wide/dual link lvds etc., and we definitely need to clip the
 planes against the real pipe src size.
 
  and at the time of plane validation the crtc validation hasn't run yet,
  so pipe_src_* contains outdated values which break in interesting ways.
 
 Just check crtc first. Or actually we probably need pre + post crtc
 checks since we want to do wm compute and such after the planes have
 been handled.

Yeah in the end the sequence should be:
1. compute pipe_config for modeset changes on any crtcs where mode_changed
   is set.
2. call helper_check_planes which should use the values computed in step
   1. to compute the nuclear plane flip states. It's important that the
   depencies flow one-way since only then can we skip all the modeset
   state recomputation (and possible serialization because we need more
   crtc states) for pure plane flips.

Where exactly do the pipe_src_w/h values get out of sync here? Imo better
to patch them up someplace in the legacy modeset code than add hacks to
plane atomic functions.

Cheers, 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: [Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-12 Thread Daniel Vetter
On Tue, May 12, 2015 at 03:33:12PM +0200, Maarten Lankhorst wrote:
 Op 12-05-15 om 10:18 schreef Daniel Vetter:
  On Mon, May 11, 2015 at 04:24:46PM +0200, Maarten Lankhorst wrote:
  This kills off most of the transitional helpers and uses atomic plane 
  updates
  in the modeset path to update everything.
 
  Getting rid of the transitional plane helpers meant that planes had to be 
  added
  in the crtc check function. On modeset a connector can be moved to a 
  different
  crtc, and this is not handled correctly otherwise.
 
  Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
  Ok pile of comments on this one. I don't yet fully grasp it all, but at
  the very bottom I've jotted down a list of ideas for how to move forward
  with this one.
 
  ---
   drivers/gpu/drm/i915/intel_atomic_plane.c |  59 ++-
   drivers/gpu/drm/i915/intel_display.c  | 655 
  ++
   drivers/gpu/drm/i915/intel_drv.h  |   2 +-
   drivers/gpu/drm/i915/intel_sprite.c   |  80 +---
   4 files changed, 441 insertions(+), 355 deletions(-)
 
  diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
  b/drivers/gpu/drm/i915/intel_atomic_plane.c
  index 86ba4b2c3a65..85b87e4d4b6e 100644
  --- a/drivers/gpu/drm/i915/intel_atomic_plane.c
  +++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
  @@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane 
  *plane,
 struct drm_plane_state *state)
   {
 struct drm_crtc *crtc = state-crtc;
  -  struct intel_crtc *intel_crtc;
  -  struct intel_crtc_state *crtc_state;
  +  struct drm_crtc_state *crtc_state;
 struct intel_plane *intel_plane = to_intel_plane(plane);
 struct intel_plane_state *intel_state = to_intel_plane_state(state);
   
  -  crtc = crtc ? crtc : plane-crtc;
  -  intel_crtc = to_intel_crtc(crtc);
  -
  +  intel_state-visible = false;
 /*
  * Both crtc and plane-crtc could be NULL if we're updating a
  * property while the plane is disabled.  We don't actually have
  * anything driver-specific we need to test in that case, so
  * just return success.
  */
  -  if (!crtc)
  +  if (!crtc) {
  +  DRM_DEBUG_ATOMIC(Invisible: no crtc\n);
 return 0;
  +  }
  +
  +  crtc_state = state-state-crtc_states[drm_crtc_index(crtc)];
  Please reuse drm_atomic_get_crtc_state, this deref magic is hard to read.
  Maybe we should have a nofail variant of those to encode the below WARN_ON
  even ...
 Yeah, there are a few places where I use it like this, because in those cases 
 the relevant state should already exist.

Imo get_foo_state + WARN_ON(IS_ERR) is better than open-coding it. It's a
tricky deref chain and I've gotten it wrong multiple times when writing
atomic helpers.

  +  if (WARN_ON(!crtc_state))
  +  return 0;
  +
  +  if (!crtc_state-enable) {
  +  DRM_DEBUG_ATOMIC(Invisible: crtc off\n);
   
  -  /* FIXME: temporary hack necessary while we still use the plane update
  -   * helper. */
  -  if (state-state) {
  -  crtc_state =
  -  intel_atomic_get_crtc_state(state-state, intel_crtc);
  -  if (IS_ERR(crtc_state))
  -  return PTR_ERR(crtc_state);
  -  } else {
  -  crtc_state = intel_crtc-config;
  +  /*
  +   * Probably allowed after converting to atomic. Right
  +   * now it probably means we have the state confused.
  +   */
  +  I915_STATE_WARN_ON(plane-type == DRM_PLANE_TYPE_PRIMARY);
  This is already possible with the primary plane support - you can disable
  it (though not change the mode at the same time).
 That explains a few warnings, thanks. :-)
  +  return 0;
  +  }
  +
  +  if (!crtc_state-active) {
  +  DRM_DEBUG_ATOMIC(Invisible: dpms off\n);
  +  return 0;
 }
   
 /*
  @@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane 
  *plane,
 /* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
 intel_state-clip.x1 = 0;
 intel_state-clip.y1 = 0;
  -  intel_state-clip.x2 =
  -  crtc_state-base.active ? crtc_state-pipe_src_w : 0;
  -  intel_state-clip.y2 =
  -  crtc_state-base.active ? crtc_state-pipe_src_h : 0;
  -
  -  /*
  -   * Disabling a plane is always okay; we just need to update
  -   * fb tracking in a special way since cleanup_fb() won't
  -   * get called by the plane helpers.
  -   */
  -  if (state-fb == NULL  plane-state-fb != NULL) {
  -  /*
  -   * 'prepare' is never called when plane is being disabled, so
  -   * we need to handle frontbuffer tracking as a special case
  -   */
  -  intel_crtc-atomic.disabled_planes |=
  -  (1  drm_plane_index(plane));
  -  }
  +  drm_crtc_get_hv_timing(crtc_state-mode,
  + intel_state-clip.x2,
  + intel_state-clip.y2);
  Imo this is obfuscating things a bit, why not just 

[Intel-gfx] [PATCH 10/42] drm/i915: make plane helpers fully atomic

2015-05-11 Thread Maarten Lankhorst
This kills off most of the transitional helpers and uses atomic plane updates
in the modeset path to update everything.

Getting rid of the transitional plane helpers meant that planes had to be added
in the crtc check function. On modeset a connector can be moved to a different
crtc, and this is not handled correctly otherwise.

Signed-off-by: Maarten Lankhorst maarten.lankho...@linux.intel.com
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  59 ++-
 drivers/gpu/drm/i915/intel_display.c  | 655 ++
 drivers/gpu/drm/i915/intel_drv.h  |   2 +-
 drivers/gpu/drm/i915/intel_sprite.c   |  80 +---
 4 files changed, 441 insertions(+), 355 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c 
b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 86ba4b2c3a65..85b87e4d4b6e 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -110,32 +110,40 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
struct drm_plane_state *state)
 {
struct drm_crtc *crtc = state-crtc;
-   struct intel_crtc *intel_crtc;
-   struct intel_crtc_state *crtc_state;
+   struct drm_crtc_state *crtc_state;
struct intel_plane *intel_plane = to_intel_plane(plane);
struct intel_plane_state *intel_state = to_intel_plane_state(state);
 
-   crtc = crtc ? crtc : plane-crtc;
-   intel_crtc = to_intel_crtc(crtc);
-
+   intel_state-visible = false;
/*
 * Both crtc and plane-crtc could be NULL if we're updating a
 * property while the plane is disabled.  We don't actually have
 * anything driver-specific we need to test in that case, so
 * just return success.
 */
-   if (!crtc)
+   if (!crtc) {
+   DRM_DEBUG_ATOMIC(Invisible: no crtc\n);
return 0;
+   }
+
+   crtc_state = state-state-crtc_states[drm_crtc_index(crtc)];
+   if (WARN_ON(!crtc_state))
+   return 0;
+
+   if (!crtc_state-enable) {
+   DRM_DEBUG_ATOMIC(Invisible: crtc off\n);
 
-   /* FIXME: temporary hack necessary while we still use the plane update
-* helper. */
-   if (state-state) {
-   crtc_state =
-   intel_atomic_get_crtc_state(state-state, intel_crtc);
-   if (IS_ERR(crtc_state))
-   return PTR_ERR(crtc_state);
-   } else {
-   crtc_state = intel_crtc-config;
+   /*
+* Probably allowed after converting to atomic. Right
+* now it probably means we have the state confused.
+*/
+   I915_STATE_WARN_ON(plane-type == DRM_PLANE_TYPE_PRIMARY);
+   return 0;
+   }
+
+   if (!crtc_state-active) {
+   DRM_DEBUG_ATOMIC(Invisible: dpms off\n);
+   return 0;
}
 
/*
@@ -155,24 +163,9 @@ static int intel_plane_atomic_check(struct drm_plane 
*plane,
/* Clip all planes to CRTC size, or 0x0 if CRTC is disabled */
intel_state-clip.x1 = 0;
intel_state-clip.y1 = 0;
-   intel_state-clip.x2 =
-   crtc_state-base.active ? crtc_state-pipe_src_w : 0;
-   intel_state-clip.y2 =
-   crtc_state-base.active ? crtc_state-pipe_src_h : 0;
-
-   /*
-* Disabling a plane is always okay; we just need to update
-* fb tracking in a special way since cleanup_fb() won't
-* get called by the plane helpers.
-*/
-   if (state-fb == NULL  plane-state-fb != NULL) {
-   /*
-* 'prepare' is never called when plane is being disabled, so
-* we need to handle frontbuffer tracking as a special case
-*/
-   intel_crtc-atomic.disabled_planes |=
-   (1  drm_plane_index(plane));
-   }
+   drm_crtc_get_hv_timing(crtc_state-mode,
+  intel_state-clip.x2,
+  intel_state-clip.y2);
 
if (state-fb  intel_rotation_90_or_270(state-rotation)) {
if (!(state-fb-modifier[0] == I915_FORMAT_MOD_Y_TILED ||
diff --git a/drivers/gpu/drm/i915/intel_display.c 
b/drivers/gpu/drm/i915/intel_display.c
index 956c9964275d..9610f76a2489 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -100,14 +100,16 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
const struct intel_crtc_state *pipe_config);
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+  struct drm_crtc_state *crtc_state);
 static void intel_begin_crtc_commit(struct drm_crtc *crtc);
 static void intel_finish_crtc_commit(struct drm_crtc *crtc);
 static