[Mesa-dev] [PATCH 08/13] i965: Convert reloc.target_handle into an index for I915_EXEC_HANDLE_LUT

[Chris Wilson]

Quoting Kenneth Graunke (2017-07-20 17:57:22)
> On Thursday, July 20, 2017 8:05:19 AM PDT Chris Wilson wrote:
> > Quoting Kenneth Graunke (2017-07-19 23:36:58)
> > > On Wednesday, July 19, 2017 3:09:16 AM PDT Chris Wilson wrote:
> > > >  #define READ_ONCE(x) (*(volatile __typeof__(x) *)&(x))
> > > > @@ -117,21 +125,12 @@ add_exec_bo(struct intel_batchbuffer *batch, struct brw_bo *bo)
> > > >                   batch->exec_array_size * sizeof(batch->exec_objects[0]));
> > > >     }
> > > >  
> > > > -   struct drm_i915_gem_exec_object2 *validation_entry =
> > > > -      &batch->exec_objects[batch->exec_count];
> > > > -   validation_entry->handle = bo->gem_handle;
> > > > -   if (bo == batch->bo) {
> > > > -      validation_entry->relocation_count = batch->reloc_count;
> > > > -      validation_entry->relocs_ptr = (uintptr_t) batch->relocs;
> > > > -   } else {
> > > > -      validation_entry->relocation_count = 0;
> > > > -      validation_entry->relocs_ptr = 0;
> > > > -   }
> > > > -   validation_entry->alignment = bo->align;
> > > > -   validation_entry->offset = bo->offset64;
> > > > -   validation_entry->flags = bo->kflags;
> > > > -   validation_entry->rsvd1 = 0;
> > > > -   validation_entry->rsvd2 = 0;
> > > > +   struct drm_i915_gem_exec_object2 *exec =
> > > > +      memset(&batch->exec_objects[batch->exec_count], 0, sizeof(*exec));
> > > > +   exec->handle = bo->gem_handle;
> > > > +   exec->alignment = bo->align;
> > > > +   exec->offset = bo->offset64;
> > > > +   exec->flags = bo->kflags;
> > > 
> > > I liked the name "validation_entry" given that we call this the "validation
> > > list"...exec matches the struct name better, but I think validation_entry
> > > helps distinguish the two lists...
> > 
> > Hmm, how about
> > 
> > -   struct drm_i915_gem_exec_object2 *exec =
> > -      memset(&batch->exec_objects[batch->exec_count], 0, sizeof(*exec));
> > -   exec->handle = bo->gem_handle;
> > -   exec->alignment = bo->align;
> > -   exec->offset = bo->offset64;
> > -   exec->flags = bo->kflags;
> > +   batch->exec_objects[batch->exec_count] = (struct drm_i915_gem_exec_object2){
> > +          .handle = bo->gem_handle,
> > +          .alignment = bo->align,
> > +          .offset = bo->offset64,
> > +          .flags = bo->kflags,
> > +   };
> > 
> > and skip the impossible problem of naming?
> > 
> > But we still end up with a couple of 
> >       struct drm_i915_gem_exec_object2 *
> >               validation_entry = &batch->exec_objects[index];
> > Could I just call those exec_object?
> > -Chris
> 
> I'm not objecting too strongly, call it exec or exec_object if you like. 
> The initializer use is pretty nice.
> 
> "validation list" is a bit of a weird name anyway...

As you've seen, I think there's some merit to a distinct name so we
don't get confused with exec_bos, I've settled for

  struct drm_i915_gem_exec_object2 *entry = &batch->validation_list[index];

as that fits into 80cols :)
-Chris