On Wed, Mar 19, 2014 at 4:05 PM, Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> wrote: > On Tue, Mar 18, 2014 at 10:26:04AM +0100, Daniel Vetter wrote: >> Extract all this logic into a new helper function >> semaphore_wait_to_signaller_ring because: >> >> - The current code has way too much magic. >> >> - The current code doesn't look at bi16, which encodes VECS signallers >> on HSW. Those are just added after the fact, so can't be encoded in >> a neat formula. >> >> - It blindly trust the hardware for the dev_priv->ring array >> derefence, which given that we have a gpu hang at hand is scary. The >> current logic can't blow up since it limits its value range >> sufficiently, but that's a bit too tricky to rely on in my opinion. >> Especially when we start to add bdw support. > > No, it does not blindly trust. What you just mean here is that to extend > it for hsw+ it is simpler to reimplement differently. Note that I say that the current logic can't blow up due to the limited value range ... I can drop the "blindly trust" and leave it at "too tricky" when merging. >> - I'm not a big fan of the explicit ring->semaphore_register list, but >> I think it's more robust to use the same mapping both when >> constructing the semaphore commands and when decoding them. >> >> - Finally add a FIXME comment about lack of broadwell support here, >> like in the earlier ipehr semaphore cmd detection function. >> >> Cc: Mika Kuoppala <mika.kuoppala@xxxxxxxxx> >> Cc: Ben Widawsky <ben@xxxxxxxxxxxx> >> Cc: Chris Wilson <chris@xxxxxxxxxxxxxxxxxx> >> Signed-off-by: Daniel Vetter <daniel.vetter@xxxxxxxx> > > Other than the digression above, where did MI_SEMAPHORE_SYNC_MASK spring > from? Early patch? Yeah, the two patches this one here should be in-reply-to have that #define. -Daniel -- Daniel Vetter Software Engineer, Intel Corporation +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx