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