[Mesa-dev] [PATCH 08/10] i965: Introduce an OUT_RELOC64 macro.

[Eric Anholt]

Kenneth Graunke <kenneth at whitecape.org> writes:

> On 12/13/2013 09:28 AM, Daniel Vetter wrote:
>> On Thu, Dec 12, 2013 at 01:26:40AM -0800, Kenneth Graunke wrote:
>>> Broadwell uses 48-bit addresses.  The first DWord is the low 32 bits,
>>> and the second DWord is the high 16 bits.
>>>
>>> Since individual buffers shouldn't be larger than 4GB in size, any
>>> offsets into those buffers (buffer->offset + delta) should fit in the
>>> low 32 bits.  So I believe we can simply emit 0 for the high 16-bits,
>>> and drm_intel_bo_emit_reloc() should patch it up.
>>>
>>> Signed-off-by: Kenneth Graunke <kenneth at whitecape.org>
>>> ---
>>>  src/mesa/drivers/dri/i965/intel_batchbuffer.h | 5 +++++
>>>  1 file changed, 5 insertions(+)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/intel_batchbuffer.h b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
>>> index 159f928..128eed9 100644
>>> --- a/src/mesa/drivers/dri/i965/intel_batchbuffer.h
>>> +++ b/src/mesa/drivers/dri/i965/intel_batchbuffer.h
>>> @@ -178,6 +178,11 @@ void intel_batchbuffer_cached_advance(struct brw_context *brw);
>>>  				read_domains, write_domain, delta);	\
>>>  } while (0)
>>>  
>>> +/* Handle 48-bit address relocations for Gen8+ */
>>> +#define OUT_RELOC64(buf, read_domains, write_domain, delta) \
>>> +   OUT_RELOC(buf, read_domains, write_domain, delta);       \
>>> +   OUT_BATCH(0);
>> 
>> Please not. The presumed_offset that libdrm uses is 64bits, and you need
>> to emit the full presumed address (and correctly shifted). Atm the kernel
>> never gives you a presumed reloc offset with the high bits set so it
>> doesn't matter. But I'd prefer if we don't need to make this opt-in
>> behaviour once we enable address spaces with more than 4G.
>> 
>> i-g-t gets away with the cheap hack since we're allowed to break igt.
>> Let me check ddx and libva whether I've lost this fight already ...
>> -Daniel
>
> I'm more than happy to do the right thing, I just don't know what that
> is.  I don't see any uint64_t values in the interface we use at all:
>
> OUT_RELOC becomes
>    ret = drm_intel_bo_emit_reloc(brw->batch.bo, 4*brw->batch.used,
>                                  buffer, delta,
>                                  read_domains, write_domain);

The libdrm ABI is a disaster.  bo->offset is a long, so we're keeping 32
bits of the kernel's returned value on 32 bit userspace, and 64 bits on
64 bit userspace.  This means that on 32-bit we'll write in an
expected-incorrect offset in the presumed offset for a >4g-located BO,
which the kernel will map and fix up at exec time.  On 64-bit, your
patch would write an expected-incorrect 32-bit value into the batch, but
libdrm would tell the kernel the full expected 64 bit value in the
presumed_offset field, and you'll get brokenness for >4g buffers.

So, I think you do need a drm_intel_bo_emit_reloc64 that returns a
uint64_t value that the kernel wrote into the presumed offset, which you
then plug into your batchbuffer.

(In other news, while thinking about this, there are some obscure races
with buffer migration due to presumed_offset being read at a separate
time from when we look up bo->offset to actually write the offset into
the batch, in the presence of context sharing in GL).
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 835 bytes
Desc: not available
URL: <http://lists.freedesktop.org/archives/mesa-dev/attachments/20140110/b38b5300/attachment.pgp>