[Intel-gfx] [RFC PATCH] drm/vblanks: Deal with HW vblank counter resets.

[Pandiyan, Dhinakaran]

On Tue, 2017-11-07 at 13:39 +0100, Daniel Vetter wrote:
> On Tue, Nov 07, 2017 at 10:47:00AM +0100, Michel Dänzer wrote:
> > On 07/11/17 07:26 AM, Dhinakaran Pandiyan wrote:
> > > Some HW vblank counters reset due to power management events, which messes
> > > up the vblank counting logic. This leads to screen freezes with user space
> > > waiting on vblank events that may not occur if the counter keeps resetting.
> > > 
> > > For e.g., After the HW vblank counter resets
> > > [    9.007359] [drm:drm_update_vblank_count [drm]] updating vblank count
> > > on crtc 0: current=297, diff=4294965389, hw=5 hw_last=1912
> > > 
> > > So, fall back to the SW counter, computed using  vblank timestamps
> > > and frame duration, when the HW counter value deviates by 50% of the SW
> > > computed value.
> > > 
> > > I have tested this patch on my SKL laptop with i915.enable_psr=1 and it
> > > *seems* to solve the screen freeze issue seen with PSR when DMC is loaded.
> > > 
> > > Known issues:
> > > 1) The 50% deviation margin is arbitrary.
> > > 2) "Redundant vblirq ignored" messages are more frequent.
> > > 
> > > I am sending this as an RFC to get feedback on whether the fall back
> > > approach is sane and if it should be implemented in the core.
> > 
> > Is there no way for the driver to know under which circumstances the
> > reset to 0 might happen? 

Not precisely, but the driver does need to allow the firmware to go into
lower power states.

> If there is, maybe it could be solved by
> > calling drm_crtc_vblank_off() before it might happen and
> > drm_crtc_vblank_on() after it might have happened.
> > 
> > Otherwise, might it be better not to use the HW counter at all when it's
> > known not to be reliable?
> 
> We know when it happens, so agreed this isn't a good/workable solution
> really. I thought the plan to fix that was to fix up our runtime pm to
> make sure the vblank counter doesn't get reset while we need it (pending
> flip or vblank). And in-between (when the vblank counter is totally off)
> we'd fix any mismatch by adjusting the sw vblank counter with an explicit
> call (where we can use the elapsed time to estimate the elapsed vblank
> counts well enough). Adding a magic hack like this doesn't sound like a
> good plan to me indeed.
> -Daniel

Thanks for the feedback, I wasn't aware of an agreed plan. I just went
through patchwork history now and realized that the full-time SW counter
approach was tried earlier, but not much information on why it was
abandoned.

Allowing runtime PM (PSR + DMC) to do it's job while letting the SW
estimate vblanks if the hardware counter is unreliable didn't seem like
a terrible plan. It could save more power fwiw. But, I understand your
concerns, will see what I can do.

-DK