Re: [PATCH 5/5] drm/i915: Estimate and update missed vblanks.

[Rodrigo Vivi <rodrigo.vivi@intel.com>]

On Fri, Jan 12, 2018 at 09:57:07PM +0000, Dhinakaran Pandiyan wrote:
> The frame counter may have got reset between disabling and enabling vblank
> interrupts due to DMC putting the hardware to DC5/6 state if PSR was
> active. The frame counter also could have stalled if PSR is active in cases
> where there is no DMC. The frame counter resetting as a user visible impact
> of screen freezes. Use drm_vblank_restore() to compute missed vblanks
> in the duration for which vblank interrupts are disabled. There's no need
> particularly check if PSR was active in the interrupt disabled duration.
> 
> Enabling vblank interrupts wakes up the hardware from DC5/6 and prevents it
> from going back again as long as the there are pending interrupts. So, we
> don't have to explicity disallow DC5/6 after enabling vblank interrupts
> to keep the counter running.
> 
> Let's not apply this to CHV for now, as enabling interrupts does not
> prevent the hardware from activating PSR and thereby stalling the counter.
> 
> Cc: Rodrigo Vivi <rodrigo.vivi@intel.com>
> Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@intel.com>
> ---
>  drivers/gpu/drm/i915/i915_irq.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/i915_irq.c b/drivers/gpu/drm/i915/i915_irq.c
> index 3517c6548e2c..db3466ec6faa 100644
> --- a/drivers/gpu/drm/i915/i915_irq.c
> +++ b/drivers/gpu/drm/i915/i915_irq.c
> @@ -2956,6 +2956,9 @@ static int ironlake_enable_vblank(struct drm_device *dev, unsigned int pipe)
>  	ilk_enable_display_irq(dev_priv, bit);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
> +	if (HAS_PSR(dev_priv))
> +		drm_vblank_restore(dev, pipe);
> +

I don't believe we need this one here.

pre-gen9 platform has psr but not dmc, so the case
where we need to restore the counter doesn't exist.

>  	return 0;
>  }
>  
> @@ -2968,6 +2971,9 @@ static int gen8_enable_vblank(struct drm_device *dev, unsigned int pipe)
>  	bdw_enable_pipe_irq(dev_priv, pipe, GEN8_PIPE_VBLANK);
>  	spin_unlock_irqrestore(&dev_priv->irq_lock, irqflags);
>  
> +	if (HAS_PSR(dev_priv))

HAS_PSR(dev_priv) && HAS_CSR(dev_priv)
maybe?
So it gets clear that it is not because PSR that we need to restore
the counter, but also don't do it when not needed.

> +		drm_vblank_restore(dev, pipe);
> +
>  	return 0;
>  }
>  
> -- 
> 2.11.0
> 
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel