Jesse, cc'ing you in case you have thoughts about this for the intel
side of things?
On Nov 17, 2011, at 4:39 PM, Andrew Lutomirski wrote:
> 2011/11/17 Michel D?nzer <michel at daenzer.net>:
>> [ Dropping intel-gfx list from CC, as it automatically rejects posts
>> from non-subscribers ]
>>
>> On Mit, 2011-11-16 at 21:39 -0800, Andy Lutomirski wrote:
>>> There are two possible races when disabling vblanks. If the IRQ
>>> fired but the hardware didn't update its counter yet, then we store
>>> too low a hardware counter. (Sensible hardware never does this.
>>> Apparently not all hardware is sensible.)
>>
>> The thing is, 'the' IRQ and 'the' hardware counter aren't necessarily
>> about the same thing. We want an IRQ which triggers at the
>> beginning of
>> vertical blank, but the Radeon hardware counters increment at the
>> beginning of scanout, i.e. at the end of vertical blank. Does that
>> make
>> the hardware 'broken' or 'not sensible'?
>
> Perhaps not. I think that using that counter as the return value of
> get_vblank_counter is a bit unfortunate in that case, though. Anyway,
> I'm not particularly attached to the comment as written.
>
> Am I correct in guessing that Radeon fires its vblank irq at the
> beginning of vblank instead of at the end And doesn't this mean that
> Radeon has a separate issue when vblank_get happens during the vblank
> interval? A trick similar to drm_calc_vbltimestamp_from_scanoutpos
> ought to be able to fix that.
>
The Radeon's i tested (R500, R600) fire the vblank irq even a few
scanlines before the start of vblank. I think most gpu's fire very
close to the beginning of vblank, because one wants to use the vblank
irq to trigger some work that needs to be done within the vblank
interval. Some Radeon's sometimes fire a spurious vblank irq at the
moment vblank irq's get enabled, althought that could also be some
tiny bug in the kms driver, maybe missing some acknowledge of a
pending irq somewhere in the vblank off or on function. That's the
reason for some of the fun inside
drm_calc_vbltimestamp_from_scanoutpos() and drm_handle_vblank(), to
associate a vblank irq and the vblank timestamps with the correct
vblank interval, even if the irq handler executes a bit outside the
vblank interval.
>>
>>> If, on the other hand, the counter updated but the IRQ didn't fire
>>> yet, we store too high a counter.
>>>
>>> We handled the former case with a heuristic based on timestamps and
>>> we did not handle the latter case. By saving a little more state,
>>> we can handle both cases exactly: all we need to do is watch for
>>> changes in the difference between the hardware and software vblank
>>> counts.
>>
>> I'm afraid that can't work:
>>
>> Some (AFAIR also Intel) hardware resets the counter to 0 when the
>> CRTC
>> is disabled / enabled (e.g. for DPMS, or a modeset). That's why we
>> ended
>> up only counting interrupts while the IRQ is enabled, and only
>> using the
>> hardware counter to fill in while the IRQ is disabled. The hardware
>> counter cannot be assumed to have any defined behaviour between
>> enabling
>> and disabling the IRQ.
>>
>> To compensate for this, the drivers call drm_vblank_pre_modeset
>> (which
>> enables the IRQ, which also updates the virtual counter from the
>> hardware counter) before disabling the CRTC and
>> drm_vblank_post_modeset
>> (which disables the IRQ, which also records the hardware counter)
>> after
>> enabling the CRTC.
>>
>
> Shouldn't this be fixable, though, by adding appropriate logic in
> drm_vblank_pre_modeset and drm_vblank_post_modeset?
>
>
> Would this be a lot simpler if drm had a function (or call to the
> driver) to compute both the vblank count and the last vblank
> timestamp? That way the irq handler, drm_update_vblank_count,
> vblank_disable_and_save, and pre/post modeset could keep everything
> exactly in sync. IIRC i915 has this capability in hardware, and if
> Radeon indeed updates the frame count exactly at the end of the vblank
> interval, it could compute this exactly as well.
I think Andrews idea is very close to what i proposed in Matthews RFC
e-mail thread, if we use the scanout position as a "clock", except we
could get rid of the calibration loop, the part i like least about my
proposal, if we know for sure when each gpu increments its hardware
counter.
At least intel, radeon and nouveau (hopefully soonish - patches
almost ready for submission) expose the dev->driver-
>get_scanout_position() hook, so we know the scanline at time of
vblank counter query. If we knew at which scanline each gpu
increments its counter we could simply compare against scanline at
counter query time and decide if we need to add 1 to the value or
not. We could make the hardware counter value look like as if it is
from a hardware counter that always increments at leading edge of
vblank, which i think would be consistent with what the irq driven
vblank increment does.
Pseudocode (with bonus goto included!):
// To be called from vblank irq on/off routines instead of driver-
>get_vblank_count(crtc):
uint32 get_effective_hw_vblank_count(crtc)
{
retry:
uint32 scanline_pre = driver->get_scanout_position(crtc);
uint32 hw_count = driver->get_vblank_count(crtc);
uint32 scanline_post = driver->get_scanout_position(crtc);
if ((scanline_pre outside vblank) && (scanline_post outside
vblank) {
// Inside active scanout -> All counters stable -> no problem:
return hw_count;
}
// Inside vblank -> careful!
if (scanline_where_gpu_increments_its_counter intersects
interval [scanline_pre ; scanline_post]) {
// Exact time of get_vblank_count() query "uncertain" wrt. hw
counter increment.
cpu_relax(); // or something similar...
goto retry;
}
// Inside vblank. Query happened before or after hw increment?
if (scanline_post < scanline_where_gpu_increments_its_counter) {
// Query happened before hw counter increment for sure. Fake
// the increment has already happened:
hw_count++;
}
return hw_count;
}
Something like this? The worst case busy waiting time would be maybe
2 scanline durations ( < 50 microseconds) in the very rare case where
we hit exactly the scanline where hw counter increments. That amount
of busy waiting sounds acceptable to me, given that we'd still keep a
vblank off delay of a few milliseconds to avoid many redundant on/off
transitions during a typical OpenGL bufferswap and that this never
gets directly called from interrupt context.
We could put this into the drm, then the drivers would need to tell
the drm the scanline where increment happens, or we could replace the
driver->get_vblank_count() hook in each driver with some variant of
this? Drivers with such a thing could lower the vblank off delay to a
few msecs, other drivers would leave it at a high value.
Thoughts?
-mario
