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