[Intel-gfx] [PATCH 2/2] Set alignment value in drm_intel_add_validate_buffer()

[Ben Widawsky]

On Fri, Jun 19, 2015 at 03:52:01PM -0700, Anuj Phogat wrote:
> +Ben
> 
> On Fri, Apr 10, 2015 at 5:20 PM, Anuj Phogat <anuj.phogat at gmail.com> wrote:
> > Signed-off-by: Anuj Phogat <anuj.phogat at gmail.com>
> > ---
> >  intel/intel_bufmgr_gem.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/intel/intel_bufmgr_gem.c b/intel/intel_bufmgr_gem.c
> > index 51d87ae..92701a5 100644
> > --- a/intel/intel_bufmgr_gem.c
> > +++ b/intel/intel_bufmgr_gem.c
> > @@ -459,7 +459,7 @@ drm_intel_add_validate_buffer(drm_intel_bo *bo)
> >         bufmgr_gem->exec_objects[index].handle = bo_gem->gem_handle;
> >         bufmgr_gem->exec_objects[index].relocation_count = bo_gem->reloc_count;
> >         bufmgr_gem->exec_objects[index].relocs_ptr = (uintptr_t) bo_gem->relocs;
> > -       bufmgr_gem->exec_objects[index].alignment = 0;
> > +       bufmgr_gem->exec_objects[index].alignment = bo->align;
> >         bufmgr_gem->exec_objects[index].offset = 0;
> >         bufmgr_gem->exec_bos[index] = bo;
> >         bufmgr_gem->exec_count++;

I'm a bit hesitant about this hunk. We're never going to use this from a mesa
that supports Yf/Ys - and going this path wouldn't be expected. Maybe add a
warning if bo->align? (From your other patch I don't think it can ever happen,
but just to future proof it.

> > @@ -501,7 +501,7 @@ drm_intel_add_validate_buffer2(drm_intel_bo *bo, int need_fence)
> >         bufmgr_gem->exec2_objects[index].handle = bo_gem->gem_handle;
> >         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].alignment = bo->align;
> >         bufmgr_gem->exec2_objects[index].offset = 0;
> >         bufmgr_gem->exec_bos[index] = bo;
> >         bufmgr_gem->exec2_objects[index].flags = 0;

I was about to argue this should be part of patch 1, but nope, it should be a
separate patch :-)

I started digging into whether we have a reasonable way to determine if a bo
alignment failed, and fall back to a softer restriction. It didn't seem doable
with the current interfaces, but it's something to think about.

With or without the first recommendation:
Reviewed-by: Ben Widawsky <ben at bwidawsk.net>