op 23-07-14 09:15, Christian K?nig schreef: > Am 23.07.2014 09:09, schrieb Daniel Vetter: >> On Wed, Jul 23, 2014 at 9:06 AM, Maarten Lankhorst >> <maarten.lankhorst at canonical.com> wrote: >>>> Can we somehow avoid the need to call fence_signal() at all? The >>>> interrupts at least on radeon are way to unreliable for such a thing. Can >>>> enable_signalling fail? What's the reason for fence_signaled() in the >>>> first place? >>> It doesn't need to be completely reliable, or finish immediately. >>> >>> And any time wake_up_all(&rdev->fence_queue) is called all the fences that >>> were enabled will be rechecked. >> I raised this already somewhere else, but should we have some common >> infrastructure in the core fence code to recheck fences periodically? >> radeon doesn't seem to be the only hw where this isn't reliable >> enough. Of course timer-based rechecking would only work if the driver >> provides the fence->signalled callback to recheck actual fence state. > > Yeah, agree. The proposal won't work reliable at all with radeon. > > Interrupts are accumulated before sending them to the CPU, e.g. you can get > one interrupt for multiple fences finished. If it's just the interrupt for > the last fence submitted that gets lost you are completely screwed up because > you won't get another interrupt. > > I had that problem multiple times while working on UVD support, resulting in > the driver thinking that it can't submit more jobs because non of the > interrupts for the already submitted fence cam through. Yeah but all the fences that have .enable_signaling will get signaled from a single interrupt, or when any waiter calls radeon_fence_process.
> Apart from that interrupts on Macs usually don't work at all, so we really > need a solution where calling fence_signaled() is completely optional. I haven't had a problem with interrupts on my mbp after d1f9809ed1315c4cdc5760cf2f59626fd3276952, but it should be trivial to start a timer that periodically does wake_up_all, and gets its timeout reset in a call to radeon_fence_process. It could either be added as a work item, or as a normal timer (disabled during gpu lockup recovery to prevent checks from fiddling with things it shouldn't). ~Maarten
