[Intel-gfx] [PATCH 5/5] drm/i915: Estimate and update missed vblanks.

[Pandiyan, Dhinakaran]

On Thu, 2018-01-18 at 23:26 -0800, Rodrigo Vivi wrote:
> 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 at intel.com>
> > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan at 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.

Even without DMC, counter should be stuck when PSR is active as no
frames are generated by the pipe. I am using drm_vblank_restore_count()
to take care of that.

> 
> >  	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.

Same reason as above, let me test this again by disabling DMC.

> 
> > +		drm_vblank_restore(dev, pipe);
> > +
> >  	return 0;
> >  }
> >  
> > -- 
> > 2.11.0
> > 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx