On Wed, Jul 27, 2016 at 01:13:54PM +0300, Joonas Lahtinen wrote:
> On ma, 2016-07-25 at 18:32 +0100, Chris Wilson wrote:
> > --- a/drivers/gpu/drm/i915/i915_drv.h
> > +++ b/drivers/gpu/drm/i915/i915_drv.h
> > @@ -907,6 +907,7 @@ struct i915_gem_context {
> >     struct list_head link;
> >  
> >     u8 remap_slice;
> > +   bool closed:1;
> 
> Not a whole bool? I think it could be.

Why? It's read mostly. So a bitfield is not going to adverse affect code
size or add rmw.

> > +static void i915_ppgtt_close(struct i915_address_space *vm)
> > +{
> > +   struct list_head *phases[] = {
> > +           &vm->active_list,
> > +           &vm->inactive_list,
> > +           &vm->unbound_list,
> > +           NULL,
> > +   }, **phase;
> > +
> > +   GEM_BUG_ON(vm->closed);
> > +   vm->closed = true;
> > +
> > +   for (phase = phases; *phase; phase++) {
> > +           struct i915_vma *vma, *vn;
> > +
> > +           list_for_each_entry_safe(vma, vn, *phase, vm_link)
> > +                   if (!vma->closed)
> > +                           i915_vma_close(vma);
> > +   }
> 
> Pretty sure rather listing three three function calls here would be
> nicer;
> 
> i915_ppgtt_do_close(vm->active_list);
> i915_ppgtt_do_close(vm->inactive_list);

But with the above loop structure the compiler generates reasonable code.
That's relevant for improving code generation for eviction later.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre
_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to