All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now
@ 2015-04-15 14:34 Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 2/8] drm/i915: Add a way to disable planes without updating state Maarten Lankhorst
                   ` (6 more replies)
  0 siblings, 7 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

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>
---
 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 082be7161203..ec12948e76aa 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 e3d41c096dc6..612d8e0b3e02 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 2/8] drm/i915: Add a way to disable planes without updating state
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 3/8] drm/i915: Use the disable callback for disabling planes Maarten Lankhorst
                   ` (5 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

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>
---
 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 75afa6ef22c7..84e21efe10cf 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2669,7 +2669,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);
@@ -2798,7 +2798,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);
@@ -2976,7 +2976,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));
@@ -12978,6 +12978,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;
@@ -13119,6 +13133,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;
@@ -13224,6 +13239,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)
 {
@@ -13282,6 +13313,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 ec12948e76aa..27dbd8145610 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 612d8e0b3e02..631645420683 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 3/8] drm/i915: Use the disable callback for disabling planes.
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 2/8] drm/i915: Add a way to disable planes without updating state Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 4/8] drm/i915: get rid of primary_enabled and use atomic state Maarten Lankhorst
                   ` (4 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

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

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.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 89231aee31c0..61b756bdbaad 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 84e21efe10cf..742829f3bb1d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2229,32 +2229,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
@@ -4516,38 +4490,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;
@@ -4701,6 +4643,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);
@@ -4711,9 +4654,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
@@ -12957,24 +12906,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 4/8] drm/i915: get rid of primary_enabled and use atomic state
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 2/8] drm/i915: Add a way to disable planes without updating state Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 3/8] drm/i915: Use the disable callback for disabling planes Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 5/8] drm/i915: Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use it there Maarten Lankhorst
                   ` (3 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.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 742829f3bb1d..0a34ac731f0e 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2211,11 +2211,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);
@@ -2636,6 +2632,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;
@@ -2643,7 +2641,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);
@@ -2765,6 +2763,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;
@@ -2772,7 +2772,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);
@@ -2941,6 +2941,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;
@@ -2948,9 +2950,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));
@@ -3010,7 +3011,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):
@@ -4654,7 +4654,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;
@@ -12536,6 +12535,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);
@@ -12548,7 +12550,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);
 		}
@@ -12846,6 +12849,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;
 
 		/*
@@ -12858,20 +12864,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;
 		}
 
@@ -12906,8 +12912,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);
@@ -12925,9 +12929,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);
 }
 
@@ -14385,8 +14386,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;
 
@@ -14563,6 +14564,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;
@@ -14572,7 +14576,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 27dbd8145610..1a528cd1cdbb 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 4165ce0644f7..6abb83432d4d 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 5/8] drm/i915: Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use it there.
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
                   ` (2 preceding siblings ...)
  2015-04-15 14:34 ` [PATCH 4/8] drm/i915: get rid of primary_enabled and use atomic state Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 6/8] drm/i915: Rename intel_crtc_dpms_overlay Maarten Lankhorst
                   ` (2 subsequent siblings)
  6 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

They're the same code, so why not?

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.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 0a34ac731f0e..d6f14765cf7d 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -2215,14 +2215,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)
@@ -4613,17 +4605,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);
@@ -4631,27 +4644,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) {
@@ -5500,9 +5581,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)
@@ -5561,19 +5639,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)
@@ -5602,25 +5667,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);
 
 	/*
@@ -12553,7 +12599,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 1a528cd1cdbb..eb1195a1de99 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -1360,8 +1360,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 631645420683..7419e04b113f 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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 6/8] drm/i915: Rename intel_crtc_dpms_overlay.
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
                   ` (3 preceding siblings ...)
  2015-04-15 14:34 ` [PATCH 5/8] drm/i915: Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use it there Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable Maarten Lankhorst
  2015-04-15 14:34 ` [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function Maarten Lankhorst
  6 siblings, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

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

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.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 d6f14765cf7d..47fbdb5c71cb 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -4587,9 +4587,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;
 
@@ -4718,7 +4718,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);
 }
@@ -4734,7 +4733,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable.
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
                   ` (4 preceding siblings ...)
  2015-04-15 14:34 ` [PATCH 6/8] drm/i915: Rename intel_crtc_dpms_overlay Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-16  7:32   ` Ander Conselvan De Oliveira
  2015-04-15 14:34 ` [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function Maarten Lankhorst
  6 siblings, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

This makes disabling planes more explicit.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/i915_debugfs.c  |  4 ++++
 drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
 drivers/gpu/drm/i915/intel_drv.h     |  2 ++
 3 files changed, 24 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_debugfs.c b/drivers/gpu/drm/i915/i915_debugfs.c
index 07a71c0ff775..2d63e15a8669 100644
--- a/drivers/gpu/drm/i915/i915_debugfs.c
+++ b/drivers/gpu/drm/i915/i915_debugfs.c
@@ -3594,8 +3594,10 @@ 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));
 
+		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);
 	}
 	drm_modeset_unlock_all(dev);
 }
@@ -3616,8 +3618,10 @@ 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;
 
+		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);
 
 		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 47fbdb5c71cb..cb677aff4245 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -3123,8 +3123,8 @@ 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);
+		intel_crtc_disable_planes(&crtc->base);
+		dev_priv->display.crtc_disable(&crtc->base);
 	}
 }
 
@@ -4711,10 +4711,13 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
 	hsw_disable_ips(intel_crtc);
 }
 
-static void intel_crtc_enable_planes(struct drm_crtc *crtc)
+void intel_crtc_enable_planes(struct drm_crtc *crtc)
 {
 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
 
+	if (intel_crtc->active)
+		return;
+
 	intel_enable_primary_hw_plane(crtc->primary, crtc);
 	intel_enable_sprite_planes(crtc);
 	intel_crtc_update_cursor(crtc, true);
@@ -4722,13 +4725,16 @@ static void intel_crtc_enable_planes(struct drm_crtc *crtc)
 	intel_post_enable_primary(crtc);
 }
 
-static void intel_crtc_disable_planes(struct drm_crtc *crtc)
+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;
 
+	if (!intel_crtc->active)
+		return;
+
 	intel_crtc_wait_for_pending_flips(crtc);
 
 	intel_pre_disable_primary(crtc);
@@ -4820,8 +4826,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. */
@@ -4943,7 +4947,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)
@@ -4973,8 +4976,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);
 
@@ -5037,8 +5038,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);
@@ -5578,8 +5577,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)
@@ -5636,8 +5633,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)
@@ -5666,8 +5661,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.
@@ -5731,9 +5724,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;
@@ -5768,6 +5763,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);
 
@@ -11991,8 +11987,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
@@ -12040,6 +12038,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 */
@@ -14433,6 +14432,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 eb1195a1de99..f521d21d04e9 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -987,6 +987,8 @@ 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_enable_planes(struct drm_crtc *crtc);
+void intel_crtc_disable_planes(struct drm_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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function.
  2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
                   ` (5 preceding siblings ...)
  2015-04-15 14:34 ` [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable Maarten Lankhorst
@ 2015-04-15 14:34 ` Maarten Lankhorst
  2015-04-16 10:28   ` [PATCH v2 " Maarten Lankhorst
  2015-04-20  8:20   ` [PATCH " Maarten Lankhorst
  6 siblings, 2 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-15 14:34 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
 drivers/gpu/drm/i915/intel_atomic_plane.c |  18 +--
 drivers/gpu/drm/i915/intel_display.c      | 196 ++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_sprite.c       |  25 +---
 3 files changed, 134 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index a27ee8cbb627..4b639b54583d 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -123,8 +123,10 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	 * anything driver-specific we need to test in that case, so
 	 * just return success.
 	 */
-	if (!crtc)
+	if (!crtc || !intel_crtc->active || !state->fb) {
+		intel_state->visible = 0;
 		return 0;
+	}
 
 	/*
 	 * The original src/dest coordinates are stored in state->base, but
@@ -148,20 +150,6 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	intel_state->clip.y2 =
 		intel_crtc->active ? intel_crtc->config->pipe_src_h : 0;
 
-	/*
-	 * Disabling a plane is always okay; we just need to update
-	 * fb tracking in a special way since cleanup_fb() won't
-	 * get called by the plane helpers.
-	 */
-	if (state->fb == NULL && plane->state->fb != NULL) {
-		/*
-		 * 'prepare' is never called when plane is being disabled, so
-		 * we need to handle frontbuffer tracking as a special case
-		 */
-		intel_crtc->atomic.disabled_planes |=
-			(1 << drm_plane_index(plane));
-	}
-
 	if (state->fb && intel_rotation_90_or_270(state->rotation)) {
 		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index cb677aff4245..4f27597486d8 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -101,6 +101,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state);
 static void intel_begin_crtc_commit(struct drm_crtc *crtc);
 static void intel_finish_crtc_commit(struct drm_crtc *crtc);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
@@ -10646,6 +10648,7 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.load_lut = intel_crtc_load_lut,
 	.atomic_begin = intel_begin_crtc_commit,
 	.atomic_flush = intel_finish_crtc_commit,
+	.atomic_check = intel_atomic_check_crtc,
 };
 
 /**
@@ -12773,6 +12776,9 @@ bool intel_wm_need_update(struct drm_plane *plane,
 	    plane->state->rotation != state->rotation)
 		return true;
 
+	if (plane->type == DRM_PLANE_TYPE_CURSOR)
+		return plane->state->crtc_w != state->crtc_w;
+
 	return false;
 }
 
@@ -12867,7 +12873,6 @@ intel_check_primary_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
 {
 	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_crtc *intel_crtc;
 	struct drm_framebuffer *fb = state->base.fb;
@@ -12875,7 +12880,6 @@ intel_check_primary_plane(struct drm_plane *plane,
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
 	bool can_position = false;
-	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
@@ -12883,58 +12887,12 @@ intel_check_primary_plane(struct drm_plane *plane,
 	if (INTEL_INFO(dev)->gen >= 9)
 		can_position = true;
 
-	ret = drm_plane_helper_check_update(plane, crtc, fb,
-					    src, dest, clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    can_position, true,
-					    &state->visible);
-	if (ret)
-		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;
-
-		/*
-		 * FBC does not work on some platforms for rotated
-		 * planes, so disable it when rotation is not 0 and
-		 * update it when rotation is set back to 0.
-		 *
-		 * FIXME: This is redundant with the fbc update done in
-		 * the primary plane enable function except that that
-		 * one is done too late. We eventually need to unify
-		 * this.
-		 */
-		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 && !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->atomic.wait_vblank = true;
-		}
-
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
-
-		intel_crtc->atomic.update_fbc = true;
-
-		if (intel_wm_need_update(plane, &state->base))
-			intel_crtc->atomic.update_wm = true;
-	}
-
-	return 0;
+	return drm_plane_helper_check_update(plane, crtc, fb,
+					     src, dest, clip,
+					     DRM_PLANE_HELPER_NO_SCALING,
+					     DRM_PLANE_HELPER_NO_SCALING,
+					     can_position, true,
+					     &state->visible);
 }
 
 static void
@@ -12976,6 +12934,121 @@ intel_disable_primary_plane(struct drm_plane *plane,
 	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
 }
 
+
+/* Transitional checking here, mostly for plane updates */
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state)
+{
+	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_atomic_state *state = crtc_state->state;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned plane_mask;
+	int i, nplanes = dev->mode_config.num_total_plane;
+	bool mode_changed = crtc_state->mode_changed;
+
+	if (!crtc_state->planes_changed)
+		return 0;
+
+	if (!intel_crtc->active)
+		return 0;
+
+	intel_crtc->atomic.update_wm = mode_changed;
+
+	DRM_DEBUG_ATOMIC("Crtc %i was enabled %i now enabled: %i\n", drm_crtc_index(crtc), crtc->state->enable, crtc_state->enable);
+
+	plane_mask = crtc_state->plane_mask | crtc->state->plane_mask;
+	for (i = 0; i < nplanes; i++) {
+		struct intel_plane_state *plane_state, *old_plane_state;
+		struct intel_plane *plane = to_intel_plane(state->planes[i]);
+		bool turn_off, turn_on, visible, was_visible;
+
+		if (!plane)
+			continue;
+
+		plane_state = to_intel_plane_state(state->plane_states[i]);
+		old_plane_state = to_intel_plane_state(plane->base.state);
+
+		was_visible = old_plane_state->visible && crtc->state->enable;
+		visible = plane_state->visible && crtc_state->enable;
+
+		turn_off = was_visible && (!visible || mode_changed);
+		turn_on = visible && (!was_visible || mode_changed);
+
+		DRM_DEBUG_ATOMIC("Plane %i with fb %p and crtc %p, was_visible %i, visible %i, turn_off %i, turn_on %i, modeset: %i\n",
+				 i, plane_state->base.fb, plane_state->base.crtc, was_visible, visible, turn_off, turn_on, mode_changed);
+
+		/* plane being turned off as part of modeset or changes? */
+		if (intel_wm_need_update(&plane->base, &plane_state->base))
+			intel_crtc->atomic.update_wm = true;
+
+		if (old_plane_state->base.fb && !plane_state->base.fb)
+		/*
+		 * 'prepare' is never called when plane is being disabled, so
+		 * we need to handle frontbuffer tracking as a special case
+		 */
+			intel_crtc->atomic.disabled_planes |=
+				(1 << drm_plane_index(&plane->base));
+
+		switch (plane->base.type) {
+		case DRM_PLANE_TYPE_PRIMARY:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
+
+			intel_crtc->atomic.wait_for_flips = true;
+			intel_crtc->atomic.pre_disable_primary = turn_off;
+			intel_crtc->atomic.post_enable_primary = turn_on;
+
+			if (turn_off)
+				intel_crtc->atomic.disable_fbc = true;
+
+			/*
+			 * FBC does not work on some platforms for rotated
+			 * planes, so disable it when rotation is not 0 and
+			 * update it when rotation is set back to 0.
+			 *
+			 * FIXME: This is redundant with the fbc update done in
+			 * the primary plane enable function except that that
+			 * one is done too late. We eventually need to unify
+			 * this.
+			 */
+
+			if (visible &&
+			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+			    dev_priv->fbc.crtc == intel_crtc &&
+			    plane_state->base.rotation != BIT(DRM_ROTATE_0))
+				intel_crtc->atomic.disable_fbc = 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 (turn_on && IS_BROADWELL(dev))
+				intel_crtc->atomic.wait_vblank = true;
+
+			intel_crtc->atomic.update_fbc = true;
+			break;
+		case DRM_PLANE_TYPE_CURSOR:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+			break;
+		case DRM_PLANE_TYPE_OVERLAY:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+			if (turn_off) {
+				intel_crtc->atomic.wait_vblank = true;
+				intel_crtc->atomic.update_sprite_watermarks |=
+					(1 << drm_plane_index(&plane->base));
+			}
+			break;
+		}
+	}
+	return 0;
+}
+
 static void intel_begin_crtc_commit(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -13190,7 +13263,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
 	/* if we want to turn off the cursor ignore width and height */
 	if (!obj)
-		goto finish;
+		return 0;
 
 	/* Check for which cursor types we support */
 	if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
@@ -13207,19 +13280,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
 	if (fb->modifier[0] != DRM_FORMAT_MOD_NONE) {
 		DRM_DEBUG_KMS("cursor cannot be tiled\n");
-		ret = -EINVAL;
-	}
-
-finish:
-	if (intel_crtc->active) {
-		if (plane->state->crtc_w != state->base.crtc_w)
-			intel_crtc->atomic.update_wm = true;
-
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+		return -EINVAL;
 	}
 
-	return ret;
+	return 0;
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7419e04b113f..9cf35c33bc61 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -815,7 +815,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 
 	if (!fb) {
 		state->visible = false;
-		goto finish;
+		return 0;
 	}
 
 	/* Don't modify another pipe's plane */
@@ -952,29 +952,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	dst->y1 = crtc_y;
 	dst->y2 = crtc_y + crtc_h;
 
-finish:
-	/*
-	 * If the sprite is completely covering the primary plane,
-	 * we can disable the primary and save power.
-	 */
-	if (intel_crtc->active) {
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
-
-		if (intel_wm_need_update(plane, &state->base))
-			intel_crtc->atomic.update_wm = true;
-
-		if (!state->visible) {
-			/*
-			 * Avoid underruns when disabling the sprite.
-			 * FIXME remove once watermark updates are done properly.
-			 */
-			intel_crtc->atomic.wait_vblank = true;
-			intel_crtc->atomic.update_sprite_watermarks |=
-				(1 << drm_plane_index(plane));
-		}
-	}
-
 	return 0;
 }
 
-- 
2.1.0

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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable.
  2015-04-15 14:34 ` [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable Maarten Lankhorst
@ 2015-04-16  7:32   ` Ander Conselvan De Oliveira
  2015-04-16  8:05     ` Maarten Lankhorst
  2015-04-16 10:26     ` [PATCH v2 " Maarten Lankhorst
  0 siblings, 2 replies; 14+ messages in thread
From: Ander Conselvan De Oliveira @ 2015-04-16  7:32 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Wed, 2015-04-15 at 16:34 +0200, Maarten Lankhorst wrote:
> This makes disabling planes more explicit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/i915_debugfs.c  |  4 ++++
>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>  3 files changed, 24 insertions(+), 18 deletions(-)

[...]

> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 47fbdb5c71cb..cb677aff4245 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4711,10 +4711,13 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
>  	hsw_disable_ips(intel_crtc);
>  }
>  
> -static void intel_crtc_enable_planes(struct drm_crtc *crtc)
> +void intel_crtc_enable_planes(struct drm_crtc *crtc)
>  {
>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>  
> +	if (intel_crtc->active)
> +		return;
> +
>  	intel_enable_primary_hw_plane(crtc->primary, crtc);
>  	intel_enable_sprite_planes(crtc);
>  	intel_crtc_update_cursor(crtc, true);

[...]

> @@ -12040,6 +12038,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);
>  	}

The crtc_enable hooks set intel_crtc->active, so the call to
intel_crtc_enable_planes() doesn't actually do anything because of the
new check in the hunk above.

Ander


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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable.
  2015-04-16  7:32   ` Ander Conselvan De Oliveira
@ 2015-04-16  8:05     ` Maarten Lankhorst
  2015-04-16 10:26     ` [PATCH v2 " Maarten Lankhorst
  1 sibling, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-16  8:05 UTC (permalink / raw)
  To: Ander Conselvan De Oliveira; +Cc: intel-gfx

Op 16-04-15 om 09:32 schreef Ander Conselvan De Oliveira:
> On Wed, 2015-04-15 at 16:34 +0200, Maarten Lankhorst wrote:
>> This makes disabling planes more explicit.
>>
>> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
>> ---
>>  drivers/gpu/drm/i915/i915_debugfs.c  |  4 ++++
>>  drivers/gpu/drm/i915/intel_display.c | 36 ++++++++++++++++++------------------
>>  drivers/gpu/drm/i915/intel_drv.h     |  2 ++
>>  3 files changed, 24 insertions(+), 18 deletions(-)
> [...]
>
>> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>> index 47fbdb5c71cb..cb677aff4245 100644
>> --- a/drivers/gpu/drm/i915/intel_display.c
>> +++ b/drivers/gpu/drm/i915/intel_display.c
>> @@ -4711,10 +4711,13 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
>>  	hsw_disable_ips(intel_crtc);
>>  }
>>  
>> -static void intel_crtc_enable_planes(struct drm_crtc *crtc)
>> +void intel_crtc_enable_planes(struct drm_crtc *crtc)
>>  {
>>  	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
>>  
>> +	if (intel_crtc->active)
>> +		return;
>> +
>>  	intel_enable_primary_hw_plane(crtc->primary, crtc);
>>  	intel_enable_sprite_planes(crtc);
>>  	intel_crtc_update_cursor(crtc, true);
> [...]
>
>> @@ -12040,6 +12038,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);
>>  	}
> The crtc_enable hooks set intel_crtc->active, so the call to
> intel_crtc_enable_planes() doesn't actually do anything because of the
> new check in the hunk above.
Argh, you're right. I'll respin this patch.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2 7/8] drm/i915: Move toggling planes out of crtc enable/disable.
  2015-04-16  7:32   ` Ander Conselvan De Oliveira
  2015-04-16  8:05     ` Maarten Lankhorst
@ 2015-04-16 10:26     ` Maarten Lankhorst
  2015-04-16 14:10       ` Daniel Vetter
  1 sibling, 1 reply; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-16 10:26 UTC (permalink / raw)
  To: Ander Conselvan De Oliveira; +Cc: intel-gfx

This makes disabling planes more explicit.

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Changes since v1:
- Create a intel_crtc_reset function for i915_debugfs.c instead of calling .crtc members directly.
- Remove the checks for intel_crtc->active, they're wrong. (thanks anderco)

 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 07a71c0ff775..60e86f331313 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->base);
 	}
 	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->base);
 
 		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 47fbdb5c71cb..61a29f142c70 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -105,6 +105,8 @@ static void intel_begin_crtc_commit(struct drm_crtc *crtc);
 static void intel_finish_crtc_commit(struct drm_crtc *crtc);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
 	struct intel_crtc_state *crtc_state);
+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)
 {
@@ -3103,6 +3105,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);
@@ -3123,8 +3138,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);
 	}
 }
 
@@ -4713,8 +4731,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);
@@ -4820,8 +4836,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. */
@@ -4943,7 +4957,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)
@@ -4973,8 +4986,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);
 
@@ -5037,8 +5048,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);
@@ -5578,8 +5587,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)
@@ -5636,8 +5643,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)
@@ -5666,8 +5671,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.
@@ -5731,9 +5734,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;
@@ -5768,6 +5773,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);
 
@@ -11991,8 +11997,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
@@ -12040,6 +12048,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 */
@@ -14433,6 +14442,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 eb1195a1de99..70b70e9be167 100644
--- a/drivers/gpu/drm/i915/intel_drv.h
+++ b/drivers/gpu/drm/i915/intel_drv.h
@@ -987,6 +987,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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* [PATCH v2 8/8] drm/i915: Move atomic crtc update checking to the check crtc function.
  2015-04-15 14:34 ` [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function Maarten Lankhorst
@ 2015-04-16 10:28   ` Maarten Lankhorst
  2015-04-20  8:20   ` [PATCH " Maarten Lankhorst
  1 sibling, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-16 10:28 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
---
Changes since v1:
 - Clear intel_crtc->atomic at the start of the check function (paranoia).
 
 drivers/gpu/drm/i915/intel_atomic_plane.c |  18 +--
 drivers/gpu/drm/i915/intel_display.c      | 196 ++++++++++++++++++++----------
 drivers/gpu/drm/i915/intel_sprite.c       |  25 +---
 3 files changed, 134 insertions(+), 105 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_atomic_plane.c b/drivers/gpu/drm/i915/intel_atomic_plane.c
index a27ee8cbb627..4b639b54583d 100644
--- a/drivers/gpu/drm/i915/intel_atomic_plane.c
+++ b/drivers/gpu/drm/i915/intel_atomic_plane.c
@@ -123,8 +123,10 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	 * anything driver-specific we need to test in that case, so
 	 * just return success.
 	 */
-	if (!crtc)
+	if (!crtc || !intel_crtc->active || !state->fb) {
+		intel_state->visible = 0;
 		return 0;
+	}
 
 	/*
 	 * The original src/dest coordinates are stored in state->base, but
@@ -148,20 +150,6 @@ static int intel_plane_atomic_check(struct drm_plane *plane,
 	intel_state->clip.y2 =
 		intel_crtc->active ? intel_crtc->config->pipe_src_h : 0;
 
-	/*
-	 * Disabling a plane is always okay; we just need to update
-	 * fb tracking in a special way since cleanup_fb() won't
-	 * get called by the plane helpers.
-	 */
-	if (state->fb == NULL && plane->state->fb != NULL) {
-		/*
-		 * 'prepare' is never called when plane is being disabled, so
-		 * we need to handle frontbuffer tracking as a special case
-		 */
-		intel_crtc->atomic.disabled_planes |=
-			(1 << drm_plane_index(plane));
-	}
-
 	if (state->fb && intel_rotation_90_or_270(state->rotation)) {
 		if (!(state->fb->modifier[0] == I915_FORMAT_MOD_Y_TILED ||
 			state->fb->modifier[0] == I915_FORMAT_MOD_Yf_TILED)) {
diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
index 61a29f142c70..4965743053b1 100644
--- a/drivers/gpu/drm/i915/intel_display.c
+++ b/drivers/gpu/drm/i915/intel_display.c
@@ -101,6 +101,8 @@ static void vlv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
 static void chv_prepare_pll(struct intel_crtc *crtc,
 			    const struct intel_crtc_state *pipe_config);
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state);
 static void intel_begin_crtc_commit(struct drm_crtc *crtc);
 static void intel_finish_crtc_commit(struct drm_crtc *crtc);
 static void skl_init_scalers(struct drm_device *dev, struct intel_crtc *intel_crtc,
@@ -10656,6 +10658,7 @@ static struct drm_crtc_helper_funcs intel_helper_funcs = {
 	.load_lut = intel_crtc_load_lut,
 	.atomic_begin = intel_begin_crtc_commit,
 	.atomic_flush = intel_finish_crtc_commit,
+	.atomic_check = intel_atomic_check_crtc,
 };
 
 /**
@@ -12783,6 +12786,9 @@ bool intel_wm_need_update(struct drm_plane *plane,
 	    plane->state->rotation != state->rotation)
 		return true;
 
+	if (plane->type == DRM_PLANE_TYPE_CURSOR)
+		return plane->state->crtc_w != state->crtc_w;
+
 	return false;
 }
 
@@ -12877,7 +12883,6 @@ intel_check_primary_plane(struct drm_plane *plane,
 			  struct intel_plane_state *state)
 {
 	struct drm_device *dev = plane->dev;
-	struct drm_i915_private *dev_priv = dev->dev_private;
 	struct drm_crtc *crtc = state->base.crtc;
 	struct intel_crtc *intel_crtc;
 	struct drm_framebuffer *fb = state->base.fb;
@@ -12885,7 +12890,6 @@ intel_check_primary_plane(struct drm_plane *plane,
 	struct drm_rect *src = &state->src;
 	const struct drm_rect *clip = &state->clip;
 	bool can_position = false;
-	int ret;
 
 	crtc = crtc ? crtc : plane->crtc;
 	intel_crtc = to_intel_crtc(crtc);
@@ -12893,58 +12897,12 @@ intel_check_primary_plane(struct drm_plane *plane,
 	if (INTEL_INFO(dev)->gen >= 9)
 		can_position = true;
 
-	ret = drm_plane_helper_check_update(plane, crtc, fb,
-					    src, dest, clip,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    DRM_PLANE_HELPER_NO_SCALING,
-					    can_position, true,
-					    &state->visible);
-	if (ret)
-		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;
-
-		/*
-		 * FBC does not work on some platforms for rotated
-		 * planes, so disable it when rotation is not 0 and
-		 * update it when rotation is set back to 0.
-		 *
-		 * FIXME: This is redundant with the fbc update done in
-		 * the primary plane enable function except that that
-		 * one is done too late. We eventually need to unify
-		 * this.
-		 */
-		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 && !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->atomic.wait_vblank = true;
-		}
-
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
-
-		intel_crtc->atomic.update_fbc = true;
-
-		if (intel_wm_need_update(plane, &state->base))
-			intel_crtc->atomic.update_wm = true;
-	}
-
-	return 0;
+	return drm_plane_helper_check_update(plane, crtc, fb,
+					     src, dest, clip,
+					     DRM_PLANE_HELPER_NO_SCALING,
+					     DRM_PLANE_HELPER_NO_SCALING,
+					     can_position, true,
+					     &state->visible);
 }
 
 static void
@@ -12986,6 +12944,121 @@ intel_disable_primary_plane(struct drm_plane *plane,
 	dev_priv->display.update_primary_plane(crtc, NULL, 0, 0);
 }
 
+/* Transitional checking here, mostly for plane updates */
+static int intel_atomic_check_crtc(struct drm_crtc *crtc,
+				   struct drm_crtc_state *crtc_state)
+{
+	struct intel_crtc_state *pipe_config = to_intel_crtc_state(crtc_state);
+	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
+	struct drm_atomic_state *state = crtc_state->state;
+	struct drm_device *dev = crtc->dev;
+	struct drm_i915_private *dev_priv = dev->dev_private;
+	unsigned plane_mask;
+	int i, nplanes = dev->mode_config.num_total_plane;
+	bool mode_changed = crtc_state->mode_changed;
+
+	if (!crtc_state->planes_changed)
+		return 0;
+
+	if (!intel_crtc->active)
+		return 0;
+
+	memset(&intel_crtc->atomic, 0, sizeof(intel_crtc->atomic));
+	intel_crtc->atomic.update_wm = mode_changed;
+
+	DRM_DEBUG_ATOMIC("Crtc %i was enabled %i now enabled: %i\n", drm_crtc_index(crtc), crtc->state->enable, crtc_state->enable);
+
+	plane_mask = crtc_state->plane_mask | crtc->state->plane_mask;
+	for (i = 0; i < nplanes; i++) {
+		struct intel_plane_state *plane_state, *old_plane_state;
+		struct intel_plane *plane = to_intel_plane(state->planes[i]);
+		bool turn_off, turn_on, visible, was_visible;
+
+		if (!plane)
+			continue;
+
+		plane_state = to_intel_plane_state(state->plane_states[i]);
+		old_plane_state = to_intel_plane_state(plane->base.state);
+
+		was_visible = old_plane_state->visible && crtc->state->enable;
+		visible = plane_state->visible && crtc_state->enable;
+
+		turn_off = was_visible && (!visible || mode_changed);
+		turn_on = visible && (!was_visible || mode_changed);
+
+		DRM_DEBUG_ATOMIC("Plane %i with fb %p and crtc %p, was_visible %i, visible %i, turn_off %i, turn_on %i, modeset: %i\n",
+				 i, plane_state->base.fb, plane_state->base.crtc, was_visible, visible, turn_off, turn_on, mode_changed);
+
+		/* plane being turned off as part of modeset or changes? */
+		if (intel_wm_need_update(&plane->base, &plane_state->base))
+			intel_crtc->atomic.update_wm = true;
+
+		if (old_plane_state->base.fb && !plane_state->base.fb)
+		/*
+		 * 'prepare' is never called when plane is being disabled, so
+		 * we need to handle frontbuffer tracking as a special case
+		 */
+			intel_crtc->atomic.disabled_planes |=
+				(1 << drm_plane_index(&plane->base));
+
+		switch (plane->base.type) {
+		case DRM_PLANE_TYPE_PRIMARY:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_PRIMARY(intel_crtc->pipe);
+
+			intel_crtc->atomic.wait_for_flips = true;
+			intel_crtc->atomic.pre_disable_primary = turn_off;
+			intel_crtc->atomic.post_enable_primary = turn_on;
+
+			if (turn_off)
+				intel_crtc->atomic.disable_fbc = true;
+
+			/*
+			 * FBC does not work on some platforms for rotated
+			 * planes, so disable it when rotation is not 0 and
+			 * update it when rotation is set back to 0.
+			 *
+			 * FIXME: This is redundant with the fbc update done in
+			 * the primary plane enable function except that that
+			 * one is done too late. We eventually need to unify
+			 * this.
+			 */
+
+			if (visible &&
+			    INTEL_INFO(dev)->gen <= 4 && !IS_G4X(dev) &&
+			    dev_priv->fbc.crtc == intel_crtc &&
+			    plane_state->base.rotation != BIT(DRM_ROTATE_0))
+				intel_crtc->atomic.disable_fbc = 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 (turn_on && IS_BROADWELL(dev))
+				intel_crtc->atomic.wait_vblank = true;
+
+			intel_crtc->atomic.update_fbc = true;
+			break;
+		case DRM_PLANE_TYPE_CURSOR:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+			break;
+		case DRM_PLANE_TYPE_OVERLAY:
+			intel_crtc->atomic.fb_bits |=
+				INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
+
+			if (turn_off) {
+				intel_crtc->atomic.wait_vblank = true;
+				intel_crtc->atomic.update_sprite_watermarks |=
+					(1 << drm_plane_index(&plane->base));
+			}
+			break;
+		}
+	}
+	return 0;
+}
+
 static void intel_begin_crtc_commit(struct drm_crtc *crtc)
 {
 	struct drm_device *dev = crtc->dev;
@@ -13200,7 +13273,7 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
 	/* if we want to turn off the cursor ignore width and height */
 	if (!obj)
-		goto finish;
+		return 0;
 
 	/* Check for which cursor types we support */
 	if (!cursor_size_ok(dev, state->base.crtc_w, state->base.crtc_h)) {
@@ -13217,19 +13290,10 @@ intel_check_cursor_plane(struct drm_plane *plane,
 
 	if (fb->modifier[0] != DRM_FORMAT_MOD_NONE) {
 		DRM_DEBUG_KMS("cursor cannot be tiled\n");
-		ret = -EINVAL;
-	}
-
-finish:
-	if (intel_crtc->active) {
-		if (plane->state->crtc_w != state->base.crtc_w)
-			intel_crtc->atomic.update_wm = true;
-
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_CURSOR(intel_crtc->pipe);
+		return -EINVAL;
 	}
 
-	return ret;
+	return 0;
 }
 
 static void
diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
index 7419e04b113f..9cf35c33bc61 100644
--- a/drivers/gpu/drm/i915/intel_sprite.c
+++ b/drivers/gpu/drm/i915/intel_sprite.c
@@ -815,7 +815,7 @@ intel_check_sprite_plane(struct drm_plane *plane,
 
 	if (!fb) {
 		state->visible = false;
-		goto finish;
+		return 0;
 	}
 
 	/* Don't modify another pipe's plane */
@@ -952,29 +952,6 @@ intel_check_sprite_plane(struct drm_plane *plane,
 	dst->y1 = crtc_y;
 	dst->y2 = crtc_y + crtc_h;
 
-finish:
-	/*
-	 * If the sprite is completely covering the primary plane,
-	 * we can disable the primary and save power.
-	 */
-	if (intel_crtc->active) {
-		intel_crtc->atomic.fb_bits |=
-			INTEL_FRONTBUFFER_SPRITE(intel_crtc->pipe);
-
-		if (intel_wm_need_update(plane, &state->base))
-			intel_crtc->atomic.update_wm = true;
-
-		if (!state->visible) {
-			/*
-			 * Avoid underruns when disabling the sprite.
-			 * FIXME remove once watermark updates are done properly.
-			 */
-			intel_crtc->atomic.wait_vblank = true;
-			intel_crtc->atomic.update_sprite_watermarks |=
-				(1 << drm_plane_index(plane));
-		}
-	}
-
 	return 0;
 }
 
-- 
2.1.0


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

^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2 7/8] drm/i915: Move toggling planes out of crtc enable/disable.
  2015-04-16 10:26     ` [PATCH v2 " Maarten Lankhorst
@ 2015-04-16 14:10       ` Daniel Vetter
  0 siblings, 0 replies; 14+ messages in thread
From: Daniel Vetter @ 2015-04-16 14:10 UTC (permalink / raw)
  To: Maarten Lankhorst; +Cc: intel-gfx

On Thu, Apr 16, 2015 at 12:26:52PM +0200, Maarten Lankhorst wrote:
> This makes disabling planes more explicit.
> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
> Changes since v1:
> - Create a intel_crtc_reset function for i915_debugfs.c instead of calling .crtc members directly.
> - Remove the checks for intel_crtc->active, they're wrong. (thanks anderco)

Style recommendation in drm/i915 land is to put the per-patch changelog
before the cutoff. I know that often it contains just noise, but often the
commit message doesn't get updated with the new reasonings and these
breadcrumbs tend to be really valuable when trying to understand why a
patch was done in the future.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function.
  2015-04-15 14:34 ` [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function Maarten Lankhorst
  2015-04-16 10:28   ` [PATCH v2 " Maarten Lankhorst
@ 2015-04-20  8:20   ` Maarten Lankhorst
  1 sibling, 0 replies; 14+ messages in thread
From: Maarten Lankhorst @ 2015-04-20  8:20 UTC (permalink / raw)
  To: intel-gfx; +Cc: Ander Conselvan de Oliveira

Op 15-04-15 om 16:34 schreef Maarten Lankhorst:
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_atomic_plane.c |  18 +--
>  drivers/gpu/drm/i915/intel_display.c      | 196 ++++++++++++++++++++----------
>  drivers/gpu/drm/i915/intel_sprite.c       |  25 +---
>  3 files changed, 134 insertions(+), 105 deletions(-)
Ignore this patch for now, the transitional helpers don't call crtc check so it's useless to do until all transitional plane helpers are converted.
The other patches should still apply.

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2015-04-20  8:20 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-04-15 14:34 [PATCH 1/8] drm/i915: Remove implicitly disabling primary plane for now Maarten Lankhorst
2015-04-15 14:34 ` [PATCH 2/8] drm/i915: Add a way to disable planes without updating state Maarten Lankhorst
2015-04-15 14:34 ` [PATCH 3/8] drm/i915: Use the disable callback for disabling planes Maarten Lankhorst
2015-04-15 14:34 ` [PATCH 4/8] drm/i915: get rid of primary_enabled and use atomic state Maarten Lankhorst
2015-04-15 14:34 ` [PATCH 5/8] drm/i915: Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use it there Maarten Lankhorst
2015-04-15 14:34 ` [PATCH 6/8] drm/i915: Rename intel_crtc_dpms_overlay Maarten Lankhorst
2015-04-15 14:34 ` [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable Maarten Lankhorst
2015-04-16  7:32   ` Ander Conselvan De Oliveira
2015-04-16  8:05     ` Maarten Lankhorst
2015-04-16 10:26     ` [PATCH v2 " Maarten Lankhorst
2015-04-16 14:10       ` Daniel Vetter
2015-04-15 14:34 ` [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function Maarten Lankhorst
2015-04-16 10:28   ` [PATCH v2 " Maarten Lankhorst
2015-04-20  8:20   ` [PATCH " Maarten Lankhorst

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.