Attachment 133422 Details for Bug 101664 – Only enable IPS if primary plane is enabled, v2.

[patch] Only enable IPS if primary plane is enabled, v2.

file_101664.txt (text/plain), 9.71 KB, created by Maarten Lankhorst on 2017-08-10 09:27:54 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Maarten Lankhorst

Created: 2017-08-10 09:27:54 UTC

Size: 9.71 KB

patch

obsolete

>diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
>index a8775ed817d1..88dfb4bd8db0 100644
>--- a/drivers/gpu/drm/i915/intel_display.c
>+++ b/drivers/gpu/drm/i915/intel_display.c
>@@ -117,7 +117,8 @@ static void ironlake_pfit_disable(struct intel_crtc *crtc, bool force);
> static void ironlake_pfit_enable(struct intel_crtc *crtc);
> static void intel_modeset_setup_hw_state(struct drm_device *dev,
> 					 struct drm_modeset_acquire_ctx *ctx);
>-static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc);
>+static void intel_pre_disable_primary_noatomic(struct drm_crtc *crtc,
>+					       struct intel_crtc_state *pipe_config);
> 
> struct intel_limit {
> 	struct {
>@@ -2723,7 +2724,8 @@ intel_find_initial_plane_obj(struct intel_crtc *intel_crtc,
> 	intel_set_plane_visible(to_intel_crtc_state(crtc_state),
> 				to_intel_plane_state(plane_state),
> 				false);
>-	intel_pre_disable_primary_noatomic(&intel_crtc->base);
>+	intel_pre_disable_primary_noatomic(&intel_crtc->base,
>+					   to_intel_crtc_state(crtc_state));
> 	trace_intel_disable_plane(primary, intel_crtc);
> 	intel_plane->disable_plane(intel_plane, intel_crtc);
> 
>@@ -4668,9 +4670,6 @@ void hsw_enable_ips(struct intel_crtc *crtc)
> 	struct drm_device *dev = crtc->base.dev;
> 	struct drm_i915_private *dev_priv = to_i915(dev);
> 
>-	if (!crtc->config->ips_enabled)
>-		return;
>-
> 	/*
> 	 * We can only enable IPS after we enable a plane and wait for a vblank
> 	 * This function is called from post_plane_update, which is run after
>@@ -4706,9 +4705,6 @@ void hsw_disable_ips(struct intel_crtc *crtc)
> 	struct drm_device *dev = crtc->base.dev;
> 	struct drm_i915_private *dev_priv = to_i915(dev);
> 
>-	if (!crtc->config->ips_enabled)
>-		return;
>-
> 	assert_plane_enabled(dev_priv, crtc->plane);
> 	if (IS_BROADWELL(dev_priv)) {
> 		mutex_lock(&dev_priv->rps.hw_lock);
>@@ -4754,7 +4750,7 @@ static void intel_crtc_dpms_overlay_disable(struct intel_crtc *intel_crtc)
>  * completely hide the primary plane.
>  */
> static void
>-intel_post_enable_primary(struct drm_crtc *crtc)
>+intel_post_enable_primary(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config)
> {
> 	struct drm_device *dev = crtc->dev;
> 	struct drm_i915_private *dev_priv = to_i915(dev);
>@@ -4767,7 +4763,8 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> 	 * when going from primary only to sprite only and vice
> 	 * versa.
> 	 */
>-	hsw_enable_ips(intel_crtc);
>+	if (pipe_config->ips_enabled && !intel_crtc->ips_disabled)
>+		hsw_enable_ips(intel_crtc);
> 
> 	/*
> 	 * Gen2 reports pipe underruns whenever all planes are disabled.
>@@ -4786,7 +4783,7 @@ intel_post_enable_primary(struct drm_crtc *crtc)
> 
> /* FIXME move all this to pre_plane_update() with proper state tracking */
> static void
>-intel_pre_disable_primary(struct drm_crtc *crtc)
>+intel_pre_disable_primary(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config)
> {
> 	struct drm_device *dev = crtc->dev;
> 	struct drm_i915_private *dev_priv = to_i915(dev);
>@@ -4808,19 +4805,20 @@ intel_pre_disable_primary(struct drm_crtc *crtc)
> 	 * when going from primary only to sprite only and vice
> 	 * versa.
> 	 */
>-	hsw_disable_ips(intel_crtc);
>+	if (pipe_config->ips_enabled)
>+		hsw_disable_ips(intel_crtc);
> }
> 
> /* FIXME get rid of this and use pre_plane_update */
> static void
>-intel_pre_disable_primary_noatomic(struct drm_crtc *crtc)
>+intel_pre_disable_primary_noatomic(struct drm_crtc *crtc, struct intel_crtc_state *pipe_config)
> {
> 	struct drm_device *dev = crtc->dev;
> 	struct drm_i915_private *dev_priv = to_i915(dev);
> 	struct intel_crtc *intel_crtc = to_intel_crtc(crtc);
> 	int pipe = intel_crtc->pipe;
> 
>-	intel_pre_disable_primary(crtc);
>+	intel_pre_disable_primary(crtc, pipe_config);
> 
> 	/*
> 	 * Vblank time updates from the shadow to live plane control register
>@@ -4858,7 +4856,7 @@ static void intel_post_plane_update(struct intel_crtc_state *old_crtc_state)
> 		if (new_primary_state->visible &&
> 		    (needs_modeset(&pipe_config->base) ||
> 		     !old_primary_state->visible))
>-			intel_post_enable_primary(&crtc->base);
>+			intel_post_enable_primary(&crtc->base, pipe_config);
> 	}
> }
> 
>@@ -4885,7 +4883,7 @@ static void intel_pre_plane_update(struct intel_crtc_state *old_crtc_state,
> 
> 		if (old_primary_state->visible &&
> 		    (modeset || !new_primary_state->visible))
>-			intel_pre_disable_primary(&crtc->base);
>+			intel_pre_disable_primary(&crtc->base, pipe_config);
> 	}
> 
> 	/*
>@@ -5700,13 +5698,6 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> 	if (!intel_crtc->active)
> 		return;
> 
>-	if (crtc->primary->state->visible) {
>-		intel_pre_disable_primary_noatomic(crtc);
>-
>-		intel_crtc_disable_planes(crtc, 1 << drm_plane_index(crtc->primary));
>-		crtc->primary->state->visible = false;
>-	}
>-
> 	state = drm_atomic_state_alloc(crtc->dev);
> 	if (!state) {
> 		DRM_DEBUG_KMS("failed to disable [CRTC:%d:%s], out of memory",
>@@ -5722,6 +5713,14 @@ static void intel_crtc_disable_noatomic(struct drm_crtc *crtc,
> 
> 	WARN_ON(IS_ERR(crtc_state) || ret);
> 
>+	if (crtc->primary->state->visible) {
>+		intel_pre_disable_primary_noatomic(crtc, crtc_state);
>+
>+		intel_crtc_disable_planes(crtc, 1 << drm_plane_index(crtc->primary));
>+		crtc->primary->state->visible = false;
>+	}
>+
>+
> 	dev_priv->display.crtc_disable(crtc_state, state);
> 
> 	drm_atomic_state_put(state);
>diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
>index a07d06fbc4b4..872b1fce1d0e 100644
>--- a/drivers/gpu/drm/i915/intel_drv.h
>+++ b/drivers/gpu/drm/i915/intel_drv.h
>@@ -828,6 +828,9 @@ struct intel_crtc {
> 	bool cpu_fifo_underrun_disabled;
> 	bool pch_fifo_underrun_disabled;
> 
>+	/* Whether ips is force-disabled for collecting CRC */
>+	bool ips_disabled;
>+
> 	/* per-pipe watermark state */
> 	struct {
> 		/* watermarks currently being used  */
>diff --git a/drivers/gpu/drm/i915/intel_pipe_crc.c b/drivers/gpu/drm/i915/intel_pipe_crc.c
>index 8fbd2bd0877f..fc3682c4c055 100644
>--- a/drivers/gpu/drm/i915/intel_pipe_crc.c
>+++ b/drivers/gpu/drm/i915/intel_pipe_crc.c
>@@ -601,6 +601,33 @@ static int get_new_crc_ctl_reg(struct drm_i915_private *dev_priv,
> 		return ivb_pipe_crc_ctl_reg(dev_priv, pipe, source, val);
> }
> 
>+static int toggle_ips_and_test_active(struct intel_crtc *crtc,
>+					      bool has_source)
>+{
>+	bool has_ips = false;
>+	int ret;
>+
>+
>+	ret = drm_modeset_lock_interruptible(&crtc->base.mutex, NULL);
>+	if (ret)
>+		return ret;
>+
>+	if (to_intel_crtc_state(crtc->base.state)->ips_enabled &&
>+	    crtc->base.state->plane_mask & BIT(drm_plane_index(crtc->base.primary)))
>+		has_ips = crtc->base.primary->state->visible;
>+
>+	if (!crtc->base.state->active && has_source) {
>+		DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
>+		ret = -EIO;
>+	}
>+
>+	crtc->ips_disabled = has_source;
>+
>+	drm_modeset_unlock(&crtc->base.mutex);
>+
>+	return ret ?: has_ips;
>+}
>+
> static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> 			       enum pipe pipe,
> 			       enum intel_pipe_crc_source source)
>@@ -610,6 +637,7 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> 	enum intel_display_power_domain power_domain;
> 	u32 val = 0; /* shut up gcc */
> 	int ret;
>+	int has_ips;
> 
> 	if (pipe_crc->source == source)
> 		return 0;
>@@ -618,11 +646,12 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> 	if (pipe_crc->source && source)
> 		return -EINVAL;
> 
>+	has_ips = toggle_ips_and_test_active(crtc, source);
>+	if (has_ips < 0)
>+		return has_ips;
>+
> 	power_domain = POWER_DOMAIN_PIPE(pipe);
>-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>-		DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
>-		return -EIO;
>-	}
>+	intel_display_power_get(dev_priv, power_domain);
> 
> 	ret = get_new_crc_ctl_reg(dev_priv, pipe, &source, &val);
> 	if (ret != 0)
>@@ -649,7 +678,8 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> 		 * user space can't make reliable use of the CRCs, so let's just
> 		 * completely disable it.
> 		 */
>-		hsw_disable_ips(crtc);
>+		if (has_ips)
>+			hsw_disable_ips(crtc);
> 
> 		spin_lock_irq(&pipe_crc->lock);
> 		kfree(pipe_crc->entries);
>@@ -694,7 +724,8 @@ static int pipe_crc_set_source(struct drm_i915_private *dev_priv,
> 		else if (IS_HASWELL(dev_priv) && pipe == PIPE_A)
> 			hsw_trans_edp_pipe_A_crc_wa(dev_priv, false);
> 
>-		hsw_enable_ips(crtc);
>+		if (has_ips)
>+			hsw_enable_ips(crtc);
> 	}
> 
> 	ret = 0;
>@@ -919,23 +950,25 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> 	enum intel_pipe_crc_source source;
> 	u32 val = 0; /* shut up gcc */
> 	int ret = 0;
>+	int has_ips;
> 
> 	if (display_crc_ctl_parse_source(source_name, &source) < 0) {
> 		DRM_DEBUG_DRIVER("unknown source %s\n", source_name);
> 		return -EINVAL;
> 	}
> 
>+	has_ips = toggle_ips_and_test_active(intel_crtc, source);
>+	if (has_ips < 0)
>+		return has_ips;
>+
> 	power_domain = POWER_DOMAIN_PIPE(crtc->index);
>-	if (!intel_display_power_get_if_enabled(dev_priv, power_domain)) {
>-		DRM_DEBUG_KMS("Trying to capture CRC while pipe is off\n");
>-		return -EIO;
>-	}
>+	intel_display_power_get(dev_priv, power_domain);
> 
> 	ret = get_new_crc_ctl_reg(dev_priv, crtc->index, &source, &val);
> 	if (ret != 0)
> 		goto out;
> 
>-	if (source) {
>+	if (source && has_ips) {
> 		/*
> 		 * When IPS gets enabled, the pipe CRC changes. Since IPS gets
> 		 * enabled and disabled dynamically based on package C states,
>@@ -956,7 +989,8 @@ int intel_crtc_set_crc_source(struct drm_crtc *crtc, const char *source_name,
> 		else if (IS_HASWELL(dev_priv) && crtc->index == PIPE_A)
> 			hsw_trans_edp_pipe_A_crc_wa(dev_priv, false);
> 
>-		hsw_enable_ips(intel_crtc);
>+		if (has_ips)
>+			hsw_enable_ips(intel_crtc);
> 	}
> 
> 	pipe_crc->skipped = 0;

Actions: View | Splinter Review

Attachments on bug 101664: 133408 | 133422

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.