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@xxxxxxxxx> > > Signed-off-by: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > > --- > > 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@xxxxxxxxxxxxxxxxxxxxx > https://lists.freedesktop.org/mailman/listinfo/intel-gfx _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
