On 26 March 2018 at 13:31, Chris Wilson <ch...@chris-wilson.co.uk> wrote:
> Quoting Sergii Romantsov (2018-03-26 13:16:24)
>> Negative deltas are used to fake a range in a large buffer.
>> See 900a5c91eeb3
>> "i965: Use negative relocation deltas to minimise vertex uploads"
>>
>> Gen8+ use 48-bit address relocations so need to extend the sign
>
> Note that 48-bit relocations were only switched on in
> commit cee9f3890351 ("i965: Allow 48-bit addressing on Gen8+.")
> to save having to backport too far (although the patch is trivial).
>
Agreed, one could backport it for older instances, although it seems unneeded.
Please use a simple fixes tag like:
Fixes: cee9f3890351 ("i965: Allow 48-bit addressing on Gen8+.")
>> to 64-bit return value. Without it we have higher bits zeroed
>> and missing the negavive values.
>> Haswell and older use 32-bit deltas so are unaffected by this issue.
>>
>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101408
>> Signed-off-by: Sergii Romantsov <sergii.romant...@globallogic.com>
>> Tested-by: Andriy Khulap <andriy.khu...@globallogic.com>
>> ---
>> src/mesa/drivers/dri/i965/intel_batchbuffer.c | 4 +++-
>> 1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> index d824ff2..128da77 100644
>> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.c
>> @@ -1124,8 +1124,10 @@ emit_reloc(struct intel_batchbuffer *batch,
>> /* Using the old buffer offset, write in what the right data would be, in
>> * case the buffer doesn't move and we can short-circuit the relocation
>> * processing in the kernel
>> + *
>> + * Some target_offsets may be negative, so extend the sign to 64 bits.
>> */
>> - return entry->offset + target_offset;
>> + return entry->offset + (int64_t)((int32_t)target_offset);
>
> Although just changing s/uint32_t target_offset/int32_t target_offset/
> may be cleaner.
Seems like there's plenty of users that opt for unsigned. Might be
better to address the issue as-is and as a follow-up do a consistent
to-signed sweep.
It keeps the fix small and simple, plus avoids a ton of extra warnings
[for those using -W*sign*] ;-)
-Emil
_______________________________________________
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
