Re: [PATCH 25/34] drm/i915: Track active timelines

[Chris Wilson <chris@chris-wilson.co.uk>]

Quoting John Harrison (2019-01-23 22:32:54)
> On 1/22/2019 07:17, Chris Wilson wrote:
> > Quoting Tvrtko Ursulin (2019-01-22 14:56:32)
> >> On 21/01/2019 22:21, Chris Wilson wrote:
> >>> +static void timeline_active(struct i915_timeline *tl)
> >>> +{
> >>> +     struct i915_gt_timelines *gt = &tl->i915->gt.timelines;
> >>> +
> >>> +     mutex_lock(&gt->mutex);
> >>> +     list_add(&tl->link, &gt->active_list);
> >>> +     mutex_unlock(&gt->mutex);
> >>> +}
> >>> +
> >>> +static void timeline_inactive(struct i915_timeline *tl)
> >>> +{
> >>> +     struct i915_gt_timelines *gt = &tl->i915->gt.timelines;
> >>> +
> >>> +     mutex_lock(&gt->mutex);
> >>> +     list_del(&tl->link);
> >>> +     mutex_unlock(&gt->mutex);
> >>> +}
> >> Bike shedding comments only:
> >> Would it be better to use a verb suffix? Even though timeline_activate
> >> also wouldn't sound perfect. Since it is file local - activate_timeline?
> >> Or even just inline to pin/unpin. Unless more gets put into them later..
> > Haven't got any plans for more here, yet, and was thinking this is a
> > pinned_list myself. I picked active_list since I was using 'active'
> > elsewhere for active_ring, active_engines, active_contexts, etc.
> >
> > I didn't like activate/deactivate enough to switch, and was trying to
> > avoid reusing pin/unpin along this path:
> >       i915_timeline_pin -> timeline_pin
> > begged confusion
> >
> > [snip]
> >> Never mind the bikeshedding comments:
> > There's time enough for someone to open a new pot of paint.
> I agree that having a verb in there would make things clearer. Maybe 
> timeline_make_(in)active? Or timeline_mark_(in)active?

mark more so than make (since the action we are doing is external).

How about

	timeline_track_active()
	timeline_untrack_active() / timeline_cancel_active()

or
	timeline_add_to_active()
	timeline_remove_from_active()

Finally!
-Chris
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx