[Intel-gfx] [PATCH 2/2] intel: Use I915_EXEC_NO_RELOC when available

[Kristian Høgsberg]

On Sat, Jan 17, 2015 at 1:49 AM, Chris Wilson <chris at chris-wilson.co.uk> wrote:
> On Sat, Jan 17, 2015 at 05:23:59AM +0100, Daniel Vetter wrote:
>> On Fri, Jan 16, 2015 at 05:46:00PM -0800, Kristian Høgsberg wrote:
>> > The I915_EXEC_NO_RELOC flag lets us tell the kernel that the offset we
>> > provide in the validate list entry is what we've used in all relocations
>> > to the bo in question.  If the bo hasn't moved, the kernel can skip
>> > relocations completely.
>> >
>> > Signed-off-by: Kristian Høgsberg <krh at bitplanet.net>
>> > ---
>> >  intel/intel_bufmgr_gem.c | 17 ++++++++++++++++-
>> >  1 file changed, 16 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
>> > index 8a51cea..a657a4d 100644
>> > --- a/intel/intel_bufmgr_gem.c
>> > +++ b/intel/intel_bufmgr_gem.c
>> > @@ -131,6 +131,7 @@ typedef struct _drm_intel_bufmgr_gem {
>> >   unsigned int no_exec : 1;
>> >   unsigned int has_vebox : 1;
>> >   unsigned int has_handle_lut : 1;
>> > + unsigned int has_no_reloc : 1;
>> >   bool fenced_relocs;
>> >
>> >   char *aub_filename;
>> > @@ -504,7 +505,15 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence)
>> >   bufmgr_gem->exec2_objects[index].relocation_count = bo_gem->reloc_count;
>> >   bufmgr_gem->exec2_objects[index].relocs_ptr = (uintptr_t)bo_gem->relocs;
>> >   bufmgr_gem->exec2_objects[index].alignment = 0;
>> > - bufmgr_gem->exec2_objects[index].offset = 0;
>> > +
>> > + /* If the kernel supports I915_EXEC_NO_RELOC, it will compare
>> > + * offset in struct drm_i915_gem_exec_object2 against the bos
>> > + * current offset and if all bos haven't moved it will skip
>> > + * relocation processing alltogether.  If I915_EXEC_NO_RELOC
>> > + * is not supported, the kernel ignores the incoming value of
>> > + * offset so we can set it either way.
>> > + */
>> > + bufmgr_gem->exec2_objects[index].offset = bo->offset64;
>> >   bufmgr_gem->exec_bos[index] = bo;
>> >   bufmgr_gem->exec2_objects[index].flags = 0;
>> >   bufmgr_gem->exec2_objects[index].rsvd1 = 0;
>> > @@ -2471,6 +2480,8 @@ do_exec2(drm_intel_bo *bo, int used, drm_intel_context *ctx,
>> >
>> >   if (bufmgr_gem->has_handle_lut)
>> >   execbuf.flags |= I915_EXEC_HANDLE_LUT;
>> > + if (bufmgr_gem->has_no_reloc)
>> > + execbuf.flags |= I915_EXEC_NO_RELOC;
>>
>> You need some opt-in flag to not break existing userspace: Iirc both UXA
>> and libva retain reloc trees partially, which means that we might have
>> different presumed offsets for the same bo in different relocs.
>
> A bigger challenge is that you have to use execlist flags to indicate
> read/write domains (actually just read or write!), and a special flag for
> the SNB pipecontrol w/a. (This is because the kernel no longer even
> scans the relocation trees if the buffers haven't moved and so we don't
> have the chance to construct the read/write domains from the relocs.)

You're referring to having to set the EXEC_OBJECT_WRITE flag for
buffers that are in any write domain?  That seems doable - we're
already scanning all relocs before submitting.  I didn't find anything
in i915_drm.h about the SNB pipecontrol... can you elaborate?

thanks,
Kristian

> -Chris
>
> --
> Chris Wilson, Intel Open Source Technology Centre