[Intel-gfx] [PATCH 25/34] drm/i915: Track active timelines
Tvrtko Ursulin
tvrtko.ursulin at linux.intel.com
Tue Jan 22 14:56:32 UTC 2019
On 21/01/2019 22:21, Chris Wilson wrote:
> Now that we pin timelines around use, we have a clearly defined lifetime
> and convenient points at which we can track only the active timelines.
> This allows us to reduce the list iteration to only consider those
> active timelines and not all.
>
> Signed-off-by: Chris Wilson <chris at chris-wilson.co.uk>
> ---
> drivers/gpu/drm/i915/i915_drv.h | 2 +-
> drivers/gpu/drm/i915/i915_gem.c | 4 +--
> drivers/gpu/drm/i915/i915_reset.c | 2 +-
> drivers/gpu/drm/i915/i915_timeline.c | 39 ++++++++++++++++++----------
> 4 files changed, 29 insertions(+), 18 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index c00eaf2889fb..5577e0e1034f 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -1977,7 +1977,7 @@ struct drm_i915_private {
>
> struct i915_gt_timelines {
> struct mutex mutex; /* protects list, tainted by GPU */
> - struct list_head list;
> + struct list_head active_list;
>
> /* Pack multiple timelines' seqnos into the same page */
> spinlock_t hwsp_lock;
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 4e0de22f0166..9c499edb4c13 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -3246,7 +3246,7 @@ wait_for_timelines(struct drm_i915_private *i915,
> return timeout;
>
> mutex_lock(>->mutex);
> - list_for_each_entry(tl, >->list, link) {
> + list_for_each_entry(tl, >->active_list, link) {
> struct i915_request *rq;
>
> rq = i915_gem_active_get_unlocked(&tl->last_request);
> @@ -3274,7 +3274,7 @@ wait_for_timelines(struct drm_i915_private *i915,
>
> /* restart after reacquiring the lock */
> mutex_lock(>->mutex);
> - tl = list_entry(>->list, typeof(*tl), link);
> + tl = list_entry(>->active_list, typeof(*tl), link);
> }
> mutex_unlock(>->mutex);
>
> diff --git a/drivers/gpu/drm/i915/i915_reset.c b/drivers/gpu/drm/i915/i915_reset.c
> index 09edf488f711..9b9169508139 100644
> --- a/drivers/gpu/drm/i915/i915_reset.c
> +++ b/drivers/gpu/drm/i915/i915_reset.c
> @@ -852,7 +852,7 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
> * No more can be submitted until we reset the wedged bit.
> */
> mutex_lock(&i915->gt.timelines.mutex);
> - list_for_each_entry(tl, &i915->gt.timelines.list, link) {
> + list_for_each_entry(tl, &i915->gt.timelines.active_list, link) {
> struct i915_request *rq;
> long timeout;
>
> diff --git a/drivers/gpu/drm/i915/i915_timeline.c b/drivers/gpu/drm/i915/i915_timeline.c
> index 69ee33dfa340..007348b1b469 100644
> --- a/drivers/gpu/drm/i915/i915_timeline.c
> +++ b/drivers/gpu/drm/i915/i915_timeline.c
> @@ -117,7 +117,6 @@ int i915_timeline_init(struct drm_i915_private *i915,
> const char *name,
> struct i915_vma *hwsp)
> {
> - struct i915_gt_timelines *gt = &i915->gt.timelines;
> void *vaddr;
>
> /*
> @@ -161,10 +160,6 @@ int i915_timeline_init(struct drm_i915_private *i915,
>
> i915_syncmap_init(&timeline->sync);
>
> - mutex_lock(>->mutex);
> - list_add(&timeline->link, >->list);
> - mutex_unlock(>->mutex);
> -
> return 0;
> }
>
> @@ -173,7 +168,7 @@ void i915_timelines_init(struct drm_i915_private *i915)
> struct i915_gt_timelines *gt = &i915->gt.timelines;
>
> mutex_init(>->mutex);
> - INIT_LIST_HEAD(>->list);
> + INIT_LIST_HEAD(>->active_list);
>
> spin_lock_init(>->hwsp_lock);
> INIT_LIST_HEAD(>->hwsp_free_list);
> @@ -182,6 +177,24 @@ void i915_timelines_init(struct drm_i915_private *i915)
> i915_gem_shrinker_taints_mutex(i915, >->mutex);
> }
>
> +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..
> +
> /**
> * i915_timelines_park - called when the driver idles
> * @i915: the drm_i915_private device
> @@ -198,7 +211,7 @@ void i915_timelines_park(struct drm_i915_private *i915)
> struct i915_timeline *timeline;
>
> mutex_lock(>->mutex);
> - list_for_each_entry(timeline, >->list, link) {
> + list_for_each_entry(timeline, >->active_list, link) {
> /*
> * All known fences are completed so we can scrap
> * the current sync point tracking and start afresh,
> @@ -212,15 +225,9 @@ void i915_timelines_park(struct drm_i915_private *i915)
>
> void i915_timeline_fini(struct i915_timeline *timeline)
> {
> - struct i915_gt_timelines *gt = &timeline->i915->gt.timelines;
> -
> GEM_BUG_ON(timeline->pin_count);
> GEM_BUG_ON(!list_empty(&timeline->requests));
>
> - mutex_lock(>->mutex);
> - list_del(&timeline->link);
> - mutex_unlock(>->mutex);
> -
> i915_syncmap_free(&timeline->sync);
> hwsp_free(timeline);
>
> @@ -263,6 +270,8 @@ int i915_timeline_pin(struct i915_timeline *tl)
> if (err)
> goto unpin;
>
> + timeline_active(tl);
> +
> return 0;
>
> unpin:
> @@ -276,6 +285,8 @@ void i915_timeline_unpin(struct i915_timeline *tl)
> if (--tl->pin_count)
> return;
>
> + timeline_inactive(tl);
> +
> /*
> * Since this timeline is idle, all bariers upon which we were waiting
> * must also be complete and so we can discard the last used barriers
> @@ -299,7 +310,7 @@ void i915_timelines_fini(struct drm_i915_private *i915)
> {
> struct i915_gt_timelines *gt = &i915->gt.timelines;
>
> - GEM_BUG_ON(!list_empty(>->list));
> + GEM_BUG_ON(!list_empty(>->active_list));
> GEM_BUG_ON(!list_empty(>->hwsp_free_list));
>
> mutex_destroy(>->mutex);
>
Never mind the bikeshedding comments:
Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin at intel.com>
Regards,
Tvrtko
More information about the Intel-gfx
mailing list