Mark Kettenis wrote:
>I have to think through the consequences of simply doing a delay
>without checking the condition here though.
Right now __wait_event_intr_timeout has a KASSERT(!cold), so, if its code
is changed to have an "if (cold) { delay(tick); ret = 1; }" then we know
that this new code is only going to be called from drm_wait_one_vblank
because that's the only place where a corresponding "if (... || cold)
return;" is removed. So we always know exactly what the condition is going
to be in __wait_event_intr_timeout when "cold" is 1: the condition from
drm_wait_one_vblank, which we know we can ignore because it was never used
before in drm_wait_one_vblank when "cold" is 1 anyway (because
drm_wait_one_vblank simply directly returns in that case).
Doing it this way might be a problem in the future though: if code
somewhere else is changed to call __wait_event_intr_timeout when "cold" is
1 and with a different condition then the condition will be silently
ignored, which is not great.
I still think the patch ought to be directly in drm_wait_one_vblank in
drm_irq.c, because that's the function that promises to wait for one vblank
but does not when "cold" is 1. So that's where I think "if (cold) {
delay(tick); return; }" ought to be, and then there would be no need to
worry about __wait_event_intr_timeout's condition or about vblank
interrupts. I'm not the one who has to merge that code with the Linux code
though :-)
Philippe
