On Fri, Feb 19, 2021 at 04:08:09PM +0100, Daniel Vetter wrote: > On Thu, Feb 18, 2021 at 06:03:05PM +0200, Ville Syrjala wrote: > > From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > > > drm_vblank_restore() exists because certain power saving states > > can clobber the hardware frame counter. The way it does this is > > by guesstimating how many frames were missed purely based on > > the difference between the last stored timestamp vs. a newly > > sampled timestamp. > > > > If we should call this function before a full frame has > > elapsed since we sampled the last timestamp we would end up > > with a possibly slightly different timestamp value for the > > same frame. Currently we will happily overwrite the already > > stored timestamp for the frame with the new value. This > > could cause userspace to observe two different timestamps > > for the same frame (and the timestamp could even go > > backwards depending on how much error we introduce when > > correcting the timestamp based on the scanout position). > > > > To avoid that let's not update the stored timestamp at all, > > and instead we just fix up the last recorded hw vblank counter > > value such that the already stored timestamp/seq number will > > match. Thus the next time a vblank irq happens it will calculate > > the correct diff between the current and stored hw vblank counter > > values. > > > > Sidenote: Another possible idea that came to mind would be to > > do this correction only if the power really was removed since > > the last time we sampled the hw frame counter. But to do that > > we would need a robust way to detect when it has occurred. Some > > possibilities could involve some kind of hardare power well > > transition counter, or potentially we could store a magic value > > in a scratch register that lives in the same power well. But > > I'm not sure either of those exist, so would need an actual > > investigation to find out. All of that is very hardware specific > > of course, so would have to be done in the driver code. > > > > Cc: Dhinakaran Pandiyan <dhinakaran.pandiyan@xxxxxxxxx> > > Cc: Rodrigo Vivi <rodrigo.vivi@xxxxxxxxx> > > Cc: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Signed-off-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx> > > Reviewed-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > For testing, there's nothing else than hsw psr that needs this, or that's > just the box you have locally? Just the one I happen to have. Any machine with PSR should be able to hit this. But now that I refresh my memory I guess HSW/BDW don't actually fully reset the hw frame counter since they don't have the DC5/6 stuff. But even on HSW/BDW the frame counter would certainly stop while in PSR, so maintaining sensible vblank seq numbers will still require drm_vblank_restore(). Just my further idea of checking some power well counter/scratch register would not help in cases where DC states are not used. Instead we'd need some kind of PSR residency counter/etc. -- Ville Syrjälä Intel _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx
