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 _______________________________________________ Intel-gfx mailing list Intel-gfx@xxxxxxxxxxxxxxxxxxxxx https://lists.freedesktop.org/mailman/listinfo/intel-gfx