[Intel-gfx] [PATCH 19/22] drm/i915: Move obj->active:5 to obj->flags

[Chris Wilson]

On Fri, Jul 29, 2016 at 10:40:09AM +0300, Joonas Lahtinen wrote:
> On ke, 2016-07-27 at 12:14 +0100, Chris Wilson wrote:
> > +static inline void
> > +i915_gem_object_set_active(struct drm_i915_gem_object *obj, int engine)
> > +{
> > +	obj->flags |= 1 << (engine + I915_BO_ACTIVE_SHIFT);
> 
> BIT(engine) << I915_BO_ACTIVE_SHIFT would be more readable to my taste,
> but I guess it's debatable.

I didn't change this to be BIT(engine + I915_BO_ACTIVE_SHIFT) because of
i915_gem_object_is_active() not following the pattern.

> >  	if (!readonly) {
> >  		active = obj->last_read;
> > -		active_mask = obj->active;
> > +		active_mask = i915_gem_object_is_active(obj);
> 
> _is_active() does not really fit to be assigned to _mask. maybe have
> object_active_mask() and then
> 
> _is_idle/inactive/whatever() { return !object_active_mask() }
> 
> Because the negation is used lot more.

10 i915_gem_object_is_active(), 1 !i915_gem_object_is_active(). Of which
4 use the mask and the rest as a boolean.

I'm still liking i915_gem_object_is_active() over
	i915_gem_object_active
	i915_gem_object_active_mask
	i915_gem_object_has_active

> > @@ -993,7 +993,7 @@ static int
> >  i915_gem_execbuffer_move_to_gpu(struct drm_i915_gem_request *req,
> >  				struct list_head *vmas)
> >  {
> > -	const unsigned other_rings = ~intel_engine_flag(req->engine);
> > +	const unsigned int other_rings = (~intel_engine_flag(req->engine) & I915_BO_ACTIVE_MASK) << I915_BO_ACTIVE_SHIFT;
> 
> Horribly long line, is this intermediary?

No. Sadly not, requires

eb_other_engines() /* the rings were an allusion to something that will break later */
{
	unsigned int mask;

	mask = ~intel_engine_flag(req->engine) & I915_BO_ACTIVE_MASK;
	mask <<= I915_BO_ACTIVE_SHIFT;

	return mask;
}

to get a reasonable split.
-Chris

-- 
Chris Wilson, Intel Open Source Technology Centre