Skip to contentSkip to conversation

drm/i915: Remove implicitly disabling primary plane for now

PATCH In review Maarten Lankhorst 3 participants 14 messages Apr 15, 2015 – Apr 20, 2015

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

PATCH 1/8 drm/i915: Remove implicitly disabling primary plane for now

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.

---
 drivers/gpu/drm/i915/intel_drv.h    |  6 -----
 drivers/gpu/drm/i915/intel_sprite.c | 45 +------------------------------------
 2 files changed, 1 insertion(+), 50 deletions(-)
drivers/gpu/drm/i915/intel_drv.h +0 -6
index 082be7161203..ec12948e76aa 100644
@@ -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
drivers/gpu/drm/i915/intel_sprite.c +1 -44

115 lines not on this page · raw message

Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#1 lore raw

PATCH 2/8 drm/i915: Add a way to disable planes without updating state

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

---
 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(-)
drivers/gpu/drm/i915/intel_display.c +35 -3
index 75afa6ef22c7..84e21efe10cf 100644
@@ -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,
drivers/gpu/drm/i915/intel_drv.h +1 -1
index ec12948e76aa..27dbd8145610 100644
@@ -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,
drivers/gpu/drm/i915/intel_sprite.c +8 -8
index 612d8e0b3e02..631645420683 100644
@@ -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);
 		}
 	}
 }
Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#2 lore raw

PATCH 3/8 drm/i915: Use the disable callback for disabling planes.

In reply to Maarten Lankhorst

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

---
 drivers/gpu/drm/i915/i915_drv.h      |  5 ++
 drivers/gpu/drm/i915/intel_display.c | 91 ++++++------------------------------
 2 files changed, 20 insertions(+), 76 deletions(-)
drivers/gpu/drm/i915/i915_drv.h +5 -0
index 89231aee31c0..61b756bdbaad 100644
@@ -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)
 
drivers/gpu/drm/i915/intel_display.c +15 -76

124 lines not on this page · raw message

Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#3 lore raw

PATCH 4/8 drm/i915: get rid of primary_enabled and use atomic state

In reply to Maarten Lankhorst
---
 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(-)
drivers/gpu/drm/i915/intel_display.c +28 -22
index 742829f3bb1d..0a34ac731f0e 100644
@@ -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,
drivers/gpu/drm/i915/intel_drv.h +0 -1
index 27dbd8145610..1a528cd1cdbb 100644
@@ -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;
drivers/gpu/drm/i915/intel_fbc.c +1 -1
index 4165ce0644f7..6abb83432d4d 100644
@@ -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");
Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#4 lore raw

PATCH 5/8 drm/i915: Move intel_(pre_disable/post_enable)_primary to intel_display.c, and use it there.

In reply to Maarten Lankhorst

They're the same code, so why not?

---
 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(-)
drivers/gpu/drm/i915/intel_display.c +102 -56
index 0a34ac731f0e..d6f14765cf7d 100644
@@ -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);
 		}
 
 		/*
drivers/gpu/drm/i915/intel_drv.h +0 -2
index 1a528cd1cdbb..eb1195a1de99 100644
@@ -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);
drivers/gpu/drm/i915/intel_sprite.c +0 -68

74 lines not on this page · raw message

Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#5 lore raw

PATCH 6/8 drm/i915: Rename intel_crtc_dpms_overlay.

In reply to Maarten Lankhorst

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

---
 drivers/gpu/drm/i915/intel_display.c | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)
drivers/gpu/drm/i915/intel_display.c +3 -4
index d6f14765cf7d..47fbdb5c71cb 100644
@@ -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;
Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#6 lore raw

PATCH 7/8 drm/i915: Move toggling planes out of crtc enable/disable.

In reply to Maarten Lankhorst

This makes disabling planes more explicit.

---
 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(-)
drivers/gpu/drm/i915/i915_debugfs.c +4 -0
index 07a71c0ff775..2d63e15a8669 100644
@@ -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));
drivers/gpu/drm/i915/intel_display.c +18 -18
index 47fbdb5c71cb..cb677aff4245 100644
@@ -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;
 
drivers/gpu/drm/i915/intel_drv.h +2 -0
index eb1195a1de99..f521d21d04e9 100644
@@ -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 *);
Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#7 lore raw

Re: [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable.

Quoted 8 lines · Maarten Lankhorst

This makes disabling planes more explicit.

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

[...]

Quoted 19 lines
drivers/gpu/drm/i915/intel_display.c +4 -1
index 47fbdb5c71cb..cb677aff4245 100644
@@ -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);

[...]

drivers/gpu/drm/i915/intel_display.c static int __intel_set_mode(struct drm_crtc *crtc, · line 12042

Quoted 6 lines
drivers/gpu/drm/i915/intel_display.c +1 -0
@@ -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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

#8 lore raw

Re: [PATCH 7/8] drm/i915: Move toggling planes out of crtc enable/disable.

Op 16-04-15 om 09:32 schreef Ander Conselvan De Oliveira:

Quoted 35 lines · Maarten Lankhorst
Quoted 8 lines

This makes disabling planes more explicit.

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

[...]

Quoted 19 lines
drivers/gpu/drm/i915/intel_display.c +4 -1
index 47fbdb5c71cb..cb677aff4245 100644
@@ -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);

[...]

Quoted 3 lines
drivers/gpu/drm/i915/intel_display.c +0 -0
@@ -12040,6 +12038,7 @@ static int __intel_set_mode(struct drm_crtc *crtc,
 		update_scanline_offset(intel_crtc);
 
Quoted 6 lines

drivers/gpu/drm/i915/intel_display.c static int __intel_set_mode(struct drm_crtc *crtc, · line 12042

Quoted 3 lines
drivers/gpu/drm/i915/intel_display.c +1 -0
 		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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx

#9 lore raw

PATCH v2 7/8 drm/i915: Move toggling planes out of crtc enable/disable.

In reply to Ander Conselvan De Oliveira

This makes disabling planes more explicit.

---
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(-)
drivers/gpu/drm/i915/i915_debugfs.c +2 -4
index 07a71c0ff775..60e86f331313 100644
@@ -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));
drivers/gpu/drm/i915/intel_display.c +28 -18

146 lines not on this page · raw message

drivers/gpu/drm/i915/intel_drv.h +1 -0
index eb1195a1de99..70b70e9be167 100644
@@ -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 *);
Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#10 lore raw

Re: [PATCH v2 7/8] drm/i915: Move toggling planes out of crtc enable/disable.

Quoted 7 lines · Maarten Lankhorst

This makes disabling planes more explicit.

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

Signature · 7 lines
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#11 lore raw

PATCH 8/8 drm/i915: Move atomic crtc update checking to the check crtc function.

In reply to Maarten Lankhorst
---
 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(-)
drivers/gpu/drm/i915/intel_atomic_plane.c +3 -15
index a27ee8cbb627..4b639b54583d 100644
@@ -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)) {
drivers/gpu/drm/i915/intel_display.c +130 -66

252 lines not on this page · raw message

drivers/gpu/drm/i915/intel_sprite.c +1 -24
index 7419e04b113f..9cf35c33bc61 100644
@@ -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;
 }
 
Signature · 6 lines
2.1.0

_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#12 lore raw

PATCH v2 8/8 drm/i915: Move atomic crtc update checking to the check crtc function.

---
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(-)
drivers/gpu/drm/i915/intel_atomic_plane.c +3 -15
index a27ee8cbb627..4b639b54583d 100644
@@ -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)) {
drivers/gpu/drm/i915/intel_display.c +130 -66

252 lines not on this page · raw message

drivers/gpu/drm/i915/intel_sprite.c +1 -24
index 7419e04b113f..9cf35c33bc61 100644
@@ -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;
 }
 
Signature · 7 lines
2.1.0


_______________________________________________
Intel-gfx mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx
#13 lore raw

Re: [PATCH 8/8] drm/i915: Move atomic crtc update checking to the check crtc function.

In reply to Maarten Lankhorst

Op 15-04-15 om 16:34 schreef Maarten Lankhorst:

Quoted 6 lines
---
 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
[email protected]
http://lists.freedesktop.org/mailman/listinfo/intel-gfx