Skip to content

Commit 1e34556

Browse files
ickleickle
committed
drm/i915: Move list of timelines under its own lock
Currently, the list of timelines is serialised by the struct_mutex, but to alleviate difficulties with using that mutex in future, move the list management under its own dedicated mutex. Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk> Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com> Link: https://patchwork.freedesktop.org/patch/msgid/20190128102356.15037-5-chris@chris-wilson.co.uk
1 parent 0ca88ba commit 1e34556

File tree

7 files changed

+109
-58
lines changed

7 files changed

+109
-58
lines changed

‎drivers/gpu/drm/i915/i915_drv.h

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1975,7 +1975,10 @@ struct drm_i915_private {
19751975
void (*resume)(struct drm_i915_private *);
19761976
void (*cleanup_engine)(struct intel_engine_cs *engine);
19771977

1978-
struct list_head timelines;
1978+
struct i915_gt_timelines {
1979+
struct mutex mutex; /* protects list, tainted by GPU */
1980+
struct list_head list;
1981+
} timelines;
19791982

19801983
struct list_head active_rings;
19811984
struct list_head closed_vma;

‎drivers/gpu/drm/i915/i915_gem.c

Lines changed: 56 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -3222,33 +3222,6 @@ i915_gem_wait_ioctl(struct drm_device *dev, void *data, struct drm_file *file)
32223222
return ret;
32233223
}
32243224

3225-
static long wait_for_timeline(struct i915_timeline *tl,
3226-
unsigned int flags, long timeout)
3227-
{
3228-
struct i915_request *rq;
3229-
3230-
rq = i915_gem_active_get_unlocked(&tl->last_request);
3231-
if (!rq)
3232-
return timeout;
3233-
3234-
/*
3235-
* "Race-to-idle".
3236-
*
3237-
* Switching to the kernel context is often used a synchronous
3238-
* step prior to idling, e.g. in suspend for flushing all
3239-
* current operations to memory before sleeping. These we
3240-
* want to complete as quickly as possible to avoid prolonged
3241-
* stalls, so allow the gpu to boost to maximum clocks.
3242-
*/
3243-
if (flags & I915_WAIT_FOR_IDLE_BOOST)
3244-
gen6_rps_boost(rq, NULL);
3245-
3246-
timeout = i915_request_wait(rq, flags, timeout);
3247-
i915_request_put(rq);
3248-
3249-
return timeout;
3250-
}
3251-
32523225
static int wait_for_engines(struct drm_i915_private *i915)
32533226
{
32543227
if (wait_for(intel_engines_are_idle(i915), I915_IDLE_ENGINES_TIMEOUT)) {
@@ -3262,6 +3235,52 @@ static int wait_for_engines(struct drm_i915_private *i915)
32623235
return 0;
32633236
}
32643237

3238+
static long
3239+
wait_for_timelines(struct drm_i915_private *i915,
3240+
unsigned int flags, long timeout)
3241+
{
3242+
struct i915_gt_timelines *gt = &i915->gt.timelines;
3243+
struct i915_timeline *tl;
3244+
3245+
if (!READ_ONCE(i915->gt.active_requests))
3246+
return timeout;
3247+
3248+
mutex_lock(&gt->mutex);
3249+
list_for_each_entry(tl, &gt->list, link) {
3250+
struct i915_request *rq;
3251+
3252+
rq = i915_gem_active_get_unlocked(&tl->last_request);
3253+
if (!rq)
3254+
continue;
3255+
3256+
mutex_unlock(&gt->mutex);
3257+
3258+
/*
3259+
* "Race-to-idle".
3260+
*
3261+
* Switching to the kernel context is often used a synchronous
3262+
* step prior to idling, e.g. in suspend for flushing all
3263+
* current operations to memory before sleeping. These we
3264+
* want to complete as quickly as possible to avoid prolonged
3265+
* stalls, so allow the gpu to boost to maximum clocks.
3266+
*/
3267+
if (flags & I915_WAIT_FOR_IDLE_BOOST)
3268+
gen6_rps_boost(rq, NULL);
3269+
3270+
timeout = i915_request_wait(rq, flags, timeout);
3271+
i915_request_put(rq);
3272+
if (timeout < 0)
3273+
return timeout;
3274+
3275+
/* restart after reacquiring the lock */
3276+
mutex_lock(&gt->mutex);
3277+
tl = list_entry(&gt->list, typeof(*tl), link);
3278+
}
3279+
mutex_unlock(&gt->mutex);
3280+
3281+
return timeout;
3282+
}
3283+
32653284
int i915_gem_wait_for_idle(struct drm_i915_private *i915,
32663285
unsigned int flags, long timeout)
32673286
{
@@ -3273,17 +3292,15 @@ int i915_gem_wait_for_idle(struct drm_i915_private *i915,
32733292
if (!READ_ONCE(i915->gt.awake))
32743293
return 0;
32753294

3295+
timeout = wait_for_timelines(i915, flags, timeout);
3296+
if (timeout < 0)
3297+
return timeout;
3298+
32763299
if (flags & I915_WAIT_LOCKED) {
3277-
struct i915_timeline *tl;
32783300
int err;
32793301

32803302
lockdep_assert_held(&i915->drm.struct_mutex);
32813303

3282-
list_for_each_entry(tl, &i915->gt.timelines, link) {
3283-
timeout = wait_for_timeline(tl, flags, timeout);
3284-
if (timeout < 0)
3285-
return timeout;
3286-
}
32873304
if (GEM_SHOW_DEBUG() && !timeout) {
32883305
/* Presume that timeout was non-zero to begin with! */
32893306
dev_warn(&i915->drm.pdev->dev,
@@ -3297,17 +3314,6 @@ int i915_gem_wait_for_idle(struct drm_i915_private *i915,
32973314

32983315
i915_retire_requests(i915);
32993316
GEM_BUG_ON(i915->gt.active_requests);
3300-
} else {
3301-
struct intel_engine_cs *engine;
3302-
enum intel_engine_id id;
3303-
3304-
for_each_engine(engine, i915, id) {
3305-
struct i915_timeline *tl = &engine->timeline;
3306-
3307-
timeout = wait_for_timeline(tl, flags, timeout);
3308-
if (timeout < 0)
3309-
return timeout;
3310-
}
33113317
}
33123318

33133319
return 0;
@@ -5008,6 +5014,8 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
50085014
dev_priv->gt.cleanup_engine = intel_engine_cleanup;
50095015
}
50105016

5017+
i915_timelines_init(dev_priv);
5018+
50115019
ret = i915_gem_init_userptr(dev_priv);
50125020
if (ret)
50135021
return ret;
@@ -5130,8 +5138,10 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
51305138
err_uc_misc:
51315139
intel_uc_fini_misc(dev_priv);
51325140

5133-
if (ret != -EIO)
5141+
if (ret != -EIO) {
51345142
i915_gem_cleanup_userptr(dev_priv);
5143+
i915_timelines_fini(dev_priv);
5144+
}
51355145

51365146
if (ret == -EIO) {
51375147
mutex_lock(&dev_priv->drm.struct_mutex);
@@ -5182,6 +5192,7 @@ void i915_gem_fini(struct drm_i915_private *dev_priv)
51825192

51835193
intel_uc_fini_misc(dev_priv);
51845194
i915_gem_cleanup_userptr(dev_priv);
5195+
i915_timelines_fini(dev_priv);
51855196

51865197
i915_gem_drain_freed_objects(dev_priv);
51875198

@@ -5284,7 +5295,6 @@ int i915_gem_init_early(struct drm_i915_private *dev_priv)
52845295
if (!dev_priv->priorities)
52855296
goto err_dependencies;
52865297

5287-
INIT_LIST_HEAD(&dev_priv->gt.timelines);
52885298
INIT_LIST_HEAD(&dev_priv->gt.active_rings);
52895299
INIT_LIST_HEAD(&dev_priv->gt.closed_vma);
52905300

@@ -5328,7 +5338,6 @@ void i915_gem_cleanup_early(struct drm_i915_private *dev_priv)
53285338
GEM_BUG_ON(!llist_empty(&dev_priv->mm.free_list));
53295339
GEM_BUG_ON(atomic_read(&dev_priv->mm.free_count));
53305340
WARN_ON(dev_priv->mm.object_count);
5331-
WARN_ON(!list_empty(&dev_priv->gt.timelines));
53325341

53335342
kmem_cache_destroy(dev_priv->priorities);
53345343
kmem_cache_destroy(dev_priv->dependencies);

‎drivers/gpu/drm/i915/i915_reset.c

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -854,7 +854,8 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
854854
*
855855
* No more can be submitted until we reset the wedged bit.
856856
*/
857-
list_for_each_entry(tl, &i915->gt.timelines, link) {
857+
mutex_lock(&i915->gt.timelines.mutex);
858+
list_for_each_entry(tl, &i915->gt.timelines.list, link) {
858859
struct i915_request *rq;
859860
long timeout;
860861

@@ -876,9 +877,12 @@ bool i915_gem_unset_wedged(struct drm_i915_private *i915)
876877
timeout = dma_fence_default_wait(&rq->fence, true,
877878
MAX_SCHEDULE_TIMEOUT);
878879
i915_request_put(rq);
879-
if (timeout < 0)
880+
if (timeout < 0) {
881+
mutex_unlock(&i915->gt.timelines.mutex);
880882
goto unlock;
883+
}
881884
}
885+
mutex_unlock(&i915->gt.timelines.mutex);
882886

883887
intel_engines_sanitize(i915, false);
884888

‎drivers/gpu/drm/i915/i915_timeline.c

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ void i915_timeline_init(struct drm_i915_private *i915,
1313
struct i915_timeline *timeline,
1414
const char *name)
1515
{
16-
lockdep_assert_held(&i915->drm.struct_mutex);
16+
struct i915_gt_timelines *gt = &i915->gt.timelines;
1717

1818
/*
1919
* Ideally we want a set of engines on a single leaf as we expect
@@ -23,9 +23,12 @@ void i915_timeline_init(struct drm_i915_private *i915,
2323
*/
2424
BUILD_BUG_ON(KSYNCMAP < I915_NUM_ENGINES);
2525

26+
timeline->i915 = i915;
2627
timeline->name = name;
2728

28-
list_add(&timeline->link, &i915->gt.timelines);
29+
mutex_lock(&gt->mutex);
30+
list_add(&timeline->link, &gt->list);
31+
mutex_unlock(&gt->mutex);
2932

3033
/* Called during early_init before we know how many engines there are */
3134

@@ -39,6 +42,17 @@ void i915_timeline_init(struct drm_i915_private *i915,
3942
i915_syncmap_init(&timeline->sync);
4043
}
4144

45+
void i915_timelines_init(struct drm_i915_private *i915)
46+
{
47+
struct i915_gt_timelines *gt = &i915->gt.timelines;
48+
49+
mutex_init(&gt->mutex);
50+
INIT_LIST_HEAD(&gt->list);
51+
52+
/* via i915_gem_wait_for_idle() */
53+
i915_gem_shrinker_taints_mutex(i915, &gt->mutex);
54+
}
55+
4256
/**
4357
* i915_timelines_park - called when the driver idles
4458
* @i915: the drm_i915_private device
@@ -51,11 +65,11 @@ void i915_timeline_init(struct drm_i915_private *i915,
5165
*/
5266
void i915_timelines_park(struct drm_i915_private *i915)
5367
{
68+
struct i915_gt_timelines *gt = &i915->gt.timelines;
5469
struct i915_timeline *timeline;
5570

56-
lockdep_assert_held(&i915->drm.struct_mutex);
57-
58-
list_for_each_entry(timeline, &i915->gt.timelines, link) {
71+
mutex_lock(&gt->mutex);
72+
list_for_each_entry(timeline, &gt->list, link) {
5973
/*
6074
* All known fences are completed so we can scrap
6175
* the current sync point tracking and start afresh,
@@ -64,15 +78,20 @@ void i915_timelines_park(struct drm_i915_private *i915)
6478
*/
6579
i915_syncmap_free(&timeline->sync);
6680
}
81+
mutex_unlock(&gt->mutex);
6782
}
6883

6984
void i915_timeline_fini(struct i915_timeline *timeline)
7085
{
86+
struct i915_gt_timelines *gt = &timeline->i915->gt.timelines;
87+
7188
GEM_BUG_ON(!list_empty(&timeline->requests));
7289

7390
i915_syncmap_free(&timeline->sync);
7491

92+
mutex_lock(&gt->mutex);
7593
list_del(&timeline->link);
94+
mutex_unlock(&gt->mutex);
7695
}
7796

7897
struct i915_timeline *
@@ -99,6 +118,15 @@ void __i915_timeline_free(struct kref *kref)
99118
kfree(timeline);
100119
}
101120

121+
void i915_timelines_fini(struct drm_i915_private *i915)
122+
{
123+
struct i915_gt_timelines *gt = &i915->gt.timelines;
124+
125+
GEM_BUG_ON(!list_empty(&gt->list));
126+
127+
mutex_destroy(&gt->mutex);
128+
}
129+
102130
#if IS_ENABLED(CONFIG_DRM_I915_SELFTEST)
103131
#include "selftests/mock_timeline.c"
104132
#include "selftests/i915_timeline.c"

‎drivers/gpu/drm/i915/i915_timeline.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ struct i915_timeline {
6666

6767
struct list_head link;
6868
const char *name;
69+
struct drm_i915_private *i915;
6970

7071
struct kref kref;
7172
};
@@ -134,6 +135,8 @@ static inline bool i915_timeline_sync_is_later(struct i915_timeline *tl,
134135
return __i915_timeline_sync_is_later(tl, fence->context, fence->seqno);
135136
}
136137

138+
void i915_timelines_init(struct drm_i915_private *i915);
137139
void i915_timelines_park(struct drm_i915_private *i915);
140+
void i915_timelines_fini(struct drm_i915_private *i915);
138141

139142
#endif

‎drivers/gpu/drm/i915/selftests/mock_gem_device.c

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -68,13 +68,14 @@ static void mock_device_release(struct drm_device *dev)
6868
i915_gem_contexts_fini(i915);
6969
mutex_unlock(&i915->drm.struct_mutex);
7070

71+
i915_timelines_fini(i915);
72+
7173
drain_workqueue(i915->wq);
7274
i915_gem_drain_freed_objects(i915);
7375

7476
mutex_lock(&i915->drm.struct_mutex);
7577
mock_fini_ggtt(&i915->ggtt);
7678
mutex_unlock(&i915->drm.struct_mutex);
77-
WARN_ON(!list_empty(&i915->gt.timelines));
7879

7980
destroy_workqueue(i915->wq);
8081

@@ -226,7 +227,8 @@ struct drm_i915_private *mock_gem_device(void)
226227
if (!i915->priorities)
227228
goto err_dependencies;
228229

229-
INIT_LIST_HEAD(&i915->gt.timelines);
230+
i915_timelines_init(i915);
231+
230232
INIT_LIST_HEAD(&i915->gt.active_rings);
231233
INIT_LIST_HEAD(&i915->gt.closed_vma);
232234

@@ -253,6 +255,7 @@ struct drm_i915_private *mock_gem_device(void)
253255
i915_gem_contexts_fini(i915);
254256
err_unlock:
255257
mutex_unlock(&i915->drm.struct_mutex);
258+
i915_timelines_fini(i915);
256259
kmem_cache_destroy(i915->priorities);
257260
err_dependencies:
258261
kmem_cache_destroy(i915->dependencies);

‎drivers/gpu/drm/i915/selftests/mock_timeline.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010

1111
void mock_timeline_init(struct i915_timeline *timeline, u64 context)
1212
{
13+
timeline->i915 = NULL;
1314
timeline->fence_context = context;
1415

1516
spin_lock_init(&timeline->lock);
@@ -24,5 +25,5 @@ void mock_timeline_init(struct i915_timeline *timeline, u64 context)
2425

2526
void mock_timeline_fini(struct i915_timeline *timeline)
2627
{
27-
i915_timeline_fini(timeline);
28+
i915_syncmap_free(&timeline->sync);
2829
}

0 commit comments

Comments
 (0)