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

[Ville Syrjälä]

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

Yeah, I think we could just avoid using the HW counter whenever there's
a possibility of PSR being used on the crtc.

Assuming we still want to use the HW counter on crtcs that can't do PSR,
we're going to need some kind of per-crtc mechanism to tell drm_vblank.c
which method to use. hwmode.private_flags is one option. We already use
it for choosing between the scanline counter and hardware timestamps when
calculating the scanout position. But in this case the flag wouldn't be
exactly private since drm_vblank.c would have to know about it as well.
If that is deemed to be a problem, then we might just need to add a bool
to struct drm_vblank_crtc to indicate that the hw counter is good, or
maybe even move the dev->max_vblank_count to live inside
struct drm_vblank_crtc.

-- 
Ville Syrjälä
Intel OTC