Re: [PATCH 07/10] drm/i915: Try to make sure cxsr is disabled around plane enable/disable

[Clint Taylor <clinton.a.taylor@intel.com>]

On 06/24/2015 12:00 PM, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> CxSR (or maxfifo on VLV/CHV) blocks somne changes to the plane control
> register (enable bit at least, not quite sure about the rest). So in
> order to have the plane enable/disable when we want we need to first
> kick the hardware out of cxsr.
>
> Unfortunateloy this requires some extra vblank waits. For the CxSR
> enable after the plane update we should eventually use an async
> vblank worker, but since we don't have that just do sync vblank
> waits. For the disable case we have no choice but to do it
> synchronously.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 36 +++++++++++++++++++++++++++++++-----
>   drivers/gpu/drm/i915/intel_drv.h     |  3 +++
>   drivers/gpu/drm/i915/intel_pm.c      | 11 ++++-------
>   3 files changed, 38 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index d67b5f1..19aedf9 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4716,6 +4716,9 @@ static void intel_post_plane_update(struct intel_crtc *crtc)
>
>   	intel_frontbuffer_flip(dev, atomic->fb_bits);
>
> +	if (atomic->disable_cxsr)
> +		crtc->wm.cxsr_allowed = true;
> +
>   	if (crtc->atomic.update_wm_post)
>   		intel_update_watermarks(&crtc->base);
>
> @@ -4765,6 +4768,11 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
>
>   	if (atomic->pre_disable_primary)
>   		intel_pre_disable_primary(&crtc->base);
> +
> +	if (atomic->disable_cxsr) {
> +		crtc->wm.cxsr_allowed = false;
> +		intel_set_memory_cxsr(dev_priv, false);
> +	}
>   }
>
>   static void intel_crtc_disable_planes(struct drm_crtc *crtc, unsigned plane_mask)
> @@ -11646,12 +11654,26 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>   			 plane->base.id, was_visible, visible,
>   			 turn_off, turn_on, mode_changed);
>
> -	if (turn_on)
> +	if (turn_on) {
>   		intel_crtc->atomic.update_wm_pre = true;
> -	else if (turn_off)
> +		/* must disable cxsr around plane enable/disable */
> +		if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> +			intel_crtc->atomic.disable_cxsr = true;
> +			/* to potentially re-enable cxsr */
> +			intel_crtc->atomic.wait_vblank = true;
> +			intel_crtc->atomic.update_wm_post = true;
> +		}
> +	} else if (turn_off) {
>   		intel_crtc->atomic.update_wm_post = true;
> -	else if (intel_wm_need_update(plane, plane_state))
> +		/* must disable cxsr around plane enable/disable */
> +		if (plane->type != DRM_PLANE_TYPE_CURSOR) {
> +			if (is_crtc_enabled)
> +				intel_crtc->atomic.wait_vblank = true;
> +			intel_crtc->atomic.disable_cxsr = true;
> +		}
> +	} else if (intel_wm_need_update(plane, plane_state)) {
>   		intel_crtc->atomic.update_wm_pre = true;
> +	}
>
>   	if (visible)
>   		intel_crtc->atomic.fb_bits |=
> @@ -11808,8 +11830,8 @@ static int intel_crtc_atomic_check(struct drm_crtc *crtc,
>   	if (pipe_config->quirks & PIPE_CONFIG_QUIRK_INITIAL_PLANES)
>   		intel_crtc_check_initial_planes(crtc, crtc_state);
>
> -	if (mode_changed)
> -		intel_crtc->atomic.update_wm_post = !crtc_state->active;
> +	if (mode_changed && !crtc_state->active)
> +		intel_crtc->atomic.update_wm_post = true;
>
>   	if (mode_changed && crtc_state->enable &&
>   	    dev_priv->display.crtc_compute_clock &&
> @@ -13129,6 +13151,8 @@ static int __intel_set_mode(struct drm_atomic_state *state)
>   		if (!needs_modeset(crtc->state))
>   			continue;
>
> +		intel_pre_plane_update(intel_crtc);
> +
>   		any_ms = true;
>   		intel_pre_plane_update(intel_crtc);
>
> @@ -14089,6 +14113,8 @@ static void intel_crtc_init(struct drm_device *dev, int pipe)
>   	intel_crtc->cursor_cntl = ~0;
>   	intel_crtc->cursor_size = ~0;
>
> +	intel_crtc->wm.cxsr_allowed = true;
> +
>   	BUG_ON(pipe >= ARRAY_SIZE(dev_priv->plane_to_crtc_mapping) ||
>   	       dev_priv->plane_to_crtc_mapping[intel_crtc->plane] != NULL);
>   	dev_priv->plane_to_crtc_mapping[intel_crtc->plane] = &intel_crtc->base;
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index f26a680..4e8d13e 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -507,6 +507,7 @@ struct intel_crtc_atomic_commit {
>   	/* Sleepable operations to perform before commit */
>   	bool wait_for_flips;
>   	bool disable_fbc;
> +	bool disable_cxsr;
>   	bool pre_disable_primary;
>   	bool update_wm_pre, update_wm_post;
>   	unsigned disabled_planes;
> @@ -565,6 +566,8 @@ struct intel_crtc {
>   		struct intel_pipe_wm active;
>   		/* SKL wm values currently in use */
>   		struct skl_pipe_wm skl_active;
> +		/* allow CxSR on this pipe */
> +		bool cxsr_allowed;
>   	} wm;
>
>   	int scanline_offset;
> diff --git a/drivers/gpu/drm/i915/intel_pm.c b/drivers/gpu/drm/i915/intel_pm.c
> index c7c90ce..b65817d 100644
> --- a/drivers/gpu/drm/i915/intel_pm.c
> +++ b/drivers/gpu/drm/i915/intel_pm.c
> @@ -335,6 +335,7 @@ void intel_set_memory_cxsr(struct drm_i915_private *dev_priv, bool enable)
>   	if (IS_VALLEYVIEW(dev)) {
>   		I915_WRITE(FW_BLC_SELF_VLV, enable ? FW_CSPWRDWNEN : 0);
>   		POSTING_READ(FW_BLC_SELF_VLV);
> +		dev_priv->wm.vlv.cxsr = enable;
>   	} else if (IS_G4X(dev) || IS_CRESTLINE(dev)) {
>   		I915_WRITE(FW_BLC_SELF, enable ? FW_BLC_SELF_EN : 0);
>   		POSTING_READ(FW_BLC_SELF);
> @@ -1116,7 +1117,7 @@ static void vlv_compute_wm(struct intel_crtc *crtc)
>
>   	memset(wm_state, 0, sizeof(*wm_state));
>
> -	wm_state->cxsr = crtc->pipe != PIPE_C;
> +	wm_state->cxsr = crtc->pipe != PIPE_C && crtc->wm.cxsr_allowed;
>   	if (IS_CHERRYVIEW(dev))
>   		wm_state->num_levels = CHV_WM_NUM_LEVELS;
>   	else
> @@ -1369,10 +1370,8 @@ static void vlv_update_wm(struct drm_crtc *crtc)
>   	    dev_priv->wm.vlv.level >= VLV_WM_LEVEL_PM5)
>   		chv_set_memory_pm5(dev_priv, false);
>
> -	if (!wm.cxsr && dev_priv->wm.vlv.cxsr) {
> +	if (!wm.cxsr && dev_priv->wm.vlv.cxsr)
>   		intel_set_memory_cxsr(dev_priv, false);
> -		intel_wait_for_vblank(dev, pipe);
> -	}
>
>   	/* FIXME should be part of crtc atomic commit */
>   	vlv_pipe_set_fifo_size(intel_crtc);
> @@ -1385,10 +1384,8 @@ static void vlv_update_wm(struct drm_crtc *crtc)
>   		      wm.pipe[pipe].sprite[0], wm.pipe[pipe].sprite[1],
>   		      wm.sr.plane, wm.sr.cursor, wm.level, wm.cxsr);
>
> -	if (wm.cxsr && !dev_priv->wm.vlv.cxsr) {
> -		intel_wait_for_vblank(dev, pipe);
> +	if (wm.cxsr && !dev_priv->wm.vlv.cxsr)
>   		intel_set_memory_cxsr(dev_priv, true);
> -	}
>
>   	if (wm.level >= VLV_WM_LEVEL_PM5 &&
>   	    dev_priv->wm.vlv.level < VLV_WM_LEVEL_PM5)
>

Reviewed-by: Clint Taylor <Clinton.A.Taylor@intel.com>
Tested-by: Clint Taylor <Clinton.A.Taylor@intel.com>

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