<div dir="ltr"><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Hello, Emil. Thanks for involving.</div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">> Fixes: cee9f3890351 ("i965: Allow 48-bit addressing on Gen8+.") <div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"> </div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">Actually that patch doesn't fix exactly <span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">cee9f3890351. Also I wrote earlier (probably, my e-mails may go into spam :)):</div><div><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline"><span style="color:rgb(34,34,34);font-family:arial,sans-serif;font-size:12.8px;font-style:normal;font-variant-ligatures:normal;font-variant-caps:normal;font-weight:400;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;background-color:rgb(255,255,255);text-decoration-style:initial;text-decoration-color:initial;float:none;display:inline">"That issue is present even on mesa 13-0.0 and also solved with similar type-conversion fix."</div></div><div class="gmail_extra"> <div class="gmail_quote">On Mon, Mar 26, 2018 at 4:57 PM, Emil Velikov <<a href="mailto:emil.l.velikov@gmail.com" target="_blank">emil.l.velikov@gmail.com</a>> wrote: <blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">On 26 March 2018 at 13:31, Chris Wilson <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> 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: <a href="https://bugs.freedesktop.org/show_bug.cgi?id=101408" rel="noreferrer" target="_blank">https://bugs.freedesktop.org/show_bug.cgi?id=101408</a> >> Signed-off-by: Sergii Romantsov <<a href="mailto:sergii.romantsov@globallogic.com">sergii.romantsov@globallogic.com</a>> >> Tested-by: Andriy Khulap <<a href="mailto:andriy.khulap@globallogic.com">andriy.khulap@globallogic.com</a>> >> --- >> 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 </blockquote></div> </div>