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

2014-01-13 Thread Kenneth Graunke
On 01/13/2014 01:04 PM, Eric Anholt wrote:
> Kenneth Graunke  writes:
> 
>> On 01/09/2014 09:31 PM, Eric Anholt wrote:
>>> Kenneth Graunke  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 
>> ---
>>  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).
>>
>> I'd really like to land this patch as-is, since I need it to land the
>> rest of my Broadwell code.  I would update the commit message to note
>> that it's broken for >4G currently.
> 
> I don't like landing known-broken code that will give you mysterious
> hangs under memory pressure.  I could possibly ack this if there was a
> WARN_ON_ONCE or just having it be a stub or something, but "kind of
> works except when you start running a big app or run something for a
> long time" is not cool.

Well, hooray for double standards, given that every other userspace
component has landed this code, but didn't bother to even consolidate it
into one easily fixable place...

It's been over a year since I wrote most of this code, and I would
REALLY like to actually land some things.

But fine, I'll go write some libdrm patches...

--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-01-13 Thread Eric Anholt
Kenneth Graunke  writes:

> On 01/09/2014 09:31 PM, Eric Anholt wrote:
>> Kenneth Graunke  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 
> ---
>  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).
>
> I'd really like to land this patch as-is, since I need it to land the
> rest of my Broadwell code.  I would update the commit message to note
> that it's broken for >4G currently.

I don't like landing known-broken code that will give you mysterious
hangs under memory pressure.  I could possibly ack this if there was a
WARN_ON_ONCE or just having it be a stub or something, but "kind of
works except when you start running a big app or run something for a
long time" is not cool.


pgpFRf2zYI20f.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-01-10 Thread Kenneth Graunke
On 01/09/2014 09:31 PM, Eric Anholt wrote:
> Kenneth Graunke  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 
 ---
  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).

I'd really like to land this patch as-is, since I need it to land the
rest of my Broadwell code.  I would update the commit message to note
that it's broken for >4G currently.

Then, I could write the new libdrm API and send out a separate series
which fixes OUT_RELOC64 to work for >4G.

Keep in mind that Mesa doesn't actually advertise Broadwell support
(i.e. recognize the PCI IDs) yet.  This would need to be fixed before we
actually turn it on.

--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2014-01-09 Thread Eric Anholt
Kenneth Graunke  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 
>>> ---
>>>  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).


pgpgcFjDmr7u0.pgp
Description: PGP signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2013-12-13 Thread Daniel Vetter
On Fri, Dec 13, 2013 at 10:04:53AM -0800, Kenneth Graunke wrote:
> 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 
> >> ---
> >>  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);

This function here only adds the relocation entry to the batch buffer, so
that the kernel can fix stuff up in case anything moved. But you still
need to write the 64bit value into the batch itself, under the assumption
that the target buffer has not moved. For that the kernel tells you the
last offset in target_bo->presumed_offset. Of course you also need to add
the delta and all that.

On a very quick read (it's getting late here, please double check) you
need to add an OUT_RELOC64 and intel_batchbuffer_emit_reloc64. The call to
drm_intel_bo_emit_reloc stays the same, but you then need a new variant of
the function which writes the presumed reloc stuff into the batch, so a
new intel_batchbuffer_emit_qword which writes the 2 dwords for the reloc
entry.

> 
> which is:
> 
> static int
> drm_intel_gem_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
> drm_intel_bo *target_bo, uint32_t target_offset,
> uint32_t read_domains, uint32_t write_domain)
> 
> ...all 32-bit values.  It sounds like you're thinking we use Chris's
> "let's presume it hasn't moved since last time" relocation stuff?  We
> don't (at least not yet)...

The presumed_offset handling is all in libdrm and the above mentioned
stuff in OUT_RELOC. So I'm pretty sure you're using it ;-)

SNA on top uses a slightly different layout of the actual relocations
entries submitted to the kernel. The upshot of that is that the fastpath
can get a bit better (the kernel essentially ends up doing nothing besides
checking that all the objects are still at the same spot), the downside is
that userspace needs to follow stricter rules around emitting relocs and
reusing buffers. Last time I've chatted with Eric he said it's not worth
the trouble for mesa. Anyway, completely different topic.
 
> I'm more than happy to take suggestions.

Hope this helps.

Cheers, Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2013-12-13 Thread Kenneth Graunke
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 
>> ---
>>  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);

which is:

static int
drm_intel_gem_bo_emit_reloc(drm_intel_bo *bo, uint32_t offset,
drm_intel_bo *target_bo, uint32_t target_offset,
uint32_t read_domains, uint32_t write_domain)

...all 32-bit values.  It sounds like you're thinking we use Chris's
"let's presume it hasn't moved since last time" relocation stuff?  We
don't (at least not yet)...

I'm more than happy to take suggestions.

--Ken
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


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

2013-12-13 Thread Daniel Vetter
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 
> ---
>  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
> +
>  #define ADVANCE_BATCH() intel_batchbuffer_advance(brw);
>  #define CACHED_BATCH() intel_batchbuffer_cached_advance(brw);
>  
> -- 
> 1.8.4.4
> 
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev

-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev