Re: [PATCH v2 3/3] drm/i915: gmch: fix stuck primary plane due to memory self-refresh mode

[Deepak S <deepak.s@linux.intel.com>]

On Friday 13 June 2014 05:24 PM, Imre Deak wrote:
> Blanking/unblanking the console in a loop on an Asus T100 sometimes
> leaves the console blank. After some digging I found that applying
>
> commit 61bc95c1fbbb6a08b55bbe161fdf1ea5493fc595
> Author: Egbert Eich <eich@suse.com>
> Date:   Mon Mar 4 09:24:38 2013 -0500
>
>      DRM/i915: On G45 enable cursor plane briefly after enabling the display plane.
>
> fixed VLV too.
>
> In my case the problem seemed to happen already during the previous crtc
> disabling and went away if I disabled self-refresh mode before disabling
> the primary plane.
>
> The root cause for this is that updates from the shadow to live plane
> control register are blocked at vblank time if the memory self-refresh
> mode (aka max-fifo mode on VLV) is active at that moment. The controller
> checks at frame start time if the CPU is in C0 and the self-refresh mode
> enable bit is set and if so activates self-reresh mode, otherwise
> deactivates it. So to make sure that the plane truly gets disabled before
> pipe-off we have to:
>
> 1. disable memory self-refresh mode
> 2. disable plane
> 3. wait for vblank
> 4. disable pipe
> 5. wait for pipe-off
>
> v2:
> - add explanation for the root cause from HW team (Cesar Mancini et al)
> - remove note about the CPU C7S state, in my latest tests disabling it
>    alone didn't make a difference
> - add vblank between disabling plane and pipe (Ville)
> - apply the same workaround for all gmch platforms (Ville)
>
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/intel_display.c | 15 +++++++++++++--
>   1 file changed, 13 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index b9251c8..5eb8afe 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -4799,6 +4799,16 @@ static void i9xx_crtc_disable(struct drm_crtc *crtc)
>   	if (IS_GEN2(dev))
>   		intel_set_cpu_fifo_underrun_reporting(dev, 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);
>   
>   	for_each_encoder_on_crtc(dev, crtc, encoder)
> @@ -4807,9 +4817,10 @@ static void i9xx_crtc_disable(struct drm_crtc *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.
> +	 * We also need to wait on all gmch platforms because of the
> +	 * self-refresh mode constraint explained above.
>   	 */
> -	if (IS_GEN2(dev))
> -		intel_wait_for_vblank(dev, pipe);
> +	intel_wait_for_vblank(dev, pipe);
>   
>   	intel_disable_pipe(dev_priv, pipe);
>   

Reviewed-by: Deepak S<deepak.s@linux.intel.com>