[Intel-gfx] [PATCH 25/34] drm/i915: Track active timelines
Chris Wilson
chris at chris-wilson.co.uk
Wed Jan 23 23:08:09 UTC 2019
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(>->mutex);
> >>> + list_add(&tl->link, >->active_list);
> >>> + mutex_unlock(>->mutex);
> >>> +}
> >>> +
> >>> +static void timeline_inactive(struct i915_timeline *tl)
> >>> +{
> >>> + struct i915_gt_timelines *gt = &tl->i915->gt.timelines;
> >>> +
> >>> + mutex_lock(>->mutex);
> >>> + list_del(&tl->link);
> >>> + mutex_unlock(>->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
More information about the Intel-gfx
mailing list