[PATCH 00/35] Make legacy modeset a lot more atomic-like

[PATCH 00/35] Make legacy modeset a lot more atomic-like

[Ander Conselvan de Oliveira]

Hi,

This patch series changes the legacy modeset path to be a lot more
atomic like. Among other things, it

 - unifies the flip-only and the modeset path;
 - implements a full state swap as part of the modeset;
 - gets rid of the recovery logic in case of a failed modeset;
 - replaces some i915 functions with drm atomic helper ones.

Some of these patches were sent previously and the feedback so far has
been addressed. I also rebased this on top Maarten's series that
simplifies plane enabling/disabling.

Thanks,
Ander

Ander Conselvan de Oliveira (28):
  drm/i915: Don't check for NULL before freeing state
  drm/i915: Call drm helpers when duplicating crtc and plane states
  drm/i915: Use for_each_connector_in_state helper macro
  drm/i915: Extract mode_changed computation out of
    stage_output_config()
  drm/i915: Add crtc states before calling compute_config()
  drm/i915: Don't pretend we can calculate multiple pipe_configs
  drm/i915: Calculate a new pipe_config based on new enabled state
  drm/i915: Remove all *_pipes flags from modeset
  drm/i915: Remove saved_mode from __intel_set_mode()
  drm/i915: Move compute part of __intel_set_mode() to separate function
  drm/i915: Simplify error handling in __intel_set_mode()
  drm/i915: Don't modeset with old mode when set_crtc fails
  drm/i915: Add primary plane to atomic state in legacy modeset
  drm/i915: Delete fb, x and y parameters from mode set functions
  drm/i915: Don't use struct intel_set_config *_changed flags
  drm/i915: Don't use staged config to calculate mode_changed flags
  drm/i915: Unify modeset and flip paths of intel_crtc_set_config()
  drm/i915: Simplify intel_set_config_compute_mode_changes() a bit
  drm/i915: Stage new modeset state straight into atomic state
  drm/i915: Remove save/restore logic from intel_crtc_set_config()
  drm/i915: Update crtc state active flag based on DPMS
  drm/atomic: Make mode_fixup() optional for check_modeset()
  drm/i915: Use atomic helpers for computing changed flags
  drm/i915: Take ownership of atomic state on success in
    intel_set_mode()
  drm/i915: Preserve shared DPLL information in new pipe_config
  drm/i915: Don't use plane update helper in legacy mode set
  drm/i915: Swap atomic state in legacy modeset
  drm/i915: Get rid of intel_crtc_set_state()

Maarten Lankhorst (7):
  drm/i915: Remove implicitly disabling primary plane for now
  drm/i915: Add a way to disable planes without updating state
  drm/i915: Use the disable callback for disabling planes.
  drm/i915: get rid of primary_enabled and use atomic state
  drm/i915: Move intel_(pre_disable/post_enable)_primary to
    intel_display.c, and use it there.
  drm/i915: Rename intel_crtc_dpms_overlay.
  drm/i915: Move toggling planes out of crtc enable/disable.

 drivers/gpu/drm/drm_atomic_helper.c       |    5 +
 drivers/gpu/drm/i915/i915_debugfs.c       |    6 +-
 drivers/gpu/drm/i915/i915_drv.h           |    5 +
 drivers/gpu/drm/i915/intel_atomic.c       |    8 +-
 drivers/gpu/drm/i915/intel_atomic_plane.c |   20 +-
 drivers/gpu/drm/i915/intel_ddi.c          |    9 +-
 drivers/gpu/drm/i915/intel_display.c      | 1491 ++++++++++++-----------------
 drivers/gpu/drm/i915/intel_dp_mst.c       |   13 +-
 drivers/gpu/drm/i915/intel_drv.h          |   21 +-
 drivers/gpu/drm/i915/intel_fbc.c          |    2 +-
 drivers/gpu/drm/i915/intel_hdmi.c         |    7 +-
 drivers/gpu/drm/i915/intel_sprite.c       |  127 +--
 12 files changed, 683 insertions(+), 1031 deletions(-)

-- 
2.1.0

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

[PATCH 01/35] drm/i915: Remove implicitly disabling primary plane for now

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Some of the flags that were used are still useful when transitioning
to atomic, so keep those around for now. This removes some of the
complications of crtc->primary_enabled, making it easier to remove.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_drv.h    |  6 -----
 drivers/gpu/drm/i915/intel_sprite.c | 45 +------------------------------------
 2 files changed, 1 insertion(+), 50 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 23a42a4..9fb2d0b 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -252,12 +252,6 @@ struct intel_plane_state {
 	bool visible;
 
 	/*
-	 * used only for sprite planes to determine when to implicitly
-	 * enable/disable the primary plane
-	 */
-	bool hides_primary;
-
-	/*
 	 * scaler_id
 	 *    = -1 : not using a scaler
 	 *    >=  0 : using a scalers
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index e3d41c0..612d8e0 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -165,17 +165,6 @@ void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count)
 			  pipe_name(pipe), start_vbl_count, end_vbl_count);
 }
 
-static void intel_update_primary_plane(struct intel_crtc *crtc)
-{
-	struct drm_i915_private *dev_priv = crtc->base.dev->dev_private;
-	int reg = DSPCNTR(crtc->plane);
-
-	if (crtc->primary_enabled)
-		I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
-	else
-		I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
-}
-
 static void
 skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 		 struct drm_framebuffer *fb,
@@ -479,8 +468,6 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
 	}
 
-	intel_update_primary_plane(intel_crtc);
-
 	if (key->flags) {
 		I915_WRITE(SPKEYMINVAL(pipe, plane), key->min_value);
 		I915_WRITE(SPKEYMAXVAL(pipe, plane), key->max_value);
@@ -521,8 +508,6 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
 	int pipe = intel_plane->pipe;
 	int plane = intel_plane->plane;
 
-	intel_update_primary_plane(intel_crtc);
-
 	I915_WRITE(SPCNTR(pipe, plane), 0);
 
 	/* Activate double buffered register update */
@@ -626,8 +611,6 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		}
 	}
 
-	intel_update_primary_plane(intel_crtc);
-
 	if (key->flags) {
 		I915_WRITE(SPRKEYVAL(pipe), key->min_value);
 		I915_WRITE(SPRKEYMAX(pipe), key->max_value);
@@ -670,8 +653,6 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 
-	intel_update_primary_plane(intel_crtc);
-
 	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
 	/* Can't leave the scaler enabled... */
 	if (intel_plane->can_scale)
@@ -766,8 +747,6 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 		linear_offset += src_h * fb->pitches[0] + src_w * pixel_size;
 	}
 
-	intel_update_primary_plane(intel_crtc);
-
 	if (key->flags) {
 		I915_WRITE(DVSKEYVAL(pipe), key->min_value);
 		I915_WRITE(DVSKEYMAX(pipe), key->max_value);
@@ -805,8 +784,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_plane->pipe;
 
-	intel_update_primary_plane(intel_crtc);
-
 	I915_WRITE(DVSCNTR(pipe), 0);
 	/* Disable the scaler */
 	I915_WRITE(DVSSCALE(pipe), 0);
@@ -859,7 +836,7 @@ intel_post_enable_primary(struct drm_crtc *crtc)
  * @crtc: the CRTC whose primary plane is to be disabled
  *
  * Performs potentially sleeping operations that must be done before the
- * primary plane is enabled, such as updating FBC and IPS.  Note that this may
+ * primary plane is disabled, such as updating FBC and IPS.  Note that this may
  * be called due to an explicit primary plane update, or due to an implicit
  * disable that is caused when a sprite plane completely hides the primary
  * plane.
@@ -885,11 +862,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 }
 
-static bool colorkey_enabled(struct intel_plane *intel_plane)
-{
-	return intel_plane->ckey.flags != I915_SET_COLORKEY_NONE;
-}
-
 static int
 intel_check_sprite_plane(struct drm_plane *plane,
 			 struct intel_plane_state *state)
@@ -1053,23 +1025,10 @@ finish:
 	 * If the sprite is completely covering the primary plane,
 	 * we can disable the primary and save power.
 	 */
-	state->hides_primary = fb != NULL && drm_rect_equals(dst, clip) &&
-		!colorkey_enabled(intel_plane);
-	WARN_ON(state->hides_primary && !state->visible && intel_crtc->active);
-
 	if (intel_crtc->active) {
-		if (intel_crtc->primary_enabled == state->hides_primary)
-			intel_crtc->atomic.wait_for_flips = true;
-
-		if (intel_crtc->primary_enabled && state->hides_primary)
-			intel_crtc->atomic.pre_disable_primary = true;
-
 		intel_crtc->atomic.fb_bits |=
 			INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
 
-		if (!intel_crtc->primary_enabled && !state->hides_primary)
-			intel_crtc->atomic.post_enable_primary = true;
-
 		if (intel_wm_need_update(plane, &state->base))
 			intel_crtc->atomic.update_wm = true;
 
@@ -1105,8 +1064,6 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 	plane->fb = fb;
 
 	if (intel_crtc->active) {
-		intel_crtc->primary_enabled = !state->hides_primary;
-
 		if (state->visible) {
 			crtc_x = state->dst.x1;
 			crtc_y = state->dst.y1;
-- 
2.1.0

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

[PATCH 02/35] drm/i915: Add a way to disable planes without updating state

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This is used by the next commit to disable all planes on a crtc
without caring what type it is.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 38 +++++++++++++++++++++++++++++++++---
 drivers/gpu/drm/i915/intel_drv.h     |  2 +-
 drivers/gpu/drm/i915/intel_sprite.c  | 16 +++++++--------
 3 files changed, 44 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3094b08..97db75b 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2694,7 +2694,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	u32 reg = DSPCNTR(plane);
 	int pixel_size;
 
-	if (!intel_crtc->primary_enabled) {
+	if (!intel_crtc->primary_enabled || !fb) {
 		I915_WRITE(reg, 0);
 		if (INTEL_INFO(dev)->gen >= 4)
 			I915_WRITE(DSPSURF(plane), 0);
@@ -2823,7 +2823,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	u32 reg = DSPCNTR(plane);
 	int pixel_size;
 
-	if (!intel_crtc->primary_enabled) {
+	if (!intel_crtc->primary_enabled || !fb) {
 		I915_WRITE(reg, 0);
 		I915_WRITE(DSPSURF(plane), 0);
 		POSTING_READ(reg);
@@ -3001,7 +3001,7 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	unsigned long surf_addr;
 	struct drm_plane *plane;
 
-	if (!intel_crtc->primary_enabled) {
+	if (!intel_crtc->primary_enabled || !fb) {
 		I915_WRITE(PLANE_CTL(pipe, 0), 0);
 		I915_WRITE(PLANE_SURF(pipe, 0), 0);
 		POSTING_READ(PLANE_CTL(pipe, 0));
@@ -13245,6 +13245,20 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	}
 }
 
+static void
+intel_disable_primary_plane(struct drm_plane *plane,
+			    struct drm_crtc *crtc,
+			    bool force)
+{
+	struct drm_device *dev = plane->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+
+	if (!force)
+		to_intel_crtc(crtc)->primary_enabled = false;
+
+	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
+}
+
 static void intel_begin_crtc_commit(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -13386,6 +13400,7 @@ static struct drm_plane *intel_primary_plane_create(struct drm_device *dev,
 	primary->plane = pipe;
 	primary->check_plane = intel_check_primary_plane;
 	primary->commit_plane = intel_commit_primary_plane;
+	primary->disable_plane = intel_disable_primary_plane;
 	primary->ckey.flags = I915_SET_COLORKEY_NONE;
 	if (HAS_FBC(dev) && INTEL_INFO(dev)->gen < 4)
 		primary->plane = !pipe;
@@ -13491,6 +13506,22 @@ finish:
 }
 
 static void
+intel_disable_cursor_plane(struct drm_plane *plane,
+			   struct drm_crtc *crtc,
+			   bool force)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	if (!force) {
+		plane->fb = NULL;
+		intel_crtc->cursor_bo = NULL;
+		intel_crtc->cursor_addr = 0;
+	}
+
+	intel_crtc_update_cursor(crtc, false);
+}
+
+static void
 intel_commit_cursor_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
 {
@@ -13549,6 +13580,7 @@ static struct drm_plane *intel_cursor_plane_create(struct drm_device *dev,
 	state->scaler_id = -1;
 	cursor->check_plane = intel_check_cursor_plane;
 	cursor->commit_plane = intel_commit_cursor_plane;
+	cursor->disable_plane = intel_disable_cursor_plane;
 
 	drm_universal_plane_init(dev, &cursor->base, 0,
 				 &intel_plane_funcs,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 9fb2d0b..42548bc 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -597,7 +597,7 @@ struct intel_plane {
 			     uint32_t x, uint32_t y,
 			     uint32_t src_w, uint32_t src_h);
 	void (*disable_plane)(struct drm_plane *plane,
-			      struct drm_crtc *crtc);
+			      struct drm_crtc *crtc, bool force);
 	int (*check_plane)(struct drm_plane *plane,
 			   struct intel_plane_state *state);
 	void (*commit_plane)(struct drm_plane *plane,
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 612d8e0..6316454 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -313,11 +313,11 @@ skl_update_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc,
 }
 
 static void
-skl_disable_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc)
+skl_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc, bool force)
 {
-	struct drm_device *dev = drm_plane->dev;
+	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_plane *intel_plane = to_intel_plane(drm_plane);
+	struct intel_plane *intel_plane = to_intel_plane(dplane);
 	const int pipe = intel_plane->pipe;
 	const int plane = intel_plane->plane + 1;
 
@@ -327,7 +327,7 @@ skl_disable_plane(struct drm_plane *drm_plane, struct drm_crtc *crtc)
 	I915_WRITE(PLANE_SURF(pipe, plane), 0);
 	POSTING_READ(PLANE_SURF(pipe, plane));
 
-	intel_update_sprite_watermarks(drm_plane, crtc, 0, 0, 0, false, false);
+	intel_update_sprite_watermarks(dplane, crtc, 0, 0, 0, false, false);
 }
 
 static void
@@ -499,7 +499,7 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
 }
 
 static void
-vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
+vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc, bool force)
 {
 	struct drm_device *dev = dplane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -645,7 +645,7 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 }
 
 static void
-ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc, bool force)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -776,7 +776,7 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
 }
 
 static void
-ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
+ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc, bool force)
 {
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
@@ -1077,7 +1077,7 @@ intel_commit_sprite_plane(struct drm_plane *plane,
 						  crtc_x, crtc_y, crtc_w, crtc_h,
 						  src_x, src_y, src_w, src_h);
 		} else {
-			intel_plane->disable_plane(plane, crtc);
+			intel_plane->disable_plane(plane, crtc, false);
 		}
 	}
 }
-- 
2.1.0

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

[PATCH 03/35] drm/i915: Use the disable callback for disabling planes.

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This allows disabling all planes affecting a crtc without caring what type it is.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_drv.h      |  5 ++
 drivers/gpu/drm/i915/intel_display.c | 91 ++++++------------------------------
 2 files changed, 20 insertions(+), 76 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index f5020a8..32439ae 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -238,6 +238,11 @@ enum hpd_pin {
 #define for_each_crtc(dev, crtc) \
 	list_for_each_entry(crtc, &dev->mode_config.crtc_list, head)
 
+#define for_each_intel_plane(dev, intel_plane) \
+	list_for_each_entry(intel_plane,			\
+			    &dev->mode_config.plane_list,	\
+			    base.head)
+
 #define for_each_intel_crtc(dev, intel_crtc) \
 	list_for_each_entry(intel_crtc, &dev->mode_config.crtc_list, base.head)
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 97db75b..acf3494 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2254,32 +2254,6 @@ static void intel_enable_primary_hw_plane(struct drm_plane *plane,
 		intel_wait_for_vblank(dev, intel_crtc->pipe);
 }
 
-/**
- * intel_disable_primary_hw_plane - disable the primary hardware plane
- * @plane: plane to be disabled
- * @crtc: crtc for the plane
- *
- * Disable @plane on @crtc, making sure that the pipe is running first.
- */
-static void intel_disable_primary_hw_plane(struct drm_plane *plane,
-					   struct drm_crtc *crtc)
-{
-	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	if (WARN_ON(!intel_crtc->active))
-		return;
-
-	if (!intel_crtc->primary_enabled)
-		return;
-
-	intel_crtc->primary_enabled = false;
-
-	dev_priv->display.update_primary_plane(crtc, plane->fb,
-					       crtc->x, crtc->y);
-}
-
 static bool need_vtd_wa(struct drm_device *dev)
 {
 #ifdef CONFIG_INTEL_IOMMU
@@ -4561,38 +4535,6 @@ static void intel_enable_sprite_planes(struct drm_crtc *crtc)
 	}
 }
 
-/*
- * Disable a plane internally without actually modifying the plane's state.
- * This will allow us to easily restore the plane later by just reprogramming
- * its state.
- */
-static void disable_plane_internal(struct drm_plane *plane)
-{
-	struct intel_plane *intel_plane = to_intel_plane(plane);
-	struct drm_plane_state *state =
-		plane->funcs->atomic_duplicate_state(plane);
-	struct intel_plane_state *intel_state = to_intel_plane_state(state);
-
-	intel_state->visible = false;
-	intel_plane->commit_plane(plane, intel_state);
-
-	intel_plane_destroy_state(plane, state);
-}
-
-static void intel_disable_sprite_planes(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	enum pipe pipe = to_intel_crtc(crtc)->pipe;
-	struct drm_plane *plane;
-	struct intel_plane *intel_plane;
-
-	drm_for_each_legacy_plane(plane, &dev->mode_config.plane_list) {
-		intel_plane = to_intel_plane(plane);
-		if (plane->fb && intel_plane->pipe == pipe)
-			disable_plane_internal(plane);
-	}
-}
-
 void hsw_enable_ips(struct intel_crtc *crtc)
 {
 	struct drm_device *dev = crtc->base.dev;
@@ -4746,6 +4688,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 	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_plane *intel_plane;
 	int pipe = intel_crtc->pipe;
 
 	intel_crtc_wait_for_pending_flips(crtc);
@@ -4756,9 +4699,15 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 
 	intel_crtc_dpms_overlay(intel_crtc, false);
-	intel_crtc_update_cursor(crtc, false);
-	intel_disable_sprite_planes(crtc);
-	intel_disable_primary_hw_plane(crtc->primary, crtc);
+	intel_crtc->primary_enabled = false;
+	for_each_intel_plane(dev, intel_plane) {
+		if (intel_plane->pipe == pipe) {
+			struct drm_crtc *from = intel_plane->base.crtc;
+
+			intel_plane->disable_plane(&intel_plane->base,
+						   from ?: crtc, true);
+		}
+	}
 
 	/*
 	 * FIXME: Once we grow proper nuclear flip support out of this we need
@@ -13224,24 +13173,14 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	crtc->y = src->y1 >> 16;
 
 	if (intel_crtc->active) {
-		if (state->visible) {
+		intel_crtc->primary_enabled = state->visible;
+
+		if (state->visible)
 			/* FIXME: kill this fastboot hack */
 			intel_update_pipe_size(intel_crtc);
 
-			intel_crtc->primary_enabled = true;
-
-			dev_priv->display.update_primary_plane(crtc, plane->fb,
-					crtc->x, crtc->y);
-		} else {
-			/*
-			 * If clipping results in a non-visible primary plane,
-			 * we'll disable the primary plane.  Note that this is
-			 * a bit different than what happens if userspace
-			 * explicitly disables the plane by passing fb=0
-			 * because plane->fb still gets set and pinned.
-			 */
-			intel_disable_primary_hw_plane(plane, crtc);
-		}
+		dev_priv->display.update_primary_plane(crtc, plane->fb,
+						       crtc->x, crtc->y);
 	}
 }
 
-- 
2.1.0

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

[PATCH 04/35] drm/i915: get rid of primary_enabled and use atomic state

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 50 ++++++++++++++++++++----------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 -
 drivers/gpu/drm/i915/intel_fbc.c     |  2 +-
 3 files changed, 29 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index acf3494..40e3c62 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2236,11 +2236,7 @@ static void intel_enable_primary_hw_plane(struct drm_plane *plane,
 
 	/* If the pipe isn't enabled, we can't pump pixels and may hang */
 	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
-
-	if (intel_crtc->primary_enabled)
-		return;
-
-	intel_crtc->primary_enabled = true;
+	to_intel_plane_state(plane->state)->visible = true;
 
 	dev_priv->display.update_primary_plane(crtc, plane->fb,
 					       crtc->x, crtc->y);
@@ -2661,6 +2657,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	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 drm_plane *primary = crtc->primary;
+	bool visible = to_intel_plane_state(primary->state)->visible;
 	struct drm_i915_gem_object *obj;
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
@@ -2668,7 +2666,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
 	u32 reg = DSPCNTR(plane);
 	int pixel_size;
 
-	if (!intel_crtc->primary_enabled || !fb) {
+	if (!visible || !fb) {
 		I915_WRITE(reg, 0);
 		if (INTEL_INFO(dev)->gen >= 4)
 			I915_WRITE(DSPSURF(plane), 0);
@@ -2790,6 +2788,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	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 drm_plane *primary = crtc->primary;
+	bool visible = to_intel_plane_state(primary->state)->visible;
 	struct drm_i915_gem_object *obj;
 	int plane = intel_crtc->plane;
 	unsigned long linear_offset;
@@ -2797,7 +2797,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
 	u32 reg = DSPCNTR(plane);
 	int pixel_size;
 
-	if (!intel_crtc->primary_enabled || !fb) {
+	if (!visible || !fb) {
 		I915_WRITE(reg, 0);
 		I915_WRITE(DSPSURF(plane), 0);
 		POSTING_READ(reg);
@@ -2966,6 +2966,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	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 drm_plane *plane = crtc->primary;
+	bool visible = to_intel_plane_state(plane->state)->visible;
 	struct drm_i915_gem_object *obj;
 	int pipe = intel_crtc->pipe;
 	u32 plane_ctl, stride_div, stride;
@@ -2973,9 +2975,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 	unsigned int rotation;
 	int x_offset, y_offset;
 	unsigned long surf_addr;
-	struct drm_plane *plane;
 
-	if (!intel_crtc->primary_enabled || !fb) {
+	if (!visible || !fb) {
 		I915_WRITE(PLANE_CTL(pipe, 0), 0);
 		I915_WRITE(PLANE_SURF(pipe, 0), 0);
 		POSTING_READ(PLANE_CTL(pipe, 0));
@@ -3035,7 +3036,6 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
 
 	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
 
-	plane = crtc->primary;
 	rotation = plane->state->rotation;
 	switch (rotation) {
 	case BIT(DRM_ROTATE_90):
@@ -4699,7 +4699,6 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 
 	intel_crtc_dpms_overlay(intel_crtc, false);
-	intel_crtc->primary_enabled = false;
 	for_each_intel_plane(dev, intel_plane) {
 		if (intel_plane->pipe == pipe) {
 			struct drm_crtc *from = intel_plane->base.crtc;
@@ -12803,6 +12802,9 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	} else if (config->fb_changed) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 		struct drm_plane *primary = set->crtc->primary;
+		struct intel_plane_state *plane_state =
+				to_intel_plane_state(primary->state);
+		bool was_visible = plane_state->visible;
 		int vdisplay, hdisplay;
 
 		drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
@@ -12815,7 +12817,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		 * We need to make sure the primary plane is re-enabled if it
 		 * has previously been turned off.
 		 */
-		if (!intel_crtc->primary_enabled && ret == 0) {
+		plane_state = to_intel_plane_state(primary->state);
+		if (ret == 0 && !was_visible && plane_state->visible) {
 			WARN_ON(!intel_crtc->active);
 			intel_enable_primary_hw_plane(set->crtc->primary, set->crtc);
 		}
@@ -13113,6 +13116,9 @@ intel_check_primary_plane(struct drm_plane *plane,
 		return ret;
 
 	if (intel_crtc->active) {
+		struct intel_plane_state *old_state =
+			to_intel_plane_state(plane->state);
+
 		intel_crtc->atomic.wait_for_flips = true;
 
 		/*
@@ -13125,20 +13131,20 @@ intel_check_primary_plane(struct drm_plane *plane,
 		 * one is done too late. We eventually need to unify
 		 * this.
 		 */
-		if (intel_crtc->primary_enabled &&
+		if (state->visible &&
 		    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
 		    dev_priv->fbc.crtc == intel_crtc &&
 		    state->base.rotation != BIT(DRM_ROTATE_0)) {
 			intel_crtc->atomic.disable_fbc = true;
 		}
 
-		if (state->visible) {
+		if (state->visible && !old_state->visible) {
 			/*
 			 * BDW signals flip done immediately if the plane
 			 * is disabled, even if the plane enable is already
 			 * armed to occur at the next vblank :(
 			 */
-			if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled)
+			if (IS_BROADWELL(dev))
 				intel_crtc->atomic.wait_vblank = true;
 		}
 
@@ -13173,8 +13179,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
 	crtc->y = src->y1 >> 16;
 
 	if (intel_crtc->active) {
-		intel_crtc->primary_enabled = state->visible;
-
 		if (state->visible)
 			/* FIXME: kill this fastboot hack */
 			intel_update_pipe_size(intel_crtc);
@@ -13192,9 +13196,6 @@ intel_disable_primary_plane(struct drm_plane *plane,
 	struct drm_device *dev = plane->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 
-	if (!force)
-		to_intel_crtc(crtc)->primary_enabled = false;
-
 	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
 }
 
@@ -14655,8 +14656,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		 * Temporarily change the plane mapping and disable everything
 		 * ...  */
 		plane = crtc->plane;
+		to_intel_plane_state(crtc->base.primary->state)->visible = true;
 		crtc->plane = !plane;
-		crtc->primary_enabled = true;
 		dev_priv->display.crtc_disable(&crtc->base);
 		crtc->plane = plane;
 
@@ -14833,6 +14834,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 	int i;
 
 	for_each_intel_crtc(dev, crtc) {
+		struct drm_plane *primary = crtc->base.primary;
+		struct intel_plane_state *plane_state;
+
 		memset(crtc->config, 0, sizeof(*crtc->config));
 
 		crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
@@ -14842,7 +14846,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
 
 		crtc->base.state->enable = crtc->active;
 		crtc->base.enabled = crtc->active;
-		crtc->primary_enabled = primary_get_hw_state(crtc);
+
+		plane_state = to_intel_plane_state(primary->state);
+		plane_state->visible = primary_get_hw_state(crtc);
 
 		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
 			      crtc->base.base.id,
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 42548bc..cb0f29e 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -510,7 +510,6 @@ struct intel_crtc {
 	 */
 	bool active;
 	unsigned long enabled_power_domains;
-	bool primary_enabled; /* is the primary plane (partially) visible? */
 	bool lowfreq_avail;
 	struct intel_overlay *overlay;
 	struct intel_unpin_work *unpin_work;
diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
index 4165ce0..6abb834 100644
--- a/drivers/gpu/drm/i915/intel_fbc.c
+++ b/drivers/gpu/drm/i915/intel_fbc.c
@@ -457,7 +457,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
 		tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
 
 		if (intel_crtc_active(tmp_crtc) &&
-		    to_intel_crtc(tmp_crtc)->primary_enabled) {
+		    to_intel_plane_state(tmp_crtc->primary->state)->visible) {
 			if (one_pipe_only && crtc) {
 				if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
 					DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
-- 
2.1.0

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

[PATCH 05/35] drm/i915: Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use it there.

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

They're the same code, so why not?

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 158 ++++++++++++++++++++++-------------
 drivers/gpu/drm/i915/intel_drv.h     |   2 -
 drivers/gpu/drm/i915/intel_sprite.c  |  68 ---------------
 3 files changed, 102 insertions(+), 126 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 40e3c62..8c9cede 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2240,14 +2240,6 @@ static void intel_enable_primary_hw_plane(struct drm_plane *plane,
 
 	dev_priv->display.update_primary_plane(crtc, plane->fb,
 					       crtc->x, crtc->y);
-
-	/*
-	 * BDW signals flip done immediately if the plane
-	 * is disabled, even if the plane enable is already
-	 * armed to occur at the next vblank :(
-	 */
-	if (IS_BROADWELL(dev))
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
 }
 
 static bool need_vtd_wa(struct drm_device *dev)
@@ -4658,17 +4650,38 @@ static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable)
 	 */
 }
 
-static void intel_crtc_enable_planes(struct drm_crtc *crtc)
+/**
+ * intel_post_enable_primary - Perform operations after enabling primary plane
+ * @crtc: the CRTC whose primary plane was just enabled
+ *
+ * Performs potentially sleeping operations that must be done after the primary
+ * plane is enabled, such as updating FBC and IPS.  Note that this may be
+ * called due to an explicit primary plane update, or due to an implicit
+ * re-enable that is caused when a sprite plane is updated to no longer
+ * completely hide the primary plane.
+ */
+static void
+intel_post_enable_primary(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	int pipe = intel_crtc->pipe;
 
-	intel_enable_primary_hw_plane(crtc->primary, crtc);
-	intel_enable_sprite_planes(crtc);
-	intel_crtc_update_cursor(crtc, true);
-	intel_crtc_dpms_overlay(intel_crtc, true);
+	/*
+	 * BDW signals flip done immediately if the plane
+	 * is disabled, even if the plane enable is already
+	 * armed to occur at the next vblank :(
+	 */
+	if (IS_BROADWELL(dev))
+		intel_wait_for_vblank(dev, pipe);
 
+	/*
+	 * FIXME IPS should be fine as long as one plane is
+	 * enabled, but in practice it seems to have problems
+	 * when going from primary only to sprite only and vice
+	 * versa.
+	 */
 	hsw_enable_ips(intel_crtc);
 
 	mutex_lock(&dev->struct_mutex);
@@ -4676,27 +4689,95 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
 	mutex_unlock(&dev->struct_mutex);
 
 	/*
-	 * FIXME: Once we grow proper nuclear flip support out of this we need
-	 * to compute the mask of flip planes precisely. For the time being
-	 * consider this a flip from a NULL plane.
+	 * Gen2 reports pipe underruns whenever all planes are disabled.
+	 * So don't enable underrun reporting before at least some planes
+	 * are enabled.
+	 * FIXME: Need to fix the logic to work when we turn off all planes
+	 * but leave the pipe running.
 	 */
-	intel_frontbuffer_flip(dev, INTEL_FRONTBUFFER_ALL_MASK(pipe));
+	if (IS_GEN2(dev))
+		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
+
+	/* Underruns don't raise interrupts, so check manually. */
+	if (HAS_GMCH_DISPLAY(dev))
+		i9xx_check_fifo_underruns(dev_priv);
 }
 
-static void intel_crtc_disable_planes(struct drm_crtc *crtc)
+/**
+ * intel_pre_disable_primary - Perform operations before disabling primary plane
+ * @crtc: the CRTC whose primary plane is to be disabled
+ *
+ * Performs potentially sleeping operations that must be done before the
+ * primary plane is disabled, such as updating FBC and IPS.  Note that this may
+ * be called due to an explicit primary plane update, or due to an implicit
+ * disable that is caused when a sprite plane completely hides the primary
+ * plane.
+ */
+static void
+intel_pre_disable_primary(struct drm_crtc *crtc)
 {
 	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_plane *intel_plane;
 	int pipe = intel_crtc->pipe;
 
-	intel_crtc_wait_for_pending_flips(crtc);
+	/*
+	 * Gen2 reports pipe underruns whenever all planes are disabled.
+	 * So diasble underrun reporting before all the planes get disabled.
+	 * FIXME: Need to fix the logic to work when we turn off all planes
+	 * but leave the pipe running.
+	 */
+	if (IS_GEN2(dev))
+		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
+
+	/*
+	 * Vblank time updates from the shadow to live plane control register
+	 * are blocked if the memory self-refresh mode is active at that
+	 * moment. So to make sure the plane gets truly disabled, disable
+	 * first the self-refresh mode. The self-refresh enable bit in turn
+	 * will be checked/applied by the HW only at the next frame start
+	 * event which is after the vblank start event, so we need to have a
+	 * wait-for-vblank between disabling the plane and the pipe.
+	 */
+	if (HAS_GMCH_DISPLAY(dev))
+		intel_set_memory_cxsr(dev_priv, false);
 
+	mutex_lock(&dev->struct_mutex);
 	if (dev_priv->fbc.crtc == intel_crtc)
 		intel_fbc_disable(dev);
+	mutex_unlock(&dev->struct_mutex);
 
+	/*
+	 * FIXME IPS should be fine as long as one plane is
+	 * enabled, but in practice it seems to have problems
+	 * when going from primary only to sprite only and vice
+	 * versa.
+	 */
 	hsw_disable_ips(intel_crtc);
+}
+
+static void intel_crtc_enable_planes(struct drm_crtc *crtc)
+{
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+
+	intel_enable_primary_hw_plane(crtc->primary, crtc);
+	intel_enable_sprite_planes(crtc);
+	intel_crtc_update_cursor(crtc, true);
+	intel_crtc_dpms_overlay(intel_crtc, true);
+
+	intel_post_enable_primary(crtc);
+}
+
+static void intel_crtc_disable_planes(struct drm_crtc *crtc)
+{
+	struct drm_device *dev = crtc->dev;
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct intel_plane *intel_plane;
+	int pipe = intel_crtc->pipe;
+
+	intel_crtc_wait_for_pending_flips(crtc);
+
+	intel_pre_disable_primary(crtc);
 
 	intel_crtc_dpms_overlay(intel_crtc, false);
 	for_each_intel_plane(dev, intel_plane) {
@@ -5751,9 +5832,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 		encoder->enable(encoder);
 
 	intel_crtc_enable_planes(crtc);
-
-	/* Underruns don't raise interrupts, so check manually. */
-	i9xx_check_fifo_underruns(dev_priv);
 }
 
 static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
@@ -5812,19 +5890,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 		encoder->enable(encoder);
 
 	intel_crtc_enable_planes(crtc);
-
-	/*
-	 * Gen2 reports pipe underruns whenever all planes are disabled.
-	 * So don't enable underrun reporting before at least some planes
-	 * are enabled.
-	 * FIXME: Need to fix the logic to work when we turn off all planes
-	 * but leave the pipe running.
-	 */
-	if (IS_GEN2(dev))
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, true);
-
-	/* Underruns don't raise interrupts, so check manually. */
-	i9xx_check_fifo_underruns(dev_priv);
 }
 
 static void i9xx_pfit_disable(struct intel_crtc *crtc)
@@ -5853,25 +5918,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	/*
-	 * Gen2 reports pipe underruns whenever all planes are disabled.
-	 * So diasble underrun reporting before all the planes get disabled.
-	 * FIXME: Need to fix the logic to work when we turn off all planes
-	 * but leave the pipe running.
-	 */
-	if (IS_GEN2(dev))
-		intel_set_cpu_fifo_underrun_reporting(dev_priv, pipe, false);
-
-	/*
-	 * Vblank time updates from the shadow to live plane control register
-	 * are blocked if the memory self-refresh mode is active at that
-	 * moment. So to make sure the plane gets truly disabled, disable
-	 * first the self-refresh mode. The self-refresh enable bit in turn
-	 * will be checked/applied by the HW only at the next frame start
-	 * event which is after the vblank start event, so we need to have a
-	 * wait-for-vblank between disabling the plane and the pipe.
-	 */
-	intel_set_memory_cxsr(dev_priv, false);
 	intel_crtc_disable_planes(crtc);
 
 	/*
@@ -12820,7 +12866,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 		plane_state = to_intel_plane_state(primary->state);
 		if (ret == 0 && !was_visible && plane_state->visible) {
 			WARN_ON(!intel_crtc->active);
-			intel_enable_primary_hw_plane(set->crtc->primary, set->crtc);
+			intel_post_enable_primary(set->crtc);
 		}
 
 		/*
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index cb0f29e..4c48451 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1376,8 +1376,6 @@ int intel_sprite_set_colorkey(struct drm_device *dev, void *data,
 bool intel_pipe_update_start(struct intel_crtc *crtc,
 			     uint32_t *start_vbl_count);
 void intel_pipe_update_end(struct intel_crtc *crtc, u32 start_vbl_count);
-void intel_post_enable_primary(struct drm_crtc *crtc);
-void intel_pre_disable_primary(struct drm_crtc *crtc);
 
 /* intel_tv.c */
 void intel_tv_init(struct drm_device *dev);
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 6316454..7419e04 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -794,74 +794,6 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc, bool force)
 	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
 }
 
-/**
- * intel_post_enable_primary - Perform operations after enabling primary plane
- * @crtc: the CRTC whose primary plane was just enabled
- *
- * Performs potentially sleeping operations that must be done after the primary
- * plane is enabled, such as updating FBC and IPS.  Note that this may be
- * called due to an explicit primary plane update, or due to an implicit
- * re-enable that is caused when a sprite plane is updated to no longer
- * completely hide the primary plane.
- */
-void
-intel_post_enable_primary(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	/*
-	 * BDW signals flip done immediately if the plane
-	 * is disabled, even if the plane enable is already
-	 * armed to occur at the next vblank :(
-	 */
-	if (IS_BROADWELL(dev))
-		intel_wait_for_vblank(dev, intel_crtc->pipe);
-
-	/*
-	 * FIXME IPS should be fine as long as one plane is
-	 * enabled, but in practice it seems to have problems
-	 * when going from primary only to sprite only and vice
-	 * versa.
-	 */
-	hsw_enable_ips(intel_crtc);
-
-	mutex_lock(&dev->struct_mutex);
-	intel_fbc_update(dev);
-	mutex_unlock(&dev->struct_mutex);
-}
-
-/**
- * intel_pre_disable_primary - Perform operations before disabling primary plane
- * @crtc: the CRTC whose primary plane is to be disabled
- *
- * Performs potentially sleeping operations that must be done before the
- * primary plane is disabled, such as updating FBC and IPS.  Note that this may
- * be called due to an explicit primary plane update, or due to an implicit
- * disable that is caused when a sprite plane completely hides the primary
- * plane.
- */
-void
-intel_pre_disable_primary(struct drm_crtc *crtc)
-{
-	struct drm_device *dev = crtc->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
-	mutex_lock(&dev->struct_mutex);
-	if (dev_priv->fbc.crtc == intel_crtc)
-		intel_fbc_disable(dev);
-	mutex_unlock(&dev->struct_mutex);
-
-	/*
-	 * FIXME IPS should be fine as long as one plane is
-	 * enabled, but in practice it seems to have problems
-	 * when going from primary only to sprite only and vice
-	 * versa.
-	 */
-	hsw_disable_ips(intel_crtc);
-}
-
 static int
 intel_check_sprite_plane(struct drm_plane *plane,
 			 struct intel_plane_state *state)
-- 
2.1.0

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

[PATCH 06/35] drm/i915: Rename intel_crtc_dpms_overlay.

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

To make it clear that it isn't called during crtc enable.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8c9cede..12c02a6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4632,9 +4632,9 @@ static void intel_crtc_load_lut(struct drm_crtc *crtc)
 		hsw_enable_ips(intel_crtc);
 }
 
-static void intel_crtc_dpms_overlay(struct intel_crtc *intel_crtc, bool enable)
+static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
 {
-	if (!enable && intel_crtc->overlay) {
+	if (intel_crtc->overlay) {
 		struct drm_device *dev = intel_crtc->base.dev;
 		struct drm_i915_private *dev_priv = dev->dev_private;
 
@@ -4763,7 +4763,6 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
 	intel_enable_primary_hw_plane(crtc->primary, crtc);
 	intel_enable_sprite_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
-	intel_crtc_dpms_overlay(intel_crtc, true);
 
 	intel_post_enable_primary(crtc);
 }
@@ -4779,7 +4778,7 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
 
 	intel_pre_disable_primary(crtc);
 
-	intel_crtc_dpms_overlay(intel_crtc, false);
+	intel_crtc_dpms_overlay_disable(intel_crtc);
 	for_each_intel_plane(dev, intel_plane) {
 		if (intel_plane->pipe == pipe) {
 			struct drm_crtc *from = intel_plane->base.crtc;
-- 
2.1.0

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

[PATCH 07/35] drm/i915: Move toggling planes out of crtc enable/disable.

[Ander Conselvan de Oliveira]

From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

This makes disabling planes more explicit.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
[anderco: fixed warning due to using drm_crtc instead of intel_crtc]
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  6 ++---
 drivers/gpu/drm/i915/intel_display.c | 46 ++++++++++++++++++++++--------------
 drivers/gpu/drm/i915/intel_drv.h     |  1 +
 3 files changed, 31 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 9c2b9e4..d263d0f 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3594,8 +3594,7 @@ static void hsw_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
 		intel_display_power_get(dev_priv,
 					POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
 
-		dev_priv->display.crtc_disable(&crtc->base);
-		dev_priv->display.crtc_enable(&crtc->base);
+		intel_crtc_reset(crtc);
 	}
 	drm_modeset_unlock_all(dev);
 }
@@ -3616,8 +3615,7 @@ static void hsw_undo_trans_edp_pipe_A_crc_wa(struct drm_device *dev)
 	if (crtc->config->pch_pfit.force_thru) {
 		crtc->config->pch_pfit.force_thru = false;
 
-		dev_priv->display.crtc_disable(&crtc->base);
-		dev_priv->display.crtc_enable(&crtc->base);
+		intel_crtc_reset(crtc);
 
 		intel_display_power_put(dev_priv,
 					POWER_DOMAIN_PIPE_PANEL_FITTER(PIPE_A));
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 12c02a6..1a0ec52 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -107,6 +107,8 @@ static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_cr
 	struct intel_crtc_state *crtc_state);
 static int i9xx_get_refclk(const struct intel_crtc_state *crtc_state,
 			   int num_connectors);
+static void intel_crtc_enable_planes(struct drm_crtc *crtc);
+static void intel_crtc_disable_planes(struct drm_crtc *crtc);
 
 static struct intel_encoder *intel_find_encoder(struct intel_connector *connector, int pipe)
 {
@@ -3128,6 +3130,19 @@ static void intel_update_primary_planes(struct drm_device *dev)
 	}
 }
 
+void intel_crtc_reset(struct intel_crtc *crtc)
+{
+	struct drm_i915_private *dev_priv = to_i915(crtc->base.dev);
+
+	if (!crtc->active)
+		return;
+
+	intel_crtc_disable_planes(&crtc->base);
+	dev_priv->display.crtc_disable(&crtc->base);
+	dev_priv->display.crtc_enable(&crtc->base);
+	intel_crtc_enable_planes(&crtc->base);
+}
+
 void intel_prepare_reset(struct drm_device *dev)
 {
 	struct drm_i915_private *dev_priv = to_i915(dev);
@@ -3148,8 +3163,11 @@ void intel_prepare_reset(struct drm_device *dev)
 	 * g33 docs say we should at least disable all the planes.
 	 */
 	for_each_intel_crtc(dev, crtc) {
-		if (crtc->active)
-			dev_priv->display.crtc_disable(&crtc->base);
+		if (!crtc->active)
+			continue;
+
+		intel_crtc_disable_planes(&crtc->base);
+		dev_priv->display.crtc_disable(&crtc->base);
 	}
 }
 
@@ -4758,8 +4776,6 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
 
 static void intel_crtc_enable_planes(struct drm_crtc *crtc)
 {
-	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
-
 	intel_enable_primary_hw_plane(crtc->primary, crtc);
 	intel_enable_sprite_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
@@ -4865,8 +4881,6 @@ static void ironlake_crtc_enable(struct drm_crtc *crtc)
 
 	if (HAS_PCH_CPT(dev))
 		cpt_verify_modeset(dev, intel_crtc->pipe);
-
-	intel_crtc_enable_planes(crtc);
 }
 
 /* IPS only exists on ULT machines and is tied to pipe A. */
@@ -4990,7 +5004,6 @@ static void haswell_crtc_enable(struct drm_crtc *crtc)
 	/* If we change the relative order between pipe/planes enabling, we need
 	 * to change the workaround. */
 	haswell_mode_set_planes_workaround(intel_crtc);
-	intel_crtc_enable_planes(crtc);
 }
 
 static void ironlake_pfit_disable(struct intel_crtc *crtc)
@@ -5020,8 +5033,6 @@ static void ironlake_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	intel_crtc_disable_planes(crtc);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->disable(encoder);
 
@@ -5084,8 +5095,6 @@ static void haswell_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	intel_crtc_disable_planes(crtc);
-
 	for_each_encoder_on_crtc(dev, crtc, encoder) {
 		intel_opregion_notify_encoder(encoder, false);
 		encoder->disable(encoder);
@@ -5829,8 +5838,6 @@ static void valleyview_crtc_enable(struct drm_crtc *crtc)
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
-
-	intel_crtc_enable_planes(crtc);
 }
 
 static void i9xx_set_pll_dividers(struct intel_crtc *crtc)
@@ -5887,8 +5894,6 @@ static void i9xx_crtc_enable(struct drm_crtc *crtc)
 
 	for_each_encoder_on_crtc(dev, crtc, encoder)
 		encoder->enable(encoder);
-
-	intel_crtc_enable_planes(crtc);
 }
 
 static void i9xx_pfit_disable(struct intel_crtc *crtc)
@@ -5917,8 +5922,6 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
 	if (!intel_crtc->active)
 		return;
 
-	intel_crtc_disable_planes(crtc);
-
 	/*
 	 * On gen2 planes are double buffered but the pipe isn't, so we must
 	 * wait for planes to fully turn off before disabling the pipe.
@@ -5982,9 +5985,11 @@ void intel_crtc_control(struct drm_crtc *crtc, bool enable)
 			intel_crtc->enabled_power_domains = domains;
 
 			dev_priv->display.crtc_enable(crtc);
+			intel_crtc_enable_planes(crtc);
 		}
 	} else {
 		if (intel_crtc->active) {
+			intel_crtc_disable_planes(crtc);
 			dev_priv->display.crtc_disable(crtc);
 
 			domains = intel_crtc->enabled_power_domains;
@@ -6019,6 +6024,7 @@ static void intel_crtc_disable(struct drm_crtc *crtc)
 	/* crtc should still be enabled when we disable it. */
 	WARN_ON(!crtc->state->enable);
 
+	intel_crtc_disable_planes(crtc);
 	dev_priv->display.crtc_disable(crtc);
 	dev_priv->display.off(crtc);
 
@@ -12258,8 +12264,10 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 		intel_crtc_disable(&intel_crtc->base);
 
 	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
-		if (intel_crtc->base.state->enable)
+		if (intel_crtc->base.state->enable) {
+			intel_crtc_disable_planes(&intel_crtc->base);
 			dev_priv->display.crtc_disable(&intel_crtc->base);
+		}
 	}
 
 	/* crtc->mode is already used by the ->mode_set callbacks, hence we need
@@ -12307,6 +12315,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 		update_scanline_offset(intel_crtc);
 
 		dev_priv->display.crtc_enable(&intel_crtc->base);
+		intel_crtc_enable_planes(&intel_crtc->base);
 	}
 
 	/* FIXME: add subpixel order */
@@ -14703,6 +14712,7 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
 		plane = crtc->plane;
 		to_intel_plane_state(crtc->base.primary->state)->visible = true;
 		crtc->plane = !plane;
+		intel_crtc_disable_planes(&crtc->base);
 		dev_priv->display.crtc_disable(&crtc->base);
 		crtc->plane = plane;
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index 4c48451..dc53f59 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -994,6 +994,7 @@ void intel_mark_busy(struct drm_device *dev);
 void intel_mark_idle(struct drm_device *dev);
 void intel_crtc_restore_mode(struct drm_crtc *crtc);
 void intel_crtc_control(struct drm_crtc *crtc, bool enable);
+void intel_crtc_reset(struct intel_crtc *crtc);
 void intel_crtc_update_dpms(struct drm_crtc *crtc);
 void intel_encoder_destroy(struct drm_encoder *encoder);
 int intel_connector_init(struct intel_connector *);
-- 
2.1.0

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

[PATCH 08/35] drm/i915: Don't check for NULL before freeing state

[Ander Conselvan de Oliveira]

This is not necessary after the below commit.

commit a0211bb482c346820506c546a6a58b8357999a99
Author: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
Date:   Mon Mar 30 14:05:43 2015 +0300

    drm/atomic: Don't try to free a NULL state

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 1a0ec52..9d9e633 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9772,10 +9772,8 @@ retry:
  fail:
 	intel_crtc->new_enabled = crtc->state->enable;
 fail_unlock:
-	if (state) {
-		drm_atomic_state_free(state);
-		state = NULL;
-	}
+	drm_atomic_state_free(state);
+	state = NULL;
 
 	if (ret == -EDEADLK) {
 		drm_modeset_backoff(ctx);
@@ -12915,8 +12913,7 @@ fail:
 	}
 
 out_config:
-	if (state)
-		drm_atomic_state_free(state);
+	drm_atomic_state_free(state);
 
 	intel_set_config_free(config);
 	return ret;
-- 
2.1.0

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

[PATCH 09/35] drm/i915: Call drm helpers when duplicating crtc and plane states

[Ander Conselvan de Oliveira]

Use the helpers introduced by the commit below to properly initialize
the duplicated states.

commit f5e7840b0c4368f8cdbb055188c2a0eef50c3052
Author: Thierry Reding <treding@nvidia.com>
Date:   Wed Jan 28 14:54:32 2015 +0100

    drm/atomic: Add helpers for state-subclassing drivers

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic.c       | 8 ++++++--
 drivers/gpu/drm/i915/intel_atomic_plane.c | 4 ++--
 2 files changed, 8 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic.c b/drivers/gpu/drm/i915/intel_atomic.c
index 3c4b7cd..7082c41 100644
--- a/drivers/gpu/drm/i915/intel_atomic.c
+++ b/drivers/gpu/drm/i915/intel_atomic.c
@@ -247,8 +247,12 @@ intel_crtc_duplicate_state(struct drm_crtc *crtc)
 		crtc_state = kmemdup(intel_crtc->config,
 				     sizeof(*intel_crtc->config), GFP_KERNEL);
 
-	if (crtc_state)
-		crtc_state->base.crtc = crtc;
+	if (!crtc_state)
+		return NULL;
+
+	__drm_atomic_helper_crtc_duplicate_state(crtc, &crtc_state->base);
+
+	crtc_state->base.crtc = crtc;
 
 	return &crtc_state->base;
 }
diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index dc8e136..77462e1 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -85,8 +85,8 @@ intel_plane_duplicate_state(struct drm_plane *plane)
 		return NULL;
 
 	state = &intel_state->base;
-	if (state->fb)
-		drm_framebuffer_reference(state->fb);
+
+	__drm_atomic_helper_plane_duplicate_state(plane, state);
 
 	return state;
 }
-- 
2.1.0

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

[PATCH 10/35] drm/i915: Use for_each_connector_in_state helper macro

[Ander Conselvan de Oliveira]

Simplifies looping over connector states a bit.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_ddi.c     |  9 +++--
 drivers/gpu/drm/i915/intel_display.c | 72 +++++++++++-------------------------
 drivers/gpu/drm/i915/intel_dp_mst.c  | 13 ++++---
 drivers/gpu/drm/i915/intel_hdmi.c    |  7 +---
 4 files changed, 36 insertions(+), 65 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_ddi.c b/drivers/gpu/drm/i915/intel_ddi.c
index 455d44b..4aede05 100644
--- a/drivers/gpu/drm/i915/intel_ddi.c
+++ b/drivers/gpu/drm/i915/intel_ddi.c
@@ -566,17 +566,18 @@ intel_ddi_get_crtc_new_encoder(struct intel_crtc_state *crtc_state)
 	struct intel_crtc *crtc = to_intel_crtc(crtc_state->base.crtc);
 	struct intel_encoder *ret = NULL;
 	struct drm_atomic_state *state;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
 	int num_encoders = 0;
 	int i;
 
 	state = crtc_state->base.state;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i] ||
-		    state->connector_states[i]->crtc != crtc_state->base.crtc)
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
-		ret = to_intel_encoder(state->connector_states[i]->best_encoder);
+		ret = to_intel_encoder(connector_state->best_encoder);
 		num_encoders++;
 	}
 
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9d9e633..4ced5a4 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -453,15 +453,12 @@ static bool intel_pipe_will_have_type(const struct intel_crtc_state *crtc_state,
 				      int type)
 {
 	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
 	int i, num_connectors = 0;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
@@ -7373,14 +7370,11 @@ static int i9xx_crtc_compute_clock(struct intel_crtc *crtc,
 	struct intel_encoder *encoder;
 	const intel_limit_t *limit;
 	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	int i;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != &crtc->base)
 			continue;
 
@@ -8064,16 +8058,13 @@ static int ironlake_get_refclk(struct intel_crtc_state *crtc_state)
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
 	int num_connectors = 0, i;
 	bool is_lvds = false;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
@@ -8327,17 +8318,14 @@ static uint32_t ironlake_compute_dpll(struct intel_crtc *intel_crtc,
 	struct drm_device *dev = crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_atomic_state *state = crtc_state->base.state;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
 	uint32_t dpll;
 	int factor, num_connectors = 0, i;
 	bool is_lvds = false, is_sdvo = false;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
@@ -11061,7 +11049,8 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 {
 	struct drm_device *dev = crtc->base.dev;
 	struct drm_atomic_state *state;
-	struct intel_connector *connector;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
 	int bpp, i;
 
 	if ((IS_G4X(dev) || IS_VALLEYVIEW(dev)))
@@ -11077,15 +11066,12 @@ compute_baseline_pipe_bpp(struct intel_crtc *crtc,
 	state = pipe_config->base.state;
 
 	/* Clamp display bpp to EDID value */
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector = to_intel_connector(state->connectors[i]);
-		if (state->connector_states[i]->crtc != &crtc->base)
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->crtc != &crtc->base)
 			continue;
 
-		connected_sink_compute_bpp(connector, pipe_config);
+		connected_sink_compute_bpp(to_intel_connector(connector),
+					   pipe_config);
 	}
 
 	return bpp;
@@ -11212,14 +11198,11 @@ static bool check_single_encoder_cloning(struct drm_atomic_state *state,
 					 struct intel_encoder *encoder)
 {
 	struct intel_encoder *source_encoder;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	int i;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != &crtc->base)
 			continue;
 
@@ -11236,14 +11219,11 @@ static bool check_encoder_cloning(struct drm_atomic_state *state,
 				  struct intel_crtc *crtc)
 {
 	struct intel_encoder *encoder;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	int i;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != &crtc->base)
 			continue;
 
@@ -11259,6 +11239,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
 	struct intel_encoder *encoder;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	unsigned int used_ports = 0;
 	int i;
@@ -11268,11 +11249,7 @@ static bool check_digital_port_conflicts(struct drm_atomic_state *state)
 	 * list to detect the problem on ddi platforms
 	 * where there's just one encoder per digital port.
 	 */
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (!connector_state->best_encoder)
 			continue;
 
@@ -11323,7 +11300,7 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 			  struct drm_atomic_state *state)
 {
 	struct intel_encoder *encoder;
-	struct intel_connector *connector;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	struct intel_crtc_state *pipe_config;
 	int base_bpp, ret = -EINVAL;
@@ -11401,12 +11378,7 @@ encoder_retry:
 	 * adjust it according to limitations or connector properties, and also
 	 * a chance to reject the mode entirely.
 	 */
-	for (i = 0; i < state->num_connector; i++) {
-		connector = to_intel_connector(state->connectors[i]);
-		if (!connector)
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != crtc)
 			continue;
 
diff --git a/drivers/gpu/drm/i915/intel_dp_mst.c b/drivers/gpu/drm/i915/intel_dp_mst.c
index 3945057..6e4cc53 100644
--- a/drivers/gpu/drm/i915/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/intel_dp_mst.c
@@ -40,7 +40,9 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 	int bpp, i;
 	int lane_count, slots, rate;
 	struct drm_display_mode *adjusted_mode = &pipe_config->base.adjusted_mode;
-	struct intel_connector *found = NULL;
+	struct drm_connector *drm_connector;
+	struct intel_connector *connector, *found = NULL;
+	struct drm_connector_state *connector_state;
 	int mst_pbn;
 
 	pipe_config->dp_encoder_is_mst = true;
@@ -70,12 +72,11 @@ static bool intel_dp_mst_compute_config(struct intel_encoder *encoder,
 
 	state = pipe_config->base.state;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
+	for_each_connector_in_state(state, drm_connector, connector_state, i) {
+		connector = to_intel_connector(drm_connector);
 
-		if (state->connector_states[i]->best_encoder == &encoder->base) {
-			found = to_intel_connector(state->connectors[i]);
+		if (connector_state->best_encoder == &encoder->base) {
+			found = connector;
 			break;
 		}
 	}
diff --git a/drivers/gpu/drm/i915/intel_hdmi.c b/drivers/gpu/drm/i915/intel_hdmi.c
index e63f0b9..9458ea4 100644
--- a/drivers/gpu/drm/i915/intel_hdmi.c
+++ b/drivers/gpu/drm/i915/intel_hdmi.c
@@ -961,6 +961,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 	struct drm_device *dev = crtc_state->base.crtc->dev;
 	struct drm_atomic_state *state;
 	struct intel_encoder *encoder;
+	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
 	int count = 0, count_hdmi = 0;
 	int i;
@@ -970,11 +971,7 @@ static bool hdmi_12bpc_possible(struct intel_crtc_state *crtc_state)
 
 	state = crtc_state->base.state;
 
-	for (i = 0; i < state->num_connector; i++) {
-		if (!state->connectors[i])
-			continue;
-
-		connector_state = state->connector_states[i];
+	for_each_connector_in_state(state, connector, connector_state, i) {
 		if (connector_state->crtc != crtc_state->base.crtc)
 			continue;
 
-- 
2.1.0

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

[PATCH 11/35] drm/i915: Extract mode_changed computation out of stage_output_config()

[Ander Conselvan de Oliveira]

This should make the conversion to atomic easier, by splitting the
initialization of the atomic state from the logic that decides if a
modeset is needed.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 73 ++++++++++++++++++++----------------
 1 file changed, 41 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4ced5a4..9c43df1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12519,6 +12519,10 @@ static void
 intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 				      struct intel_set_config *config)
 {
+	struct drm_device *dev = set->crtc->dev;
+	struct intel_connector *connector;
+	struct intel_encoder *encoder;
+	struct intel_crtc *crtc;
 
 	/* We should be able to check here if the fb has the same properties
 	 * and then just flip_or_move it */
@@ -12562,6 +12566,36 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 		config->mode_changed = true;
 	}
 
+	for_each_intel_connector(dev, connector) {
+		if (&connector->new_encoder->base == connector->base.encoder)
+			continue;
+
+		config->mode_changed = true;
+		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
+			      connector->base.base.id,
+			      connector->base.name);
+	}
+
+	for_each_intel_encoder(dev, encoder) {
+		if (&encoder->new_crtc->base == encoder->base.crtc)
+			continue;
+
+		DRM_DEBUG_KMS("[ENCODER:%d:%s] crtc changed, full mode switch\n",
+			      encoder->base.base.id,
+			      encoder->base.name);
+		config->mode_changed = true;
+	}
+
+	for_each_intel_crtc(dev, crtc) {
+		if (crtc->new_enabled == crtc->base.state->enable)
+			continue;
+
+		DRM_DEBUG_KMS("[CRTC:%d] %sabled, full mode switch\n",
+			      crtc->base.base.id,
+			      crtc->new_enabled ? "en" : "dis");
+		config->mode_changed = true;
+	}
+
 	DRM_DEBUG_KMS("computed changes for [CRTC:%d], mode_changed=%d, fb_changed=%d\n",
 			set->crtc->base.id, config->mode_changed, config->fb_changed);
 }
@@ -12569,7 +12603,6 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 static int
 intel_modeset_stage_output_state(struct drm_device *dev,
 				 struct drm_mode_set *set,
-				 struct intel_set_config *config,
 				 struct drm_atomic_state *state)
 {
 	struct intel_connector *connector;
@@ -12605,14 +12638,6 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 				connector->base.base.id,
 				connector->base.name);
 		}
-
-
-		if (&connector->new_encoder->base != connector->base.encoder) {
-			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
-				      connector->base.base.id,
-				      connector->base.name);
-			config->mode_changed = true;
-		}
 	}
 	/* connector->new_encoder is now updated for all connectors. */
 
@@ -12665,15 +12690,6 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 			encoder->new_crtc = NULL;
 		else if (num_connectors > 1)
 			return -EINVAL;
-
-		/* Only now check for crtc changes so we don't miss encoders
-		 * that will be disabled. */
-		if (&encoder->new_crtc->base != encoder->base.crtc) {
-			DRM_DEBUG_KMS("[ENCODER:%d:%s] crtc changed, full mode switch\n",
-				      encoder->base.base.id,
-				      encoder->base.name);
-			config->mode_changed = true;
-		}
 	}
 	/* Now we've also updated encoder->new_crtc for all encoders. */
 	for_each_intel_connector(dev, connector) {
@@ -12699,13 +12715,6 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 				break;
 			}
 		}
-
-		if (crtc->new_enabled != crtc->base.state->enable) {
-			DRM_DEBUG_KMS("[CRTC:%d] %sabled, full mode switch\n",
-				      crtc->base.base.id,
-				      crtc->new_enabled ? "en" : "dis");
-			config->mode_changed = true;
-		}
 	}
 
 	return 0;
@@ -12777,12 +12786,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	save_set.y = set->crtc->y;
 	save_set.fb = set->crtc->primary->fb;
 
-	/* Compute whether we need a full modeset, only an fb base update or no
-	 * change at all. In the future we might also check whether only the
-	 * mode changed, e.g. for LVDS where we only change the panel fitter in
-	 * such cases. */
-	intel_set_config_compute_mode_changes(set, config);
-
 	state = drm_atomic_state_alloc(dev);
 	if (!state) {
 		ret = -ENOMEM;
@@ -12791,10 +12794,16 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	state->acquire_ctx = dev->mode_config.acquire_ctx;
 
-	ret = intel_modeset_stage_output_state(dev, set, config, state);
+	ret = intel_modeset_stage_output_state(dev, set, state);
 	if (ret)
 		goto fail;
 
+	/* Compute whether we need a full modeset, only an fb base update or no
+	 * change at all. In the future we might also check whether only the
+	 * mode changed, e.g. for LVDS where we only change the panel fitter in
+	 * such cases. */
+	intel_set_config_compute_mode_changes(set, config);
+
 	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
 						   state,
 						   &modeset_pipes,
-- 
2.1.0

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

[PATCH 12/35] drm/i915: Add crtc states before calling compute_config()

[Ander Conselvan de Oliveira]

The function intel_modeset_compute_config() needs to eventually become
part of atomic_check(). At that point, all the affected crtcs need to be
in the atomic state with the new values. So move the logic of adding
crtc states out of that function.

v2: Set crtc_state->enable in all cases. (Ander)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 52 +++++++++++++++++++++++++++++-------
 1 file changed, 42 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 9c43df1..e6e7c63 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9628,6 +9628,7 @@ bool intel_get_load_detect_pipe(struct drm_connector *connector,
 	struct drm_mode_config *config = &dev->mode_config;
 	struct drm_atomic_state *state = NULL;
 	struct drm_connector_state *connector_state;
+	struct intel_crtc_state *crtc_state;
 	int ret, i = -1;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
@@ -9723,6 +9724,14 @@ retry:
 	connector_state->crtc = crtc;
 	connector_state->best_encoder = &intel_encoder->base;
 
+	crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
+	if (IS_ERR(crtc_state)) {
+		ret = PTR_ERR(crtc_state);
+		goto fail;
+	}
+
+	crtc_state->base.enable = true;
+
 	if (!mode)
 		mode = &load_detect_mode;
 
@@ -9783,6 +9792,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 	struct drm_atomic_state *state;
 	struct drm_connector_state *connector_state;
+	struct intel_crtc_state *crtc_state;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
@@ -9799,6 +9809,10 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		if (IS_ERR(connector_state))
 			goto fail;
 
+		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
+		if (IS_ERR(crtc_state))
+			goto fail;
+
 		to_intel_connector(connector)->new_encoder = NULL;
 		intel_encoder->new_crtc = NULL;
 		intel_crtc->new_enabled = false;
@@ -9806,6 +9820,8 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		connector_state->best_encoder = NULL;
 		connector_state->crtc = NULL;
 
+		crtc_state->base.enable = false;
+
 		intel_set_mode(crtc, NULL, 0, 0, NULL, state);
 
 		drm_atomic_state_free(state);
@@ -12112,14 +12128,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	intel_modeset_affected_pipes(crtc, modeset_pipes,
 				     prepare_pipes, disable_pipes);
 
-	for_each_intel_crtc_masked(dev, *disable_pipes, intel_crtc) {
-		pipe_config = intel_atomic_get_crtc_state(state, intel_crtc);
-		if (IS_ERR(pipe_config))
-			return pipe_config;
-
-		pipe_config->base.enable = false;
-	}
-
 	/*
 	 * Note this needs changes when we start tracking multiple modes
 	 * and crtcs.  At that point we'll need to compute the whole config
@@ -12136,8 +12144,6 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 		if (IS_ERR(pipe_config))
 			return pipe_config;
 
-		pipe_config->base.enable = true;
-
 		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
 				       "[modeset]");
 	}
@@ -12362,9 +12368,11 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
 	struct drm_atomic_state *state;
+	struct intel_crtc *intel_crtc;
 	struct intel_encoder *encoder;
 	struct intel_connector *connector;
 	struct drm_connector_state *connector_state;
+	struct intel_crtc_state *crtc_state;
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state) {
@@ -12402,6 +12410,21 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 		}
 	}
 
+	for_each_intel_crtc(dev, intel_crtc) {
+		if (intel_crtc->new_enabled == intel_crtc->base.enabled)
+			continue;
+
+		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
+		if (IS_ERR(crtc_state)) {
+			DRM_DEBUG_KMS("Failed to add [CRTC:%d] to state: %ld\n",
+				      intel_crtc->base.base.id,
+				      PTR_ERR(crtc_state));
+			continue;
+		}
+
+		crtc_state->base.enable = intel_crtc->new_enabled;
+	}
+
 	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb,
 		       state);
 
@@ -12609,6 +12632,7 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
+	struct intel_crtc_state *crtc_state;
 	int ro;
 
 	/* The upper layers ensure that we either disable a crtc or have a list
@@ -12715,6 +12739,14 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 				break;
 			}
 		}
+
+		if (crtc->new_enabled != crtc->base.state->enable) {
+			crtc_state = intel_atomic_get_crtc_state(state, crtc);
+			if (IS_ERR(crtc_state))
+				return PTR_ERR(crtc_state);
+
+			crtc_state->base.enable = crtc->new_enabled;
+		}
 	}
 
 	return 0;
-- 
2.1.0

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

[PATCH 13/35] drm/i915: Don't pretend we can calculate multiple pipe_configs

[Ander Conselvan de Oliveira]

The code in intel_modeset_pipe_config() still needs changes before it
can calculate more than just one pipe_config, and pretending it can will
only make those changes more difficult.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 45 +++++++++++++++---------------------
 1 file changed, 19 insertions(+), 26 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index e6e7c63..d1253b1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11310,33 +11310,29 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 	crtc_state->scaler_state = scaler_state;
 }
 
-static struct intel_crtc_state *
+static int
 intel_modeset_pipe_config(struct drm_crtc *crtc,
 			  struct drm_display_mode *mode,
-			  struct drm_atomic_state *state)
+			  struct drm_atomic_state *state,
+			  struct intel_crtc_state *pipe_config)
 {
 	struct intel_encoder *encoder;
 	struct drm_connector *connector;
 	struct drm_connector_state *connector_state;
-	struct intel_crtc_state *pipe_config;
 	int base_bpp, ret = -EINVAL;
 	int i;
 	bool retry = true;
 
 	if (!check_encoder_cloning(state, to_intel_crtc(crtc))) {
 		DRM_DEBUG_KMS("rejecting invalid cloning configuration\n");
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 	}
 
 	if (!check_digital_port_conflicts(state)) {
 		DRM_DEBUG_KMS("rejecting conflicting digital port configuration\n");
-		return ERR_PTR(-EINVAL);
+		return -EINVAL;
 	}
 
-	pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
-	if (IS_ERR(pipe_config))
-		return pipe_config;
-
 	clear_intel_crtc_state(pipe_config);
 
 	pipe_config->base.crtc = crtc;
@@ -11433,9 +11429,9 @@ encoder_retry:
 	DRM_DEBUG_KMS("plane bpp: %i, pipe bpp: %i, dithering: %i\n",
 		      base_bpp, pipe_config->pipe_bpp, pipe_config->dither);
 
-	return pipe_config;
+	return 0;
 fail:
-	return ERR_PTR(ret);
+	return ret;
 }
 
 /* Computes which crtcs are affected and sets the relevant bits in the mask. For
@@ -12116,9 +12112,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 			     unsigned *prepare_pipes,
 			     unsigned *disable_pipes)
 {
-	struct drm_device *dev = crtc->dev;
-	struct intel_crtc_state *pipe_config = NULL;
-	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *pipe_config;
 	int ret = 0;
 
 	ret = drm_atomic_add_affected_connectors(state, crtc);
@@ -12134,21 +12128,20 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	 * (i.e. one pipe_config for each crtc) rather than just the one
 	 * for this crtc.
 	 */
-	for_each_intel_crtc_masked(dev, *modeset_pipes, intel_crtc) {
-		/* FIXME: For now we still expect modeset_pipes has at most
-		 * one bit set. */
-		if (WARN_ON(&intel_crtc->base != crtc))
-			continue;
+	pipe_config = intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));
+	if (IS_ERR(pipe_config))
+		return pipe_config;
 
-		pipe_config = intel_modeset_pipe_config(crtc, mode, state);
-		if (IS_ERR(pipe_config))
-			return pipe_config;
+	if (!(*modeset_pipes & (1 << to_intel_crtc(crtc)->pipe)))
+		return pipe_config;
 
-		intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,
-				       "[modeset]");
-	}
+	ret = intel_modeset_pipe_config(crtc, mode, state, pipe_config);
+	if (ret)
+		return ERR_PTR(ret);
+
+	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");
 
-	return intel_atomic_get_crtc_state(state, to_intel_crtc(crtc));;
+	return pipe_config;
 }
 
 static int __intel_set_mode_setup_plls(struct drm_atomic_state *state,
-- 
2.1.0

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

[PATCH 14/35] drm/i915: Calculate a new pipe_config based on new enabled state

[Ander Conselvan de Oliveira]

With the current implementation of intel_modeset_affected_pipes(), if a
pipe will be enabled then it is in modeset_pipes. We'll remove that mask
in a follow up patch, but want to preserve this behavior, so just make
that explicit.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index d1253b1..b0755eb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12132,7 +12132,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	if (IS_ERR(pipe_config))
 		return pipe_config;
 
-	if (!(*modeset_pipes & (1 << to_intel_crtc(crtc)->pipe)))
+	if (!pipe_config->base.enable)
 		return pipe_config;
 
 	ret = intel_modeset_pipe_config(crtc, mode, state, pipe_config);
-- 
2.1.0

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

[PATCH 15/35] drm/i915: Remove all *_pipes flags from modeset

[Ander Conselvan de Oliveira]

Set the mode_changed field on the crtc_states and use that instead.

Note that even though this patch doesn't completely replace the logic in
intel_modeset_affected_pipes(), that logic was never fully used to its
full extent. Since the commit mentioned below, modeset_pipes and
prepare_pipes would only contain at most the pipe for which the set_crtc
ioctl was called. We can grow back that logic when the time comes.

commit b6c5164d7bf624f3e1b750787ddb983150c5117c
Author: Daniel Vetter <daniel.vetter@ffwll.ch>
Date:   Fri Apr 12 18:48:43 2013 +0200

    drm/i915: Fixup Oops in the pipe config computation

v2: Don't set mode_changed unconditionally for modeset_crtc. (Ander)
    Check for needs_modeset() before trying to allocate a PLL. (Ander)
    Only call .crtc_enable() for pipes that were disabled. (Maarten)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 311 +++++++++++++++--------------------
 1 file changed, 132 insertions(+), 179 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index b0755eb..dc604c9 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5664,13 +5664,13 @@ static int intel_mode_max_pixclk(struct drm_atomic_state *state)
 	return max_pixclk;
 }
 
-static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
-					    unsigned *prepare_pipes)
+static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
 {
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
-	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
 	int max_pixclk = intel_mode_max_pixclk(state);
-	int cdclk;
+	int cdclk, i;
 
 	if (max_pixclk < 0)
 		return max_pixclk;
@@ -5683,10 +5683,20 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state,
 	if (cdclk == dev_priv->cdclk_freq)
 		return 0;
 
+	/* add all active pipes to the state */
+	for_each_crtc(state->dev, crtc) {
+		if (!crtc->state->enable)
+			continue;
+
+		crtc_state = drm_atomic_get_crtc_state(state, crtc);
+		if (IS_ERR(crtc_state))
+			return PTR_ERR(crtc_state);
+	}
+
 	/* disable/enable all currently active pipes while we change cdclk */
-	for_each_intel_crtc(state->dev, intel_crtc)
-		if (intel_crtc->base.state->enable)
-			*prepare_pipes |= (1 << intel_crtc->pipe);
+	for_each_crtc_in_state(state, crtc, crtc_state, i)
+		if (crtc_state->enable)
+			crtc_state->mode_changed = true;
 
 	return 0;
 }
@@ -11434,94 +11444,6 @@ fail:
 	return ret;
 }
 
-/* Computes which crtcs are affected and sets the relevant bits in the mask. For
- * simplicity we use the crtc's pipe number (because it's easier to obtain). */
-static void
-intel_modeset_affected_pipes(struct drm_crtc *crtc, unsigned *modeset_pipes,
-			     unsigned *prepare_pipes, unsigned *disable_pipes)
-{
-	struct intel_crtc *intel_crtc;
-	struct drm_device *dev = crtc->dev;
-	struct intel_encoder *encoder;
-	struct intel_connector *connector;
-	struct drm_crtc *tmp_crtc;
-
-	*disable_pipes = *modeset_pipes = *prepare_pipes = 0;
-
-	/* Check which crtcs have changed outputs connected to them, these need
-	 * to be part of the prepare_pipes mask. We don't (yet) support global
-	 * modeset across multiple crtcs, so modeset_pipes will only have one
-	 * bit set at most. */
-	for_each_intel_connector(dev, connector) {
-		if (connector->base.encoder == &connector->new_encoder->base)
-			continue;
-
-		if (connector->base.encoder) {
-			tmp_crtc = connector->base.encoder->crtc;
-
-			*prepare_pipes |= 1 << to_intel_crtc(tmp_crtc)->pipe;
-		}
-
-		if (connector->new_encoder)
-			*prepare_pipes |=
-				1 << connector->new_encoder->new_crtc->pipe;
-	}
-
-	for_each_intel_encoder(dev, encoder) {
-		if (encoder->base.crtc == &encoder->new_crtc->base)
-			continue;
-
-		if (encoder->base.crtc) {
-			tmp_crtc = encoder->base.crtc;
-
-			*prepare_pipes |= 1 << to_intel_crtc(tmp_crtc)->pipe;
-		}
-
-		if (encoder->new_crtc)
-			*prepare_pipes |= 1 << encoder->new_crtc->pipe;
-	}
-
-	/* Check for pipes that will be enabled/disabled ... */
-	for_each_intel_crtc(dev, intel_crtc) {
-		if (intel_crtc->base.state->enable == intel_crtc->new_enabled)
-			continue;
-
-		if (!intel_crtc->new_enabled)
-			*disable_pipes |= 1 << intel_crtc->pipe;
-		else
-			*prepare_pipes |= 1 << intel_crtc->pipe;
-	}
-
-
-	/* set_mode is also used to update properties on life display pipes. */
-	intel_crtc = to_intel_crtc(crtc);
-	if (intel_crtc->new_enabled)
-		*prepare_pipes |= 1 << intel_crtc->pipe;
-
-	/*
-	 * For simplicity do a full modeset on any pipe where the output routing
-	 * changed. We could be more clever, but that would require us to be
-	 * more careful with calling the relevant encoder->mode_set functions.
-	 */
-	if (*prepare_pipes)
-		*modeset_pipes = *prepare_pipes;
-
-	/* ... and mask these out. */
-	*modeset_pipes &= ~(*disable_pipes);
-	*prepare_pipes &= ~(*disable_pipes);
-
-	/*
-	 * HACK: We don't (yet) fully support global modesets. intel_set_config
-	 * obies this rule, but the modeset restore mode of
-	 * intel_modeset_setup_hw_state does not.
-	 */
-	*modeset_pipes &= 1 << intel_crtc->pipe;
-	*prepare_pipes &= 1 << intel_crtc->pipe;
-
-	DRM_DEBUG_KMS("set mode pipe masks: modeset: %x, prepare: %x, disable: %x\n",
-		      *modeset_pipes, *prepare_pipes, *disable_pipes);
-}
-
 static bool intel_crtc_in_use(struct drm_crtc *crtc)
 {
 	struct drm_encoder *encoder;
@@ -11534,13 +11456,22 @@ static bool intel_crtc_in_use(struct drm_crtc *crtc)
 	return false;
 }
 
+static bool
+needs_modeset(struct drm_crtc_state *state)
+{
+	return state->mode_changed || state->active_changed;
+}
+
 static void
-intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
+intel_modeset_update_state(struct drm_atomic_state *state)
 {
+	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct intel_encoder *intel_encoder;
-	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
 	struct drm_connector *connector;
+	int i;
 
 	intel_shared_dpll_commit(dev_priv);
 
@@ -11548,26 +11479,36 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
 		if (!intel_encoder->base.crtc)
 			continue;
 
-		intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
+		for_each_crtc_in_state(state, crtc, crtc_state, i)
+			if (crtc == intel_encoder->base.crtc)
+				break;
 
-		if (prepare_pipes & (1 << intel_crtc->pipe))
+		if (crtc != intel_encoder->base.crtc)
+			continue;
+
+		if (crtc_state->enable && needs_modeset(crtc_state))
 			intel_encoder->connectors_active = false;
 	}
 
 	intel_modeset_commit_output_state(dev);
 
 	/* Double check state. */
-	for_each_intel_crtc(dev, intel_crtc) {
-		WARN_ON(intel_crtc->base.state->enable != intel_crtc_in_use(&intel_crtc->base));
+	for_each_crtc(dev, crtc) {
+		WARN_ON(crtc->state->enable != intel_crtc_in_use(crtc));
 	}
 
 	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
 		if (!connector->encoder || !connector->encoder->crtc)
 			continue;
 
-		intel_crtc = to_intel_crtc(connector->encoder->crtc);
+		for_each_crtc_in_state(state, crtc, crtc_state, i)
+			if (crtc == connector->encoder->crtc)
+				break;
+
+		if (crtc != connector->encoder->crtc)
+			continue;
 
-		if (prepare_pipes & (1 << intel_crtc->pipe)) {
+		if (crtc_state->enable && needs_modeset(crtc_state)) {
 			struct drm_property *dpms_property =
 				dev->mode_config.dpms_property;
 
@@ -12104,13 +12045,28 @@ static void update_scanline_offset(struct intel_crtc *crtc)
 		crtc->scanline_offset = 1;
 }
 
+static void
+intel_atomic_modeset_compute_changed_flags(struct drm_atomic_state *state,
+					   struct drm_crtc *modeset_crtc)
+{
+	struct drm_crtc_state *crtc_state;
+	struct drm_crtc *crtc;
+	int i;
+
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		if (crtc_state->enable != crtc->state->enable)
+			crtc_state->mode_changed = true;
+
+		/* FIXME: Do we need to always set mode_changed for
+		 * modeset_crtc if it is enabled? modeset_affect_pipes()
+		 * did that. */
+	}
+}
+
 static struct intel_crtc_state *
 intel_modeset_compute_config(struct drm_crtc *crtc,
 			     struct drm_display_mode *mode,
-			     struct drm_atomic_state *state,
-			     unsigned *modeset_pipes,
-			     unsigned *prepare_pipes,
-			     unsigned *disable_pipes)
+			     struct drm_atomic_state *state)
 {
 	struct intel_crtc_state *pipe_config;
 	int ret = 0;
@@ -12119,8 +12075,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	if (ret)
 		return ERR_PTR(ret);
 
-	intel_modeset_affected_pipes(crtc, modeset_pipes,
-				     prepare_pipes, disable_pipes);
+	intel_atomic_modeset_compute_changed_flags(state, crtc);
 
 	/*
 	 * Note this needs changes when we start tracking multiple modes
@@ -12144,33 +12099,41 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	return pipe_config;
 }
 
-static int __intel_set_mode_setup_plls(struct drm_atomic_state *state,
-				       unsigned modeset_pipes,
-				       unsigned disable_pipes)
+static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
 {
 	struct drm_device *dev = state->dev;
 	struct drm_i915_private *dev_priv = to_i915(dev);
-	unsigned clear_pipes = modeset_pipes | disable_pipes;
+	unsigned clear_pipes = 0;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *intel_crtc_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
 	int ret = 0;
+	int i;
 
 	if (!dev_priv->display.crtc_compute_clock)
 		return 0;
 
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		intel_crtc = to_intel_crtc(crtc);
+
+		if (needs_modeset(crtc_state))
+			clear_pipes |= 1 << intel_crtc->pipe;
+	}
+
 	ret = intel_shared_dpll_start_config(dev_priv, clear_pipes);
 	if (ret)
 		goto done;
 
-	for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
-		struct intel_crtc_state *crtc_state =
-			intel_atomic_get_crtc_state(state, intel_crtc);
-
-		/* Modeset pipes should have a new state by now */
-		if (WARN_ON(IS_ERR(crtc_state)))
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		if (!needs_modeset(crtc_state) || !crtc_state->enable)
 			continue;
 
+		intel_crtc = to_intel_crtc(crtc);
+		intel_crtc_state = to_intel_crtc_state(crtc_state);
+
 		ret = dev_priv->display.crtc_compute_clock(intel_crtc,
-							   crtc_state);
+							   intel_crtc_state);
 		if (ret) {
 			intel_shared_dpll_abort_config(dev_priv);
 			goto done;
@@ -12181,21 +12144,21 @@ done:
 	return ret;
 }
 
-static int __intel_set_mode(struct drm_crtc *crtc,
+static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 			    struct drm_display_mode *mode,
 			    int x, int y, struct drm_framebuffer *fb,
-			    struct intel_crtc_state *pipe_config,
-			    unsigned modeset_pipes,
-			    unsigned prepare_pipes,
-			    unsigned disable_pipes)
+			    struct intel_crtc_state *pipe_config)
 {
-	struct drm_device *dev = crtc->dev;
+	struct drm_device *dev = modeset_crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_display_mode *saved_mode;
 	struct drm_atomic_state *state = pipe_config->base.state;
 	struct intel_crtc_state *crtc_state_copy = NULL;
 	struct intel_crtc *intel_crtc;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
 	int ret = 0;
+	int i;
 
 	saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
 	if (!saved_mode)
@@ -12207,7 +12170,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 		goto done;
 	}
 
-	*saved_mode = crtc->mode;
+	*saved_mode = modeset_crtc->mode;
 
 	/*
 	 * See if the config requires any additional preparation, e.g.
@@ -12217,25 +12180,24 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	 * adjusted_mode bits in the crtc directly.
 	 */
 	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
-		ret = valleyview_modeset_global_pipes(state, &prepare_pipes);
+		ret = valleyview_modeset_global_pipes(state);
 		if (ret)
 			goto done;
-
-		/* may have added more to prepare_pipes than we should */
-		prepare_pipes &= ~disable_pipes;
 	}
 
-	ret = __intel_set_mode_setup_plls(state, modeset_pipes, disable_pipes);
+	ret = __intel_set_mode_setup_plls(state);
 	if (ret)
 		goto done;
 
-	for_each_intel_crtc_masked(dev, disable_pipes, intel_crtc)
-		intel_crtc_disable(&intel_crtc->base);
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		if (!needs_modeset(crtc_state))
+			continue;
 
-	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
-		if (intel_crtc->base.state->enable) {
-			intel_crtc_disable_planes(&intel_crtc->base);
-			dev_priv->display.crtc_disable(&intel_crtc->base);
+		if (!crtc_state->enable) {
+			intel_crtc_disable(crtc);
+		} else if (crtc->state->enable) {
+			intel_crtc_disable_planes(crtc);
+			dev_priv->display.crtc_disable(crtc);
 		}
 	}
 
@@ -12246,32 +12208,36 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	 * pipes; here we assume a single modeset_pipe and only track the
 	 * single crtc and mode.
 	 */
-	if (modeset_pipes) {
-		crtc->mode = *mode;
+	if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) {
+		modeset_crtc->mode = *mode;
 		/* mode_set/enable/disable functions rely on a correct pipe
 		 * config. */
-		intel_crtc_set_state(to_intel_crtc(crtc), pipe_config);
+		intel_crtc_set_state(to_intel_crtc(modeset_crtc), pipe_config);
 
 		/*
 		 * Calculate and store various constants which
 		 * are later needed by vblank and swap-completion
 		 * timestamping. They are derived from true hwmode.
 		 */
-		drm_calc_timestamping_constants(crtc,
+		drm_calc_timestamping_constants(modeset_crtc,
 						&pipe_config->base.adjusted_mode);
 	}
 
 	/* Only after disabling all output pipelines that will be changed can we
 	 * update the the output configuration. */
-	intel_modeset_update_state(dev, prepare_pipes);
+	intel_modeset_update_state(state);
 
 	modeset_update_crtc_power_domains(state);
 
-	for_each_intel_crtc_masked(dev, modeset_pipes, intel_crtc) {
-		struct drm_plane *primary = intel_crtc->base.primary;
+	if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) {
+		struct drm_plane *primary;
 		int vdisplay, hdisplay;
 
+		intel_crtc = to_intel_crtc(modeset_crtc);
+		primary = intel_crtc->base.primary;
+
 		drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
+
 		ret = drm_plane_helper_update(primary, &intel_crtc->base,
 					      fb, 0, 0,
 					      hdisplay, vdisplay,
@@ -12280,20 +12246,23 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 	}
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
-	for_each_intel_crtc_masked(dev, prepare_pipes, intel_crtc) {
-		update_scanline_offset(intel_crtc);
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		if (!needs_modeset(crtc_state) || !crtc_state->enable)
+			continue;
+
+		update_scanline_offset(to_intel_crtc(crtc));
 
-		dev_priv->display.crtc_enable(&intel_crtc->base);
-		intel_crtc_enable_planes(&intel_crtc->base);
+		dev_priv->display.crtc_enable(crtc);
+		intel_crtc_enable_planes(crtc);
 	}
 
 	/* FIXME: add subpixel order */
 done:
-	if (ret && crtc->state->enable)
-		crtc->mode = *saved_mode;
+	if (ret && modeset_crtc->state->enable)
+		modeset_crtc->mode = *saved_mode;
 
 	if (ret == 0 && pipe_config) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+		struct intel_crtc *intel_crtc = to_intel_crtc(modeset_crtc);
 
 		/* The pipe_config will be freed with the atomic state, so
 		 * make a copy. */
@@ -12309,18 +12278,14 @@ done:
 	return ret;
 }
 
-static int intel_set_mode_pipes(struct drm_crtc *crtc,
-				struct drm_display_mode *mode,
-				int x, int y, struct drm_framebuffer *fb,
-				struct intel_crtc_state *pipe_config,
-				unsigned modeset_pipes,
-				unsigned prepare_pipes,
-				unsigned disable_pipes)
+static int intel_set_mode_with_config(struct drm_crtc *crtc,
+				      struct drm_display_mode *mode,
+				      int x, int y, struct drm_framebuffer *fb,
+				      struct intel_crtc_state *pipe_config)
 {
 	int ret;
 
-	ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config, modeset_pipes,
-			       prepare_pipes, disable_pipes);
+	ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config);
 
 	if (ret == 0)
 		intel_modeset_check_state(crtc->dev);
@@ -12334,22 +12299,15 @@ static int intel_set_mode(struct drm_crtc *crtc,
 			  struct drm_atomic_state *state)
 {
 	struct intel_crtc_state *pipe_config;
-	unsigned modeset_pipes, prepare_pipes, disable_pipes;
 	int ret = 0;
 
-	pipe_config = intel_modeset_compute_config(crtc, mode, state,
-						   &modeset_pipes,
-						   &prepare_pipes,
-						   &disable_pipes);
-
+	pipe_config = intel_modeset_compute_config(crtc, mode, state);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
 		goto out;
 	}
 
-	ret = intel_set_mode_pipes(crtc, mode, x, y, fb, pipe_config,
-				   modeset_pipes, prepare_pipes,
-				   disable_pipes);
+	ret = intel_set_mode_with_config(crtc, mode, x, y, fb, pipe_config);
 	if (ret)
 		goto out;
 
@@ -12775,7 +12733,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	struct drm_atomic_state *state = NULL;
 	struct intel_set_config *config;
 	struct intel_crtc_state *pipe_config;
-	unsigned modeset_pipes, prepare_pipes, disable_pipes;
 	int ret;
 
 	BUG_ON(!set);
@@ -12830,10 +12787,7 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	intel_set_config_compute_mode_changes(set, config);
 
 	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
-						   state,
-						   &modeset_pipes,
-						   &prepare_pipes,
-						   &disable_pipes);
+						   state);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
 		goto fail;
@@ -12853,10 +12807,9 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
 	if (config->mode_changed) {
-		ret = intel_set_mode_pipes(set->crtc, set->mode,
-					   set->x, set->y, set->fb, pipe_config,
-					   modeset_pipes, prepare_pipes,
-					   disable_pipes);
+		ret = intel_set_mode_with_config(set->crtc, set->mode,
+						 set->x, set->y, set->fb,
+						 pipe_config);
 	} else if (config->fb_changed) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 		struct drm_plane *primary = set->crtc->primary;
-- 
2.1.0

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

[PATCH 16/35] drm/i915: Remove saved_mode from __intel_set_mode()

[Ander Conselvan de Oliveira]

There's no way that function can fail after it sets crtc->mode anymore,
so there's no need to save the old mode for the failure case.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 11 -----------
 1 file changed, 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index dc604c9..69e07c1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12151,7 +12151,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 {
 	struct drm_device *dev = modeset_crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	struct drm_display_mode *saved_mode;
 	struct drm_atomic_state *state = pipe_config->base.state;
 	struct intel_crtc_state *crtc_state_copy = NULL;
 	struct intel_crtc *intel_crtc;
@@ -12160,18 +12159,12 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	int ret = 0;
 	int i;
 
-	saved_mode = kmalloc(sizeof(*saved_mode), GFP_KERNEL);
-	if (!saved_mode)
-		return -ENOMEM;
-
 	crtc_state_copy = kmalloc(sizeof(*crtc_state_copy), GFP_KERNEL);
 	if (!crtc_state_copy) {
 		ret = -ENOMEM;
 		goto done;
 	}
 
-	*saved_mode = modeset_crtc->mode;
-
 	/*
 	 * See if the config requires any additional preparation, e.g.
 	 * to adjust global state with pipes off.  We need to do this
@@ -12258,9 +12251,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 	/* FIXME: add subpixel order */
 done:
-	if (ret && modeset_crtc->state->enable)
-		modeset_crtc->mode = *saved_mode;
-
 	if (ret == 0 && pipe_config) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(modeset_crtc);
 
@@ -12274,7 +12264,6 @@ done:
 		kfree(crtc_state_copy);
 	}
 
-	kfree(saved_mode);
 	return ret;
 }
 
-- 
2.1.0

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

[PATCH 17/35] drm/i915: Move compute part of __intel_set_mode() to separate function

[Ander Conselvan de Oliveira]

The first function calls done in that function can still cause changes
to the atomic state and may fail. This should eventually be part of our
atomic check function, while the rest of the code in __intel_set_mode()
is the commit hook. So this makes the legacy mode set more atomic-y.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++++++-------------
 1 file changed, 30 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 69e07c1..4767ff6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12144,6 +12144,32 @@ done:
 	return ret;
 }
 
+/* Code that should eventually be part of atomic_check() */
+static int __intel_set_mode_checks(struct drm_atomic_state *state)
+{
+	struct drm_device *dev = state->dev;
+	int ret;
+
+	/*
+	 * See if the config requires any additional preparation, e.g.
+	 * to adjust global state with pipes off.  We need to do this
+	 * here so we can get the modeset_pipe updated config for the new
+	 * mode set on this crtc.  For other crtcs we need to use the
+	 * adjusted_mode bits in the crtc directly.
+	 */
+	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
+		ret = valleyview_modeset_global_pipes(state);
+		if (ret)
+			return ret;
+	}
+
+	ret = __intel_set_mode_setup_plls(state);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
 static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 			    struct drm_display_mode *mode,
 			    int x, int y, struct drm_framebuffer *fb,
@@ -12159,29 +12185,16 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	int ret = 0;
 	int i;
 
+	ret = __intel_set_mode_checks(state);
+	if (ret < 0)
+		return ret;
+
 	crtc_state_copy = kmalloc(sizeof(*crtc_state_copy), GFP_KERNEL);
 	if (!crtc_state_copy) {
 		ret = -ENOMEM;
 		goto done;
 	}
 
-	/*
-	 * See if the config requires any additional preparation, e.g.
-	 * to adjust global state with pipes off.  We need to do this
-	 * here so we can get the modeset_pipe updated config for the new
-	 * mode set on this crtc.  For other crtcs we need to use the
-	 * adjusted_mode bits in the crtc directly.
-	 */
-	if (IS_VALLEYVIEW(dev) || IS_BROXTON(dev)) {
-		ret = valleyview_modeset_global_pipes(state);
-		if (ret)
-			goto done;
-	}
-
-	ret = __intel_set_mode_setup_plls(state);
-	if (ret)
-		goto done;
-
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!needs_modeset(crtc_state))
 			continue;
-- 
2.1.0

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

[PATCH 18/35] drm/i915: Simplify error handling in __intel_set_mode()

[Ander Conselvan de Oliveira]

The remaining parts of the failure path could only be reached if the
allocation of crtc_state_copy would fail. In that case, there is nothing
to undo, so just get rid of the label for error handling and return an
error code immediately.

We also always allocate a pipe_config, even if the pipe is being
disabled, so the remaining part of what was the error/done case can be
simplified a little too.

v2: Ignore return value from drm_plane_helper_update(). (Ander)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 30 ++++++++++++------------------
 1 file changed, 12 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4767ff6..083cf23 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12190,10 +12190,8 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 		return ret;
 
 	crtc_state_copy = kmalloc(sizeof(*crtc_state_copy), GFP_KERNEL);
-	if (!crtc_state_copy) {
-		ret = -ENOMEM;
-		goto done;
-	}
+	if (!crtc_state_copy)
+		return -ENOMEM;
 
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!needs_modeset(crtc_state))
@@ -12249,6 +12247,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 					      hdisplay, vdisplay,
 					      x << 16, y << 16,
 					      hdisplay << 16, vdisplay << 16);
+		WARN_ON(ret != 0);
 	}
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
@@ -12263,21 +12262,16 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	}
 
 	/* FIXME: add subpixel order */
-done:
-	if (ret == 0 && pipe_config) {
-		struct intel_crtc *intel_crtc = to_intel_crtc(modeset_crtc);
-
-		/* The pipe_config will be freed with the atomic state, so
-		 * make a copy. */
-		memcpy(crtc_state_copy, intel_crtc->config,
-		       sizeof *crtc_state_copy);
-		intel_crtc->config = crtc_state_copy;
-		intel_crtc->base.state = &crtc_state_copy->base;
-	} else {
-		kfree(crtc_state_copy);
-	}
 
-	return ret;
+	intel_crtc = to_intel_crtc(modeset_crtc);
+
+	/* The pipe_config will be freed with the atomic state, so
+	 * make a copy. */
+	memcpy(crtc_state_copy, intel_crtc->config, sizeof *crtc_state_copy);
+	intel_crtc->config = crtc_state_copy;
+	intel_crtc->base.state = &crtc_state_copy->base;
+
+	return 0;
 }
 
 static int intel_set_mode_with_config(struct drm_crtc *crtc,
-- 
2.1.0

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

[PATCH 19/35] drm/i915: Don't modeset with old mode when set_crtc fails

[Ander Conselvan de Oliveira]

The modeset code is now properly divided in two phases, so that it only
changes hardware state if it succeeds, so there's no ill-effect that
needs to be undone on failure anymore.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 48 ------------------------------------
 1 file changed, 48 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 083cf23..902870a 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12699,33 +12699,9 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	return 0;
 }
 
-static void disable_crtc_nofb(struct intel_crtc *crtc)
-{
-	struct drm_device *dev = crtc->base.dev;
-	struct intel_encoder *encoder;
-	struct intel_connector *connector;
-
-	DRM_DEBUG_KMS("Trying to restore without FB -> disabling pipe %c\n",
-		      pipe_name(crtc->pipe));
-
-	for_each_intel_connector(dev, connector) {
-		if (connector->new_encoder &&
-		    connector->new_encoder->new_crtc == crtc)
-			connector->new_encoder = NULL;
-	}
-
-	for_each_intel_encoder(dev, encoder) {
-		if (encoder->new_crtc == crtc)
-			encoder->new_crtc = NULL;
-	}
-
-	crtc->new_enabled = false;
-}
-
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
-	struct drm_mode_set save_set;
 	struct drm_atomic_state *state = NULL;
 	struct intel_set_config *config;
 	struct intel_crtc_state *pipe_config;
@@ -12758,12 +12734,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret)
 		goto out_config;
 
-	save_set.crtc = set->crtc;
-	save_set.mode = &set->crtc->mode;
-	save_set.x = set->crtc->x;
-	save_set.y = set->crtc->y;
-	save_set.fb = set->crtc->primary->fb;
-
 	state = drm_atomic_state_alloc(dev);
 	if (!state) {
 		ret = -ENOMEM;
@@ -12847,24 +12817,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 			      set->crtc->base.id, ret);
 fail:
 		intel_set_config_restore_state(dev, config);
-
-		drm_atomic_state_clear(state);
-
-		/*
-		 * HACK: if the pipe was on, but we didn't have a framebuffer,
-		 * force the pipe off to avoid oopsing in the modeset code
-		 * due to fb==NULL. This should only happen during boot since
-		 * we don't yet reconstruct the FB from the hardware state.
-		 */
-		if (to_intel_crtc(save_set.crtc)->new_enabled && !save_set.fb)
-			disable_crtc_nofb(to_intel_crtc(save_set.crtc));
-
-		/* Try to restore the config */
-		if (config->mode_changed &&
-		    intel_set_mode(save_set.crtc, save_set.mode,
-				   save_set.x, save_set.y, save_set.fb,
-				   state))
-			DRM_ERROR("failed to restore config after modeset failure\n");
 	}
 
 out_config:
-- 
2.1.0

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

[PATCH 20/35] drm/i915: Add primary plane to atomic state in legacy modeset

[Ander Conselvan de Oliveira]

Add the primary plane state to the legacy modeset atomic state and use
it when configuring the primary plane in __intel_set_mode(). This is a
first step towards merging the flip path in intel_crtc_set_config() and
__intel_set_mode().

v2: Set crtc to NULL if fb is NULL. (Maarten)

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 83 ++++++++++++++++++++++++++++++------
 1 file changed, 71 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 902870a..382b621 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9622,6 +9622,41 @@ mode_fits_in_fbdev(struct drm_device *dev,
 #endif
 }
 
+static int intel_modeset_setup_plane_state(struct drm_atomic_state *state,
+					   struct drm_crtc *crtc,
+					   struct drm_display_mode *mode,
+					   struct drm_framebuffer *fb,
+					   int x, int y)
+{
+	struct drm_plane_state *plane_state;
+	int hdisplay, vdisplay;
+	int ret;
+
+	plane_state = drm_atomic_get_plane_state(state, crtc->primary);
+	if (IS_ERR(plane_state))
+		return PTR_ERR(plane_state);
+
+	if (mode)
+		drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
+	else
+		hdisplay = vdisplay = 0;
+
+	ret = drm_atomic_set_crtc_for_plane(plane_state, fb ? crtc : NULL);
+	if (ret)
+		return ret;
+	drm_atomic_set_fb_for_plane(plane_state, fb);
+	plane_state->crtc_x = 0;
+	plane_state->crtc_y = 0;
+	plane_state->crtc_w = hdisplay;
+	plane_state->crtc_h = vdisplay;
+	plane_state->src_x = x << 16;
+	plane_state->src_y = y << 16;
+	plane_state->src_w = hdisplay << 16;
+	plane_state->src_h = vdisplay << 16;
+
+	return 0;
+}
+
 bool intel_get_load_detect_pipe(struct drm_connector *connector,
 				struct drm_display_mode *mode,
 				struct intel_load_detect_pipe *old,
@@ -9764,6 +9799,10 @@ retry:
 		goto fail;
 	}
 
+	ret = intel_modeset_setup_plane_state(state, crtc, mode, fb, 0, 0);
+	if (ret)
+		goto fail;
+
 	if (intel_set_mode(crtc, mode, 0, 0, fb, state)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
 		if (old->release_fb)
@@ -9803,6 +9842,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 	struct drm_atomic_state *state;
 	struct drm_connector_state *connector_state;
 	struct intel_crtc_state *crtc_state;
+	int ret;
 
 	DRM_DEBUG_KMS("[CONNECTOR:%d:%s], [ENCODER:%d:%s]\n",
 		      connector->base.id, connector->name,
@@ -9832,6 +9872,11 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 
 		crtc_state->base.enable = false;
 
+		ret = intel_modeset_setup_plane_state(state, crtc, NULL, NULL,
+						      0, 0);
+		if (ret)
+			goto fail;
+
 		intel_set_mode(crtc, NULL, 0, 0, NULL, state);
 
 		drm_atomic_state_free(state);
@@ -12182,6 +12227,8 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
+	struct drm_plane *plane;
+	struct drm_plane_state *plane_state;
 	int ret = 0;
 	int i;
 
@@ -12233,20 +12280,24 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 	modeset_update_crtc_power_domains(state);
 
-	if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) {
-		struct drm_plane *primary;
-		int vdisplay, hdisplay;
-
-		intel_crtc = to_intel_crtc(modeset_crtc);
-		primary = intel_crtc->base.primary;
+	for_each_plane_in_state(state, plane, plane_state, i) {
+		if (WARN_ON(plane != modeset_crtc->primary))
+			continue;
 
-		drm_crtc_get_hv_timing(mode, &hdisplay, &vdisplay);
+		/* Primary plane is disabled in intel_crtc_disable() */
+		if (!pipe_config->base.enable)
+			continue;
 
-		ret = drm_plane_helper_update(primary, &intel_crtc->base,
-					      fb, 0, 0,
-					      hdisplay, vdisplay,
-					      x << 16, y << 16,
-					      hdisplay << 16, vdisplay << 16);
+		ret = drm_plane_helper_update(plane, plane_state->crtc,
+					      plane_state->fb,
+					      plane_state->crtc_x,
+					      plane_state->crtc_y,
+					      plane_state->crtc_w,
+					      plane_state->crtc_h,
+					      plane_state->src_x,
+					      plane_state->src_y,
+					      plane_state->src_w,
+					      plane_state->src_h);
 		WARN_ON(ret != 0);
 	}
 
@@ -12372,6 +12423,9 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 		crtc_state->base.enable = intel_crtc->new_enabled;
 	}
 
+	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
+					crtc->primary->fb, crtc->x, crtc->y);
+
 	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb,
 		       state);
 
@@ -12746,6 +12800,11 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret)
 		goto fail;
 
+	ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,
+					      set->fb, set->x, set->y);
+	if (ret)
+		goto fail;
+
 	/* Compute whether we need a full modeset, only an fb base update or no
 	 * change at all. In the future we might also check whether only the
 	 * mode changed, e.g. for LVDS where we only change the panel fitter in
-- 
2.1.0

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

[PATCH 21/35] drm/i915: Delete fb, x and y parameters from mode set functions

[Ander Conselvan de Oliveira]

We don't need to pass it down the call chain anymore now that the plane
state is set up properly.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 +++++-----------
 1 file changed, 5 insertions(+), 11 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 382b621..08694f8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -83,7 +83,6 @@ static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config);
 
 static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
-			  int x, int y, struct drm_framebuffer *old_fb,
 			  struct drm_atomic_state *state);
 static int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *ifb,
@@ -9803,7 +9802,7 @@ retry:
 	if (ret)
 		goto fail;
 
-	if (intel_set_mode(crtc, mode, 0, 0, fb, state)) {
+	if (intel_set_mode(crtc, mode, state)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
 		if (old->release_fb)
 			old->release_fb->funcs->destroy(old->release_fb);
@@ -9877,7 +9876,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		if (ret)
 			goto fail;
 
-		intel_set_mode(crtc, NULL, 0, 0, NULL, state);
+		intel_set_mode(crtc, NULL, state);
 
 		drm_atomic_state_free(state);
 
@@ -12217,7 +12216,6 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 
 static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 			    struct drm_display_mode *mode,
-			    int x, int y, struct drm_framebuffer *fb,
 			    struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = modeset_crtc->dev;
@@ -12327,12 +12325,11 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 static int intel_set_mode_with_config(struct drm_crtc *crtc,
 				      struct drm_display_mode *mode,
-				      int x, int y, struct drm_framebuffer *fb,
 				      struct intel_crtc_state *pipe_config)
 {
 	int ret;
 
-	ret = __intel_set_mode(crtc, mode, x, y, fb, pipe_config);
+	ret = __intel_set_mode(crtc, mode, pipe_config);
 
 	if (ret == 0)
 		intel_modeset_check_state(crtc->dev);
@@ -12342,7 +12339,6 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc,
 
 static int intel_set_mode(struct drm_crtc *crtc,
 			  struct drm_display_mode *mode,
-			  int x, int y, struct drm_framebuffer *fb,
 			  struct drm_atomic_state *state)
 {
 	struct intel_crtc_state *pipe_config;
@@ -12354,7 +12350,7 @@ static int intel_set_mode(struct drm_crtc *crtc,
 		goto out;
 	}
 
-	ret = intel_set_mode_with_config(crtc, mode, x, y, fb, pipe_config);
+	ret = intel_set_mode_with_config(crtc, mode, pipe_config);
 	if (ret)
 		goto out;
 
@@ -12426,8 +12422,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
 					crtc->primary->fb, crtc->x, crtc->y);
 
-	intel_set_mode(crtc, &crtc->mode, crtc->x, crtc->y, crtc->primary->fb,
-		       state);
+	intel_set_mode(crtc, &crtc->mode, state);
 
 	drm_atomic_state_free(state);
 }
@@ -12833,7 +12828,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	if (config->mode_changed) {
 		ret = intel_set_mode_with_config(set->crtc, set->mode,
-						 set->x, set->y, set->fb,
 						 pipe_config);
 	} else if (config->fb_changed) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
-- 
2.1.0

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

[PATCH 22/35] drm/i915: Don't use struct intel_set_config *_changed flags

[Ander Conselvan de Oliveira]

Use the similar fields in crtc_state instead, so that this code can be
moved to our future implementation of atomic_check().

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 66 +++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 08694f8..8caddc5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12138,6 +12138,18 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	if (ret)
 		return ERR_PTR(ret);
 
+	/* Check things that can only be changed through modeset */
+	if (pipe_config->has_audio !=
+	    to_intel_crtc(crtc)->config->has_audio)
+		pipe_config->base.mode_changed = true;
+
+	/*
+	 * Note we have an issue here with infoframes: current code
+	 * only updates them on the full mode set path per hw
+	 * requirements.  So here we should be checking for any
+	 * required changes and forcing a mode set.
+	 */
+
 	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");
 
 	return pipe_config;
@@ -12536,7 +12548,7 @@ is_crtc_connector_off(struct drm_mode_set *set)
 
 static void
 intel_set_config_compute_mode_changes(struct drm_mode_set *set,
-				      struct intel_set_config *config)
+				      struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = set->crtc->dev;
 	struct intel_connector *connector;
@@ -12546,7 +12558,7 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 	/* We should be able to check here if the fb has the same properties
 	 * and then just flip_or_move it */
 	if (is_crtc_connector_off(set)) {
-		config->mode_changed = true;
+		pipe_config->base.mode_changed = true;
 	} else if (set->crtc->primary->fb != set->fb) {
 		/*
 		 * If we have no fb, we can only flip as long as the crtc is
@@ -12560,36 +12572,36 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 
 			if (intel_crtc->active) {
 				DRM_DEBUG_KMS("crtc has no fb, will flip\n");
-				config->fb_changed = true;
+				pipe_config->base.planes_changed = true;
 			} else {
 				DRM_DEBUG_KMS("inactive crtc, full mode set\n");
-				config->mode_changed = true;
+				pipe_config->base.mode_changed = true;
 			}
 		} else if (set->fb == NULL) {
-			config->mode_changed = true;
+			pipe_config->base.mode_changed = true;
 		} else if (set->fb->pixel_format !=
 			   set->crtc->primary->fb->pixel_format) {
-			config->mode_changed = true;
+			pipe_config->base.mode_changed = true;
 		} else {
-			config->fb_changed = true;
+			pipe_config->base.planes_changed = true;
 		}
 	}
 
 	if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))
-		config->fb_changed = true;
+		pipe_config->base.planes_changed = true;
 
 	if (set->mode && !drm_mode_equal(set->mode, &set->crtc->mode)) {
 		DRM_DEBUG_KMS("modes are different, full mode set\n");
 		drm_mode_debug_printmodeline(&set->crtc->mode);
 		drm_mode_debug_printmodeline(set->mode);
-		config->mode_changed = true;
+		pipe_config->base.mode_changed = true;
 	}
 
 	for_each_intel_connector(dev, connector) {
 		if (&connector->new_encoder->base == connector->base.encoder)
 			continue;
 
-		config->mode_changed = true;
+		pipe_config->base.mode_changed = true;
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
 			      connector->base.base.id,
 			      connector->base.name);
@@ -12602,7 +12614,7 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 		DRM_DEBUG_KMS("[ENCODER:%d:%s] crtc changed, full mode switch\n",
 			      encoder->base.base.id,
 			      encoder->base.name);
-		config->mode_changed = true;
+		pipe_config->base.mode_changed = true;
 	}
 
 	for_each_intel_crtc(dev, crtc) {
@@ -12612,11 +12624,12 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 		DRM_DEBUG_KMS("[CRTC:%d] %sabled, full mode switch\n",
 			      crtc->base.base.id,
 			      crtc->new_enabled ? "en" : "dis");
-		config->mode_changed = true;
+		pipe_config->base.mode_changed = true;
 	}
 
 	DRM_DEBUG_KMS("computed changes for [CRTC:%d], mode_changed=%d, fb_changed=%d\n",
-			set->crtc->base.id, config->mode_changed, config->fb_changed);
+			set->crtc->base.id, pipe_config->base.mode_changed,
+			pipe_config->base.planes_changed);
 }
 
 static int
@@ -12800,36 +12813,25 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret)
 		goto fail;
 
-	/* Compute whether we need a full modeset, only an fb base update or no
-	 * change at all. In the future we might also check whether only the
-	 * mode changed, e.g. for LVDS where we only change the panel fitter in
-	 * such cases. */
-	intel_set_config_compute_mode_changes(set, config);
-
 	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
 						   state);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
 		goto fail;
-	} else if (pipe_config) {
-		if (pipe_config->has_audio !=
-		    to_intel_crtc(set->crtc)->config->has_audio)
-			config->mode_changed = true;
-
-		/*
-		 * Note we have an issue here with infoframes: current code
-		 * only updates them on the full mode set path per hw
-		 * requirements.  So here we should be checking for any
-		 * required changes and forcing a mode set.
-		 */
 	}
 
+	/* Compute whether we need a full modeset, only an fb base update or no
+	 * change at all. In the future we might also check whether only the
+	 * mode changed, e.g. for LVDS where we only change the panel fitter in
+	 * such cases. */
+	intel_set_config_compute_mode_changes(set, pipe_config);
+
 	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
-	if (config->mode_changed) {
+	if (pipe_config->base.mode_changed) {
 		ret = intel_set_mode_with_config(set->crtc, set->mode,
 						 pipe_config);
-	} else if (config->fb_changed) {
+	} else if (pipe_config->base.planes_changed) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
 		struct drm_plane *primary = set->crtc->primary;
 		struct intel_plane_state *plane_state =
-- 
2.1.0

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

[PATCH 23/35] drm/i915: Don't use staged config to calculate mode_changed flags

[Ander Conselvan de Oliveira]

Use the atomic state instead.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 49 ++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 24 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 8caddc5..3cc2da6 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12550,10 +12550,12 @@ static void
 intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 				      struct intel_crtc_state *pipe_config)
 {
-	struct drm_device *dev = set->crtc->dev;
-	struct intel_connector *connector;
-	struct intel_encoder *encoder;
-	struct intel_crtc *crtc;
+	struct drm_atomic_state *state;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i;
 
 	/* We should be able to check here if the fb has the same properties
 	 * and then just flip_or_move it */
@@ -12597,33 +12599,32 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 		pipe_config->base.mode_changed = true;
 	}
 
-	for_each_intel_connector(dev, connector) {
-		if (&connector->new_encoder->base == connector->base.encoder)
-			continue;
-
-		pipe_config->base.mode_changed = true;
-		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
-			      connector->base.base.id,
-			      connector->base.name);
-	}
+	state = pipe_config->base.state;
 
-	for_each_intel_encoder(dev, encoder) {
-		if (&encoder->new_crtc->base == encoder->base.crtc)
-			continue;
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		if (connector_state->best_encoder !=
+		    connector->state->best_encoder) {
+			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
+				      connector->base.id,
+				      connector->name);
+			pipe_config->base.mode_changed = true;
+		}
 
-		DRM_DEBUG_KMS("[ENCODER:%d:%s] crtc changed, full mode switch\n",
-			      encoder->base.base.id,
-			      encoder->base.name);
-		pipe_config->base.mode_changed = true;
+		if (connector_state->crtc != connector->state->crtc) {
+			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] crtc changed, full mode switch\n",
+				      connector->base.id,
+				      connector->name);
+			pipe_config->base.mode_changed = true;
+		}
 	}
 
-	for_each_intel_crtc(dev, crtc) {
-		if (crtc->new_enabled == crtc->base.state->enable)
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		if (crtc_state->enable == crtc->state->enable)
 			continue;
 
 		DRM_DEBUG_KMS("[CRTC:%d] %sabled, full mode switch\n",
-			      crtc->base.base.id,
-			      crtc->new_enabled ? "en" : "dis");
+			      crtc->base.id,
+			      crtc_state->enable ? "en" : "dis");
 		pipe_config->base.mode_changed = true;
 	}
 
-- 
2.1.0

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

[PATCH 24/35] drm/i915: Unify modeset and flip paths of intel_crtc_set_config()

[Ander Conselvan de Oliveira]

Call intel_set_mode() uncondionally from intel_crtc_set_config(), since
the former function is now properly wired to ignore all the modesets if
the mode_changed and active_changed flags are false in crtc_state.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 37 +++++++++++++++++++-----------------
 1 file changed, 20 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 3cc2da6..4949c25 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12762,12 +12762,21 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	return 0;
 }
 
+static bool primary_plane_visible(struct drm_crtc *crtc)
+{
+	struct intel_plane_state *plane_state =
+		to_intel_plane_state(crtc->primary->state);
+
+	return plane_state->visible;
+}
+
 static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
 	struct drm_atomic_state *state = NULL;
 	struct intel_set_config *config;
 	struct intel_crtc_state *pipe_config;
+	bool primary_plane_was_visible;
 	int ret;
 
 	BUG_ON(!set);
@@ -12829,29 +12838,23 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
-	if (pipe_config->base.mode_changed) {
-		ret = intel_set_mode_with_config(set->crtc, set->mode,
-						 pipe_config);
-	} else if (pipe_config->base.planes_changed) {
+	primary_plane_was_visible = primary_plane_visible(set->crtc);
+
+	ret = intel_set_mode_with_config(set->crtc, set->mode,
+					 pipe_config);
+
+	if (ret == 0 &&
+	    pipe_config->base.enable &&
+	    pipe_config->base.planes_changed &&
+	    !needs_modeset(&pipe_config->base)) {
 		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
-		struct drm_plane *primary = set->crtc->primary;
-		struct intel_plane_state *plane_state =
-				to_intel_plane_state(primary->state);
-		bool was_visible = plane_state->visible;
-		int vdisplay, hdisplay;
-
-		drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
-		ret = drm_plane_helper_update(primary, set->crtc, set->fb,
-					      0, 0, hdisplay, vdisplay,
-					      set->x << 16, set->y << 16,
-					      hdisplay << 16, vdisplay << 16);
 
 		/*
 		 * We need to make sure the primary plane is re-enabled if it
 		 * has previously been turned off.
 		 */
-		plane_state = to_intel_plane_state(primary->state);
-		if (ret == 0 && !was_visible && plane_state->visible) {
+		if (ret == 0 && !primary_plane_was_visible &&
+		    primary_plane_visible(set->crtc)) {
 			WARN_ON(!intel_crtc->active);
 			intel_post_enable_primary(set->crtc);
 		}
-- 
2.1.0

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

[PATCH 25/35] drm/i915: Simplify intel_set_config_compute_mode_changes() a bit

[Ander Conselvan de Oliveira]

Add a helper function to make the code slightly more readable.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 44 ++++++++++++++++++++++--------------
 1 file changed, 27 insertions(+), 17 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 4949c25..172801d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12633,6 +12633,18 @@ intel_set_config_compute_mode_changes(struct drm_mode_set *set,
 			pipe_config->base.planes_changed);
 }
 
+static bool intel_connector_in_mode_set(struct intel_connector *connector,
+					struct drm_mode_set *set)
+{
+	int ro;
+
+	for (ro = 0; ro < set->num_connectors; ro++)
+		if (set->connectors[ro] == &connector->base)
+			return true;
+
+	return false;
+}
+
 static int
 intel_modeset_stage_output_state(struct drm_device *dev,
 				 struct drm_mode_set *set,
@@ -12643,7 +12655,6 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	struct intel_encoder *encoder;
 	struct intel_crtc *crtc;
 	struct intel_crtc_state *crtc_state;
-	int ro;
 
 	/* The upper layers ensure that we either disable a crtc or have a list
 	 * of connectors. For paranoia, double-check this. */
@@ -12651,21 +12662,22 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	WARN_ON(set->fb && (set->num_connectors == 0));
 
 	for_each_intel_connector(dev, connector) {
-		/* Otherwise traverse passed in connector list and get encoders
-		 * for them. */
-		for (ro = 0; ro < set->num_connectors; ro++) {
-			if (set->connectors[ro] == &connector->base) {
-				connector->new_encoder = intel_find_encoder(connector, to_intel_crtc(set->crtc)->pipe);
-				break;
-			}
+		bool in_mode_set = intel_connector_in_mode_set(connector, set);
+
+		if (in_mode_set) {
+			int pipe = to_intel_crtc(set->crtc)->pipe;
+			connector->new_encoder =
+				intel_find_encoder(connector, pipe);
 		}
 
+		if (!connector->base.encoder ||
+		    connector->base.encoder->crtc != set->crtc)
+			continue;
+
 		/* If we disable the crtc, disable all its connectors. Also, if
 		 * the connector is on the changing crtc but not on the new
 		 * connector list, disable it. */
-		if ((!set->fb || ro == set->num_connectors) &&
-		    connector->base.encoder &&
-		    connector->base.encoder->crtc == set->crtc) {
+		if (!set->fb || !in_mode_set) {
 			connector->new_encoder = NULL;
 
 			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [NOCRTC]\n",
@@ -12682,12 +12694,10 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 		if (!connector->new_encoder)
 			continue;
 
-		new_crtc = connector->new_encoder->base.crtc;
-
-		for (ro = 0; ro < set->num_connectors; ro++) {
-			if (set->connectors[ro] == &connector->base)
-				new_crtc = set->crtc;
-		}
+		if (intel_connector_in_mode_set(connector, set))
+			new_crtc = set->crtc;
+		else
+			new_crtc = connector->new_encoder->base.crtc;
 
 		/* Make sure the new CRTC will work with the encoder */
 		if (!drm_encoder_crtc_ok(&connector->new_encoder->base,
-- 
2.1.0

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

[PATCH 26/35] drm/i915: Stage new modeset state straight into atomic state

[Ander Conselvan de Oliveira]

The logic that stages the state before the modeset was still updating
first the old staged config and then populating the atomic state based
on that. Change this to use only the atomic state.

Note that now the staged config is updated in the function
intel_modeset_commit_output_state(). This is done so that the modeset
check and the force restore path in the hw state read out code continue
to work.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 170 +++++++++++++++++------------------
 1 file changed, 85 insertions(+), 85 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 172801d..f556761 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11064,27 +11064,47 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
  * intel_modeset_commit_output_state
  *
  * This function copies the stage display pipe configuration to the real one.
+ *
+ * FIXME: we want to replace this with a proper state swap in the future
  */
-static void intel_modeset_commit_output_state(struct drm_device *dev)
+static void intel_modeset_commit_output_state(struct drm_atomic_state *state)
 {
-	struct intel_crtc *crtc;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	struct drm_connector *connector;
+	struct drm_connector_state *connector_state;
 	struct intel_encoder *encoder;
-	struct intel_connector *connector;
+	struct intel_connector *intel_connector;
+	int i;
 
-	for_each_intel_connector(dev, connector) {
-		connector->base.encoder = &connector->new_encoder->base;
+	for_each_connector_in_state(state, connector, connector_state, i) {
+		*connector->state = *connector_state;
+
+		connector->encoder = connector_state->best_encoder;
+		if (connector->encoder)
+			connector->encoder->crtc = connector_state->crtc;
 	}
 
-	for_each_intel_encoder(dev, encoder) {
-		encoder->base.crtc = &encoder->new_crtc->base;
+	/* Update crtc of disabled encoders */
+	for_each_intel_encoder(state->dev, encoder) {
+		int num_connectors = 0;
+
+		for_each_intel_connector(state->dev, intel_connector)
+			if (intel_connector->base.encoder == &encoder->base)
+				num_connectors++;
+
+		if (num_connectors == 0)
+			encoder->base.crtc = NULL;
 	}
 
-	for_each_intel_crtc(dev, crtc) {
-		crtc->base.state->enable = crtc->new_enabled;
-		crtc->base.enabled = crtc->new_enabled;
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		crtc->state->enable = crtc_state->enable;
+		crtc->enabled = crtc_state->enable;
 	}
 
-	intel_modeset_update_connector_atomic_state(dev);
+	/* Copy the new configuration to the staged state, to keep the few
+	 * pieces of code that haven't been converted yet happy */
+	intel_modeset_update_staged_output_state(state->dev);
 }
 
 static void
@@ -11534,7 +11554,7 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 			intel_encoder->connectors_active = false;
 	}
 
-	intel_modeset_commit_output_state(dev);
+	intel_modeset_commit_output_state(state);
 
 	/* Double check state. */
 	for_each_crtc(dev, crtc) {
@@ -12651,10 +12671,11 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 				 struct drm_atomic_state *state)
 {
 	struct intel_connector *connector;
+	struct drm_connector *drm_connector;
 	struct drm_connector_state *connector_state;
-	struct intel_encoder *encoder;
-	struct intel_crtc *crtc;
-	struct intel_crtc_state *crtc_state;
+	struct drm_crtc *crtc;
+	struct drm_crtc_state *crtc_state;
+	int i, ret;
 
 	/* The upper layers ensure that we either disable a crtc or have a list
 	 * of connectors. For paranoia, double-check this. */
@@ -12664,21 +12685,28 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	for_each_intel_connector(dev, connector) {
 		bool in_mode_set = intel_connector_in_mode_set(connector, set);
 
+		if (!in_mode_set && connector->base.state->crtc != set->crtc)
+			continue;
+
+		connector_state =
+			drm_atomic_get_connector_state(state, &connector->base);
+		if (IS_ERR(connector_state))
+			return PTR_ERR(connector_state);
+
 		if (in_mode_set) {
 			int pipe = to_intel_crtc(set->crtc)->pipe;
-			connector->new_encoder =
-				intel_find_encoder(connector, pipe);
+			connector_state->best_encoder =
+				&intel_find_encoder(connector, pipe)->base;
 		}
 
-		if (!connector->base.encoder ||
-		    connector->base.encoder->crtc != set->crtc)
+		if (connector->base.state->crtc != set->crtc)
 			continue;
 
 		/* If we disable the crtc, disable all its connectors. Also, if
 		 * the connector is on the changing crtc but not on the new
 		 * connector list, disable it. */
 		if (!set->fb || !in_mode_set) {
-			connector->new_encoder = NULL;
+			connector_state->best_encoder = NULL;
 
 			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [NOCRTC]\n",
 				connector->base.base.id,
@@ -12687,86 +12715,58 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 	}
 	/* connector->new_encoder is now updated for all connectors. */
 
-	/* Update crtc of enabled connectors. */
-	for_each_intel_connector(dev, connector) {
-		struct drm_crtc *new_crtc;
+	for_each_connector_in_state(state, drm_connector, connector_state, i) {
+		connector = to_intel_connector(drm_connector);
+
+		if (!connector_state->best_encoder) {
+			ret = drm_atomic_set_crtc_for_connector(connector_state,
+								NULL);
+			if (ret)
+				return ret;
 
-		if (!connector->new_encoder)
 			continue;
+		}
 
-		if (intel_connector_in_mode_set(connector, set))
-			new_crtc = set->crtc;
-		else
-			new_crtc = connector->new_encoder->base.crtc;
+		if (intel_connector_in_mode_set(connector, set)) {
+			struct drm_crtc *crtc = connector->base.state->crtc;
+
+			/* If this connector was in a previous crtc, add it
+			 * to the state. We might need to disable it. */
+			if (crtc) {
+				crtc_state =
+					drm_atomic_get_crtc_state(state, crtc);
+				if (IS_ERR(crtc_state))
+					return PTR_ERR(crtc_state);
+			}
+
+			ret = drm_atomic_set_crtc_for_connector(connector_state,
+								set->crtc);
+			if (ret)
+				return ret;
+		}
 
 		/* Make sure the new CRTC will work with the encoder */
-		if (!drm_encoder_crtc_ok(&connector->new_encoder->base,
-					 new_crtc)) {
+		if (!drm_encoder_crtc_ok(connector_state->best_encoder,
+					 connector_state->crtc)) {
 			return -EINVAL;
 		}
-		connector->new_encoder->new_crtc = to_intel_crtc(new_crtc);
-
-		connector_state =
-			drm_atomic_get_connector_state(state, &connector->base);
-		if (IS_ERR(connector_state))
-			return PTR_ERR(connector_state);
-
-		connector_state->crtc = new_crtc;
-		connector_state->best_encoder = &connector->new_encoder->base;
 
 		DRM_DEBUG_KMS("[CONNECTOR:%d:%s] to [CRTC:%d]\n",
 			connector->base.base.id,
 			connector->base.name,
-			new_crtc->base.id);
-	}
-
-	/* Check for any encoders that needs to be disabled. */
-	for_each_intel_encoder(dev, encoder) {
-		int num_connectors = 0;
-		for_each_intel_connector(dev, connector) {
-			if (connector->new_encoder == encoder) {
-				WARN_ON(!connector->new_encoder->new_crtc);
-				num_connectors++;
-			}
-		}
-
-		if (num_connectors == 0)
-			encoder->new_crtc = NULL;
-		else if (num_connectors > 1)
-			return -EINVAL;
-	}
-	/* Now we've also updated encoder->new_crtc for all encoders. */
-	for_each_intel_connector(dev, connector) {
-		connector_state =
-			drm_atomic_get_connector_state(state, &connector->base);
-		if (IS_ERR(connector_state))
-			return PTR_ERR(connector_state);
+			connector_state->crtc->base.id);
 
-		if (connector->new_encoder) {
-			if (connector->new_encoder != connector->encoder)
-				connector->encoder = connector->new_encoder;
-		} else {
-			connector_state->crtc = NULL;
-			connector_state->best_encoder = NULL;
-		}
+		if (connector_state->best_encoder != &connector->encoder->base)
+			connector->encoder =
+				to_intel_encoder(connector_state->best_encoder);
 	}
-	for_each_intel_crtc(dev, crtc) {
-		crtc->new_enabled = false;
-
-		for_each_intel_encoder(dev, encoder) {
-			if (encoder->new_crtc == crtc) {
-				crtc->new_enabled = true;
-				break;
-			}
-		}
 
-		if (crtc->new_enabled != crtc->base.state->enable) {
-			crtc_state = intel_atomic_get_crtc_state(state, crtc);
-			if (IS_ERR(crtc_state))
-				return PTR_ERR(crtc_state);
+	for_each_crtc_in_state(state, crtc, crtc_state, i) {
+		ret = drm_atomic_add_affected_connectors(state, crtc);
+		if (ret)
+			return ret;
 
-			crtc_state->base.enable = crtc->new_enabled;
-		}
+		crtc_state->enable = drm_atomic_connectors_for_crtc(state, crtc);
 	}
 
 	return 0;
-- 
2.1.0

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

[PATCH 27/35] drm/i915: Remove save/restore logic from intel_crtc_set_config()

[Ander Conselvan de Oliveira]

This is no longer necessary since we only update the staged config on
successfull modeset. The new configuration is stored in an atomic state
struct which is freed in case of failure.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 113 ++---------------------------------
 drivers/gpu/drm/i915/intel_drv.h     |   9 ---
 2 files changed, 6 insertions(+), 116 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index f556761..a5f565d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12461,91 +12461,6 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 
 #undef for_each_intel_crtc_masked
 
-static void intel_set_config_free(struct intel_set_config *config)
-{
-	if (!config)
-		return;
-
-	kfree(config->save_connector_encoders);
-	kfree(config->save_encoder_crtcs);
-	kfree(config->save_crtc_enabled);
-	kfree(config);
-}
-
-static int intel_set_config_save_state(struct drm_device *dev,
-				       struct intel_set_config *config)
-{
-	struct drm_crtc *crtc;
-	struct drm_encoder *encoder;
-	struct drm_connector *connector;
-	int count;
-
-	config->save_crtc_enabled =
-		kcalloc(dev->mode_config.num_crtc,
-			sizeof(bool), GFP_KERNEL);
-	if (!config->save_crtc_enabled)
-		return -ENOMEM;
-
-	config->save_encoder_crtcs =
-		kcalloc(dev->mode_config.num_encoder,
-			sizeof(struct drm_crtc *), GFP_KERNEL);
-	if (!config->save_encoder_crtcs)
-		return -ENOMEM;
-
-	config->save_connector_encoders =
-		kcalloc(dev->mode_config.num_connector,
-			sizeof(struct drm_encoder *), GFP_KERNEL);
-	if (!config->save_connector_encoders)
-		return -ENOMEM;
-
-	/* Copy data. Note that driver private data is not affected.
-	 * Should anything bad happen only the expected state is
-	 * restored, not the drivers personal bookkeeping.
-	 */
-	count = 0;
-	for_each_crtc(dev, crtc) {
-		config->save_crtc_enabled[count++] = crtc->state->enable;
-	}
-
-	count = 0;
-	list_for_each_entry(encoder, &dev->mode_config.encoder_list, head) {
-		config->save_encoder_crtcs[count++] = encoder->crtc;
-	}
-
-	count = 0;
-	list_for_each_entry(connector, &dev->mode_config.connector_list, head) {
-		config->save_connector_encoders[count++] = connector->encoder;
-	}
-
-	return 0;
-}
-
-static void intel_set_config_restore_state(struct drm_device *dev,
-					   struct intel_set_config *config)
-{
-	struct intel_crtc *crtc;
-	struct intel_encoder *encoder;
-	struct intel_connector *connector;
-	int count;
-
-	count = 0;
-	for_each_intel_crtc(dev, crtc) {
-		crtc->new_enabled = config->save_crtc_enabled[count++];
-	}
-
-	count = 0;
-	for_each_intel_encoder(dev, encoder) {
-		encoder->new_crtc =
-			to_intel_crtc(config->save_encoder_crtcs[count++]);
-	}
-
-	count = 0;
-	for_each_intel_connector(dev, connector) {
-		connector->new_encoder =
-			to_intel_encoder(config->save_connector_encoders[count++]);
-	}
-}
-
 static bool
 is_crtc_connector_off(struct drm_mode_set *set)
 {
@@ -12784,7 +12699,6 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 {
 	struct drm_device *dev;
 	struct drm_atomic_state *state = NULL;
-	struct intel_set_config *config;
 	struct intel_crtc_state *pipe_config;
 	bool primary_plane_was_visible;
 	int ret;
@@ -12807,37 +12721,26 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 
 	dev = set->crtc->dev;
 
-	ret = -ENOMEM;
-	config = kzalloc(sizeof(*config), GFP_KERNEL);
-	if (!config)
-		goto out_config;
-
-	ret = intel_set_config_save_state(dev, config);
-	if (ret)
-		goto out_config;
-
 	state = drm_atomic_state_alloc(dev);
-	if (!state) {
-		ret = -ENOMEM;
-		goto out_config;
-	}
+	if (!state)
+		return -ENOMEM;
 
 	state->acquire_ctx = dev->mode_config.acquire_ctx;
 
 	ret = intel_modeset_stage_output_state(dev, set, state);
 	if (ret)
-		goto fail;
+		goto out;
 
 	ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,
 					      set->fb, set->x, set->y);
 	if (ret)
-		goto fail;
+		goto out;
 
 	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
 						   state);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
-		goto fail;
+		goto out;
 	}
 
 	/* Compute whether we need a full modeset, only an fb base update or no
@@ -12884,14 +12787,10 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret) {
 		DRM_DEBUG_KMS("failed to set mode on [CRTC:%d], err = %d\n",
 			      set->crtc->base.id, ret);
-fail:
-		intel_set_config_restore_state(dev, config);
 	}
 
-out_config:
+out:
 	drm_atomic_state_free(state);
-
-	intel_set_config_free(config);
 	return ret;
 }
 
diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
index dc53f59..5ca4fb0 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -813,15 +813,6 @@ struct intel_unpin_work {
 	bool enable_stall_check;
 };
 
-struct intel_set_config {
-	struct drm_encoder **save_connector_encoders;
-	struct drm_crtc **save_encoder_crtcs;
-	bool *save_crtc_enabled;
-
-	bool fb_changed;
-	bool mode_changed;
-};
-
 struct intel_load_detect_pipe {
 	struct drm_framebuffer *release_fb;
 	bool load_detect_temp;
-- 
2.1.0

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

[PATCH 28/35] drm/i915: Update crtc state active flag based on DPMS

[Ander Conselvan de Oliveira]

In a follow up patch the function that computes mode changes will be
replaced with the one from the atomic helpers. To preserve the behavior
of legacy modeset forcing DPMS on, that function will need to detect a
change in the active state of the crtc, so that has to be kept up to
date.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index a5f565d..55041bd 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -6019,6 +6019,8 @@ void intel_crtc_update_dpms(struct drm_crtc *crtc)
 		enable |= intel_encoder->connectors_active;
 
 	intel_crtc_control(crtc, enable);
+
+	crtc->state->active = enable;
 }
 
 static void intel_crtc_disable(struct drm_crtc *crtc)
-- 
2.1.0

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

[PATCH 29/35] drm/atomic: Make mode_fixup() optional for check_modeset()

[Ander Conselvan de Oliveira]

So the i915 driver can use the same logic for setting mode and active
changed flags, without having to implement encoder helpers and the
mode_fixup() callback.

Cc: dri-devel@lists.freedestkop.org
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/drm_atomic_helper.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/gpu/drm/drm_atomic_helper.c b/drivers/gpu/drm/drm_atomic_helper.c
index 539203a..d7caa25 100644
--- a/drivers/gpu/drm/drm_atomic_helper.c
+++ b/drivers/gpu/drm/drm_atomic_helper.c
@@ -280,6 +280,8 @@ mode_fixup(struct drm_atomic_state *state)
 		 */
 		encoder = conn_state->best_encoder;
 		funcs = encoder->helper_private;
+		if (!funcs)
+			continue;
 
 		if (encoder->bridge && encoder->bridge->funcs->mode_fixup) {
 			ret = encoder->bridge->funcs->mode_fixup(
@@ -317,6 +319,9 @@ mode_fixup(struct drm_atomic_state *state)
 			continue;
 
 		funcs = crtc->helper_private;
+		if (!funcs->mode_fixup)
+			continue;
+
 		ret = funcs->mode_fixup(crtc, &crtc_state->mode,
 					&crtc_state->adjusted_mode);
 		if (!ret) {
-- 
2.1.0

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

[PATCH 30/35] drm/i915: Use atomic helpers for computing changed flags

[Ander Conselvan de Oliveira]

Replace the drivers own logic for computing mode_changed, active_changed
and planes_changed flags with the check_modeset() atomic helper. Since
that function needs to compare the crtc's new mode with the current,
this patch also moves the set up of crtc_state->mode earlier in the call
chain.

Note that for the call to check_plane() to work properly, we need to
check new plane state against new crtc state. But since we still use the
plane update helper, which doesn't have a full atomic state, we need to
hack around that in intel_plane_atomic_check().

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  16 ++-
 drivers/gpu/drm/i915/intel_display.c      | 197 ++++++------------------------
 2 files changed, 52 insertions(+), 161 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index 77462e1..86ba4b2 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -111,6 +111,7 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 {
 	struct drm_crtc *crtc = state->crtc;
 	struct intel_crtc *intel_crtc;
+	struct intel_crtc_state *crtc_state;
 	struct intel_plane *intel_plane = to_intel_plane(plane);
 	struct intel_plane_state *intel_state = to_intel_plane_state(state);
 
@@ -126,6 +127,17 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	if (!crtc)
 		return 0;
 
+	/* 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;
+	}
+
 	/*
 	 * The original src/dest coordinates are stored in state->base, but
 	 * we want to keep another copy internal to our driver that we can
@@ -144,9 +156,9 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	intel_state->clip.x1 = 0;
 	intel_state->clip.y1 = 0;
 	intel_state->clip.x2 =
-		intel_crtc->active ? intel_crtc->config->pipe_src_w : 0;
+		crtc_state->base.active ? crtc_state->pipe_src_w : 0;
 	intel_state->clip.y2 =
-		intel_crtc->active ? intel_crtc->config->pipe_src_h : 0;
+		crtc_state->base.active ? crtc_state->pipe_src_h : 0;
 
 	/*
 	 * Disabling a plane is always okay; we just need to update
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 55041bd..7667015 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -82,7 +82,7 @@ static void i9xx_crtc_clock_get(struct intel_crtc *crtc,
 static void ironlake_pch_clock_get(struct intel_crtc *crtc,
 				   struct intel_crtc_state *pipe_config);
 
-static int intel_set_mode(struct drm_crtc *crtc, struct drm_display_mode *mode,
+static int intel_set_mode(struct drm_crtc *crtc,
 			  struct drm_atomic_state *state);
 static int intel_framebuffer_init(struct drm_device *dev,
 				  struct intel_framebuffer *ifb,
@@ -9804,7 +9804,9 @@ retry:
 	if (ret)
 		goto fail;
 
-	if (intel_set_mode(crtc, mode, state)) {
+	drm_mode_copy(&crtc_state->base.mode, mode);
+
+	if (intel_set_mode(crtc, state)) {
 		DRM_DEBUG_KMS("failed to set mode on load-detect pipe\n");
 		if (old->release_fb)
 			old->release_fb->funcs->destroy(old->release_fb);
@@ -9878,7 +9880,7 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		if (ret)
 			goto fail;
 
-		intel_set_mode(crtc, NULL, state);
+		intel_set_mode(crtc, state);
 
 		drm_atomic_state_free(state);
 
@@ -11388,7 +11390,6 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 
 static int
 intel_modeset_pipe_config(struct drm_crtc *crtc,
-			  struct drm_display_mode *mode,
 			  struct drm_atomic_state *state,
 			  struct intel_crtc_state *pipe_config)
 {
@@ -11411,10 +11412,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
 	clear_intel_crtc_state(pipe_config);
 
-	pipe_config->base.crtc = crtc;
-	drm_mode_copy(&pipe_config->base.adjusted_mode, mode);
-	drm_mode_copy(&pipe_config->base.mode, mode);
-
 	pipe_config->cpu_transcoder =
 		(enum transcoder) to_intel_crtc(crtc)->pipe;
 	pipe_config->shared_dpll = DPLL_ID_PRIVATE;
@@ -12111,27 +12108,8 @@ static void update_scanline_offset(struct intel_crtc *crtc)
 		crtc->scanline_offset = 1;
 }
 
-static void
-intel_atomic_modeset_compute_changed_flags(struct drm_atomic_state *state,
-					   struct drm_crtc *modeset_crtc)
-{
-	struct drm_crtc_state *crtc_state;
-	struct drm_crtc *crtc;
-	int i;
-
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (crtc_state->enable != crtc->state->enable)
-			crtc_state->mode_changed = true;
-
-		/* FIXME: Do we need to always set mode_changed for
-		 * modeset_crtc if it is enabled? modeset_affect_pipes()
-		 * did that. */
-	}
-}
-
 static struct intel_crtc_state *
 intel_modeset_compute_config(struct drm_crtc *crtc,
-			     struct drm_display_mode *mode,
 			     struct drm_atomic_state *state)
 {
 	struct intel_crtc_state *pipe_config;
@@ -12141,7 +12119,9 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	if (ret)
 		return ERR_PTR(ret);
 
-	intel_atomic_modeset_compute_changed_flags(state, crtc);
+	ret = drm_atomic_helper_check_modeset(state->dev, state);
+	if (ret)
+		return ERR_PTR(ret);
 
 	/*
 	 * Note this needs changes when we start tracking multiple modes
@@ -12156,7 +12136,7 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 	if (!pipe_config->base.enable)
 		return pipe_config;
 
-	ret = intel_modeset_pipe_config(crtc, mode, state, pipe_config);
+	ret = intel_modeset_pipe_config(crtc, state, pipe_config);
 	if (ret)
 		return ERR_PTR(ret);
 
@@ -12174,6 +12154,10 @@ intel_modeset_compute_config(struct drm_crtc *crtc,
 
 	intel_dump_pipe_config(to_intel_crtc(crtc), pipe_config,"[modeset]");
 
+	ret = drm_atomic_helper_check_planes(state->dev, state);
+	if (ret)
+		return ERR_PTR(ret);
+
 	return pipe_config;
 }
 
@@ -12249,7 +12233,6 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 }
 
 static int __intel_set_mode(struct drm_crtc *modeset_crtc,
-			    struct drm_display_mode *mode,
 			    struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = modeset_crtc->dev;
@@ -12292,7 +12275,7 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	 * single crtc and mode.
 	 */
 	if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) {
-		modeset_crtc->mode = *mode;
+		modeset_crtc->mode = pipe_config->base.mode;
 		/* mode_set/enable/disable functions rely on a correct pipe
 		 * config. */
 		intel_crtc_set_state(to_intel_crtc(modeset_crtc), pipe_config);
@@ -12358,12 +12341,11 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 }
 
 static int intel_set_mode_with_config(struct drm_crtc *crtc,
-				      struct drm_display_mode *mode,
 				      struct intel_crtc_state *pipe_config)
 {
 	int ret;
 
-	ret = __intel_set_mode(crtc, mode, pipe_config);
+	ret = __intel_set_mode(crtc, pipe_config);
 
 	if (ret == 0)
 		intel_modeset_check_state(crtc->dev);
@@ -12372,19 +12354,18 @@ static int intel_set_mode_with_config(struct drm_crtc *crtc,
 }
 
 static int intel_set_mode(struct drm_crtc *crtc,
-			  struct drm_display_mode *mode,
 			  struct drm_atomic_state *state)
 {
 	struct intel_crtc_state *pipe_config;
 	int ret = 0;
 
-	pipe_config = intel_modeset_compute_config(crtc, mode, state);
+	pipe_config = intel_modeset_compute_config(crtc, state);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
 		goto out;
 	}
 
-	ret = intel_set_mode_with_config(crtc, mode, pipe_config);
+	ret = intel_set_mode_with_config(crtc, pipe_config);
 	if (ret)
 		goto out;
 
@@ -12451,125 +12432,21 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 		}
 
 		crtc_state->base.enable = intel_crtc->new_enabled;
+
+		if (&intel_crtc->base == crtc)
+			drm_mode_copy(&crtc_state->base.mode, &crtc->mode);
 	}
 
 	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
 					crtc->primary->fb, crtc->x, crtc->y);
 
-	intel_set_mode(crtc, &crtc->mode, state);
+	intel_set_mode(crtc, state);
 
 	drm_atomic_state_free(state);
 }
 
 #undef for_each_intel_crtc_masked
 
-static bool
-is_crtc_connector_off(struct drm_mode_set *set)
-{
-	int i;
-
-	if (set->num_connectors == 0)
-		return false;
-
-	if (WARN_ON(set->connectors == NULL))
-		return false;
-
-	for (i = 0; i < set->num_connectors; i++)
-		if (set->connectors[i]->encoder &&
-		    set->connectors[i]->encoder->crtc == set->crtc &&
-		    set->connectors[i]->dpms != DRM_MODE_DPMS_ON)
-			return true;
-
-	return false;
-}
-
-static void
-intel_set_config_compute_mode_changes(struct drm_mode_set *set,
-				      struct intel_crtc_state *pipe_config)
-{
-	struct drm_atomic_state *state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
-	int i;
-
-	/* We should be able to check here if the fb has the same properties
-	 * and then just flip_or_move it */
-	if (is_crtc_connector_off(set)) {
-		pipe_config->base.mode_changed = true;
-	} else if (set->crtc->primary->fb != set->fb) {
-		/*
-		 * If we have no fb, we can only flip as long as the crtc is
-		 * active, otherwise we need a full mode set.  The crtc may
-		 * be active if we've only disabled the primary plane, or
-		 * in fastboot situations.
-		 */
-		if (set->crtc->primary->fb == NULL) {
-			struct intel_crtc *intel_crtc =
-				to_intel_crtc(set->crtc);
-
-			if (intel_crtc->active) {
-				DRM_DEBUG_KMS("crtc has no fb, will flip\n");
-				pipe_config->base.planes_changed = true;
-			} else {
-				DRM_DEBUG_KMS("inactive crtc, full mode set\n");
-				pipe_config->base.mode_changed = true;
-			}
-		} else if (set->fb == NULL) {
-			pipe_config->base.mode_changed = true;
-		} else if (set->fb->pixel_format !=
-			   set->crtc->primary->fb->pixel_format) {
-			pipe_config->base.mode_changed = true;
-		} else {
-			pipe_config->base.planes_changed = true;
-		}
-	}
-
-	if (set->fb && (set->x != set->crtc->x || set->y != set->crtc->y))
-		pipe_config->base.planes_changed = true;
-
-	if (set->mode && !drm_mode_equal(set->mode, &set->crtc->mode)) {
-		DRM_DEBUG_KMS("modes are different, full mode set\n");
-		drm_mode_debug_printmodeline(&set->crtc->mode);
-		drm_mode_debug_printmodeline(set->mode);
-		pipe_config->base.mode_changed = true;
-	}
-
-	state = pipe_config->base.state;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		if (connector_state->best_encoder !=
-		    connector->state->best_encoder) {
-			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] encoder changed, full mode switch\n",
-				      connector->base.id,
-				      connector->name);
-			pipe_config->base.mode_changed = true;
-		}
-
-		if (connector_state->crtc != connector->state->crtc) {
-			DRM_DEBUG_KMS("[CONNECTOR:%d:%s] crtc changed, full mode switch\n",
-				      connector->base.id,
-				      connector->name);
-			pipe_config->base.mode_changed = true;
-		}
-	}
-
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (crtc_state->enable == crtc->state->enable)
-			continue;
-
-		DRM_DEBUG_KMS("[CRTC:%d] %sabled, full mode switch\n",
-			      crtc->base.id,
-			      crtc_state->enable ? "en" : "dis");
-		pipe_config->base.mode_changed = true;
-	}
-
-	DRM_DEBUG_KMS("computed changes for [CRTC:%d], mode_changed=%d, fb_changed=%d\n",
-			set->crtc->base.id, pipe_config->base.mode_changed,
-			pipe_config->base.planes_changed);
-}
-
 static bool intel_connector_in_mode_set(struct intel_connector *connector,
 					struct drm_mode_set *set)
 {
@@ -12686,6 +12563,21 @@ intel_modeset_stage_output_state(struct drm_device *dev,
 		crtc_state->enable = drm_atomic_connectors_for_crtc(state, crtc);
 	}
 
+	ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,
+					      set->fb, set->x, set->y);
+	if (ret)
+		return ret;
+
+	crtc_state = drm_atomic_get_crtc_state(state, set->crtc);
+	if (IS_ERR(crtc_state))
+		return PTR_ERR(crtc_state);
+
+	if (set->mode)
+		drm_mode_copy(&crtc_state->mode, set->mode);
+
+	if (set->num_connectors)
+		crtc_state->active = true;
+
 	return 0;
 }
 
@@ -12733,30 +12625,17 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	if (ret)
 		goto out;
 
-	ret = intel_modeset_setup_plane_state(state, set->crtc, set->mode,
-					      set->fb, set->x, set->y);
-	if (ret)
-		goto out;
-
-	pipe_config = intel_modeset_compute_config(set->crtc, set->mode,
-						   state);
+	pipe_config = intel_modeset_compute_config(set->crtc, state);
 	if (IS_ERR(pipe_config)) {
 		ret = PTR_ERR(pipe_config);
 		goto out;
 	}
 
-	/* Compute whether we need a full modeset, only an fb base update or no
-	 * change at all. In the future we might also check whether only the
-	 * mode changed, e.g. for LVDS where we only change the panel fitter in
-	 * such cases. */
-	intel_set_config_compute_mode_changes(set, pipe_config);
-
 	intel_update_pipe_size(to_intel_crtc(set->crtc));
 
 	primary_plane_was_visible = primary_plane_visible(set->crtc);
 
-	ret = intel_set_mode_with_config(set->crtc, set->mode,
-					 pipe_config);
+	ret = intel_set_mode_with_config(set->crtc, pipe_config);
 
 	if (ret == 0 &&
 	    pipe_config->base.enable &&
-- 
2.1.0

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

[PATCH 31/35] drm/i915: Take ownership of atomic state on success in intel_set_mode()

[Ander Conselvan de Oliveira]

To match the behavior of ->atomic_commit().

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7667015..7c8caf5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -9880,9 +9880,9 @@ void intel_release_load_detect_pipe(struct drm_connector *connector,
 		if (ret)
 			goto fail;
 
-		intel_set_mode(crtc, state);
-
-		drm_atomic_state_free(state);
+		ret = intel_set_mode(crtc, state);
+		if (ret)
+			goto fail;
 
 		if (old->release_fb) {
 			drm_framebuffer_unregister_private(old->release_fb);
@@ -12337,6 +12337,8 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	intel_crtc->config = crtc_state_copy;
 	intel_crtc->base.state = &crtc_state_copy->base;
 
+	drm_atomic_state_free(state);
+
 	return 0;
 }
 
@@ -12382,6 +12384,7 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 	struct intel_connector *connector;
 	struct drm_connector_state *connector_state;
 	struct intel_crtc_state *crtc_state;
+	int ret;
 
 	state = drm_atomic_state_alloc(dev);
 	if (!state) {
@@ -12440,9 +12443,9 @@ void intel_crtc_restore_mode(struct drm_crtc *crtc)
 	intel_modeset_setup_plane_state(state, crtc, &crtc->mode,
 					crtc->primary->fb, crtc->x, crtc->y);
 
-	intel_set_mode(crtc, state);
-
-	drm_atomic_state_free(state);
+	ret = intel_set_mode(crtc, state);
+	if (ret)
+		drm_atomic_state_free(state);
 }
 
 #undef for_each_intel_crtc_masked
@@ -12671,7 +12674,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
 	}
 
 out:
-	drm_atomic_state_free(state);
+	if (ret)
+		drm_atomic_state_free(state);
 	return ret;
 }
 
-- 
2.1.0

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

[PATCH 32/35] drm/i915: Preserve shared DPLL information in new pipe_config

[Ander Conselvan de Oliveira]

When a new pipe_config is calculated, the fields related to shared dplls
are reset, under the assumption that they will be recalculated as part
of the modeset, which is true with the current state of the code.

As we convert to atomic, however, it will be possible to calculate a new
pipe_config and skip the modeset. In that case, after the state swap we
still want the shared dplls to be preserved.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 7c8caf5..edfe198 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11379,13 +11379,21 @@ clear_intel_crtc_state(struct intel_crtc_state *crtc_state)
 {
 	struct drm_crtc_state tmp_state;
 	struct intel_crtc_scaler_state scaler_state;
+	struct intel_dpll_hw_state dpll_hw_state;
+	enum intel_dpll_id shared_dpll;
 
 	/* Clear only the intel specific part of the crtc state excluding scalers */
 	tmp_state = crtc_state->base;
 	scaler_state = crtc_state->scaler_state;
+	shared_dpll = crtc_state->shared_dpll;
+	dpll_hw_state = crtc_state->dpll_hw_state;
+
 	memset(crtc_state, 0, sizeof *crtc_state);
+
 	crtc_state->base = tmp_state;
 	crtc_state->scaler_state = scaler_state;
+	crtc_state->shared_dpll = shared_dpll;
+	crtc_state->dpll_hw_state = dpll_hw_state;
 }
 
 static int
@@ -11414,7 +11422,6 @@ intel_modeset_pipe_config(struct drm_crtc *crtc,
 
 	pipe_config->cpu_transcoder =
 		(enum transcoder) to_intel_crtc(crtc)->pipe;
-	pipe_config->shared_dpll = DPLL_ID_PRIVATE;
 
 	/*
 	 * Sanitize sync polarity flags based on requested ones. If neither
@@ -12178,9 +12185,14 @@ static int __intel_set_mode_setup_plls(struct drm_atomic_state *state)
 
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		intel_crtc = to_intel_crtc(crtc);
+		intel_crtc_state = to_intel_crtc_state(crtc_state);
 
-		if (needs_modeset(crtc_state))
+		if (needs_modeset(crtc_state)) {
 			clear_pipes |= 1 << intel_crtc->pipe;
+			intel_crtc_state->shared_dpll = DPLL_ID_PRIVATE;
+			memset(&intel_crtc_state->dpll_hw_state, 0,
+			       sizeof(intel_crtc_state->dpll_hw_state));
+		}
 	}
 
 	ret = intel_shared_dpll_start_config(dev_priv, clear_pipes);
-- 
2.1.0

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

[PATCH 33/35] drm/i915: Don't use plane update helper in legacy mode set

[Ander Conselvan de Oliveira]

Use lower level calls to better integrate with the modeset code and
allow a full state swap in a follow up patch.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 47 +++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 22 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index edfe198..99a3bc0 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -12244,6 +12244,23 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	return 0;
 }
 
+static void __intel_set_mode_swap_plane_state(struct drm_device *dev,
+					      struct drm_atomic_state *state)
+{
+	int i;
+
+	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
+		struct drm_plane *plane = state->planes[i];
+
+		if (!plane)
+			continue;
+
+		plane->state->state = state;
+		swap(state->plane_states[i], plane->state);
+		plane->state->state = NULL;
+	}
+}
+
 static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 			    struct intel_crtc_state *pipe_config)
 {
@@ -12254,8 +12271,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	struct intel_crtc *intel_crtc;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
-	struct drm_plane *plane;
-	struct drm_plane_state *plane_state;
 	int ret = 0;
 	int i;
 
@@ -12263,6 +12278,10 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	if (ret < 0)
 		return ret;
 
+	ret = drm_atomic_helper_prepare_planes(dev, state);
+	if (ret)
+		return ret;
+
 	crtc_state_copy = kmalloc(sizeof(*crtc_state_copy), GFP_KERNEL);
 	if (!crtc_state_copy)
 		return -ENOMEM;
@@ -12307,26 +12326,8 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 	modeset_update_crtc_power_domains(state);
 
-	for_each_plane_in_state(state, plane, plane_state, i) {
-		if (WARN_ON(plane != modeset_crtc->primary))
-			continue;
-
-		/* Primary plane is disabled in intel_crtc_disable() */
-		if (!pipe_config->base.enable)
-			continue;
-
-		ret = drm_plane_helper_update(plane, plane_state->crtc,
-					      plane_state->fb,
-					      plane_state->crtc_x,
-					      plane_state->crtc_y,
-					      plane_state->crtc_w,
-					      plane_state->crtc_h,
-					      plane_state->src_x,
-					      plane_state->src_y,
-					      plane_state->src_w,
-					      plane_state->src_h);
-		WARN_ON(ret != 0);
-	}
+	__intel_set_mode_swap_plane_state(dev, state);
+	drm_atomic_helper_commit_planes(dev, state);
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
@@ -12349,6 +12350,8 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	intel_crtc->config = crtc_state_copy;
 	intel_crtc->base.state = &crtc_state_copy->base;
 
+	drm_atomic_helper_cleanup_planes(dev, state);
+
 	drm_atomic_state_free(state);
 
 	return 0;
-- 
2.1.0

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

[PATCH 34/35] drm/i915: Swap atomic state in legacy modeset

[Ander Conselvan de Oliveira]

Replace the commit output state function with a simple swap of states.
Note that we still need to reconcile the legacy state after the swap,
since there are still code that relies on those.

Also note that even though changes to the state of a crtc different than
the one passed as an argument to __intel_set_mode() will be saved, the
modeset logic still deals with only one crtc.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 117 ++++++++++++-----------------------
 1 file changed, 39 insertions(+), 78 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 99a3bc0..5b8b66c 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -5640,16 +5640,21 @@ static int broxton_calc_cdclk(struct drm_i915_private *dev_priv,
 		return 144000;
 }
 
-/* compute the max pixel clock for new configuration */
-static int intel_mode_max_pixclk(struct drm_atomic_state *state)
+/* Compute the max pixel clock for new configuration. Uses atomic state if
+ * that's non-NULL, look at current state otherwise. */
+static int intel_mode_max_pixclk(struct drm_device *dev,
+				 struct drm_atomic_state *state)
 {
-	struct drm_device *dev = state->dev;
 	struct intel_crtc *intel_crtc;
 	struct intel_crtc_state *crtc_state;
 	int max_pixclk = 0;
 
 	for_each_intel_crtc(dev, intel_crtc) {
-		crtc_state = intel_atomic_get_crtc_state(state, intel_crtc);
+		if (state)
+			crtc_state =
+				intel_atomic_get_crtc_state(state, intel_crtc);
+		else
+			crtc_state = intel_crtc->config;
 		if (IS_ERR(crtc_state))
 			return PTR_ERR(crtc_state);
 
@@ -5668,7 +5673,7 @@ static int valleyview_modeset_global_pipes(struct drm_atomic_state *state)
 	struct drm_i915_private *dev_priv = to_i915(state->dev);
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
-	int max_pixclk = intel_mode_max_pixclk(state);
+	int max_pixclk = intel_mode_max_pixclk(state->dev, state);
 	int cdclk, i;
 
 	if (max_pixclk < 0)
@@ -5736,18 +5741,15 @@ static void vlv_program_pfi_credits(struct drm_i915_private *dev_priv)
 	WARN_ON(I915_READ(GCI_CONTROL) & PFI_CREDIT_RESEND);
 }
 
-static void valleyview_modeset_global_resources(struct drm_atomic_state *state)
+static void valleyview_modeset_global_resources(struct drm_atomic_state *old_state)
 {
-	struct drm_device *dev = state->dev;
+	struct drm_device *dev = old_state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int max_pixclk = intel_mode_max_pixclk(state);
+	int max_pixclk = intel_mode_max_pixclk(dev, NULL);
 	int req_cdclk;
 
-	/* The only reason this can fail is if we fail to add the crtc_state
-	 * to the atomic state. But that can't happen since the call to
-	 * intel_mode_max_pixclk() in valleyview_modeset_global_pipes() (which
-	 * can't have failed otherwise the mode set would be aborted) added all
-	 * the states already. */
+	/* The path in intel_mode_max_pixclk() with a NULL atomic state should
+	 * never fail. */
 	if (WARN_ON(max_pixclk < 0))
 		return;
 
@@ -9066,11 +9068,11 @@ void hsw_disable_pc8(struct drm_i915_private *dev_priv)
 	intel_prepare_ddi(dev);
 }
 
-static void broxton_modeset_global_resources(struct drm_atomic_state *state)
+static void broxton_modeset_global_resources(struct drm_atomic_state *old_state)
 {
-	struct drm_device *dev = state->dev;
+	struct drm_device *dev = old_state->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
-	int max_pixclk = intel_mode_max_pixclk(state);
+	int max_pixclk = intel_mode_max_pixclk(dev, NULL);
 	int req_cdclk;
 
 	/* see the comment in valleyview_modeset_global_resources */
@@ -11064,46 +11066,36 @@ static void intel_modeset_update_connector_atomic_state(struct drm_device *dev)
 	}
 }
 
-/**
- * intel_modeset_commit_output_state
- *
- * This function copies the stage display pipe configuration to the real one.
- *
- * FIXME: we want to replace this with a proper state swap in the future
+/* Fixup legacy state after an atomic state swap.
  */
-static void intel_modeset_commit_output_state(struct drm_atomic_state *state)
+static void intel_modeset_fixup_state(struct drm_atomic_state *state)
 {
-	struct drm_crtc *crtc;
-	struct drm_crtc_state *crtc_state;
-	struct drm_connector *connector;
-	struct drm_connector_state *connector_state;
+	struct intel_crtc *crtc;
 	struct intel_encoder *encoder;
-	struct intel_connector *intel_connector;
-	int i;
-
-	for_each_connector_in_state(state, connector, connector_state, i) {
-		*connector->state = *connector_state;
+	struct intel_connector *connector;
 
-		connector->encoder = connector_state->best_encoder;
-		if (connector->encoder)
-			connector->encoder->crtc = connector_state->crtc;
+	for_each_intel_connector(state->dev, connector) {
+		connector->base.encoder = connector->base.state->best_encoder;
+		if (connector->base.encoder)
+			connector->base.encoder->crtc =
+				connector->base.state->crtc;
 	}
 
 	/* Update crtc of disabled encoders */
 	for_each_intel_encoder(state->dev, encoder) {
 		int num_connectors = 0;
 
-		for_each_intel_connector(state->dev, intel_connector)
-			if (intel_connector->base.encoder == &encoder->base)
+		for_each_intel_connector(state->dev, connector)
+			if (connector->base.encoder == &encoder->base)
 				num_connectors++;
 
 		if (num_connectors == 0)
 			encoder->base.crtc = NULL;
 	}
 
-	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		crtc->state->enable = crtc_state->enable;
-		crtc->enabled = crtc_state->enable;
+	for_each_intel_crtc(state->dev, crtc) {
+		crtc->base.enabled = crtc->base.state->enable;
+		crtc->config = to_intel_crtc_state(crtc->base.state);
 	}
 
 	/* Copy the new configuration to the staged state, to keep the few
@@ -11560,7 +11552,8 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 			intel_encoder->connectors_active = false;
 	}
 
-	intel_modeset_commit_output_state(state);
+	drm_atomic_helper_swap_state(state->dev, state);
+	intel_modeset_fixup_state(state);
 
 	/* Double check state. */
 	for_each_crtc(dev, crtc) {
@@ -11578,7 +11571,7 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 		if (crtc != connector->encoder->crtc)
 			continue;
 
-		if (crtc_state->enable && needs_modeset(crtc_state)) {
+		if (crtc->state->enable && needs_modeset(crtc->state)) {
 			struct drm_property *dpms_property =
 				dev->mode_config.dpms_property;
 
@@ -12244,31 +12237,12 @@ static int __intel_set_mode_checks(struct drm_atomic_state *state)
 	return 0;
 }
 
-static void __intel_set_mode_swap_plane_state(struct drm_device *dev,
-					      struct drm_atomic_state *state)
-{
-	int i;
-
-	for (i = 0; i < dev->mode_config.num_total_plane; i++) {
-		struct drm_plane *plane = state->planes[i];
-
-		if (!plane)
-			continue;
-
-		plane->state->state = state;
-		swap(state->plane_states[i], plane->state);
-		plane->state->state = NULL;
-	}
-}
-
 static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 			    struct intel_crtc_state *pipe_config)
 {
 	struct drm_device *dev = modeset_crtc->dev;
 	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_atomic_state *state = pipe_config->base.state;
-	struct intel_crtc_state *crtc_state_copy = NULL;
-	struct intel_crtc *intel_crtc;
 	struct drm_crtc *crtc;
 	struct drm_crtc_state *crtc_state;
 	int ret = 0;
@@ -12282,10 +12256,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	if (ret)
 		return ret;
 
-	crtc_state_copy = kmalloc(sizeof(*crtc_state_copy), GFP_KERNEL);
-	if (!crtc_state_copy)
-		return -ENOMEM;
-
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
 		if (!needs_modeset(crtc_state))
 			continue;
@@ -12307,9 +12277,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	 */
 	if (pipe_config->base.enable && needs_modeset(&pipe_config->base)) {
 		modeset_crtc->mode = pipe_config->base.mode;
-		/* mode_set/enable/disable functions rely on a correct pipe
-		 * config. */
-		intel_crtc_set_state(to_intel_crtc(modeset_crtc), pipe_config);
 
 		/*
 		 * Calculate and store various constants which
@@ -12324,14 +12291,16 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 	 * update the the output configuration. */
 	intel_modeset_update_state(state);
 
+	/* The state has been swaped above, so state actually contains the
+	 * old state now. */
+
 	modeset_update_crtc_power_domains(state);
 
-	__intel_set_mode_swap_plane_state(dev, state);
 	drm_atomic_helper_commit_planes(dev, state);
 
 	/* Now enable the clocks, plane, pipe, and connectors that we set up. */
 	for_each_crtc_in_state(state, crtc, crtc_state, i) {
-		if (!needs_modeset(crtc_state) || !crtc_state->enable)
+		if (!needs_modeset(crtc->state) || !crtc->state->enable)
 			continue;
 
 		update_scanline_offset(to_intel_crtc(crtc));
@@ -12342,14 +12311,6 @@ static int __intel_set_mode(struct drm_crtc *modeset_crtc,
 
 	/* FIXME: add subpixel order */
 
-	intel_crtc = to_intel_crtc(modeset_crtc);
-
-	/* The pipe_config will be freed with the atomic state, so
-	 * make a copy. */
-	memcpy(crtc_state_copy, intel_crtc->config, sizeof *crtc_state_copy);
-	intel_crtc->config = crtc_state_copy;
-	intel_crtc->base.state = &crtc_state_copy->base;
-
 	drm_atomic_helper_cleanup_planes(dev, state);
 
 	drm_atomic_state_free(state);
-- 
2.1.0

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

[PATCH 35/35] drm/i915: Get rid of intel_crtc_set_state()

[Ander Conselvan de Oliveira]

Now that we do proper state swaps, we don't depend on this function
anymore to keep the state in sync.

Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 12 ++----------
 1 file changed, 2 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 5b8b66c..ee547b2 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -10168,14 +10168,6 @@ void intel_mark_idle(struct drm_device *dev)
 	intel_runtime_pm_put(dev_priv);
 }
 
-static void intel_crtc_set_state(struct intel_crtc *crtc,
-				 struct intel_crtc_state *crtc_state)
-{
-	kfree(crtc->config);
-	crtc->config = crtc_state;
-	crtc->base.state = &crtc_state->base;
-}
-
 static void intel_crtc_destroy(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
@@ -10192,7 +10184,6 @@ static void intel_crtc_destroy(struct drm_crtc *crtc)
 		kfree(work);
 	}
 
-	intel_crtc_set_state(intel_crtc, NULL);
 	drm_crtc_cleanup(crtc);
 
 	kfree(intel_crtc);
@@ -13366,7 +13357,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
 	crtc_state = kzalloc(sizeof(*crtc_state), GFP_KERNEL);
 	if (!crtc_state)
 		goto fail;
-	intel_crtc_set_state(intel_crtc, crtc_state);
+	intel_crtc->config = crtc_state;
+	intel_crtc->base.state = &crtc_state->base;
 	crtc_state->base.crtc = &intel_crtc->base;
 
 	/* initialize shared scalers */
-- 
2.1.0

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

Re: [PATCH 00/35] Make legacy modeset a lot more atomic-like

[Maarten Lankhorst]

Op 21-04-15 om 16:12 schreef Ander Conselvan de Oliveira:
> Hi,
>
> This patch series changes the legacy modeset path to be a lot more
> atomic like. Among other things, it
>
>  - unifies the flip-only and the modeset path;
>  - implements a full state swap as part of the modeset;
>  - gets rid of the recovery logic in case of a failed modeset;
>  - replaces some i915 functions with drm atomic helper ones.
>
> Some of these patches were sent previously and the feedback so far has
> been addressed. I also rebased this on top Maarten's series that
> simplifies plane enabling/disabling.
>
> Thanks,
> Ander
>
> Ander Conselvan de Oliveira (28):
>   drm/i915: Don't check for NULL before freeing state
>   drm/i915: Call drm helpers when duplicating crtc and plane states
>   drm/i915: Use for_each_connector_in_state helper macro
>   drm/i915: Extract mode_changed computation out of
>     stage_output_config()
>   drm/i915: Add crtc states before calling compute_config()
>   drm/i915: Don't pretend we can calculate multiple pipe_configs
>   drm/i915: Calculate a new pipe_config based on new enabled state
>   drm/i915: Remove all *_pipes flags from modeset
>   drm/i915: Remove saved_mode from __intel_set_mode()
>   drm/i915: Move compute part of __intel_set_mode() to separate function
>   drm/i915: Simplify error handling in __intel_set_mode()
>   drm/i915: Don't modeset with old mode when set_crtc fails
>   drm/i915: Add primary plane to atomic state in legacy modeset
>   drm/i915: Delete fb, x and y parameters from mode set functions
>   drm/i915: Don't use struct intel_set_config *_changed flags
>   drm/i915: Don't use staged config to calculate mode_changed flags
>   drm/i915: Unify modeset and flip paths of intel_crtc_set_config()
>   drm/i915: Simplify intel_set_config_compute_mode_changes() a bit
>   drm/i915: Stage new modeset state straight into atomic state
>   drm/i915: Remove save/restore logic from intel_crtc_set_config()
>   drm/i915: Update crtc state active flag based on DPMS
>   drm/atomic: Make mode_fixup() optional for check_modeset()
>   drm/i915: Use atomic helpers for computing changed flags
>   drm/i915: Take ownership of atomic state on success in
>     intel_set_mode()
>   drm/i915: Preserve shared DPLL information in new pipe_config
>   drm/i915: Don't use plane update helper in legacy mode set
>   drm/i915: Swap atomic state in legacy modeset
>   drm/i915: Get rid of intel_crtc_set_state()
For the whole series:
Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

\o/ fixing up atomic planes should be a lot more trivial now, killing the last of the plane helpers.

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

Re: [PATCH 04/35] drm/i915: get rid of primary_enabled and use atomic state

[Daniel Vetter]

On Tue, Apr 21, 2015 at 05:12:53PM +0300, Ander Conselvan de Oliveira wrote:
> From: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>

Generally a bit too terse commmit message. E.g. here a short summary of
our irc discussion about why it should be ok to remove this feature is
definitely needed. I added that.

But I prefer more verbose commit messages in general.
-Daniel

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 50 ++++++++++++++++++++----------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  drivers/gpu/drm/i915/intel_fbc.c     |  2 +-
>  3 files changed, 29 insertions(+), 24 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index acf3494..40e3c62 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -2236,11 +2236,7 @@ static void intel_enable_primary_hw_plane(struct drm_plane *plane,
>  
>  	/* If the pipe isn't enabled, we can't pump pixels and may hang */
>  	assert_pipe_enabled(dev_priv, intel_crtc->pipe);
> -
> -	if (intel_crtc->primary_enabled)
> -		return;
> -
> -	intel_crtc->primary_enabled = true;
> +	to_intel_plane_state(plane->state)->visible = true;
>  
>  	dev_priv->display.update_primary_plane(crtc, plane->fb,
>  					       crtc->x, crtc->y);
> @@ -2661,6 +2657,8 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	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 drm_plane *primary = crtc->primary;
> +	bool visible = to_intel_plane_state(primary->state)->visible;
>  	struct drm_i915_gem_object *obj;
>  	int plane = intel_crtc->plane;
>  	unsigned long linear_offset;
> @@ -2668,7 +2666,7 @@ static void i9xx_update_primary_plane(struct drm_crtc *crtc,
>  	u32 reg = DSPCNTR(plane);
>  	int pixel_size;
>  
> -	if (!intel_crtc->primary_enabled || !fb) {
> +	if (!visible || !fb) {
>  		I915_WRITE(reg, 0);
>  		if (INTEL_INFO(dev)->gen >= 4)
>  			I915_WRITE(DSPSURF(plane), 0);
> @@ -2790,6 +2788,8 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	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 drm_plane *primary = crtc->primary;
> +	bool visible = to_intel_plane_state(primary->state)->visible;
>  	struct drm_i915_gem_object *obj;
>  	int plane = intel_crtc->plane;
>  	unsigned long linear_offset;
> @@ -2797,7 +2797,7 @@ static void ironlake_update_primary_plane(struct drm_crtc *crtc,
>  	u32 reg = DSPCNTR(plane);
>  	int pixel_size;
>  
> -	if (!intel_crtc->primary_enabled || !fb) {
> +	if (!visible || !fb) {
>  		I915_WRITE(reg, 0);
>  		I915_WRITE(DSPSURF(plane), 0);
>  		POSTING_READ(reg);
> @@ -2966,6 +2966,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	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 drm_plane *plane = crtc->primary;
> +	bool visible = to_intel_plane_state(plane->state)->visible;
>  	struct drm_i915_gem_object *obj;
>  	int pipe = intel_crtc->pipe;
>  	u32 plane_ctl, stride_div, stride;
> @@ -2973,9 +2975,8 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  	unsigned int rotation;
>  	int x_offset, y_offset;
>  	unsigned long surf_addr;
> -	struct drm_plane *plane;
>  
> -	if (!intel_crtc->primary_enabled || !fb) {
> +	if (!visible || !fb) {
>  		I915_WRITE(PLANE_CTL(pipe, 0), 0);
>  		I915_WRITE(PLANE_SURF(pipe, 0), 0);
>  		POSTING_READ(PLANE_CTL(pipe, 0));
> @@ -3035,7 +3036,6 @@ static void skylake_update_primary_plane(struct drm_crtc *crtc,
>  
>  	plane_ctl |= PLANE_CTL_PLANE_GAMMA_DISABLE;
>  
> -	plane = crtc->primary;
>  	rotation = plane->state->rotation;
>  	switch (rotation) {
>  	case BIT(DRM_ROTATE_90):
> @@ -4699,7 +4699,6 @@ static void intel_crtc_disable_planes(struct drm_crtc *crtc)
>  	hsw_disable_ips(intel_crtc);
>  
>  	intel_crtc_dpms_overlay(intel_crtc, false);
> -	intel_crtc->primary_enabled = false;
>  	for_each_intel_plane(dev, intel_plane) {
>  		if (intel_plane->pipe == pipe) {
>  			struct drm_crtc *from = intel_plane->base.crtc;
> @@ -12803,6 +12802,9 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  	} else if (config->fb_changed) {
>  		struct intel_crtc *intel_crtc = to_intel_crtc(set->crtc);
>  		struct drm_plane *primary = set->crtc->primary;
> +		struct intel_plane_state *plane_state =
> +				to_intel_plane_state(primary->state);
> +		bool was_visible = plane_state->visible;
>  		int vdisplay, hdisplay;
>  
>  		drm_crtc_get_hv_timing(set->mode, &hdisplay, &vdisplay);
> @@ -12815,7 +12817,8 @@ static int intel_crtc_set_config(struct drm_mode_set *set)
>  		 * We need to make sure the primary plane is re-enabled if it
>  		 * has previously been turned off.
>  		 */
> -		if (!intel_crtc->primary_enabled && ret == 0) {
> +		plane_state = to_intel_plane_state(primary->state);
> +		if (ret == 0 && !was_visible && plane_state->visible) {
>  			WARN_ON(!intel_crtc->active);
>  			intel_enable_primary_hw_plane(set->crtc->primary, set->crtc);
>  		}
> @@ -13113,6 +13116,9 @@ intel_check_primary_plane(struct drm_plane *plane,
>  		return ret;
>  
>  	if (intel_crtc->active) {
> +		struct intel_plane_state *old_state =
> +			to_intel_plane_state(plane->state);
> +
>  		intel_crtc->atomic.wait_for_flips = true;
>  
>  		/*
> @@ -13125,20 +13131,20 @@ intel_check_primary_plane(struct drm_plane *plane,
>  		 * one is done too late. We eventually need to unify
>  		 * this.
>  		 */
> -		if (intel_crtc->primary_enabled &&
> +		if (state->visible &&
>  		    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
>  		    dev_priv->fbc.crtc == intel_crtc &&
>  		    state->base.rotation != BIT(DRM_ROTATE_0)) {
>  			intel_crtc->atomic.disable_fbc = true;
>  		}
>  
> -		if (state->visible) {
> +		if (state->visible && !old_state->visible) {
>  			/*
>  			 * BDW signals flip done immediately if the plane
>  			 * is disabled, even if the plane enable is already
>  			 * armed to occur at the next vblank :(
>  			 */
> -			if (IS_BROADWELL(dev) && !intel_crtc->primary_enabled)
> +			if (IS_BROADWELL(dev))
>  				intel_crtc->atomic.wait_vblank = true;
>  		}
>  
> @@ -13173,8 +13179,6 @@ intel_commit_primary_plane(struct drm_plane *plane,
>  	crtc->y = src->y1 >> 16;
>  
>  	if (intel_crtc->active) {
> -		intel_crtc->primary_enabled = state->visible;
> -
>  		if (state->visible)
>  			/* FIXME: kill this fastboot hack */
>  			intel_update_pipe_size(intel_crtc);
> @@ -13192,9 +13196,6 @@ intel_disable_primary_plane(struct drm_plane *plane,
>  	struct drm_device *dev = plane->dev;
>  	struct drm_i915_private *dev_priv = dev->dev_private;
>  
> -	if (!force)
> -		to_intel_crtc(crtc)->primary_enabled = false;
> -
>  	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
>  }
>  
> @@ -14655,8 +14656,8 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc)
>  		 * Temporarily change the plane mapping and disable everything
>  		 * ...  */
>  		plane = crtc->plane;
> +		to_intel_plane_state(crtc->base.primary->state)->visible = true;
>  		crtc->plane = !plane;
> -		crtc->primary_enabled = true;
>  		dev_priv->display.crtc_disable(&crtc->base);
>  		crtc->plane = plane;
>  
> @@ -14833,6 +14834,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  	int i;
>  
>  	for_each_intel_crtc(dev, crtc) {
> +		struct drm_plane *primary = crtc->base.primary;
> +		struct intel_plane_state *plane_state;
> +
>  		memset(crtc->config, 0, sizeof(*crtc->config));
>  
>  		crtc->config->quirks |= PIPE_CONFIG_QUIRK_INHERITED_MODE;
> @@ -14842,7 +14846,9 @@ static void intel_modeset_readout_hw_state(struct drm_device *dev)
>  
>  		crtc->base.state->enable = crtc->active;
>  		crtc->base.enabled = crtc->active;
> -		crtc->primary_enabled = primary_get_hw_state(crtc);
> +
> +		plane_state = to_intel_plane_state(primary->state);
> +		plane_state->visible = primary_get_hw_state(crtc);
>  
>  		DRM_DEBUG_KMS("[CRTC:%d] hw state readout: %s\n",
>  			      crtc->base.base.id,
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 42548bc..cb0f29e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -510,7 +510,6 @@ struct intel_crtc {
>  	 */
>  	bool active;
>  	unsigned long enabled_power_domains;
> -	bool primary_enabled; /* is the primary plane (partially) visible? */
>  	bool lowfreq_avail;
>  	struct intel_overlay *overlay;
>  	struct intel_unpin_work *unpin_work;
> diff --git a/drivers/gpu/drm/i915/intel_fbc.c b/drivers/gpu/drm/i915/intel_fbc.c
> index 4165ce0..6abb834 100644
> --- a/drivers/gpu/drm/i915/intel_fbc.c
> +++ b/drivers/gpu/drm/i915/intel_fbc.c
> @@ -457,7 +457,7 @@ static struct drm_crtc *intel_fbc_find_crtc(struct drm_i915_private *dev_priv)
>  		tmp_crtc = dev_priv->pipe_to_crtc_mapping[pipe];
>  
>  		if (intel_crtc_active(tmp_crtc) &&
> -		    to_intel_crtc(tmp_crtc)->primary_enabled) {
> +		    to_intel_plane_state(tmp_crtc->primary->state)->visible) {
>  			if (one_pipe_only && crtc) {
>  				if (set_no_fbc_reason(dev_priv, FBC_MULTIPLE_PIPES))
>  					DRM_DEBUG_KMS("more than one pipe active, disabling compression\n");
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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 00/35] Make legacy modeset a lot more atomic-like

[Daniel Vetter]

On Tue, Apr 21, 2015 at 05:21:22PM +0200, Maarten Lankhorst wrote:
> Op 21-04-15 om 16:12 schreef Ander Conselvan de Oliveira:
> > Hi,
> >
> > This patch series changes the legacy modeset path to be a lot more
> > atomic like. Among other things, it
> >
> >  - unifies the flip-only and the modeset path;
> >  - implements a full state swap as part of the modeset;
> >  - gets rid of the recovery logic in case of a failed modeset;
> >  - replaces some i915 functions with drm atomic helper ones.
> >
> > Some of these patches were sent previously and the feedback so far has
> > been addressed. I also rebased this on top Maarten's series that
> > simplifies plane enabling/disabling.
> >
> > Thanks,
> > Ander
> >
> > Ander Conselvan de Oliveira (28):
> >   drm/i915: Don't check for NULL before freeing state
> >   drm/i915: Call drm helpers when duplicating crtc and plane states
> >   drm/i915: Use for_each_connector_in_state helper macro
> >   drm/i915: Extract mode_changed computation out of
> >     stage_output_config()
> >   drm/i915: Add crtc states before calling compute_config()
> >   drm/i915: Don't pretend we can calculate multiple pipe_configs
> >   drm/i915: Calculate a new pipe_config based on new enabled state
> >   drm/i915: Remove all *_pipes flags from modeset
> >   drm/i915: Remove saved_mode from __intel_set_mode()
> >   drm/i915: Move compute part of __intel_set_mode() to separate function
> >   drm/i915: Simplify error handling in __intel_set_mode()
> >   drm/i915: Don't modeset with old mode when set_crtc fails
> >   drm/i915: Add primary plane to atomic state in legacy modeset
> >   drm/i915: Delete fb, x and y parameters from mode set functions
> >   drm/i915: Don't use struct intel_set_config *_changed flags
> >   drm/i915: Don't use staged config to calculate mode_changed flags
> >   drm/i915: Unify modeset and flip paths of intel_crtc_set_config()
> >   drm/i915: Simplify intel_set_config_compute_mode_changes() a bit
> >   drm/i915: Stage new modeset state straight into atomic state
> >   drm/i915: Remove save/restore logic from intel_crtc_set_config()
> >   drm/i915: Update crtc state active flag based on DPMS
> >   drm/atomic: Make mode_fixup() optional for check_modeset()
> >   drm/i915: Use atomic helpers for computing changed flags
> >   drm/i915: Take ownership of atomic state on success in
> >     intel_set_mode()
> >   drm/i915: Preserve shared DPLL information in new pipe_config
> >   drm/i915: Don't use plane update helper in legacy mode set
> >   drm/i915: Swap atomic state in legacy modeset
> >   drm/i915: Get rid of intel_crtc_set_state()
> For the whole series:
> Reviewed-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> 
> \o/ fixing up atomic planes should be a lot more trivial now, killing the last of the plane helpers.

Pulled them all in now, thanks.
-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 15/35] drm/i915: Remove all *_pipes flags from modeset

[Chris Wilson]

On Tue, Apr 21, 2015 at 05:13:04PM +0300, Ander Conselvan de Oliveira wrote:
> @@ -11548,26 +11479,36 @@ intel_modeset_update_state(struct drm_device *dev, unsigned prepare_pipes)
>  		if (!intel_encoder->base.crtc)
>  			continue;
>  
> -		intel_crtc = to_intel_crtc(intel_encoder->base.crtc);
> +		for_each_crtc_in_state(state, crtc, crtc_state, i)
> +			if (crtc == intel_encoder->base.crtc)
> +				break;
>  
> -		if (prepare_pipes & (1 << intel_crtc->pipe))
> +		if (crtc != intel_encoder->base.crtc)
> +			continue;
> +
> +		if (crtc_state->enable && needs_modeset(crtc_state))
>  			intel_encoder->connectors_active = false;

This makes the compiler gasp, and even if the compiler is wrong, the
code isn't perhaps the most clear - and this pattern gets repeated.

For example,

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 067b1dee1b90..3b049cdbb7e5 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11859,15 +11859,15 @@ intel_modeset_update_state(struct drm_atomic_state *state)
                if (!intel_encoder->base.crtc)
                        continue;
 
-               for_each_crtc_in_state(state, crtc, crtc_state, i)
-                       if (crtc == intel_encoder->base.crtc)
-                               break;
+               for_each_crtc_in_state(state, crtc, crtc_state, i) {
+                       if (crtc != intel_encoder->base.crtc)
+                               continue;
 
-               if (crtc != intel_encoder->base.crtc)
-                       continue;
+                       if (crtc_state->enable && needs_modeset(crtc_state))
+                               intel_encoder->connectors_active = false;
 
-               if (crtc_state->enable && needs_modeset(crtc_state))
-                       intel_encoder->connectors_active = false;
+                       break;
+               }
        }
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

[PATCH] drm/i915: Silence compiler warning

[Ander Conselvan de Oliveira]

Silence the following -Wmaybe-uninitialized warnings and make the code
more clear.

drivers/gpu/drm/i915/intel_display.c: In function ‘__intel_set_mode’:
drivers/gpu/drm/i915/intel_display.c:11844:14: warning: ‘crtc_state’ may be used uninitialized in this function [-Wmaybe-uninitialized]
  return state->mode_changed || state->active_changed;
              ^
drivers/gpu/drm/i915/intel_display.c:11854:25: note: ‘crtc_state’ was declared here
  struct drm_crtc_state *crtc_state;
                         ^
drivers/gpu/drm/i915/intel_display.c:11868:6: warning: ‘crtc’ may be used uninitialized in this function [-Wmaybe-uninitialized]
   if (crtc != intel_encoder->base.crtc)
      ^
drivers/gpu/drm/i915/intel_display.c:11853:19: note: ‘crtc’ was declared here
  struct drm_crtc *crtc;

Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>
---
 drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index c392e58..ee3adb3 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -11861,15 +11861,15 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 		if (!intel_encoder->base.crtc)
 			continue;
 
-		for_each_crtc_in_state(state, crtc, crtc_state, i)
-			if (crtc == intel_encoder->base.crtc)
-				break;
+		for_each_crtc_in_state(state, crtc, crtc_state, i) {
+			if (crtc != intel_encoder->base.crtc)
+				continue;
 
-		if (crtc != intel_encoder->base.crtc)
-			continue;
+			if (crtc_state->enable && needs_modeset(crtc_state))
+				intel_encoder->connectors_active = false;
 
-		if (crtc_state->enable && needs_modeset(crtc_state))
-			intel_encoder->connectors_active = false;
+			break;
+		}
 	}
 
 	drm_atomic_helper_swap_state(state->dev, state);
@@ -11884,24 +11884,24 @@ intel_modeset_update_state(struct drm_atomic_state *state)
 		if (!connector->encoder || !connector->encoder->crtc)
 			continue;
 
-		for_each_crtc_in_state(state, crtc, crtc_state, i)
-			if (crtc == connector->encoder->crtc)
-				break;
+		for_each_crtc_in_state(state, crtc, crtc_state, i) {
+			if (crtc != connector->encoder->crtc)
+				continue;
 
-		if (crtc != connector->encoder->crtc)
-			continue;
+			if (crtc->state->enable && needs_modeset(crtc->state)) {
+				struct drm_property *dpms_property =
+					dev->mode_config.dpms_property;
 
-		if (crtc->state->enable && needs_modeset(crtc->state)) {
-			struct drm_property *dpms_property =
-				dev->mode_config.dpms_property;
+				connector->dpms = DRM_MODE_DPMS_ON;
+				drm_object_property_set_value(&connector->base,
+								 dpms_property,
+								 DRM_MODE_DPMS_ON);
 
-			connector->dpms = DRM_MODE_DPMS_ON;
-			drm_object_property_set_value(&connector->base,
-							 dpms_property,
-							 DRM_MODE_DPMS_ON);
+				intel_encoder = to_intel_encoder(connector->encoder);
+				intel_encoder->connectors_active = true;
+			}
 
-			intel_encoder = to_intel_encoder(connector->encoder);
-			intel_encoder->connectors_active = true;
+			break;
 		}
 	}
 
-- 
2.1.0

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

Re: [PATCH] drm/i915: Silence compiler warning

[Daniel Vetter]

On Fri, May 29, 2015 at 02:28:09PM +0300, Ander Conselvan de Oliveira wrote:
> Silence the following -Wmaybe-uninitialized warnings and make the code
> more clear.
> 
> drivers/gpu/drm/i915/intel_display.c: In function ‘__intel_set_mode’:
> drivers/gpu/drm/i915/intel_display.c:11844:14: warning: ‘crtc_state’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>   return state->mode_changed || state->active_changed;
>               ^
> drivers/gpu/drm/i915/intel_display.c:11854:25: note: ‘crtc_state’ was declared here
>   struct drm_crtc_state *crtc_state;
>                          ^
> drivers/gpu/drm/i915/intel_display.c:11868:6: warning: ‘crtc’ may be used uninitialized in this function [-Wmaybe-uninitialized]
>    if (crtc != intel_encoder->base.crtc)
>       ^
> drivers/gpu/drm/i915/intel_display.c:11853:19: note: ‘crtc’ was declared here
>   struct drm_crtc *crtc;
> 
> Reported-by: Chris Wilson <chris@chris-wilson.co.uk>
> Suggested-by: Chris Wilson <chris@chris-wilson.co.uk>
> Signed-off-by: Ander Conselvan de Oliveira <ander.conselvan.de.oliveira@intel.com>

Queued for -next, thanks for the patch.
-Daniel

> ---
>  drivers/gpu/drm/i915/intel_display.c | 42 ++++++++++++++++++------------------
>  1 file changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index c392e58..ee3adb3 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -11861,15 +11861,15 @@ intel_modeset_update_state(struct drm_atomic_state *state)
>  		if (!intel_encoder->base.crtc)
>  			continue;
>  
> -		for_each_crtc_in_state(state, crtc, crtc_state, i)
> -			if (crtc == intel_encoder->base.crtc)
> -				break;
> +		for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +			if (crtc != intel_encoder->base.crtc)
> +				continue;
>  
> -		if (crtc != intel_encoder->base.crtc)
> -			continue;
> +			if (crtc_state->enable && needs_modeset(crtc_state))
> +				intel_encoder->connectors_active = false;
>  
> -		if (crtc_state->enable && needs_modeset(crtc_state))
> -			intel_encoder->connectors_active = false;
> +			break;
> +		}
>  	}
>  
>  	drm_atomic_helper_swap_state(state->dev, state);
> @@ -11884,24 +11884,24 @@ intel_modeset_update_state(struct drm_atomic_state *state)
>  		if (!connector->encoder || !connector->encoder->crtc)
>  			continue;
>  
> -		for_each_crtc_in_state(state, crtc, crtc_state, i)
> -			if (crtc == connector->encoder->crtc)
> -				break;
> +		for_each_crtc_in_state(state, crtc, crtc_state, i) {
> +			if (crtc != connector->encoder->crtc)
> +				continue;
>  
> -		if (crtc != connector->encoder->crtc)
> -			continue;
> +			if (crtc->state->enable && needs_modeset(crtc->state)) {
> +				struct drm_property *dpms_property =
> +					dev->mode_config.dpms_property;
>  
> -		if (crtc->state->enable && needs_modeset(crtc->state)) {
> -			struct drm_property *dpms_property =
> -				dev->mode_config.dpms_property;
> +				connector->dpms = DRM_MODE_DPMS_ON;
> +				drm_object_property_set_value(&connector->base,
> +								 dpms_property,
> +								 DRM_MODE_DPMS_ON);
>  
> -			connector->dpms = DRM_MODE_DPMS_ON;
> -			drm_object_property_set_value(&connector->base,
> -							 dpms_property,
> -							 DRM_MODE_DPMS_ON);
> +				intel_encoder = to_intel_encoder(connector->encoder);
> +				intel_encoder->connectors_active = true;
> +			}
>  
> -			intel_encoder = to_intel_encoder(connector->encoder);
> -			intel_encoder->connectors_active = true;
> +			break;
>  		}
>  	}
>  
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
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