Re: [PATCH 2/2] drm/i915: Skip load detect when intel_crtc->new_enable==true

[Daniel Vetter <daniel@ffwll.ch>]

On Mon, Aug 11, 2014 at 01:15:36PM +0300, ville.syrjala@linux.intel.com wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> During suspend we turn off the crtcs, but leave the staged config in
> place so that we can restore the display(s) to their previous state on
> resume.
> 
> During resume when we attempt to apply the force pipe A quirk we use the
> load detect mechanism. That doesn't check whether there was an already
> staged configuration for the crtc since that's not even possible during
> normal runtime load detection. But during resume it is possible, and if
> we just blindly go and overwrite the staged crtc configuration for the
> load detection we can no longer restore the display to the correct
> state.
> 
> Even worse, we don't even clear all the staged connector->encoder->crtc
> links so we may end up using a cloned setup for the load detection, and
> after we're done we just clear the links related to the VGA output
> leaving the links for the other outputs in place. This will eventually
> result in calling intel_set_mode() with mode==NULL but with valid
> connector->encoder->crtc links which will result in dereferencing the
> NULL mode since the code thinks it will have to a modeset.
> 
> To avoid these problems don't use any crtc with new_enabled==true for
> load detection.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/i915/intel_display.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 7953b46..ec12e31d1 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -8478,10 +8478,14 @@ retry:
>  		i++;
>  		if (!(encoder->possible_crtcs & (1 << i)))
>  			continue;
> -		if (!possible_crtc->enabled) {
> -			crtc = possible_crtc;
> -			break;
> -		}
> +		if (possible_crtc->enabled)
> +			continue;
> +		/* This can occur when applying the pipe A quirk on resume. */
> +		if (to_intel_crtc(possible_crtc)->new_enabled)
> +			continue;
> +
> +		crtc = possible_crtc;
> +		break;
>  	}
>  
>  	/*
> -- 
> 1.8.5.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/intel-gfx

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch