[Mesa-dev] [PATCH] i965: Extend the negative 32-bit deltas to 64-bits

[Sergii Romantsov]

Hello, Chris. Thank you for reviewing patch.
Do you mean changing just only emit_reloc() parameter type or all the
preceding callers too?

Also, please, explain what you mean about patch cee9f3890351?
That issue is present even on mesa 13-0.0 and also solved with similar
type-conversion fix.


Regards, Sergii.

On Mon, Mar 26, 2018 at 3:31 PM, Chris Wilson <chris at 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).
>
> > 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.romantsov at globallogic.com>
> > Tested-by: Andriy Khulap <andriy.khulap at 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.
> -Chris
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>



-- 
Sergii Romantsov
GlobalLogic Inc.
www.globallogic.com
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180326/65a85727/attachment-0001.html>