On Fri, Jan 19, 2018 at 09:42:14PM +0000, Pandiyan, Dhinakaran wrote: > 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. Oh oh! Indeed. Now I remember you had told me this here. Can you please add a comment with this info somewhere so I don't ask the same question again ;) anyways: Reviewed-by: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > > > > > 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
