Re: [PATCH v2 3/9] drm/i915: Remove intel_crtc->atomic.disable_ips.

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Mon, Jan 11, 2016 at 01:27:43PM +0100, Maarten Lankhorst wrote:
> This is a revert of commit 066cf55b9ce3 "drm/i915: Fix IPS related flicker".
> intel_pre_disable_primary already handles this, and now everything
> goes through the atomic path there's no need to try to disable ips twice.

If it's a revert why wasn't it done with 'git revert'?

Also I'm not sure it isn't a step backwards. Based on the spec we should
be able to keep IPS enabled as long as one plane (possibly referring to 
either primary or sprite) is enabled on the pipe. So in theory we should
keep the IPS handling out of the primary plane code, and instead we
should compute the IPS state based on the set of active planes on the
pipe.

> 
> Signed-off-by: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
> Reviewed-by: Ander Conselvan de Oliveira <conselvan2@gmail.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 16 +---------------
>  drivers/gpu/drm/i915/intel_drv.h     |  1 -
>  2 files changed, 1 insertion(+), 16 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index eac73f005a70..62044ad5c6ec 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4823,9 +4823,6 @@ static void intel_pre_plane_update(struct intel_crtc *crtc)
>  	if (atomic->disable_fbc)
>  		intel_fbc_deactivate(crtc);
>  
> -	if (crtc->atomic.disable_ips)
> -		hsw_disable_ips(crtc);
> -
>  	if (atomic->pre_disable_primary)
>  		intel_pre_disable_primary(&crtc->base);
>  
> @@ -11907,19 +11904,8 @@ int intel_plane_atomic_calc_changes(struct drm_crtc_state *crtc_state,
>  		intel_crtc->atomic.pre_disable_primary = turn_off;
>  		intel_crtc->atomic.post_enable_primary = turn_on;
>  
> -		if (turn_off) {
> -			/*
> -			 * FIXME: Actually if we will still have any other
> -			 * plane enabled on the pipe we could let IPS enabled
> -			 * still, but for now lets consider that when we make
> -			 * primary invisible by setting DSPCNTR to 0 on
> -			 * update_primary_plane function IPS needs to be
> -			 * disable.
> -			 */
> -			intel_crtc->atomic.disable_ips = true;
> -
> +		if (turn_off)
>  			intel_crtc->atomic.disable_fbc = true;
> -		}
>  
>  		/*
>  		 * FBC does not work on some platforms for rotated
> diff --git a/drivers/gpu/drm/i915/intel_drv.h b/drivers/gpu/drm/i915/intel_drv.h
> index 8940aa4cf50c..39adf7cd0b36 100644
> --- a/drivers/gpu/drm/i915/intel_drv.h
> +++ b/drivers/gpu/drm/i915/intel_drv.h
> @@ -566,7 +566,6 @@ struct intel_mmio_flip {
>  struct intel_crtc_atomic_commit {
>  	/* Sleepable operations to perform before commit */
>  	bool disable_fbc;
> -	bool disable_ips;
>  	bool pre_disable_primary;
>  
>  	/* Sleepable operations to perform after commit */
> -- 
> 2.1.0
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Ville Syrjälä
Intel OTC
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/intel-gfx