On 07/11/17 11:50 AM, Ville Syrjälä 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? 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. Or just allow setting struct drm_vblank_crtc's get_vblank_counter member to NULL? -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer _______________________________________________ dri-devel mailing list dri-devel@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/dri-devel
