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. > - 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? -Chris -- Chris Wilson, Intel Open Source Technology Centre _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx http://lists.freedesktop.org/mailman/listinfo/intel-gfx