Hi,

[...]

> > @@ -28,7 +28,6 @@ static u32 object_max_page_size(struct
> > intel_memory_region **placements,
> >                 max_page_size = max_t(u32, max_page_size, mr-
> > >min_page_size);
> >         }
> >  
> > -       GEM_BUG_ON(!max_page_size);
> >         return max_page_size;
> >  }
> 
> Should this change be separated out? It's not immediately clear to a
> reviewer why it is included.

no, it's not, indeed... and is it correct to assume that the
default size is I915_GTT_PAGE_SIZE_4K?

[...]

> > +#define assert_vm_bind_held(vm)   lockdep_assert_held(&(vm)-
> > >vm_bind_lock)
> > +
> > +static inline void i915_gem_vm_bind_lock(struct i915_address_space
> > *vm)
> > +{
> > +       mutex_lock(&vm->vm_bind_lock);
> > +}
> > +
> > +static inline int
> > +i915_gem_vm_bind_lock_interruptible(struct i915_address_space *vm)
> > +{
> > +       return mutex_lock_interruptible(&vm->vm_bind_lock);
> > +}
> > +
> > +static inline void i915_gem_vm_bind_unlock(struct i915_address_space
> > *vm)
> > +{
> > +       mutex_unlock(&vm->vm_bind_lock);
> > +}
> > +
> 
> Kerneldoc for the inlines.

do we really need these oneline wrappers?

Andi

Reply via email to