[PATCH 3/3] drm/amdgpu: busywait KIQ register accessing v2

[Christian König]

Am 17.10.2017 um 10:38 schrieb Ding, Pixel:
> [SNIP]
>>> +		if (seq >= wait_seq && wait_seq >= last_seq)
>>> +			break;
>>> +		if (seq <= last_seq &&
>>> +		    (wait_seq <= seq || wait_seq >= last_seq))
>>> +			break;
>> Why not just "(int32_t)(wait_seq - seq) > 0" ? IIRC that should be
>> sufficient for a wrap around test.
>> [pixel] seq could be larger (executed) or smaller (not yet) than wait_seq even without a wrap round, I think you mean "(int32_t)(last_seq - seq) > 0”
>> it actually can know there’s a wrap around, but it still doesn't know if the waited seq is in the range between or not, we still need other conditions. Code here is to identify cases to break as:
> ===last_seq=====wait_seq====seq===
> ===wait_seq==seq========last_seq==
> ==seq==========last_seq==wait_seq=
>
> Does it make sense?

No, let me explain a bit more. The full code I had in mind is this:

do {
     seq = amdgpu_fence_read(ring)
} while ((int32_t)(wait_seq - seq) > 0);

This should handle the following cases:
1. wait_seq is larger than seq, in this case we still need to wait.

(wait_seq - seq) is larger than zero and the loop continues.

2. wait_seq is smaller than seq, in this case the we can stop waiting.

(wait_seq -seq) is smaller or equal to zero and the loop stops.

3. wait_seq is much smaller than seq because of a wrap around, in this 
case we still need to wait.

(wait_seq - seq) is larger than zero because the substraction wraps 
around the numeric range as well and the look will continue.

4. wait_seq is much larger than seq because of a wrap around, in this 
case we can stop waiting.

(wait_seq - seq) is smaller than zero because the cast to a signed 
number makes it negative and the loop stops.

This is rather common code for wrap around testing and you don't need to 
fiddle with last_seq at all here.

If you want you can also use the __dma_fence_is_later() helper function 
which implements exactly that test as well.

Regards,
Christian.