[Intel-gfx] [PATCH 6/7] drm/i915: Perform primary enable/disable atomically with sprite updates

Jesse Barnes jbarnes at virtuousgeek.org
Thu Dec 12 22:06:01 CET 2013


On Thu, 17 Oct 2013 22:53:18 +0300
ville.syrjala at linux.intel.com wrote:

> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> Move the primary plane enable/disable to occur atomically with the
> sprite update that caused the primary plane visibility to change.
> 
> FBC and IPS enable/disable is left to happen well before or after
> the primary plane change.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 96 ++++++++++++++++++++++---------------
>  1 file changed, 57 insertions(+), 39 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index ce4e4ec..f871b8f 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -85,6 +85,19 @@ static void intel_pipe_update_end(struct drm_crtc *crtc)
>  	local_irq_enable();
>  }
>  
> +static void intel_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);
> +	int reg = DSPCNTR(intel_crtc->plane);
> +
> +	if (intel_crtc->primary_enabled)
> +		I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
> +	else
> +		I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
> +}
> +
>  static void
>  vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  		 struct drm_framebuffer *fb,
> @@ -175,6 +188,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  
>  	intel_pipe_update_start(crtc);
>  
> +	intel_update_primary_plane(crtc);
> +
>  	I915_WRITE(SPSTRIDE(pipe, plane), fb->pitches[0]);
>  	I915_WRITE(SPPOS(pipe, plane), (crtc_y << 16) | crtc_x);
>  
> @@ -187,7 +202,8 @@ vlv_update_plane(struct drm_plane *dplane, struct drm_crtc *crtc,
>  	I915_WRITE(SPCNTR(pipe, plane), sprctl);
>  	I915_MODIFY_DISPBASE(SPSURF(pipe, plane), i915_gem_obj_ggtt_offset(obj) +
>  			     sprsurf_offset);
> -	POSTING_READ(SPSURF(pipe, plane));
> +
> +	intel_flush_primary_plane(dev_priv, to_intel_crtc(crtc)->plane);
>  
>  	intel_pipe_update_end(crtc);
>  }
> @@ -203,11 +219,14 @@ vlv_disable_plane(struct drm_plane *dplane, struct drm_crtc *crtc)
>  
>  	intel_pipe_update_start(crtc);
>  
> +	intel_update_primary_plane(crtc);
> +
>  	I915_WRITE(SPCNTR(pipe, plane), I915_READ(SPCNTR(pipe, plane)) &
>  		   ~SP_ENABLE);
>  	/* Activate double buffered register update */
>  	I915_MODIFY_DISPBASE(SPSURF(pipe, plane), 0);
> -	POSTING_READ(SPSURF(pipe, plane));
> +
> +	intel_flush_primary_plane(dev_priv, to_intel_crtc(crtc)->plane);
>  
>  	intel_pipe_update_end(crtc);
>  
> @@ -359,6 +378,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	intel_pipe_update_start(crtc);
>  
> +	intel_update_primary_plane(crtc);
> +
>  	I915_WRITE(SPRSTRIDE(pipe), fb->pitches[0]);
>  	I915_WRITE(SPRPOS(pipe), (crtc_y << 16) | crtc_x);
>  
> @@ -377,7 +398,8 @@ ivb_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	I915_WRITE(SPRCTL(pipe), sprctl);
>  	I915_MODIFY_DISPBASE(SPRSURF(pipe),
>  			     i915_gem_obj_ggtt_offset(obj) + sprsurf_offset);
> -	POSTING_READ(SPRSURF(pipe));
> +
> +	intel_flush_primary_plane(dev_priv, to_intel_crtc(crtc)->plane);
>  
>  	intel_pipe_update_end(crtc);
>  
> @@ -397,13 +419,16 @@ ivb_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>  
>  	intel_pipe_update_start(crtc);
>  
> +	intel_update_primary_plane(crtc);
> +
>  	I915_WRITE(SPRCTL(pipe), I915_READ(SPRCTL(pipe)) & ~SPRITE_ENABLE);
>  	/* Can't leave the scaler enabled... */
>  	if (intel_plane->can_scale)
>  		I915_WRITE(SPRSCALE(pipe), 0);
>  	/* Activate double buffered register update */
>  	I915_MODIFY_DISPBASE(SPRSURF(pipe), 0);
> -	POSTING_READ(SPRSURF(pipe));
> +
> +	intel_flush_primary_plane(dev_priv, to_intel_crtc(crtc)->plane);
>  
>  	intel_pipe_update_end(crtc);
>  
> @@ -545,6 +570,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  
>  	intel_pipe_update_start(crtc);
>  
> +	intel_update_primary_plane(crtc);
> +
>  	I915_WRITE(DVSSTRIDE(pipe), fb->pitches[0]);
>  	I915_WRITE(DVSPOS(pipe), (crtc_y << 16) | crtc_x);
>  
> @@ -558,7 +585,8 @@ ilk_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	I915_WRITE(DVSCNTR(pipe), dvscntr);
>  	I915_MODIFY_DISPBASE(DVSSURF(pipe),
>  			     i915_gem_obj_ggtt_offset(obj) + dvssurf_offset);
> -	POSTING_READ(DVSSURF(pipe));
> +
> +	intel_flush_primary_plane(dev_priv, to_intel_crtc(crtc)->plane);
>  
>  	intel_pipe_update_end(crtc);
>  }
> @@ -573,12 +601,15 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>  
>  	intel_pipe_update_start(crtc);
>  
> +	intel_update_primary_plane(crtc);
> +
>  	I915_WRITE(DVSCNTR(pipe), I915_READ(DVSCNTR(pipe)) & ~DVS_ENABLE);
>  	/* Disable the scaler */
>  	I915_WRITE(DVSSCALE(pipe), 0);
>  	/* Flush double buffered register updates */
>  	I915_MODIFY_DISPBASE(DVSSURF(pipe), 0);
> -	POSTING_READ(DVSSURF(pipe));
> +
> +	intel_flush_primary_plane(dev_priv, to_intel_crtc(crtc)->plane);
>  
>  	intel_pipe_update_end(crtc);
>  
> @@ -586,20 +617,10 @@ ilk_disable_plane(struct drm_plane *plane, struct drm_crtc *crtc)
>  }
>  
>  static void
> -intel_enable_primary(struct drm_crtc *crtc)
> +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 reg = DSPCNTR(intel_crtc->plane);
> -
> -	if (intel_crtc->primary_enabled)
> -		return;
> -
> -	intel_crtc->primary_enabled = true;
> -
> -	I915_WRITE(reg, I915_READ(reg) | DISPLAY_PLANE_ENABLE);
> -	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
>  
>  	/*
>  	 * FIXME IPS should be fine as long as one plane is
> @@ -618,17 +639,11 @@ intel_enable_primary(struct drm_crtc *crtc)
>  }
>  
>  static void
> -intel_disable_primary(struct drm_crtc *crtc)
> +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);
> -	int reg = DSPCNTR(intel_crtc->plane);
> -
> -	if (!intel_crtc->primary_enabled)
> -		return;
> -
> -	intel_crtc->primary_enabled = false;
>  
>  	mutex_lock(&dev->struct_mutex);
>  	if (dev_priv->fbc.plane == intel_crtc->plane)
> @@ -642,9 +657,6 @@ intel_disable_primary(struct drm_crtc *crtc)
>  	 * versa.
>  	 */
>  	hsw_disable_ips(intel_crtc);
> -
> -	I915_WRITE(reg, I915_READ(reg) & ~DISPLAY_PLANE_ENABLE);
> -	intel_flush_primary_plane(dev_priv, intel_crtc->plane);
>  }
>  
>  static int
> @@ -738,7 +750,7 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	struct drm_i915_gem_object *obj = intel_fb->obj;
>  	struct drm_i915_gem_object *old_obj = intel_plane->obj;
>  	int ret;
> -	bool disable_primary = false;
> +	bool primary_enabled;
>  	bool visible;
>  	int hscale, vscale;
>  	int max_scale, min_scale;
> @@ -909,8 +921,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	 * If the sprite is completely covering the primary plane,
>  	 * we can disable the primary and save power.
>  	 */
> -	disable_primary = drm_rect_equals(&dst, &clip) && !colorkey_enabled(intel_plane);
> -	WARN_ON(disable_primary && !visible && intel_crtc->active);
> +	primary_enabled = !drm_rect_equals(&dst, &clip) || colorkey_enabled(intel_plane);
> +	WARN_ON(!primary_enabled && !visible && intel_crtc->active);
>  
>  	mutex_lock(&dev->struct_mutex);
>  
> @@ -937,12 +949,12 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  	intel_plane->obj = obj;
>  
>  	if (intel_crtc->active) {
> -		/*
> -		 * Be sure to re-enable the primary before the sprite is no longer
> -		 * covering it fully.
> -		 */
> -		if (!disable_primary)
> -			intel_enable_primary(crtc);
> +		bool primary_was_enabled = intel_crtc->primary_enabled;
> +
> +		intel_crtc->primary_enabled = primary_enabled;
> +
> +		if (primary_was_enabled && !primary_enabled)
> +			intel_pre_disable_primary(crtc);
>  
>  		if (visible)
>  			intel_plane->update_plane(plane, crtc, fb, obj,
> @@ -951,8 +963,8 @@ intel_update_plane(struct drm_plane *plane, struct drm_crtc *crtc,
>  		else
>  			intel_plane->disable_plane(plane, crtc);
>  
> -		if (disable_primary)
> -			intel_disable_primary(crtc);
> +		if (!primary_was_enabled && primary_enabled)
> +			intel_post_enable_primary(crtc);
>  	}
>  
>  	/* Unpin old obj after new one is active to avoid ugliness */
> @@ -990,8 +1002,14 @@ intel_disable_plane(struct drm_plane *plane)
>  	intel_crtc = to_intel_crtc(plane->crtc);
>  
>  	if (intel_crtc->active) {
> -		intel_enable_primary(plane->crtc);
> +		bool primary_was_enabled = intel_crtc->primary_enabled;
> +
> +		intel_crtc->primary_enabled = true;
> +
>  		intel_plane->disable_plane(plane, plane->crtc);
> +
> +		if (!primary_was_enabled && intel_crtc->primary_enabled)
> +			intel_post_enable_primary(plane->crtc);
>  	}
>  
>  	if (intel_plane->obj) {

Reviewed-by: Jesse Barnes <jbarnes at virtuousgeek.org>

-- 
Jesse Barnes, Intel Open Source Technology Center



More information about the Intel-gfx mailing list