[Intel-gfx] [PATCH] drm/i915: make semaphore signaller detection more robust

[Daniel Vetter]

On Wed, Mar 19, 2014 at 4:05 PM, Chris Wilson <chris at chris-wilson.co.uk> 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 at intel.com>
>> Cc: Ben Widawsky <ben at bwidawsk.net>
>> Cc: Chris Wilson <chris at chris-wilson.co.uk>
>> Signed-off-by: Daniel Vetter <daniel.vetter at ffwll.ch>
>
> 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