Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

drm/i915: Pull seqno started checks together

We have a few instances of checking seqno-1 to see if the HW has started
the request. Pull those together under a helper.

v2: Pull the !seqno assertion higher, as given seqno==1 we may indeed
check to see if we have started using seqno==0.

Suggested-by: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Signed-off-by: Chris Wilson <chris@chris-wilson.co.uk>
Cc: Tvrtko Ursulin <tvrtko.ursulin@intel.com>
Reviewed-by: Mika Kuoppala <mika.kuoppala@linux.intel.com>
Link: https://patchwork.freedesktop.org/patch/msgid/20180806112605.20725-1-chris@chris-wilson.co.uk

+60 -32
+4 -5
drivers/gpu/drm/i915/i915_request.c
··· 527 527 528 528 seqno = timeline_get_seqno(&engine->timeline); 529 529 GEM_BUG_ON(!seqno); 530 - GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), seqno)); 530 + GEM_BUG_ON(intel_engine_signaled(engine, seqno)); 531 531 532 532 /* We may be recursing from the signal callback of another i915 fence */ 533 533 spin_lock_nested(&request->lock, SINGLE_DEPTH_NESTING); ··· 579 579 */ 580 580 GEM_BUG_ON(!request->global_seqno); 581 581 GEM_BUG_ON(request->global_seqno != engine->timeline.seqno); 582 - GEM_BUG_ON(i915_seqno_passed(intel_engine_get_seqno(engine), 583 - request->global_seqno)); 582 + GEM_BUG_ON(intel_engine_has_completed(engine, request->global_seqno)); 584 583 engine->timeline.seqno--; 585 584 586 585 /* We may be recursing from the signal callback of another i915 fence */ ··· 1204 1205 * it is a fair assumption that it will not complete within our 1205 1206 * relatively short timeout. 1206 1207 */ 1207 - if (!i915_seqno_passed(intel_engine_get_seqno(engine), seqno - 1)) 1208 + if (!intel_engine_has_started(engine, seqno)) 1208 1209 return false; 1209 1210 1210 1211 /* ··· 1221 1222 irq = READ_ONCE(engine->breadcrumbs.irq_count); 1222 1223 timeout_us += local_clock_us(&cpu); 1223 1224 do { 1224 - if (i915_seqno_passed(intel_engine_get_seqno(engine), seqno)) 1225 + if (intel_engine_has_completed(engine, seqno)) 1225 1226 return seqno == i915_request_global_seqno(rq); 1226 1227 1227 1228 /*
+25 -14
drivers/gpu/drm/i915/i915_request.h
··· 272 272 #define I915_WAIT_ALL BIT(2) /* used by i915_gem_object_wait() */ 273 273 #define I915_WAIT_FOR_IDLE_BOOST BIT(3) 274 274 275 - static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine); 275 + static inline bool intel_engine_has_started(struct intel_engine_cs *engine, 276 + u32 seqno); 277 + static inline bool intel_engine_has_completed(struct intel_engine_cs *engine, 278 + u32 seqno); 276 279 277 280 /** 278 281 * Returns true if seq1 is later than seq2. ··· 285 282 return (s32)(seq1 - seq2) >= 0; 286 283 } 287 284 285 + /** 286 + * i915_request_started - check if the request has begun being executed 287 + * @rq: the request 288 + * 289 + * Returns true if the request has been submitted to hardware, and the hardware 290 + * has advanced passed the end of the previous request and so should be either 291 + * currently processing the request (though it may be preempted and so 292 + * not necessarily the next request to complete) or have completed the request. 293 + */ 294 + static inline bool i915_request_started(const struct i915_request *rq) 295 + { 296 + u32 seqno; 297 + 298 + seqno = i915_request_global_seqno(rq); 299 + if (!seqno) /* not yet submitted to HW */ 300 + return false; 301 + 302 + return intel_engine_has_started(rq->engine, seqno); 303 + } 304 + 288 305 static inline bool 289 306 __i915_request_completed(const struct i915_request *rq, u32 seqno) 290 307 { 291 308 GEM_BUG_ON(!seqno); 292 - return i915_seqno_passed(intel_engine_get_seqno(rq->engine), seqno) && 309 + return intel_engine_has_completed(rq->engine, seqno) && 293 310 seqno == i915_request_global_seqno(rq); 294 311 } 295 312 ··· 322 299 return false; 323 300 324 301 return __i915_request_completed(rq, seqno); 325 - } 326 - 327 - static inline bool i915_request_started(const struct i915_request *rq) 328 - { 329 - u32 seqno; 330 - 331 - seqno = i915_request_global_seqno(rq); 332 - if (!seqno) 333 - return false; 334 - 335 - return i915_seqno_passed(intel_engine_get_seqno(rq->engine), 336 - seqno - 1); 337 302 } 338 303 339 304 static inline bool i915_sched_node_signaled(const struct i915_sched_node *node)
+2 -4
drivers/gpu/drm/i915/intel_breadcrumbs.c
··· 256 256 spin_unlock(&b->irq_lock); 257 257 258 258 rbtree_postorder_for_each_entry_safe(wait, n, &b->waiters, node) { 259 - GEM_BUG_ON(!i915_seqno_passed(intel_engine_get_seqno(engine), 260 - wait->seqno)); 259 + GEM_BUG_ON(!intel_engine_signaled(engine, wait->seqno)); 261 260 RB_CLEAR_NODE(&wait->node); 262 261 wake_up_process(wait->tsk); 263 262 } ··· 507 508 return armed; 508 509 509 510 /* Make the caller recheck if its request has already started. */ 510 - return i915_seqno_passed(intel_engine_get_seqno(engine), 511 - wait->seqno - 1); 511 + return intel_engine_has_started(engine, wait->seqno); 512 512 } 513 513 514 514 static inline bool chain_wakeup(struct rb_node *rb, int priority)
+1 -2
drivers/gpu/drm/i915/intel_engine_cs.c
··· 968 968 return true; 969 969 970 970 /* Any inflight/incomplete requests? */ 971 - if (!i915_seqno_passed(intel_engine_get_seqno(engine), 972 - intel_engine_last_submit(engine))) 971 + if (!intel_engine_signaled(engine, intel_engine_last_submit(engine))) 973 972 return false; 974 973 975 974 if (I915_SELFTEST_ONLY(engine->breadcrumbs.mock))
+1 -1
drivers/gpu/drm/i915/intel_hangcheck.c
··· 142 142 if (signaller->hangcheck.deadlock >= I915_NUM_ENGINES) 143 143 return -1; 144 144 145 - if (i915_seqno_passed(intel_engine_get_seqno(signaller), seqno)) 145 + if (intel_engine_signaled(signaller, seqno)) 146 146 return 1; 147 147 148 148 /* cursory check for an unkickable deadlock */
+27 -6
drivers/gpu/drm/i915/intel_ringbuffer.h
··· 910 910 u64 intel_engine_get_active_head(const struct intel_engine_cs *engine); 911 911 u64 intel_engine_get_last_batch_head(const struct intel_engine_cs *engine); 912 912 913 - static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine) 914 - { 915 - return intel_read_status_page(engine, I915_GEM_HWS_INDEX); 916 - } 917 - 918 913 static inline u32 intel_engine_last_submit(struct intel_engine_cs *engine) 919 914 { 920 - /* We are only peeking at the tail of the submit queue (and not the 915 + /* 916 + * We are only peeking at the tail of the submit queue (and not the 921 917 * queue itself) in order to gain a hint as to the current active 922 918 * state of the engine. Callers are not expected to be taking 923 919 * engine->timeline->lock, nor are they expected to be concerned ··· 921 925 * a hint and nothing more. 922 926 */ 923 927 return READ_ONCE(engine->timeline.seqno); 928 + } 929 + 930 + static inline u32 intel_engine_get_seqno(struct intel_engine_cs *engine) 931 + { 932 + return intel_read_status_page(engine, I915_GEM_HWS_INDEX); 933 + } 934 + 935 + static inline bool intel_engine_signaled(struct intel_engine_cs *engine, 936 + u32 seqno) 937 + { 938 + return i915_seqno_passed(intel_engine_get_seqno(engine), seqno); 939 + } 940 + 941 + static inline bool intel_engine_has_completed(struct intel_engine_cs *engine, 942 + u32 seqno) 943 + { 944 + GEM_BUG_ON(!seqno); 945 + return intel_engine_signaled(engine, seqno); 946 + } 947 + 948 + static inline bool intel_engine_has_started(struct intel_engine_cs *engine, 949 + u32 seqno) 950 + { 951 + GEM_BUG_ON(!seqno); 952 + return intel_engine_signaled(engine, seqno - 1); 924 953 } 925 954 926 955 void intel_engine_get_instdone(struct intel_engine_cs *engine,