From: Jesse B. <jb...@vi...> - 2009-02-17 18:34:25
|
On Tuesday, February 17, 2009 9:43 am Michel Dänzer wrote: > On Tue, 2009-02-17 at 09:27 -0800, Jesse Barnes wrote: > > On Tuesday, February 17, 2009 9:04 am Michel Dänzer wrote: > > > On Mon, 2009-02-16 at 10:42 -0800, Jesse Barnes wrote: > > > > On Sunday, February 15, 2009 11:33 pm Michel Dänzer wrote: > > > > > On Fri, 2009-02-13 at 10:27 -0800, Jesse Barnes wrote: > > > > > > Recall our last discussion where I outlined the cases we'd have > > > > > > to deal with in the modeset ioctl if we didn't use get/put to > > > > > > just keep interrupts on around the calls: > > > > > > > > > > But we are intending to keep them on around the calls. So the > > > > > problem is that you are disabling the IRQ in between? Maybe a > > > > > solution could be not to mess with the counter when > > > > > disabling/enabling the IRQ. It needs to be guarded by the modeset > > > > > ioctl anyway, so shouldn't that work? > > > > > > > > The current problem isn't a disable between modeset ioctls, it's a > > > > disable followed by a counter reset of any kind (modeset ioctls or > > > > not), since the "last" count we track is just done in the disable > > > > function, not in the modeset ioctl. Doing in in the modeset ioctl > > > > instead may be possible, but as I said there are lots of cases to > > > > deal with. > > > > > > Isn't the actual problem that drm_irq_uninstall() updates last_vblank? > > > If it didn't (and the modeset ioctl is properly called around the > > > disabling and re-enabling of the IRQ), wouldn't it just work? > > > > No, this happens w/o an uninstall, it's just due to wraparound being > > added between the disable timer & the next drm_vblank_get. > > I still don't understand the problem then; this can only happen if there > actually was a wraparound, as the drm_vblank_get must happen (via the > modeset ioctl) before the hardware frame counter resets for any other > reason. Maybe I'm misunderstanding the problem, lemme walk through the logic: - we're running along, and at some point the vblank disable timer fires -> last_vblank = current counter (23252 for example) - at some later point we do a DPMS or mode set, pre-modeset is called: - call drm_vblank_get - enable vblank events - get current counter (should be >23252 if called correctly) - diff = cur - last (some small number) - diff is added - post mode set - just put (re-enable the timer) and clear the modeset flag - another DPMS event comes in before the timer fires - modeset flag is clear so it goes through the normal path - call drm_vblank_get - enable events - get current counter (will be small this time) - diff = cur - last (some large number, and cur < last) - wraparound value + diff is added - post mode set - just put again So now we have a counter that jumped by at least the wraparound value. We have some code in the 2D driver to avoid calling into the modeset ioctl if the display is already off, but it won't catch two DPMS on->off->on-off cycles in a row that happen before the disable timer fires and last_vblank is updated again. This might happen more often on Intel than radeon since output probing tends to turn things off & on, but you should be able to see the same jumpiness by doing a few, quick modesets after the timer fires. -- Jesse Barnes, Intel Open Source Technology Center |