> Date: Sun, 19 Aug 2018 13:08:15 -0400 > From: Philippe Meunier <[email protected]> > > Philippe Meunier wrote: > >Yes, the delay is okay. The problem is that when "cold" is 1, the vblank > >counter never changes during a call to drm_wait_one_vblank, so the > >condition inside __wait_event_intr_timeout is never true and there is a > >timeout. In fact the vblank counter does not even change across multiple > >calls to drm_wait_one_vblank, unless there is in-between a call to > >vblank_disable_and_save followed by a call to drm_vblank_enable. > > I looked at that vblank stuff more in details: > > - If you search in the (very long) debugging output below, you'll see that > the first call to drm_handle_vblank, which is the callback into the DRM > code when there's a vblank interrupt, appears long after the first call > to intel_tv_detect_type, which is the function call that triggers the > wrong console resolution problem on my computer. > So vblank interrupts, which change vblank->count (which is what the > condition inside __wait_event_intr_timeout depends on), start happening > too late to help solve the console resolution problem. > > - There's also a hardware vblank counter which is used inside > drm_update_vblank_count to change vblank->count too, to simulate the > ticking of vblank interrupts in the early stage of the DRM code before > the interrupts actually start being handled, but drm_update_vblank_count > is only called from the vblank interrupt handler drm_handle_vblank, so > too late for our purpose (see the previous paragraph) or from > vblank_disable_and_save / drm_vblank_enable, which is at the wrong time > (see the quote from my previous email at the top). > > Conclusion: the condition inside __wait_event_intr_timeout, which depends > on vblank->count changing, can never be true when intel_tv_detect_type is > called for the first time. > > I think it would be possible to change the condition "last != > drm_vblank_count(dev, pipe)" given by drm_wait_one_vblank as argument to > __wait_event_intr_timeout (through wait_event_timeout) to use the hardware > vblank counter instead of using the software vblank->count, but that would > require again some changes to drm_irq.c and it would probably be a bit messy > (see how cur_vblank and diff are computed inside drm_update_vblank_count). > > So I think using delay(tick) to fake a single vblank interval in > __wait_event_intr_timeout when "cold" is true is the simplest way to go. > > The DRM code also defines vblank->framedur_ns, which is the duration of a > frame in nanoseconds, so using delay(vblank->framedur_ns / 1000) instead of > delay(tick) would work too. In practice vblank->framedur_ns is 13 to 17 > milliseconds (see the output below) while one tick is 10 milliseconds, so I > think a tick is close enough and simpler. > > Anyway, at this point I feel I've kind of beaten this horse to death, so > it's up to you. Thoughts?
Yeah. On OpenBSD we enable interrupts late. And the vblank counter updates happen from the interrupt handler. I have to think through the consequences of simply doing a delay without checking the condition here though.
