Re: [PATCH 1/3] drm/i915: Restore vblank interrupts earlier

From: Daniel Vetter <daniel@ffwll.ch>
To: Ville Syrjala <ville.syrjala@linux.intel.com>
Cc: dri-devel@lists.freedesktop.org, intel-gfx@lists.freedesktop.org,
	Dennis <dennis.nezic@utoronto.ca>,
	stable@vger.kernel.org
Subject: Re: [PATCH 1/3] drm/i915: Restore vblank interrupts earlier
Date: Tue, 2 Oct 2018 10:48:36 +0200	[thread overview]
Message-ID: <20181002084836.GM11082@phenom.ffwll.local> (raw)
In-Reply-To: <20181001142909.5567-1-ville.syrjala@linux.intel.com>

On Mon, Oct 01, 2018 at 05:29:07PM +0300, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
> 
> Plane sanitation needs vblank interrupts (on account of CxSR disable).
> So let's restore vblank interrupts earlier.
> 
> v2: Make it actually build
> 
> Cc: stable@vger.kernel.org
> Cc: Dennis <dennis.nezic@utoronto.ca>
> Tested-by: Dennis <dennis.nezic@utoronto.ca>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=105637
> Fixes: b1e01595a66d ("drm/i915: Redo plane sanitation during readout")
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>
> ---
>  drivers/gpu/drm/i915/intel_display.c | 19 +++++++++----------
>  1 file changed, 9 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_display.c b/drivers/gpu/drm/i915/intel_display.c
> index 4c5c2b39e65c..e018b37bed39 100644
> --- a/drivers/gpu/drm/i915/intel_display.c
> +++ b/drivers/gpu/drm/i915/intel_display.c
> @@ -15551,13 +15551,9 @@ static void intel_sanitize_crtc(struct intel_crtc *crtc,
>  			   I915_READ(reg) & ~PIPECONF_FRAME_START_DELAY_MASK);
>  	}
>  
> -	/* restore vblank interrupts to correct state */
> -	drm_crtc_vblank_reset(&crtc->base);
>  	if (crtc->active) {
>  		struct intel_plane *plane;
>  
> -		drm_crtc_vblank_on(&crtc->base);
> -
>  		/* Disable everything but the primary plane */
>  		for_each_intel_plane_on_crtc(dev, crtc, plane) {
>  			const struct intel_plane_state *plane_state =
> @@ -15899,7 +15895,6 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  			     struct drm_modeset_acquire_ctx *ctx)
>  {
>  	struct drm_i915_private *dev_priv = to_i915(dev);
> -	enum pipe pipe;
>  	struct intel_crtc *crtc;
>  	struct intel_encoder *encoder;
>  	int i;
> @@ -15912,15 +15907,19 @@ intel_modeset_setup_hw_state(struct drm_device *dev,
>  	/* HW state is read out, now we need to sanitize this mess. */
>  	get_encoder_power_domains(dev_priv);
>  
> -	intel_sanitize_plane_mapping(dev_priv);

If you add a comment here why we need to do this, this has my

Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
> +		drm_crtc_vblank_reset(&crtc->base);
>  
> -	for_each_intel_encoder(dev, encoder) {
> -		intel_sanitize_encoder(encoder);
> +		if (crtc->active)
> +			drm_crtc_vblank_on(&crtc->base);
>  	}
>  
> -	for_each_pipe(dev_priv, pipe) {
> -		crtc = intel_get_crtc_for_pipe(dev_priv, pipe);
> +	intel_sanitize_plane_mapping(dev_priv);
>  
> +	for_each_intel_encoder(dev, encoder)
> +		intel_sanitize_encoder(encoder);
> +
> +	for_each_intel_crtc(&dev_priv->drm, crtc) {
>  		intel_sanitize_crtc(crtc, ctx);
>  		intel_dump_pipe_config(crtc, crtc->config,
>  				       "[setup_hw_state]");
> -- 
> 2.16.4
> 
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch