* [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags
@ 2023-10-04 18:36 Jonathan Cavitt
2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines Jonathan Cavitt
` (7 more replies)
0 siblings, 8 replies; 15+ messages in thread
From: Jonathan Cavitt @ 2023-10-04 18:36 UTC (permalink / raw)
To: intel-gfx
Cc: janusz.krzysztofik, andi.shyti, matthew.d.roper, jonathan.cavitt,
chris.p.wilson, nirmoy.das
Add pci (device info) tags for if GuC TLB Invalidation is enabled.
Since GuC based TLB invalidation is only strictly necessary for MTL
resently, only enable GuC based TLB invalidations for MTL.
Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com>
---
drivers/gpu/drm/i915/i915_drv.h | 1 +
drivers/gpu/drm/i915/i915_pci.c | 1 +
drivers/gpu/drm/i915/intel_device_info.h | 3 ++-
3 files changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 2b7a6db4d0d44..1e25cc1e3dba1 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -807,4 +807,5 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
#define HAS_LMEMBAR_SMEM_STOLEN(i915) (!HAS_LMEM(i915) && \
GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70))
+#define HAS_GUC_TLB_INVALIDATION(i915) (INTEL_INFO(i915)->has_guc_tlb_invalidation)
#endif
diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c
index df7c261410f79..c3a5d5efb45d1 100644
--- a/drivers/gpu/drm/i915/i915_pci.c
+++ b/drivers/gpu/drm/i915/i915_pci.c
@@ -837,6 +837,7 @@ static const struct intel_device_info mtl_info = {
.memory_regions = REGION_SMEM | REGION_STOLEN_LMEM,
.platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0),
.require_force_probe = 1,
+ .has_guc_tlb_invalidation = 1,
MTL_CACHELEVEL,
};
diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h
index 39817490b13fd..ad54db0a22470 100644
--- a/drivers/gpu/drm/i915/intel_device_info.h
+++ b/drivers/gpu/drm/i915/intel_device_info.h
@@ -173,7 +173,8 @@ enum intel_ppgtt_type {
func(has_coherent_ggtt); \
func(tuning_thread_rr_after_dep); \
func(unfenced_needs_alignment); \
- func(hws_needs_physical);
+ func(hws_needs_physical); \
+ func(has_guc_tlb_invalidation);
struct intel_ip_version {
u8 ver;
--
2.25.1
^ permalink raw reply related [flat|nested] 15+ messages in thread* [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt @ 2023-10-04 18:36 ` Jonathan Cavitt 2023-10-05 8:54 ` Tvrtko Ursulin 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 3/4] drm/i915: No TLB invalidation on wedged or suspended GT Jonathan Cavitt ` (6 subsequent siblings) 7 siblings, 1 reply; 15+ messages in thread From: Jonathan Cavitt @ 2023-10-04 18:36 UTC (permalink / raw) To: intel-gfx Cc: janusz.krzysztofik, andi.shyti, matthew.d.roper, jonathan.cavitt, chris.p.wilson, nirmoy.das From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> The GuC firmware had defined the interface for Translation Look-Aside Buffer (TLB) invalidation. We should use this interface when invalidating the engine and GuC TLBs. Add additional functionality to intel_gt_invalidate_tlb, invalidating the GuC TLBs and falling back to GT invalidation when the GuC is disabled. The invalidation is done by sending a request directly to the GuC tlb_lookup that invalidates the table. The invalidation is submitted as a wait request and is performed in the CT event handler. This means we cannot perform this TLB invalidation path if the CT is not enabled. If the request isn't fulfilled in two seconds, this would constitute an error in the invalidation as that would constitute either a lost request or a severe GuC overload. The tlb_lookup table is allocated as an xarray because the set of pending TLB invalidations may have no upper bound. The consequence of this is that all actions interfacting with this table need to use the xarray functions, such as xa_alloc_cyclic_irq for array insertion. With this new invalidation routine, we can perform GuC-based GGTT invalidations. GuC-based GGTT invalidation is incompatible with MMIO invalidation so we should not perform MMIO invalidation when GuC-based GGTT invalidation is expected. Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> Signed-off-by: Bruce Chang <yu.bruce.chang@intel.com> Signed-off-by: Chris Wilson <chris.p.wilson@intel.com> Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> Signed-off-by: Fei Yang <fei.yang@intel.com> CC: Andi Shyti <andi.shyti@linux.intel.com> --- drivers/gpu/drm/i915/gt/intel_ggtt.c | 42 ++-- drivers/gpu/drm/i915/gt/intel_tlb.c | 14 +- .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 33 +++ drivers/gpu/drm/i915/gt/uc/intel_guc.h | 22 ++ drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 6 + drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 211 +++++++++++++++++- 7 files changed, 313 insertions(+), 16 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c index 4d7d88b92632b..be8c216d29f91 100644 --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c @@ -206,22 +206,38 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN); } -static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) +static void guc_ggtt_ct_invalidate(struct intel_gt *gt) { - struct drm_i915_private *i915 = ggtt->vm.i915; + struct intel_uncore *uncore = gt->uncore; + intel_wakeref_t wakeref; - gen8_ggtt_invalidate(ggtt); + with_intel_runtime_pm_if_active(uncore->rpm, wakeref) { + struct intel_guc *guc = >->uc.guc; - if (GRAPHICS_VER(i915) >= 12) { - struct intel_gt *gt; + intel_guc_invalidate_tlb(guc); + } +} - list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) - intel_uncore_write_fw(gt->uncore, - GEN12_GUC_TLB_INV_CR, - GEN12_GUC_TLB_INV_CR_INVALIDATE); - } else { - intel_uncore_write_fw(ggtt->vm.gt->uncore, - GEN8_GTCR, GEN8_GTCR_INVALIDATE); +static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) +{ + struct drm_i915_private *i915 = ggtt->vm.i915; + struct intel_gt *gt; + + if (!IS_GEN9_LP(i915) && GRAPHICS_VER(i915) < 11) + gen8_ggtt_invalidate(ggtt); + + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) { + if (HAS_GUC_TLB_INVALIDATION(i915) && + intel_guc_is_ready(>->uc.guc)) { + guc_ggtt_ct_invalidate(gt); + } else if (GRAPHICS_VER(i915) >= 12) { + intel_uncore_write(gt->uncore, + GEN12_GUC_TLB_INV_CR, + GEN12_GUC_TLB_INV_CR_INVALIDATE); + } else { + intel_uncore_write(gt->uncore, + GEN8_GTCR, GEN8_GTCR_INVALIDATE); + } } } @@ -1243,7 +1259,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) ggtt->vm.raw_insert_page = gen8_ggtt_insert_page; } - if (intel_uc_wants_guc(&ggtt->vm.gt->uc)) + if (intel_uc_wants_guc_submission(&ggtt->vm.gt->uc)) ggtt->invalidate = guc_ggtt_invalidate; else ggtt->invalidate = gen8_ggtt_invalidate; diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c b/drivers/gpu/drm/i915/gt/intel_tlb.c index 139608c30d978..a84563c178bc6 100644 --- a/drivers/gpu/drm/i915/gt/intel_tlb.c +++ b/drivers/gpu/drm/i915/gt/intel_tlb.c @@ -12,6 +12,7 @@ #include "intel_gt_print.h" #include "intel_gt_regs.h" #include "intel_tlb.h" +#include "uc/intel_guc.h" /* * HW architecture suggest typical invalidation time at 40us, @@ -131,11 +132,22 @@ void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno) return; with_intel_gt_pm_if_awake(gt, wakeref) { + struct intel_guc *guc = >->uc.guc; + mutex_lock(>->tlb.invalidate_lock); if (tlb_seqno_passed(gt, seqno)) goto unlock; - mmio_invalidate_full(gt); + if (HAS_GUC_TLB_INVALIDATION(gt->i915)) { + /* + * Only perform GuC TLB invalidation if GuC is ready. + * Otherwise, skip invalidation altogeter. + */ + if (intel_guc_is_ready(guc)) + intel_guc_invalidate_tlb_full(guc); + } else { + mmio_invalidate_full(gt); + } write_seqcount_invalidate(>->tlb.seqno); unlock: diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h index f359bef046e0b..9dff8012d5e76 100644 --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h @@ -138,6 +138,8 @@ enum intel_guc_action { INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507, INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A, + INTEL_GUC_ACTION_TLB_INVALIDATION = 0x7000, + INTEL_GUC_ACTION_TLB_INVALIDATION_DONE = 0x7001, INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002, INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003, INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004, @@ -181,4 +183,35 @@ enum intel_guc_state_capture_event_status { #define INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_MASK 0x000000FF +#define INTEL_GUC_TLB_INVAL_TYPE_MASK REG_GENMASK(7, 0) +#define INTEL_GUC_TLB_INVAL_MODE_MASK REG_GENMASK(11, 8) +#define INTEL_GUC_TLB_INVAL_FLUSH_CACHE REG_BIT(31) + +enum intel_guc_tlb_invalidation_type { + INTEL_GUC_TLB_INVAL_FULL = 0x0, + INTEL_GUC_TLB_INVAL_GUC = 0x3, +}; + +/* + * 0: Heavy mode of Invalidation: + * The pipeline of the engine(s) for which the invalidation is targeted to is + * blocked, and all the in-flight transactions are guaranteed to be Globally + * Observed before completing the TLB invalidation + * 1: Lite mode of Invalidation: + * TLBs of the targeted engine(s) are immediately invalidated. + * In-flight transactions are NOT guaranteed to be Globally Observed before + * completing TLB invalidation. + * Light Invalidation Mode is to be used only when + * it can be guaranteed (by SW) that the address translations remain invariant + * for the in-flight transactions across the TLB invalidation. In other words, + * this mode can be used when the TLB invalidation is intended to clear out the + * stale cached translations that are no longer in use. Light Invalidation Mode + * is much faster than the Heavy Invalidation Mode, as it does not wait for the + * in-flight transactions to be GOd. + */ +enum intel_guc_tlb_inval_mode { + INTEL_GUC_TLB_INVAL_MODE_HEAVY = 0x0, + INTEL_GUC_TLB_INVAL_MODE_LITE = 0x1, +}; + #endif /* _ABI_GUC_ACTIONS_ABI_H */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index 6c392bad29c19..d7203ba88b0c0 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -79,6 +79,18 @@ struct intel_guc { */ atomic_t outstanding_submission_g2h; + /** @tlb_lookup: xarray to store all pending TLB invalidation requests */ + struct xarray tlb_lookup; + + /** + * @serial_slot: id to the initial waiter created in tlb_lookup, + * which is used only when failed to allocate new waiter. + */ + u32 serial_slot; + + /** @next_seqno: the next id (sequence no.) to allocate. */ + u32 next_seqno; + /** @interrupts: pointers to GuC interrupt-managing functions. */ struct { bool enabled; @@ -296,6 +308,11 @@ struct intel_guc { #define MAKE_GUC_VER_STRUCT(ver) MAKE_GUC_VER((ver).major, (ver).minor, (ver).patch) #define GUC_SUBMIT_VER(guc) MAKE_GUC_VER_STRUCT((guc)->submission_version) +struct intel_guc_tlb_wait { + struct wait_queue_head wq; + bool busy; +}; + static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) { return container_of(log, struct intel_guc, log); @@ -418,6 +435,11 @@ static inline bool intel_guc_is_supported(struct intel_guc *guc) return intel_uc_fw_is_supported(&guc->fw); } +int intel_guc_invalidate_tlb_full(struct intel_guc *guc); +int intel_guc_invalidate_tlb(struct intel_guc *guc); +int intel_guc_tlb_invalidation_done(struct intel_guc *guc, const u32 *hxg, + u32 size); + static inline bool intel_guc_is_wanted(struct intel_guc *guc) { return intel_uc_fw_is_enabled(&guc->fw); diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c index 6e22af31513a5..1ee4d4a988398 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c @@ -1115,6 +1115,11 @@ static int ct_process_request(struct intel_guc_ct *ct, struct ct_incoming_msg *r case INTEL_GUC_ACTION_NOTIFY_EXCEPTION: ret = intel_guc_crash_process_msg(guc, action); break; + case INTEL_GUC_ACTION_TLB_INVALIDATION_DONE: + ret = intel_guc_tlb_invalidation_done(ct_to_guc(ct), hxg, request->size); + if (unlikely(ret)) + ct_free_msg(request); + break; default: ret = -EOPNOTSUPP; break; @@ -1186,6 +1191,7 @@ static int ct_handle_event(struct intel_guc_ct *ct, struct ct_incoming_msg *requ switch (action) { case INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_DONE: case INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE: + case INTEL_GUC_ACTION_TLB_INVALIDATION_DONE: g2h_release_space(ct, request->size); } diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h index b4d56eccfb1f0..a7c9874e122a3 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h @@ -22,6 +22,7 @@ /* Payload length only i.e. don't include G2H header length */ #define G2H_LEN_DW_SCHED_CONTEXT_MODE_SET 2 #define G2H_LEN_DW_DEREGISTER_CONTEXT 1 +#define G2H_LEN_DW_INVALIDATE_TLB 1 #define GUC_CONTEXT_DISABLE 0 #define GUC_CONTEXT_ENABLE 1 diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 2cce5ec1ff00d..12f79abd889ce 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -1798,9 +1798,11 @@ static void __guc_reset_context(struct intel_context *ce, intel_engine_mask_t st void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stalled) { + struct intel_guc_tlb_wait *wait; struct intel_context *ce; unsigned long index; unsigned long flags; + unsigned long i; if (unlikely(!guc_submission_initialized(guc))) { /* Reset called during driver load? GuC not yet initialised! */ @@ -1826,6 +1828,14 @@ void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stall /* GuC is blown away, drop all references to contexts */ xa_destroy(&guc->context_lookup); + + /* + * The full GT reset will have cleared the TLB caches and flushed the + * G2H message queue; we can release all the blocked waiters. + */ + xa_for_each(&guc->tlb_lookup, i, wait) { + wake_up(&wait->wq); + } } static void guc_cancel_context_requests(struct intel_context *ce) @@ -1948,6 +1958,49 @@ void intel_guc_submission_reset_finish(struct intel_guc *guc) static void destroyed_worker_func(struct work_struct *w); static void reset_fail_worker_func(struct work_struct *w); +static int init_tlb_lookup(struct intel_guc *guc) +{ + struct intel_guc_tlb_wait *wait; + int err; + + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) + return 0; + + xa_init_flags(&guc->tlb_lookup, XA_FLAGS_ALLOC); + + wait = kzalloc(sizeof(*wait), GFP_KERNEL); + if (!wait) + return -ENOMEM; + + init_waitqueue_head(&wait->wq); + + /* Preallocate a shared id for use under memory pressure. */ + err = xa_alloc_cyclic_irq(&guc->tlb_lookup, &guc->serial_slot, wait, + xa_limit_32b, &guc->next_seqno, GFP_KERNEL); + if (err == -ENOMEM) { + kfree(wait); + return err; + } + + return 0; +} + +static void fini_tlb_lookup(struct intel_guc *guc) +{ + struct intel_guc_tlb_wait *wait; + + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) + return; + + wait = xa_load(&guc->tlb_lookup, guc->serial_slot); + if (wait) { + GEM_BUG_ON(wait->busy); + kfree(wait); + } + + xa_destroy(&guc->tlb_lookup); +} + /* * Set up the memory resources to be shared with the GuC (via the GGTT) * at firmware loading time. @@ -1966,11 +2019,15 @@ int intel_guc_submission_init(struct intel_guc *guc) return ret; } + ret = init_tlb_lookup(guc); + if (ret) + goto destroy_pool; + guc->submission_state.guc_ids_bitmap = bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); if (!guc->submission_state.guc_ids_bitmap) { ret = -ENOMEM; - goto destroy_pool; + goto destroy_tlb; } guc->timestamp.ping_delay = (POLL_TIME_CLKS / gt->clock_frequency + 1) * HZ; @@ -1979,9 +2036,10 @@ int intel_guc_submission_init(struct intel_guc *guc) return 0; +destroy_tlb: + fini_tlb_lookup(guc); destroy_pool: guc_lrc_desc_pool_destroy_v69(guc); - return ret; } @@ -1994,6 +2052,7 @@ void intel_guc_submission_fini(struct intel_guc *guc) guc_lrc_desc_pool_destroy_v69(guc); i915_sched_engine_put(guc->sched_engine); bitmap_free(guc->submission_state.guc_ids_bitmap); + fini_tlb_lookup(guc); guc->submission_initialized = false; } @@ -4624,6 +4683,154 @@ g2h_context_lookup(struct intel_guc *guc, u32 ctx_id) return ce; } +static void wait_wake_outstanding_tlb_g2h(struct intel_guc *guc, u32 seqno) +{ + struct intel_guc_tlb_wait *wait; + unsigned long flags; + + xa_lock_irqsave(&guc->tlb_lookup, flags); + wait = xa_load(&guc->tlb_lookup, seqno); + + /* We received a response after the waiting task did exit with a timeout */ + if (wait) + wake_up(&wait->wq); + else + drm_dbg(&guc_to_gt(guc)->i915->drm, + "Stale TLB invalidation response with seqno %d\n", seqno); + + xa_unlock_irqrestore(&guc->tlb_lookup, flags); +} + +int intel_guc_tlb_invalidation_done(struct intel_guc *guc, const u32 *hxg, u32 size) +{ + u32 seqno, hxg_len, len; + + hxg_len = size - GUC_CTB_MSG_MIN_LEN; + len = hxg_len - GUC_HXG_MSG_MIN_LEN; + + /* Check for underflow */ + if (unlikely(len < 1 || len > size)) + return -EPROTO; + + seqno = hxg[GUC_HXG_MSG_MIN_LEN]; + wait_wake_outstanding_tlb_g2h(guc, seqno); + return 0; +} + +static long must_wait_woken(struct wait_queue_entry *wq_entry, long timeout) +{ + /* + * This is equivalent to wait_woken() with the exception that + * we do not wake up early if the kthread task has been completed. + * As we are called from page reclaim in any task context, + * we may be invoked from stopped kthreads, but we *must* + * complete the wait from the HW . + * + * A second problem is that since we are called under reclaim + * and wait_woken() inspected the thread state, it makes an invalid + * assumption that all PF_KTHREAD tasks have set_kthread_struct() + * called upon them, and will trigger a GPF in is_kthread_should_stop(). + */ + do { + set_current_state(TASK_UNINTERRUPTIBLE); + if (wq_entry->flags & WQ_FLAG_WOKEN) + break; + + timeout = schedule_timeout(timeout); + } while (timeout); + __set_current_state(TASK_RUNNING); + + /* See wait_woken() and woken_wake_function() */ + smp_store_mb(wq_entry->flags, wq_entry->flags & ~WQ_FLAG_WOKEN); + + return timeout; +} + +static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 type) +{ + struct intel_guc_tlb_wait _wq, *wq = &_wq; + DEFINE_WAIT_FUNC(wait, woken_wake_function); + struct intel_gt *gt = guc_to_gt(guc); + int err; + u32 seqno; + u32 action[] = { + INTEL_GUC_ACTION_TLB_INVALIDATION, + 0, + REG_FIELD_PREP(INTEL_GUC_TLB_INVAL_TYPE_MASK, type) | + REG_FIELD_PREP(INTEL_GUC_TLB_INVAL_MODE_MASK, + INTEL_GUC_TLB_INVAL_MODE_HEAVY) | + INTEL_GUC_TLB_INVAL_FLUSH_CACHE, + }; + u32 size = ARRAY_SIZE(action); + + if (!intel_guc_ct_enabled(&guc->ct)) + return -EINVAL; + + init_waitqueue_head(&_wq.wq); + + if (xa_alloc_cyclic_irq(&guc->tlb_lookup, &seqno, wq, + xa_limit_32b, &guc->next_seqno, + GFP_ATOMIC | __GFP_NOWARN) < 0) { + /* Under severe memory pressure? Serialise TLB allocations */ + xa_lock_irq(&guc->tlb_lookup); + wq = xa_load(&guc->tlb_lookup, guc->serial_slot); + wait_event_lock_irq(wq->wq, + !READ_ONCE(wq->busy), + guc->tlb_lookup.xa_lock); + /* + * Update wq->busy under lock to ensure only one waiter can + * issue the TLB invalidation command using the serial slot at a + * time. The condition is set to true before releasing the lock + * so that other caller continue to wait until woken up again. + */ + wq->busy = true; + xa_unlock_irq(&guc->tlb_lookup); + + seqno = guc->serial_slot; + } + + action[1] = seqno; + + add_wait_queue(&wq->wq, &wait); + + err = intel_guc_send_busy_loop(guc, action, size, G2H_LEN_DW_INVALIDATE_TLB, true); + if (err) + goto out; + + /* + * GuC has a timeout of 1ms for a TLB invalidation response from GAM. On a + * timeout GuC drops the request and has no mechanism to notify the host about + * the timeout. There is also no mechanism for determining the number of + * any/all messages in the CT channel. Ergo, keep a larger timeout that accounts + * for this individual timeout and the max number of outstanding invalidation + * requests that can be queued in CT buffer. + */ +#define OUTSTANDING_GUC_TIMEOUT_PERIOD (HZ * 2) + if (!must_wait_woken(&wait, OUTSTANDING_GUC_TIMEOUT_PERIOD)) { + gt_err(gt, + "TLB invalidation response timed out for seqno %u\n", seqno); + err = -ETIME; + } +out: + remove_wait_queue(&wq->wq, &wait); + if (seqno != guc->serial_slot) + xa_erase_irq(&guc->tlb_lookup, seqno); + + return err; +} + +/* Full TLB invalidation */ +int intel_guc_invalidate_tlb_full(struct intel_guc *guc) +{ + return guc_send_invalidate_tlb(guc, INTEL_GUC_TLB_INVAL_FULL); +} + +/* GuC TLB Invalidation: Invalidate the TLB's of GuC itself. */ +int intel_guc_invalidate_tlb(struct intel_guc *guc) +{ + return guc_send_invalidate_tlb(guc, INTEL_GUC_TLB_INVAL_GUC); +} + int intel_guc_deregister_done_process_msg(struct intel_guc *guc, const u32 *msg, u32 len) -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines Jonathan Cavitt @ 2023-10-05 8:54 ` Tvrtko Ursulin 2023-10-05 15:08 ` Cavitt, Jonathan 0 siblings, 1 reply; 15+ messages in thread From: Tvrtko Ursulin @ 2023-10-05 8:54 UTC (permalink / raw) To: Jonathan Cavitt, intel-gfx Cc: janusz.krzysztofik, andi.shyti, matthew.d.roper, chris.p.wilson, nirmoy.das On 04/10/2023 19:36, Jonathan Cavitt wrote: > From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > > The GuC firmware had defined the interface for Translation Look-Aside > Buffer (TLB) invalidation. We should use this interface when > invalidating the engine and GuC TLBs. > Add additional functionality to intel_gt_invalidate_tlb, invalidating > the GuC TLBs and falling back to GT invalidation when the GuC is > disabled. > The invalidation is done by sending a request directly to the GuC > tlb_lookup that invalidates the table. The invalidation is submitted as > a wait request and is performed in the CT event handler. This means we > cannot perform this TLB invalidation path if the CT is not enabled. > If the request isn't fulfilled in two seconds, this would constitute > an error in the invalidation as that would constitute either a lost > request or a severe GuC overload. > The tlb_lookup table is allocated as an xarray because the set of > pending TLB invalidations may have no upper bound. The consequence of > this is that all actions interfacting with this table need to use the > xarray functions, such as xa_alloc_cyclic_irq for array insertion. > > With this new invalidation routine, we can perform GuC-based GGTT > invalidations. GuC-based GGTT invalidation is incompatible with > MMIO invalidation so we should not perform MMIO invalidation when > GuC-based GGTT invalidation is expected. This respun (series) includes changes some but not all points that I have raised, or at least asked about in the last round. There were no replies, to explain either that I was wrong, or whatever, against the points left un-modified. Some of those I raised more than once. And while I appreciate addressing review feedback and quick respins, I don't enjoy the silence on the unaddressed parts since that is not how the review should work. Therefore I will not be reviewing this version until all review comments are dealt with one way or the other. Regards, Tvrtko > Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > Signed-off-by: Bruce Chang <yu.bruce.chang@intel.com> > Signed-off-by: Chris Wilson <chris.p.wilson@intel.com> > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> > Signed-off-by: Fei Yang <fei.yang@intel.com> > CC: Andi Shyti <andi.shyti@linux.intel.com> > --- > drivers/gpu/drm/i915/gt/intel_ggtt.c | 42 ++-- > drivers/gpu/drm/i915/gt/intel_tlb.c | 14 +- > .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 33 +++ > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 22 ++ > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 6 + > drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 + > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 211 +++++++++++++++++- > 7 files changed, 313 insertions(+), 16 deletions(-) > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > index 4d7d88b92632b..be8c216d29f91 100644 > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > @@ -206,22 +206,38 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) > intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN); > } > > -static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) > +static void guc_ggtt_ct_invalidate(struct intel_gt *gt) > { > - struct drm_i915_private *i915 = ggtt->vm.i915; > + struct intel_uncore *uncore = gt->uncore; > + intel_wakeref_t wakeref; > > - gen8_ggtt_invalidate(ggtt); > + with_intel_runtime_pm_if_active(uncore->rpm, wakeref) { > + struct intel_guc *guc = >->uc.guc; > > - if (GRAPHICS_VER(i915) >= 12) { > - struct intel_gt *gt; > + intel_guc_invalidate_tlb(guc); > + } > +} > > - list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) > - intel_uncore_write_fw(gt->uncore, > - GEN12_GUC_TLB_INV_CR, > - GEN12_GUC_TLB_INV_CR_INVALIDATE); > - } else { > - intel_uncore_write_fw(ggtt->vm.gt->uncore, > - GEN8_GTCR, GEN8_GTCR_INVALIDATE); > +static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) > +{ > + struct drm_i915_private *i915 = ggtt->vm.i915; > + struct intel_gt *gt; > + > + if (!IS_GEN9_LP(i915) && GRAPHICS_VER(i915) < 11) > + gen8_ggtt_invalidate(ggtt); > + > + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) { > + if (HAS_GUC_TLB_INVALIDATION(i915) && > + intel_guc_is_ready(>->uc.guc)) { > + guc_ggtt_ct_invalidate(gt); > + } else if (GRAPHICS_VER(i915) >= 12) { > + intel_uncore_write(gt->uncore, > + GEN12_GUC_TLB_INV_CR, > + GEN12_GUC_TLB_INV_CR_INVALIDATE); > + } else { > + intel_uncore_write(gt->uncore, > + GEN8_GTCR, GEN8_GTCR_INVALIDATE); > + } > } > } > > @@ -1243,7 +1259,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) > ggtt->vm.raw_insert_page = gen8_ggtt_insert_page; > } > > - if (intel_uc_wants_guc(&ggtt->vm.gt->uc)) > + if (intel_uc_wants_guc_submission(&ggtt->vm.gt->uc)) > ggtt->invalidate = guc_ggtt_invalidate; > else > ggtt->invalidate = gen8_ggtt_invalidate; > diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c b/drivers/gpu/drm/i915/gt/intel_tlb.c > index 139608c30d978..a84563c178bc6 100644 > --- a/drivers/gpu/drm/i915/gt/intel_tlb.c > +++ b/drivers/gpu/drm/i915/gt/intel_tlb.c > @@ -12,6 +12,7 @@ > #include "intel_gt_print.h" > #include "intel_gt_regs.h" > #include "intel_tlb.h" > +#include "uc/intel_guc.h" > > /* > * HW architecture suggest typical invalidation time at 40us, > @@ -131,11 +132,22 @@ void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno) > return; > > with_intel_gt_pm_if_awake(gt, wakeref) { > + struct intel_guc *guc = >->uc.guc; > + > mutex_lock(>->tlb.invalidate_lock); > if (tlb_seqno_passed(gt, seqno)) > goto unlock; > > - mmio_invalidate_full(gt); > + if (HAS_GUC_TLB_INVALIDATION(gt->i915)) { > + /* > + * Only perform GuC TLB invalidation if GuC is ready. > + * Otherwise, skip invalidation altogeter. > + */ > + if (intel_guc_is_ready(guc)) > + intel_guc_invalidate_tlb_full(guc); > + } else { > + mmio_invalidate_full(gt); > + } > > write_seqcount_invalidate(>->tlb.seqno); > unlock: > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > index f359bef046e0b..9dff8012d5e76 100644 > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > @@ -138,6 +138,8 @@ enum intel_guc_action { > INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, > INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507, > INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A, > + INTEL_GUC_ACTION_TLB_INVALIDATION = 0x7000, > + INTEL_GUC_ACTION_TLB_INVALIDATION_DONE = 0x7001, > INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002, > INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003, > INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004, > @@ -181,4 +183,35 @@ enum intel_guc_state_capture_event_status { > > #define INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_MASK 0x000000FF > > +#define INTEL_GUC_TLB_INVAL_TYPE_MASK REG_GENMASK(7, 0) > +#define INTEL_GUC_TLB_INVAL_MODE_MASK REG_GENMASK(11, 8) > +#define INTEL_GUC_TLB_INVAL_FLUSH_CACHE REG_BIT(31) > + > +enum intel_guc_tlb_invalidation_type { > + INTEL_GUC_TLB_INVAL_FULL = 0x0, > + INTEL_GUC_TLB_INVAL_GUC = 0x3, > +}; > + > +/* > + * 0: Heavy mode of Invalidation: > + * The pipeline of the engine(s) for which the invalidation is targeted to is > + * blocked, and all the in-flight transactions are guaranteed to be Globally > + * Observed before completing the TLB invalidation > + * 1: Lite mode of Invalidation: > + * TLBs of the targeted engine(s) are immediately invalidated. > + * In-flight transactions are NOT guaranteed to be Globally Observed before > + * completing TLB invalidation. > + * Light Invalidation Mode is to be used only when > + * it can be guaranteed (by SW) that the address translations remain invariant > + * for the in-flight transactions across the TLB invalidation. In other words, > + * this mode can be used when the TLB invalidation is intended to clear out the > + * stale cached translations that are no longer in use. Light Invalidation Mode > + * is much faster than the Heavy Invalidation Mode, as it does not wait for the > + * in-flight transactions to be GOd. > + */ > +enum intel_guc_tlb_inval_mode { > + INTEL_GUC_TLB_INVAL_MODE_HEAVY = 0x0, > + INTEL_GUC_TLB_INVAL_MODE_LITE = 0x1, > +}; > + > #endif /* _ABI_GUC_ACTIONS_ABI_H */ > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > index 6c392bad29c19..d7203ba88b0c0 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > @@ -79,6 +79,18 @@ struct intel_guc { > */ > atomic_t outstanding_submission_g2h; > > + /** @tlb_lookup: xarray to store all pending TLB invalidation requests */ > + struct xarray tlb_lookup; > + > + /** > + * @serial_slot: id to the initial waiter created in tlb_lookup, > + * which is used only when failed to allocate new waiter. > + */ > + u32 serial_slot; > + > + /** @next_seqno: the next id (sequence no.) to allocate. */ > + u32 next_seqno; > + > /** @interrupts: pointers to GuC interrupt-managing functions. */ > struct { > bool enabled; > @@ -296,6 +308,11 @@ struct intel_guc { > #define MAKE_GUC_VER_STRUCT(ver) MAKE_GUC_VER((ver).major, (ver).minor, (ver).patch) > #define GUC_SUBMIT_VER(guc) MAKE_GUC_VER_STRUCT((guc)->submission_version) > > +struct intel_guc_tlb_wait { > + struct wait_queue_head wq; > + bool busy; > +}; > + > static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) > { > return container_of(log, struct intel_guc, log); > @@ -418,6 +435,11 @@ static inline bool intel_guc_is_supported(struct intel_guc *guc) > return intel_uc_fw_is_supported(&guc->fw); > } > > +int intel_guc_invalidate_tlb_full(struct intel_guc *guc); > +int intel_guc_invalidate_tlb(struct intel_guc *guc); > +int intel_guc_tlb_invalidation_done(struct intel_guc *guc, const u32 *hxg, > + u32 size); > + > static inline bool intel_guc_is_wanted(struct intel_guc *guc) > { > return intel_uc_fw_is_enabled(&guc->fw); > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > index 6e22af31513a5..1ee4d4a988398 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > @@ -1115,6 +1115,11 @@ static int ct_process_request(struct intel_guc_ct *ct, struct ct_incoming_msg *r > case INTEL_GUC_ACTION_NOTIFY_EXCEPTION: > ret = intel_guc_crash_process_msg(guc, action); > break; > + case INTEL_GUC_ACTION_TLB_INVALIDATION_DONE: > + ret = intel_guc_tlb_invalidation_done(ct_to_guc(ct), hxg, request->size); > + if (unlikely(ret)) > + ct_free_msg(request); > + break; > default: > ret = -EOPNOTSUPP; > break; > @@ -1186,6 +1191,7 @@ static int ct_handle_event(struct intel_guc_ct *ct, struct ct_incoming_msg *requ > switch (action) { > case INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_DONE: > case INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE: > + case INTEL_GUC_ACTION_TLB_INVALIDATION_DONE: > g2h_release_space(ct, request->size); > } > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h > index b4d56eccfb1f0..a7c9874e122a3 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h > @@ -22,6 +22,7 @@ > /* Payload length only i.e. don't include G2H header length */ > #define G2H_LEN_DW_SCHED_CONTEXT_MODE_SET 2 > #define G2H_LEN_DW_DEREGISTER_CONTEXT 1 > +#define G2H_LEN_DW_INVALIDATE_TLB 1 > > #define GUC_CONTEXT_DISABLE 0 > #define GUC_CONTEXT_ENABLE 1 > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > index 2cce5ec1ff00d..12f79abd889ce 100644 > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > @@ -1798,9 +1798,11 @@ static void __guc_reset_context(struct intel_context *ce, intel_engine_mask_t st > > void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stalled) > { > + struct intel_guc_tlb_wait *wait; > struct intel_context *ce; > unsigned long index; > unsigned long flags; > + unsigned long i; > > if (unlikely(!guc_submission_initialized(guc))) { > /* Reset called during driver load? GuC not yet initialised! */ > @@ -1826,6 +1828,14 @@ void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stall > > /* GuC is blown away, drop all references to contexts */ > xa_destroy(&guc->context_lookup); > + > + /* > + * The full GT reset will have cleared the TLB caches and flushed the > + * G2H message queue; we can release all the blocked waiters. > + */ > + xa_for_each(&guc->tlb_lookup, i, wait) { > + wake_up(&wait->wq); > + } > } > > static void guc_cancel_context_requests(struct intel_context *ce) > @@ -1948,6 +1958,49 @@ void intel_guc_submission_reset_finish(struct intel_guc *guc) > static void destroyed_worker_func(struct work_struct *w); > static void reset_fail_worker_func(struct work_struct *w); > > +static int init_tlb_lookup(struct intel_guc *guc) > +{ > + struct intel_guc_tlb_wait *wait; > + int err; > + > + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) > + return 0; > + > + xa_init_flags(&guc->tlb_lookup, XA_FLAGS_ALLOC); > + > + wait = kzalloc(sizeof(*wait), GFP_KERNEL); > + if (!wait) > + return -ENOMEM; > + > + init_waitqueue_head(&wait->wq); > + > + /* Preallocate a shared id for use under memory pressure. */ > + err = xa_alloc_cyclic_irq(&guc->tlb_lookup, &guc->serial_slot, wait, > + xa_limit_32b, &guc->next_seqno, GFP_KERNEL); > + if (err == -ENOMEM) { > + kfree(wait); > + return err; > + } > + > + return 0; > +} > + > +static void fini_tlb_lookup(struct intel_guc *guc) > +{ > + struct intel_guc_tlb_wait *wait; > + > + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) > + return; > + > + wait = xa_load(&guc->tlb_lookup, guc->serial_slot); > + if (wait) { > + GEM_BUG_ON(wait->busy); > + kfree(wait); > + } > + > + xa_destroy(&guc->tlb_lookup); > +} > + > /* > * Set up the memory resources to be shared with the GuC (via the GGTT) > * at firmware loading time. > @@ -1966,11 +2019,15 @@ int intel_guc_submission_init(struct intel_guc *guc) > return ret; > } > > + ret = init_tlb_lookup(guc); > + if (ret) > + goto destroy_pool; > + > guc->submission_state.guc_ids_bitmap = > bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); > if (!guc->submission_state.guc_ids_bitmap) { > ret = -ENOMEM; > - goto destroy_pool; > + goto destroy_tlb; > } > > guc->timestamp.ping_delay = (POLL_TIME_CLKS / gt->clock_frequency + 1) * HZ; > @@ -1979,9 +2036,10 @@ int intel_guc_submission_init(struct intel_guc *guc) > > return 0; > > +destroy_tlb: > + fini_tlb_lookup(guc); > destroy_pool: > guc_lrc_desc_pool_destroy_v69(guc); > - > return ret; > } > > @@ -1994,6 +2052,7 @@ void intel_guc_submission_fini(struct intel_guc *guc) > guc_lrc_desc_pool_destroy_v69(guc); > i915_sched_engine_put(guc->sched_engine); > bitmap_free(guc->submission_state.guc_ids_bitmap); > + fini_tlb_lookup(guc); > guc->submission_initialized = false; > } > > @@ -4624,6 +4683,154 @@ g2h_context_lookup(struct intel_guc *guc, u32 ctx_id) > return ce; > } > > +static void wait_wake_outstanding_tlb_g2h(struct intel_guc *guc, u32 seqno) > +{ > + struct intel_guc_tlb_wait *wait; > + unsigned long flags; > + > + xa_lock_irqsave(&guc->tlb_lookup, flags); > + wait = xa_load(&guc->tlb_lookup, seqno); > + > + /* We received a response after the waiting task did exit with a timeout */ > + if (wait) > + wake_up(&wait->wq); > + else > + drm_dbg(&guc_to_gt(guc)->i915->drm, > + "Stale TLB invalidation response with seqno %d\n", seqno); > + > + xa_unlock_irqrestore(&guc->tlb_lookup, flags); > +} > + > +int intel_guc_tlb_invalidation_done(struct intel_guc *guc, const u32 *hxg, u32 size) > +{ > + u32 seqno, hxg_len, len; > + > + hxg_len = size - GUC_CTB_MSG_MIN_LEN; > + len = hxg_len - GUC_HXG_MSG_MIN_LEN; > + > + /* Check for underflow */ > + if (unlikely(len < 1 || len > size)) > + return -EPROTO; > + > + seqno = hxg[GUC_HXG_MSG_MIN_LEN]; > + wait_wake_outstanding_tlb_g2h(guc, seqno); > + return 0; > +} > + > +static long must_wait_woken(struct wait_queue_entry *wq_entry, long timeout) > +{ > + /* > + * This is equivalent to wait_woken() with the exception that > + * we do not wake up early if the kthread task has been completed. > + * As we are called from page reclaim in any task context, > + * we may be invoked from stopped kthreads, but we *must* > + * complete the wait from the HW . > + * > + * A second problem is that since we are called under reclaim > + * and wait_woken() inspected the thread state, it makes an invalid > + * assumption that all PF_KTHREAD tasks have set_kthread_struct() > + * called upon them, and will trigger a GPF in is_kthread_should_stop(). > + */ > + do { > + set_current_state(TASK_UNINTERRUPTIBLE); > + if (wq_entry->flags & WQ_FLAG_WOKEN) > + break; > + > + timeout = schedule_timeout(timeout); > + } while (timeout); > + __set_current_state(TASK_RUNNING); > + > + /* See wait_woken() and woken_wake_function() */ > + smp_store_mb(wq_entry->flags, wq_entry->flags & ~WQ_FLAG_WOKEN); > + > + return timeout; > +} > + > +static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 type) > +{ > + struct intel_guc_tlb_wait _wq, *wq = &_wq; > + DEFINE_WAIT_FUNC(wait, woken_wake_function); > + struct intel_gt *gt = guc_to_gt(guc); > + int err; > + u32 seqno; > + u32 action[] = { > + INTEL_GUC_ACTION_TLB_INVALIDATION, > + 0, > + REG_FIELD_PREP(INTEL_GUC_TLB_INVAL_TYPE_MASK, type) | > + REG_FIELD_PREP(INTEL_GUC_TLB_INVAL_MODE_MASK, > + INTEL_GUC_TLB_INVAL_MODE_HEAVY) | > + INTEL_GUC_TLB_INVAL_FLUSH_CACHE, > + }; > + u32 size = ARRAY_SIZE(action); > + > + if (!intel_guc_ct_enabled(&guc->ct)) > + return -EINVAL; > + > + init_waitqueue_head(&_wq.wq); > + > + if (xa_alloc_cyclic_irq(&guc->tlb_lookup, &seqno, wq, > + xa_limit_32b, &guc->next_seqno, > + GFP_ATOMIC | __GFP_NOWARN) < 0) { > + /* Under severe memory pressure? Serialise TLB allocations */ > + xa_lock_irq(&guc->tlb_lookup); > + wq = xa_load(&guc->tlb_lookup, guc->serial_slot); > + wait_event_lock_irq(wq->wq, > + !READ_ONCE(wq->busy), > + guc->tlb_lookup.xa_lock); > + /* > + * Update wq->busy under lock to ensure only one waiter can > + * issue the TLB invalidation command using the serial slot at a > + * time. The condition is set to true before releasing the lock > + * so that other caller continue to wait until woken up again. > + */ > + wq->busy = true; > + xa_unlock_irq(&guc->tlb_lookup); > + > + seqno = guc->serial_slot; > + } > + > + action[1] = seqno; > + > + add_wait_queue(&wq->wq, &wait); > + > + err = intel_guc_send_busy_loop(guc, action, size, G2H_LEN_DW_INVALIDATE_TLB, true); > + if (err) > + goto out; > + > + /* > + * GuC has a timeout of 1ms for a TLB invalidation response from GAM. On a > + * timeout GuC drops the request and has no mechanism to notify the host about > + * the timeout. There is also no mechanism for determining the number of > + * any/all messages in the CT channel. Ergo, keep a larger timeout that accounts > + * for this individual timeout and the max number of outstanding invalidation > + * requests that can be queued in CT buffer. > + */ > +#define OUTSTANDING_GUC_TIMEOUT_PERIOD (HZ * 2) > + if (!must_wait_woken(&wait, OUTSTANDING_GUC_TIMEOUT_PERIOD)) { > + gt_err(gt, > + "TLB invalidation response timed out for seqno %u\n", seqno); > + err = -ETIME; > + } > +out: > + remove_wait_queue(&wq->wq, &wait); > + if (seqno != guc->serial_slot) > + xa_erase_irq(&guc->tlb_lookup, seqno); > + > + return err; > +} > + > +/* Full TLB invalidation */ > +int intel_guc_invalidate_tlb_full(struct intel_guc *guc) > +{ > + return guc_send_invalidate_tlb(guc, INTEL_GUC_TLB_INVAL_FULL); > +} > + > +/* GuC TLB Invalidation: Invalidate the TLB's of GuC itself. */ > +int intel_guc_invalidate_tlb(struct intel_guc *guc) > +{ > + return guc_send_invalidate_tlb(guc, INTEL_GUC_TLB_INVAL_GUC); > +} > + > int intel_guc_deregister_done_process_msg(struct intel_guc *guc, > const u32 *msg, > u32 len) ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines 2023-10-05 8:54 ` Tvrtko Ursulin @ 2023-10-05 15:08 ` Cavitt, Jonathan 0 siblings, 0 replies; 15+ messages in thread From: Cavitt, Jonathan @ 2023-10-05 15:08 UTC (permalink / raw) To: Tvrtko Ursulin, intel-gfx@lists.freedesktop.org Cc: Krzysztofik, Janusz, Shyti, Andi, Roper, Matthew D, chris.p.wilson@linux.intel.com, Das, Nirmoy -----Original Message----- From: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com> Sent: Thursday, October 5, 2023 1:55 AM To: Cavitt, Jonathan <jonathan.cavitt@intel.com>; intel-gfx@lists.freedesktop.org Cc: Dutt, Sudeep <sudeep.dutt@intel.com>; Chang, Yu bruce <yu.bruce.chang@intel.com>; chris.p.wilson@linux.intel.com; Iddamsetty, Aravind <aravind.iddamsetty@intel.com>; Yang, Fei <fei.yang@intel.com>; Shyti, Andi <andi.shyti@intel.com>; Harrison, John C <john.c.harrison@intel.com>; Das, Nirmoy <nirmoy.das@intel.com>; Krzysztofik, Janusz <janusz.krzysztofik@intel.com>; Roper, Matthew D <matthew.d.roper@intel.com> Subject: Re: [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines > > > On 04/10/2023 19:36, Jonathan Cavitt wrote: > > From: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > > > > The GuC firmware had defined the interface for Translation Look-Aside > > Buffer (TLB) invalidation. We should use this interface when > > invalidating the engine and GuC TLBs. > > Add additional functionality to intel_gt_invalidate_tlb, invalidating > > the GuC TLBs and falling back to GT invalidation when the GuC is > > disabled. > > The invalidation is done by sending a request directly to the GuC > > tlb_lookup that invalidates the table. The invalidation is submitted as > > a wait request and is performed in the CT event handler. This means we > > cannot perform this TLB invalidation path if the CT is not enabled. > > If the request isn't fulfilled in two seconds, this would constitute > > an error in the invalidation as that would constitute either a lost > > request or a severe GuC overload. > > The tlb_lookup table is allocated as an xarray because the set of > > pending TLB invalidations may have no upper bound. The consequence of > > this is that all actions interfacting with this table need to use the > > xarray functions, such as xa_alloc_cyclic_irq for array insertion. > > > > With this new invalidation routine, we can perform GuC-based GGTT > > invalidations. GuC-based GGTT invalidation is incompatible with > > MMIO invalidation so we should not perform MMIO invalidation when > > GuC-based GGTT invalidation is expected. > > This respun (series) includes changes some but not all points that I > have raised, or at least asked about in the last round. There were no > replies, to explain either that I was wrong, or whatever, against the > points left un-modified. Some of those I raised more than once. > > And while I appreciate addressing review feedback and quick respins, I > don't enjoy the silence on the unaddressed parts since that is not how > the review should work. Therefore I will not be reviewing this version > until all review comments are dealt with one way or the other. I went back through your previous revision notes just now. AFAICT, the only revision note I did not address in this patch series pertains to the necessity for if (!intel_guc_ct_enabled(&guc->ct)) in guc_send_invalidate_tlb. However, I cannot see where this is guarded against in the code prior, so it still appears necessary. If this is incorrect, please inform me of what I may have missed. I understand it's difficult to see what's changed between revisions, and I recognize that was my fault for not including a cover letter: I was under the impression I was to not include cover letters in upstream patches unless I had some additional information to provide, but the way I write cover letters gives the impression that this is not the case. I added a cover letter to version six that retroactively applies version change notes to all prior revisions. This may have been missed because I accidentally left the dii-client tag (and a stale patch duplicate patch) in the series, so it may have been seen as a series sent to the wrong email address. I'll aim to remove the tag for version seven. -Jonathan Cavitt > > Regards, > > Tvrtko > > > Signed-off-by: Prathap Kumar Valsan <prathap.kumar.valsan@intel.com> > > Signed-off-by: Bruce Chang <yu.bruce.chang@intel.com> > > Signed-off-by: Chris Wilson <chris.p.wilson@intel.com> > > Signed-off-by: Umesh Nerlige Ramappa <umesh.nerlige.ramappa@intel.com> > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > Signed-off-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> > > Signed-off-by: Fei Yang <fei.yang@intel.com> > > CC: Andi Shyti <andi.shyti@linux.intel.com> > > --- > > drivers/gpu/drm/i915/gt/intel_ggtt.c | 42 ++-- > > drivers/gpu/drm/i915/gt/intel_tlb.c | 14 +- > > .../gpu/drm/i915/gt/uc/abi/guc_actions_abi.h | 33 +++ > > drivers/gpu/drm/i915/gt/uc/intel_guc.h | 22 ++ > > drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c | 6 + > > drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h | 1 + > > .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 211 +++++++++++++++++- > > 7 files changed, 313 insertions(+), 16 deletions(-) > > > > diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c b/drivers/gpu/drm/i915/gt/intel_ggtt.c > > index 4d7d88b92632b..be8c216d29f91 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_ggtt.c > > +++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c > > @@ -206,22 +206,38 @@ static void gen8_ggtt_invalidate(struct i915_ggtt *ggtt) > > intel_uncore_write_fw(uncore, GFX_FLSH_CNTL_GEN6, GFX_FLSH_CNTL_EN); > > } > > > > -static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) > > +static void guc_ggtt_ct_invalidate(struct intel_gt *gt) > > { > > - struct drm_i915_private *i915 = ggtt->vm.i915; > > + struct intel_uncore *uncore = gt->uncore; > > + intel_wakeref_t wakeref; > > > > - gen8_ggtt_invalidate(ggtt); > > + with_intel_runtime_pm_if_active(uncore->rpm, wakeref) { > > + struct intel_guc *guc = >->uc.guc; > > > > - if (GRAPHICS_VER(i915) >= 12) { > > - struct intel_gt *gt; > > + intel_guc_invalidate_tlb(guc); > > + } > > +} > > > > - list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) > > - intel_uncore_write_fw(gt->uncore, > > - GEN12_GUC_TLB_INV_CR, > > - GEN12_GUC_TLB_INV_CR_INVALIDATE); > > - } else { > > - intel_uncore_write_fw(ggtt->vm.gt->uncore, > > - GEN8_GTCR, GEN8_GTCR_INVALIDATE); > > +static void guc_ggtt_invalidate(struct i915_ggtt *ggtt) > > +{ > > + struct drm_i915_private *i915 = ggtt->vm.i915; > > + struct intel_gt *gt; > > + > > + if (!IS_GEN9_LP(i915) && GRAPHICS_VER(i915) < 11) > > + gen8_ggtt_invalidate(ggtt); > > + > > + list_for_each_entry(gt, &ggtt->gt_list, ggtt_link) { > > + if (HAS_GUC_TLB_INVALIDATION(i915) && > > + intel_guc_is_ready(>->uc.guc)) { > > + guc_ggtt_ct_invalidate(gt); > > + } else if (GRAPHICS_VER(i915) >= 12) { > > + intel_uncore_write(gt->uncore, > > + GEN12_GUC_TLB_INV_CR, > > + GEN12_GUC_TLB_INV_CR_INVALIDATE); > > + } else { > > + intel_uncore_write(gt->uncore, > > + GEN8_GTCR, GEN8_GTCR_INVALIDATE); > > + } > > } > > } > > > > @@ -1243,7 +1259,7 @@ static int gen8_gmch_probe(struct i915_ggtt *ggtt) > > ggtt->vm.raw_insert_page = gen8_ggtt_insert_page; > > } > > > > - if (intel_uc_wants_guc(&ggtt->vm.gt->uc)) > > + if (intel_uc_wants_guc_submission(&ggtt->vm.gt->uc)) > > ggtt->invalidate = guc_ggtt_invalidate; > > else > > ggtt->invalidate = gen8_ggtt_invalidate; > > diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c b/drivers/gpu/drm/i915/gt/intel_tlb.c > > index 139608c30d978..a84563c178bc6 100644 > > --- a/drivers/gpu/drm/i915/gt/intel_tlb.c > > +++ b/drivers/gpu/drm/i915/gt/intel_tlb.c > > @@ -12,6 +12,7 @@ > > #include "intel_gt_print.h" > > #include "intel_gt_regs.h" > > #include "intel_tlb.h" > > +#include "uc/intel_guc.h" > > > > /* > > * HW architecture suggest typical invalidation time at 40us, > > @@ -131,11 +132,22 @@ void intel_gt_invalidate_tlb_full(struct intel_gt *gt, u32 seqno) > > return; > > > > with_intel_gt_pm_if_awake(gt, wakeref) { > > + struct intel_guc *guc = >->uc.guc; > > + > > mutex_lock(>->tlb.invalidate_lock); > > if (tlb_seqno_passed(gt, seqno)) > > goto unlock; > > > > - mmio_invalidate_full(gt); > > + if (HAS_GUC_TLB_INVALIDATION(gt->i915)) { > > + /* > > + * Only perform GuC TLB invalidation if GuC is ready. > > + * Otherwise, skip invalidation altogeter. > > + */ > > + if (intel_guc_is_ready(guc)) > > + intel_guc_invalidate_tlb_full(guc); > > + } else { > > + mmio_invalidate_full(gt); > > + } > > > > write_seqcount_invalidate(>->tlb.seqno); > > unlock: > > diff --git a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > > index f359bef046e0b..9dff8012d5e76 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > > +++ b/drivers/gpu/drm/i915/gt/uc/abi/guc_actions_abi.h > > @@ -138,6 +138,8 @@ enum intel_guc_action { > > INTEL_GUC_ACTION_REGISTER_CONTEXT_MULTI_LRC = 0x4601, > > INTEL_GUC_ACTION_CLIENT_SOFT_RESET = 0x5507, > > INTEL_GUC_ACTION_SET_ENG_UTIL_BUFF = 0x550A, > > + INTEL_GUC_ACTION_TLB_INVALIDATION = 0x7000, > > + INTEL_GUC_ACTION_TLB_INVALIDATION_DONE = 0x7001, > > INTEL_GUC_ACTION_STATE_CAPTURE_NOTIFICATION = 0x8002, > > INTEL_GUC_ACTION_NOTIFY_FLUSH_LOG_BUFFER_TO_FILE = 0x8003, > > INTEL_GUC_ACTION_NOTIFY_CRASH_DUMP_POSTED = 0x8004, > > @@ -181,4 +183,35 @@ enum intel_guc_state_capture_event_status { > > > > #define INTEL_GUC_STATE_CAPTURE_EVENT_STATUS_MASK 0x000000FF > > > > +#define INTEL_GUC_TLB_INVAL_TYPE_MASK REG_GENMASK(7, 0) > > +#define INTEL_GUC_TLB_INVAL_MODE_MASK REG_GENMASK(11, 8) > > +#define INTEL_GUC_TLB_INVAL_FLUSH_CACHE REG_BIT(31) > > + > > +enum intel_guc_tlb_invalidation_type { > > + INTEL_GUC_TLB_INVAL_FULL = 0x0, > > + INTEL_GUC_TLB_INVAL_GUC = 0x3, > > +}; > > + > > +/* > > + * 0: Heavy mode of Invalidation: > > + * The pipeline of the engine(s) for which the invalidation is targeted to is > > + * blocked, and all the in-flight transactions are guaranteed to be Globally > > + * Observed before completing the TLB invalidation > > + * 1: Lite mode of Invalidation: > > + * TLBs of the targeted engine(s) are immediately invalidated. > > + * In-flight transactions are NOT guaranteed to be Globally Observed before > > + * completing TLB invalidation. > > + * Light Invalidation Mode is to be used only when > > + * it can be guaranteed (by SW) that the address translations remain invariant > > + * for the in-flight transactions across the TLB invalidation. In other words, > > + * this mode can be used when the TLB invalidation is intended to clear out the > > + * stale cached translations that are no longer in use. Light Invalidation Mode > > + * is much faster than the Heavy Invalidation Mode, as it does not wait for the > > + * in-flight transactions to be GOd. > > + */ > > +enum intel_guc_tlb_inval_mode { > > + INTEL_GUC_TLB_INVAL_MODE_HEAVY = 0x0, > > + INTEL_GUC_TLB_INVAL_MODE_LITE = 0x1, > > +}; > > + > > #endif /* _ABI_GUC_ACTIONS_ABI_H */ > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > index 6c392bad29c19..d7203ba88b0c0 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h > > @@ -79,6 +79,18 @@ struct intel_guc { > > */ > > atomic_t outstanding_submission_g2h; > > > > + /** @tlb_lookup: xarray to store all pending TLB invalidation requests */ > > + struct xarray tlb_lookup; > > + > > + /** > > + * @serial_slot: id to the initial waiter created in tlb_lookup, > > + * which is used only when failed to allocate new waiter. > > + */ > > + u32 serial_slot; > > + > > + /** @next_seqno: the next id (sequence no.) to allocate. */ > > + u32 next_seqno; > > + > > /** @interrupts: pointers to GuC interrupt-managing functions. */ > > struct { > > bool enabled; > > @@ -296,6 +308,11 @@ struct intel_guc { > > #define MAKE_GUC_VER_STRUCT(ver) MAKE_GUC_VER((ver).major, (ver).minor, (ver).patch) > > #define GUC_SUBMIT_VER(guc) MAKE_GUC_VER_STRUCT((guc)->submission_version) > > > > +struct intel_guc_tlb_wait { > > + struct wait_queue_head wq; > > + bool busy; > > +}; > > + > > static inline struct intel_guc *log_to_guc(struct intel_guc_log *log) > > { > > return container_of(log, struct intel_guc, log); > > @@ -418,6 +435,11 @@ static inline bool intel_guc_is_supported(struct intel_guc *guc) > > return intel_uc_fw_is_supported(&guc->fw); > > } > > > > +int intel_guc_invalidate_tlb_full(struct intel_guc *guc); > > +int intel_guc_invalidate_tlb(struct intel_guc *guc); > > +int intel_guc_tlb_invalidation_done(struct intel_guc *guc, const u32 *hxg, > > + u32 size); > > + > > static inline bool intel_guc_is_wanted(struct intel_guc *guc) > > { > > return intel_uc_fw_is_enabled(&guc->fw); > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > > index 6e22af31513a5..1ee4d4a988398 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ct.c > > @@ -1115,6 +1115,11 @@ static int ct_process_request(struct intel_guc_ct *ct, struct ct_incoming_msg *r > > case INTEL_GUC_ACTION_NOTIFY_EXCEPTION: > > ret = intel_guc_crash_process_msg(guc, action); > > break; > > + case INTEL_GUC_ACTION_TLB_INVALIDATION_DONE: > > + ret = intel_guc_tlb_invalidation_done(ct_to_guc(ct), hxg, request->size); > > + if (unlikely(ret)) > > + ct_free_msg(request); > > + break; > > default: > > ret = -EOPNOTSUPP; > > break; > > @@ -1186,6 +1191,7 @@ static int ct_handle_event(struct intel_guc_ct *ct, struct ct_incoming_msg *requ > > switch (action) { > > case INTEL_GUC_ACTION_SCHED_CONTEXT_MODE_DONE: > > case INTEL_GUC_ACTION_DEREGISTER_CONTEXT_DONE: > > + case INTEL_GUC_ACTION_TLB_INVALIDATION_DONE: > > g2h_release_space(ct, request->size); > > } > > > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h > > index b4d56eccfb1f0..a7c9874e122a3 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h > > @@ -22,6 +22,7 @@ > > /* Payload length only i.e. don't include G2H header length */ > > #define G2H_LEN_DW_SCHED_CONTEXT_MODE_SET 2 > > #define G2H_LEN_DW_DEREGISTER_CONTEXT 1 > > +#define G2H_LEN_DW_INVALIDATE_TLB 1 > > > > #define GUC_CONTEXT_DISABLE 0 > > #define GUC_CONTEXT_ENABLE 1 > > diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > index 2cce5ec1ff00d..12f79abd889ce 100644 > > --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c > > @@ -1798,9 +1798,11 @@ static void __guc_reset_context(struct intel_context *ce, intel_engine_mask_t st > > > > void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stalled) > > { > > + struct intel_guc_tlb_wait *wait; > > struct intel_context *ce; > > unsigned long index; > > unsigned long flags; > > + unsigned long i; > > > > if (unlikely(!guc_submission_initialized(guc))) { > > /* Reset called during driver load? GuC not yet initialised! */ > > @@ -1826,6 +1828,14 @@ void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stall > > > > /* GuC is blown away, drop all references to contexts */ > > xa_destroy(&guc->context_lookup); > > + > > + /* > > + * The full GT reset will have cleared the TLB caches and flushed the > > + * G2H message queue; we can release all the blocked waiters. > > + */ > > + xa_for_each(&guc->tlb_lookup, i, wait) { > > + wake_up(&wait->wq); > > + } > > } > > > > static void guc_cancel_context_requests(struct intel_context *ce) > > @@ -1948,6 +1958,49 @@ void intel_guc_submission_reset_finish(struct intel_guc *guc) > > static void destroyed_worker_func(struct work_struct *w); > > static void reset_fail_worker_func(struct work_struct *w); > > > > +static int init_tlb_lookup(struct intel_guc *guc) > > +{ > > + struct intel_guc_tlb_wait *wait; > > + int err; > > + > > + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) > > + return 0; > > + > > + xa_init_flags(&guc->tlb_lookup, XA_FLAGS_ALLOC); > > + > > + wait = kzalloc(sizeof(*wait), GFP_KERNEL); > > + if (!wait) > > + return -ENOMEM; > > + > > + init_waitqueue_head(&wait->wq); > > + > > + /* Preallocate a shared id for use under memory pressure. */ > > + err = xa_alloc_cyclic_irq(&guc->tlb_lookup, &guc->serial_slot, wait, > > + xa_limit_32b, &guc->next_seqno, GFP_KERNEL); > > + if (err == -ENOMEM) { > > + kfree(wait); > > + return err; > > + } > > + > > + return 0; > > +} > > + > > +static void fini_tlb_lookup(struct intel_guc *guc) > > +{ > > + struct intel_guc_tlb_wait *wait; > > + > > + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) > > + return; > > + > > + wait = xa_load(&guc->tlb_lookup, guc->serial_slot); > > + if (wait) { > > + GEM_BUG_ON(wait->busy); > > + kfree(wait); > > + } > > + > > + xa_destroy(&guc->tlb_lookup); > > +} > > + > > /* > > * Set up the memory resources to be shared with the GuC (via the GGTT) > > * at firmware loading time. > > @@ -1966,11 +2019,15 @@ int intel_guc_submission_init(struct intel_guc *guc) > > return ret; > > } > > > > + ret = init_tlb_lookup(guc); > > + if (ret) > > + goto destroy_pool; > > + > > guc->submission_state.guc_ids_bitmap = > > bitmap_zalloc(NUMBER_MULTI_LRC_GUC_ID(guc), GFP_KERNEL); > > if (!guc->submission_state.guc_ids_bitmap) { > > ret = -ENOMEM; > > - goto destroy_pool; > > + goto destroy_tlb; > > } > > > > guc->timestamp.ping_delay = (POLL_TIME_CLKS / gt->clock_frequency + 1) * HZ; > > @@ -1979,9 +2036,10 @@ int intel_guc_submission_init(struct intel_guc *guc) > > > > return 0; > > > > +destroy_tlb: > > + fini_tlb_lookup(guc); > > destroy_pool: > > guc_lrc_desc_pool_destroy_v69(guc); > > - > > return ret; > > } > > > > @@ -1994,6 +2052,7 @@ void intel_guc_submission_fini(struct intel_guc *guc) > > guc_lrc_desc_pool_destroy_v69(guc); > > i915_sched_engine_put(guc->sched_engine); > > bitmap_free(guc->submission_state.guc_ids_bitmap); > > + fini_tlb_lookup(guc); > > guc->submission_initialized = false; > > } > > > > @@ -4624,6 +4683,154 @@ g2h_context_lookup(struct intel_guc *guc, u32 ctx_id) > > return ce; > > } > > > > +static void wait_wake_outstanding_tlb_g2h(struct intel_guc *guc, u32 seqno) > > +{ > > + struct intel_guc_tlb_wait *wait; > > + unsigned long flags; > > + > > + xa_lock_irqsave(&guc->tlb_lookup, flags); > > + wait = xa_load(&guc->tlb_lookup, seqno); > > + > > + /* We received a response after the waiting task did exit with a timeout */ > > + if (wait) > > + wake_up(&wait->wq); > > + else > > + drm_dbg(&guc_to_gt(guc)->i915->drm, > > + "Stale TLB invalidation response with seqno %d\n", seqno); > > + > > + xa_unlock_irqrestore(&guc->tlb_lookup, flags); > > +} > > + > > +int intel_guc_tlb_invalidation_done(struct intel_guc *guc, const u32 *hxg, u32 size) > > +{ > > + u32 seqno, hxg_len, len; > > + > > + hxg_len = size - GUC_CTB_MSG_MIN_LEN; > > + len = hxg_len - GUC_HXG_MSG_MIN_LEN; > > + > > + /* Check for underflow */ > > + if (unlikely(len < 1 || len > size)) > > + return -EPROTO; > > + > > + seqno = hxg[GUC_HXG_MSG_MIN_LEN]; > > + wait_wake_outstanding_tlb_g2h(guc, seqno); > > + return 0; > > +} > > + > > +static long must_wait_woken(struct wait_queue_entry *wq_entry, long timeout) > > +{ > > + /* > > + * This is equivalent to wait_woken() with the exception that > > + * we do not wake up early if the kthread task has been completed. > > + * As we are called from page reclaim in any task context, > > + * we may be invoked from stopped kthreads, but we *must* > > + * complete the wait from the HW . > > + * > > + * A second problem is that since we are called under reclaim > > + * and wait_woken() inspected the thread state, it makes an invalid > > + * assumption that all PF_KTHREAD tasks have set_kthread_struct() > > + * called upon them, and will trigger a GPF in is_kthread_should_stop(). > > + */ > > + do { > > + set_current_state(TASK_UNINTERRUPTIBLE); > > + if (wq_entry->flags & WQ_FLAG_WOKEN) > > + break; > > + > > + timeout = schedule_timeout(timeout); > > + } while (timeout); > > + __set_current_state(TASK_RUNNING); > > + > > + /* See wait_woken() and woken_wake_function() */ > > + smp_store_mb(wq_entry->flags, wq_entry->flags & ~WQ_FLAG_WOKEN); > > + > > + return timeout; > > +} > > + > > +static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 type) > > +{ > > + struct intel_guc_tlb_wait _wq, *wq = &_wq; > > + DEFINE_WAIT_FUNC(wait, woken_wake_function); > > + struct intel_gt *gt = guc_to_gt(guc); > > + int err; > > + u32 seqno; > > + u32 action[] = { > > + INTEL_GUC_ACTION_TLB_INVALIDATION, > > + 0, > > + REG_FIELD_PREP(INTEL_GUC_TLB_INVAL_TYPE_MASK, type) | > > + REG_FIELD_PREP(INTEL_GUC_TLB_INVAL_MODE_MASK, > > + INTEL_GUC_TLB_INVAL_MODE_HEAVY) | > > + INTEL_GUC_TLB_INVAL_FLUSH_CACHE, > > + }; > > + u32 size = ARRAY_SIZE(action); > > + > > + if (!intel_guc_ct_enabled(&guc->ct)) > > + return -EINVAL; > > + > > + init_waitqueue_head(&_wq.wq); > > + > > + if (xa_alloc_cyclic_irq(&guc->tlb_lookup, &seqno, wq, > > + xa_limit_32b, &guc->next_seqno, > > + GFP_ATOMIC | __GFP_NOWARN) < 0) { > > + /* Under severe memory pressure? Serialise TLB allocations */ > > + xa_lock_irq(&guc->tlb_lookup); > > + wq = xa_load(&guc->tlb_lookup, guc->serial_slot); > > + wait_event_lock_irq(wq->wq, > > + !READ_ONCE(wq->busy), > > + guc->tlb_lookup.xa_lock); > > + /* > > + * Update wq->busy under lock to ensure only one waiter can > > + * issue the TLB invalidation command using the serial slot at a > > + * time. The condition is set to true before releasing the lock > > + * so that other caller continue to wait until woken up again. > > + */ > > + wq->busy = true; > > + xa_unlock_irq(&guc->tlb_lookup); > > + > > + seqno = guc->serial_slot; > > + } > > + > > + action[1] = seqno; > > + > > + add_wait_queue(&wq->wq, &wait); > > + > > + err = intel_guc_send_busy_loop(guc, action, size, G2H_LEN_DW_INVALIDATE_TLB, true); > > + if (err) > > + goto out; > > + > > + /* > > + * GuC has a timeout of 1ms for a TLB invalidation response from GAM. On a > > + * timeout GuC drops the request and has no mechanism to notify the host about > > + * the timeout. There is also no mechanism for determining the number of > > + * any/all messages in the CT channel. Ergo, keep a larger timeout that accounts > > + * for this individual timeout and the max number of outstanding invalidation > > + * requests that can be queued in CT buffer. > > + */ > > +#define OUTSTANDING_GUC_TIMEOUT_PERIOD (HZ * 2) > > + if (!must_wait_woken(&wait, OUTSTANDING_GUC_TIMEOUT_PERIOD)) { > > + gt_err(gt, > > + "TLB invalidation response timed out for seqno %u\n", seqno); > > + err = -ETIME; > > + } > > +out: > > + remove_wait_queue(&wq->wq, &wait); > > + if (seqno != guc->serial_slot) > > + xa_erase_irq(&guc->tlb_lookup, seqno); > > + > > + return err; > > +} > > + > > +/* Full TLB invalidation */ > > +int intel_guc_invalidate_tlb_full(struct intel_guc *guc) > > +{ > > + return guc_send_invalidate_tlb(guc, INTEL_GUC_TLB_INVAL_FULL); > > +} > > + > > +/* GuC TLB Invalidation: Invalidate the TLB's of GuC itself. */ > > +int intel_guc_invalidate_tlb(struct intel_guc *guc) > > +{ > > + return guc_send_invalidate_tlb(guc, INTEL_GUC_TLB_INVAL_GUC); > > +} > > + > > int intel_guc_deregister_done_process_msg(struct intel_guc *guc, > > const u32 *msg, > > u32 len) > ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] [PATCH v5 3/4] drm/i915: No TLB invalidation on wedged or suspended GT 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines Jonathan Cavitt @ 2023-10-04 18:36 ` Jonathan Cavitt 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 4/4] drm/i915/gt: Increase sleep in gt_tlb selftest sanitycheck Jonathan Cavitt ` (5 subsequent siblings) 7 siblings, 0 replies; 15+ messages in thread From: Jonathan Cavitt @ 2023-10-04 18:36 UTC (permalink / raw) To: intel-gfx Cc: janusz.krzysztofik, andi.shyti, matthew.d.roper, jonathan.cavitt, chris.p.wilson, nirmoy.das In case of GT is suspended or wedged, don't allow submission of new TLB invalidation request and cancel all pending requests. The TLB entries will be invalidated either during GuC reload or on system resume. Signed-off-by: Fei Yang <fei.yang@intel.com> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> CC: John Harrison <john.c.harrison@intel.com> --- drivers/gpu/drm/i915/gt/intel_gt.h | 1 + drivers/gpu/drm/i915/gt/intel_tlb.c | 25 ++++++++++++ drivers/gpu/drm/i915/gt/intel_tlb.h | 3 ++ drivers/gpu/drm/i915/gt/uc/intel_guc.h | 1 + .../gpu/drm/i915/gt/uc/intel_guc_submission.c | 39 +++++++++++++++---- drivers/gpu/drm/i915/i915_driver.c | 6 +++ 6 files changed, 68 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/i915/gt/intel_gt.h b/drivers/gpu/drm/i915/gt/intel_gt.h index 970bedf6b78a7..4e3bb221d2f4d 100644 --- a/drivers/gpu/drm/i915/gt/intel_gt.h +++ b/drivers/gpu/drm/i915/gt/intel_gt.h @@ -179,4 +179,5 @@ enum i915_map_type intel_gt_coherent_map_type(struct intel_gt *gt, void intel_gt_bind_context_set_ready(struct intel_gt *gt); void intel_gt_bind_context_set_unready(struct intel_gt *gt); bool intel_gt_is_bind_context_ready(struct intel_gt *gt); + #endif /* __INTEL_GT_H__ */ diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.c b/drivers/gpu/drm/i915/gt/intel_tlb.c index a84563c178bc6..c6bb1b34abdaa 100644 --- a/drivers/gpu/drm/i915/gt/intel_tlb.c +++ b/drivers/gpu/drm/i915/gt/intel_tlb.c @@ -166,6 +166,31 @@ void intel_gt_fini_tlb(struct intel_gt *gt) mutex_destroy(>->tlb.invalidate_lock); } +void intel_gt_tlb_suspend_all(struct drm_i915_private *i915) +{ + struct intel_gt *gt; + int i; + + if (!HAS_GUC_TLB_INVALIDATION(i915)) + return; + for_each_gt(gt, i915, i) + wake_up_all_tlb_invalidate(>->uc.guc); +} + +void intel_gt_tlb_resume_all(struct drm_i915_private *i915) +{ + struct intel_gt *gt; + int i; + + if (!HAS_GUC_TLB_INVALIDATION(i915)) + return; + for_each_gt(gt, i915, i) { + /* Perform tlb invalidation on both GT and GuC, in that order. */ + intel_guc_invalidate_tlb_full(>->uc.guc); + intel_guc_invalidate_tlb(>->uc.guc); + } +} + #if IS_ENABLED(CONFIG_DRM_I915_SELFTEST) #include "selftest_tlb.c" #endif diff --git a/drivers/gpu/drm/i915/gt/intel_tlb.h b/drivers/gpu/drm/i915/gt/intel_tlb.h index 337327af92ac4..1a34ec0c447f2 100644 --- a/drivers/gpu/drm/i915/gt/intel_tlb.h +++ b/drivers/gpu/drm/i915/gt/intel_tlb.h @@ -26,4 +26,7 @@ static inline u32 intel_gt_next_invalidate_tlb_full(const struct intel_gt *gt) return intel_gt_tlb_seqno(gt) | 1; } +void intel_gt_tlb_suspend_all(struct drm_i915_private *i915); +void intel_gt_tlb_resume_all(struct drm_i915_private *i915); + #endif /* INTEL_TLB_H */ diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.h b/drivers/gpu/drm/i915/gt/uc/intel_guc.h index d7203ba88b0c0..e76b26095eef2 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc.h +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.h @@ -536,4 +536,5 @@ void intel_guc_dump_time_info(struct intel_guc *guc, struct drm_printer *p); int intel_guc_sched_disable_gucid_threshold_max(struct intel_guc *guc); +void wake_up_all_tlb_invalidate(struct intel_guc *guc); #endif diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c index 12f79abd889ce..a023f8d303540 100644 --- a/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c +++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c @@ -32,6 +32,7 @@ #include "i915_drv.h" #include "i915_reg.h" +#include "i915_irq.h" #include "i915_trace.h" /** @@ -1796,13 +1797,23 @@ static void __guc_reset_context(struct intel_context *ce, intel_engine_mask_t st intel_context_put(parent); } -void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stalled) +void wake_up_all_tlb_invalidate(struct intel_guc *guc) { struct intel_guc_tlb_wait *wait; + unsigned long i; + + if (!HAS_GUC_TLB_INVALIDATION(guc_to_gt(guc)->i915)) + return; + xa_for_each(&guc->tlb_lookup, i, wait) { + wake_up(&wait->wq); + } +} + +void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stalled) +{ struct intel_context *ce; unsigned long index; unsigned long flags; - unsigned long i; if (unlikely(!guc_submission_initialized(guc))) { /* Reset called during driver load? GuC not yet initialised! */ @@ -1833,9 +1844,7 @@ void intel_guc_submission_reset(struct intel_guc *guc, intel_engine_mask_t stall * The full GT reset will have cleared the TLB caches and flushed the * G2H message queue; we can release all the blocked waiters. */ - xa_for_each(&guc->tlb_lookup, i, wait) { - wake_up(&wait->wq); - } + wake_up_all_tlb_invalidate(guc); } static void guc_cancel_context_requests(struct intel_context *ce) @@ -1931,6 +1940,12 @@ void intel_guc_submission_cancel_requests(struct intel_guc *guc) /* GuC is blown away, drop all references to contexts */ xa_destroy(&guc->context_lookup); + + /* + * Wedged GT won't respond to any TLB invalidation request. Simply + * release all the blocked waiters. + */ + wake_up_all_tlb_invalidate(guc); } void intel_guc_submission_reset_finish(struct intel_guc *guc) @@ -4746,6 +4761,14 @@ static long must_wait_woken(struct wait_queue_entry *wq_entry, long timeout) return timeout; } +static bool intel_gt_is_enabled(const struct intel_gt *gt) +{ + /* Check if GT is wedged or suspended */ + if (intel_gt_is_wedged(gt) || !intel_irqs_enabled(gt->i915)) + return false; + return true; +} + static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 type) { struct intel_guc_tlb_wait _wq, *wq = &_wq; @@ -4763,7 +4786,8 @@ static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 type) }; u32 size = ARRAY_SIZE(action); - if (!intel_guc_ct_enabled(&guc->ct)) + if (!intel_gt_is_enabled(gt) || + !intel_guc_ct_enabled(&guc->ct)) return -EINVAL; init_waitqueue_head(&_wq.wq); @@ -4806,7 +4830,8 @@ static int guc_send_invalidate_tlb(struct intel_guc *guc, u32 type) * requests that can be queued in CT buffer. */ #define OUTSTANDING_GUC_TIMEOUT_PERIOD (HZ * 2) - if (!must_wait_woken(&wait, OUTSTANDING_GUC_TIMEOUT_PERIOD)) { + if (intel_gt_is_enabled(gt) && + !must_wait_woken(&wait, OUTSTANDING_GUC_TIMEOUT_PERIOD)) { gt_err(gt, "TLB invalidation response timed out for seqno %u\n", seqno); err = -ETIME; diff --git a/drivers/gpu/drm/i915/i915_driver.c b/drivers/gpu/drm/i915/i915_driver.c index ccbb2834cde07..0c9d9826d2f41 100644 --- a/drivers/gpu/drm/i915/i915_driver.c +++ b/drivers/gpu/drm/i915/i915_driver.c @@ -72,6 +72,7 @@ #include "gt/intel_gt.h" #include "gt/intel_gt_pm.h" #include "gt/intel_rc6.h" +#include "gt/intel_tlb.h" #include "pxp/intel_pxp.h" #include "pxp/intel_pxp_debugfs.h" @@ -1093,6 +1094,9 @@ static int i915_drm_suspend(struct drm_device *dev) intel_dp_mst_suspend(dev_priv); intel_runtime_pm_disable_interrupts(dev_priv); + + intel_gt_tlb_suspend_all(dev_priv); + intel_hpd_cancel_work(dev_priv); intel_suspend_encoders(dev_priv); @@ -1264,6 +1268,8 @@ static int i915_drm_resume(struct drm_device *dev) intel_gvt_resume(dev_priv); + intel_gt_tlb_resume_all(dev_priv); + enable_rpm_wakeref_asserts(&dev_priv->runtime_pm); return 0; -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [Intel-gfx] [PATCH v5 4/4] drm/i915/gt: Increase sleep in gt_tlb selftest sanitycheck 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines Jonathan Cavitt 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 3/4] drm/i915: No TLB invalidation on wedged or suspended GT Jonathan Cavitt @ 2023-10-04 18:36 ` Jonathan Cavitt 2023-10-04 19:03 ` [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Andi Shyti ` (4 subsequent siblings) 7 siblings, 0 replies; 15+ messages in thread From: Jonathan Cavitt @ 2023-10-04 18:36 UTC (permalink / raw) To: intel-gfx Cc: janusz.krzysztofik, andi.shyti, matthew.d.roper, jonathan.cavitt, chris.p.wilson, nirmoy.das For the gt_tlb live selftest, increase the timeout from 10 ms to 200 ms. 200 ms should be more than enough time, and 10 ms was too aggressive. Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> --- drivers/gpu/drm/i915/gt/selftest_tlb.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/i915/gt/selftest_tlb.c b/drivers/gpu/drm/i915/gt/selftest_tlb.c index 7e41f69fc818f..46e0a1dbecc8d 100644 --- a/drivers/gpu/drm/i915/gt/selftest_tlb.c +++ b/drivers/gpu/drm/i915/gt/selftest_tlb.c @@ -137,7 +137,7 @@ pte_tlbinv(struct intel_context *ce, i915_request_add(rq); /* Short sleep to sanitycheck the batch is spinning before we begin */ - msleep(10); + msleep(200); if (va == vb) { if (!i915_request_completed(rq)) { pr_err("%s(%s): Semaphore sanitycheck failed %llx, with alignment %llx, using PTE size %x (phys %x, sg %x)\n", -- 2.25.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt ` (2 preceding siblings ...) 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 4/4] drm/i915/gt: Increase sleep in gt_tlb selftest sanitycheck Jonathan Cavitt @ 2023-10-04 19:03 ` Andi Shyti 2023-10-04 19:10 ` John Harrison 2023-10-04 19:09 ` John Harrison ` (3 subsequent siblings) 7 siblings, 1 reply; 15+ messages in thread From: Andi Shyti @ 2023-10-04 19:03 UTC (permalink / raw) To: Jonathan Cavitt Cc: janusz.krzysztofik, matthew.d.roper, intel-gfx, chris.p.wilson, nirmoy.das Hi Jonathan, On Wed, Oct 04, 2023 at 11:36:22AM -0700, Jonathan Cavitt wrote: > Add pci (device info) tags for if GuC TLB Invalidation is enabled. > Since GuC based TLB invalidation is only strictly necessary for MTL > resently, only enable GuC based TLB invalidations for MTL. > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> Jani was mentioning that the pci tags is not a proper title. No need to resend, I think I will merge this series, so that, if you agree, I can change /pci tags/pci flag/ before pushing. In any case. Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> Andi > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_pci.c | 1 + > drivers/gpu/drm/i915/intel_device_info.h | 3 ++- > 3 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2b7a6db4d0d44..1e25cc1e3dba1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -807,4 +807,5 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > #define HAS_LMEMBAR_SMEM_STOLEN(i915) (!HAS_LMEM(i915) && \ > GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) > > +#define HAS_GUC_TLB_INVALIDATION(i915) (INTEL_INFO(i915)->has_guc_tlb_invalidation) > #endif > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index df7c261410f79..c3a5d5efb45d1 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -837,6 +837,7 @@ static const struct intel_device_info mtl_info = { > .memory_regions = REGION_SMEM | REGION_STOLEN_LMEM, > .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0), > .require_force_probe = 1, > + .has_guc_tlb_invalidation = 1, > MTL_CACHELEVEL, > }; > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 39817490b13fd..ad54db0a22470 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -173,7 +173,8 @@ enum intel_ppgtt_type { > func(has_coherent_ggtt); \ > func(tuning_thread_rr_after_dep); \ > func(unfenced_needs_alignment); \ > - func(hws_needs_physical); > + func(hws_needs_physical); \ > + func(has_guc_tlb_invalidation); > > struct intel_ip_version { > u8 ver; > -- > 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 19:03 ` [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Andi Shyti @ 2023-10-04 19:10 ` John Harrison 2023-10-04 19:24 ` Andi Shyti 0 siblings, 1 reply; 15+ messages in thread From: John Harrison @ 2023-10-04 19:10 UTC (permalink / raw) To: Andi Shyti, Jonathan Cavitt Cc: janusz.krzysztofik, matthew.d.roper, intel-gfx, chris.p.wilson, nirmoy.das On 10/4/2023 12:03, Andi Shyti wrote: > Hi Jonathan, > > On Wed, Oct 04, 2023 at 11:36:22AM -0700, Jonathan Cavitt wrote: >> Add pci (device info) tags for if GuC TLB Invalidation is enabled. >> Since GuC based TLB invalidation is only strictly necessary for MTL >> resently, only enable GuC based TLB invalidations for MTL. >> >> Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > Jani was mentioning that the pci tags is not a proper title. > > No need to resend, I think I will merge this series, so that, if > you agree, I can change /pci tags/pci flag/ before pushing. Have all the review comments been addressed? Surely it can't be pushed until it has at least an ack from everyone who has expressed concerns about the changes? John. > > In any case. > > Reviewed-by: Andi Shyti <andi.shyti@linux.intel.com> > > Andi > >> --- >> drivers/gpu/drm/i915/i915_drv.h | 1 + >> drivers/gpu/drm/i915/i915_pci.c | 1 + >> drivers/gpu/drm/i915/intel_device_info.h | 3 ++- >> 3 files changed, 4 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h >> index 2b7a6db4d0d44..1e25cc1e3dba1 100644 >> --- a/drivers/gpu/drm/i915/i915_drv.h >> +++ b/drivers/gpu/drm/i915/i915_drv.h >> @@ -807,4 +807,5 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, >> #define HAS_LMEMBAR_SMEM_STOLEN(i915) (!HAS_LMEM(i915) && \ >> GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) >> >> +#define HAS_GUC_TLB_INVALIDATION(i915) (INTEL_INFO(i915)->has_guc_tlb_invalidation) >> #endif >> diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c >> index df7c261410f79..c3a5d5efb45d1 100644 >> --- a/drivers/gpu/drm/i915/i915_pci.c >> +++ b/drivers/gpu/drm/i915/i915_pci.c >> @@ -837,6 +837,7 @@ static const struct intel_device_info mtl_info = { >> .memory_regions = REGION_SMEM | REGION_STOLEN_LMEM, >> .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0), >> .require_force_probe = 1, >> + .has_guc_tlb_invalidation = 1, >> MTL_CACHELEVEL, >> }; >> >> diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h >> index 39817490b13fd..ad54db0a22470 100644 >> --- a/drivers/gpu/drm/i915/intel_device_info.h >> +++ b/drivers/gpu/drm/i915/intel_device_info.h >> @@ -173,7 +173,8 @@ enum intel_ppgtt_type { >> func(has_coherent_ggtt); \ >> func(tuning_thread_rr_after_dep); \ >> func(unfenced_needs_alignment); \ >> - func(hws_needs_physical); >> + func(hws_needs_physical); \ >> + func(has_guc_tlb_invalidation); >> >> struct intel_ip_version { >> u8 ver; >> -- >> 2.25.1 ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 19:10 ` John Harrison @ 2023-10-04 19:24 ` Andi Shyti 2023-10-04 19:44 ` Matt Roper 2023-10-04 19:47 ` Andi Shyti 0 siblings, 2 replies; 15+ messages in thread From: Andi Shyti @ 2023-10-04 19:24 UTC (permalink / raw) To: John Harrison Cc: janusz.krzysztofik, matthew.d.roper, intel-gfx, Jonathan Cavitt, chris.p.wilson, nirmoy.das Hi John, > > > Add pci (device info) tags for if GuC TLB Invalidation is enabled. > > > Since GuC based TLB invalidation is only strictly necessary for MTL > > > resently, only enable GuC based TLB invalidations for MTL. > > > > > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > Jani was mentioning that the pci tags is not a proper title. > > > > No need to resend, I think I will merge this series, so that, if > > you agree, I can change /pci tags/pci flag/ before pushing. > Have all the review comments been addressed? Surely it can't be pushed until > it has at least an ack from everyone who has expressed concerns about the > changes? this particular patch did not receive any comment so far, except for the "pci tags" from Jani. This solution was somehow hinted by Tvrtko in one of the previous review, I guess. Personally I think that having a pci flag just for this is a bit of an overkill, but I don't have a strong opinion about it. Andi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 19:24 ` Andi Shyti @ 2023-10-04 19:44 ` Matt Roper 2023-10-04 19:47 ` Andi Shyti 1 sibling, 0 replies; 15+ messages in thread From: Matt Roper @ 2023-10-04 19:44 UTC (permalink / raw) To: Andi Shyti Cc: janusz.krzysztofik, intel-gfx, Jonathan Cavitt, chris.p.wilson, nirmoy.das On Wed, Oct 04, 2023 at 09:24:12PM +0200, Andi Shyti wrote: > Hi John, > > > > > Add pci (device info) tags for if GuC TLB Invalidation is enabled. > > > > Since GuC based TLB invalidation is only strictly necessary for MTL > > > > resently, only enable GuC based TLB invalidations for MTL. > > > > > > > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > > Jani was mentioning that the pci tags is not a proper title. > > > > > > No need to resend, I think I will merge this series, so that, if > > > you agree, I can change /pci tags/pci flag/ before pushing. > > Have all the review comments been addressed? Surely it can't be pushed until > > it has at least an ack from everyone who has expressed concerns about the > > changes? > > this particular patch did not receive any comment so far, except > for the "pci tags" from Jani. > > This solution was somehow hinted by Tvrtko in one of the previous > review, I guess. > > Personally I think that having a pci flag just for this is a bit > of an overkill, but I don't have a strong opinion about it. Drive-by comment: you probably only want to turn on the feature flag for MTL at the end of the series, not at the beginning. Otherwise bisects that land somewhere in the middle might have half the necessary changes but not all of them. Matt > > Andi -- Matt Roper Graphics Software Engineer Linux GPU Platform Enablement Intel Corporation ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 19:24 ` Andi Shyti 2023-10-04 19:44 ` Matt Roper @ 2023-10-04 19:47 ` Andi Shyti 1 sibling, 0 replies; 15+ messages in thread From: Andi Shyti @ 2023-10-04 19:47 UTC (permalink / raw) To: Andi Shyti Cc: janusz.krzysztofik, matthew.d.roper, intel-gfx, Jonathan Cavitt, chris.p.wilson, nirmoy.das > > > > Add pci (device info) tags for if GuC TLB Invalidation is enabled. > > > > Since GuC based TLB invalidation is only strictly necessary for MTL > > > > resently, only enable GuC based TLB invalidations for MTL. > > > > > > > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > > > Jani was mentioning that the pci tags is not a proper title. > > > > > > No need to resend, I think I will merge this series, so that, if > > > you agree, I can change /pci tags/pci flag/ before pushing. > > Have all the review comments been addressed? Surely it can't be pushed until > > it has at least an ack from everyone who has expressed concerns about the > > changes? and... of course... I won't push it until all the reviews and acks are in place... I just wanted to save Jonathan a v6 just for this change if, in a furtunate case, there won't be other reviews. Andi > this particular patch did not receive any comment so far, except > for the "pci tags" from Jani. > > This solution was somehow hinted by Tvrtko in one of the previous > review, I guess. > > Personally I think that having a pci flag just for this is a bit > of an overkill, but I don't have a strong opinion about it. > > Andi ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt ` (3 preceding siblings ...) 2023-10-04 19:03 ` [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Andi Shyti @ 2023-10-04 19:09 ` John Harrison 2023-10-05 2:18 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v5,1/4] " Patchwork ` (2 subsequent siblings) 7 siblings, 0 replies; 15+ messages in thread From: John Harrison @ 2023-10-04 19:09 UTC (permalink / raw) To: Jonathan Cavitt, intel-gfx Cc: janusz.krzysztofik, andi.shyti, matthew.d.roper, chris.p.wilson, nirmoy.das Why is there no cover letter for this patch series? It is at v5 but there is no history of what has changed from one version to the next. That makes it much harder to review. John. On 10/4/2023 11:36, Jonathan Cavitt wrote: > Add pci (device info) tags for if GuC TLB Invalidation is enabled. > Since GuC based TLB invalidation is only strictly necessary for MTL > resently, only enable GuC based TLB invalidations for MTL. > > Signed-off-by: Jonathan Cavitt <jonathan.cavitt@intel.com> > --- > drivers/gpu/drm/i915/i915_drv.h | 1 + > drivers/gpu/drm/i915/i915_pci.c | 1 + > drivers/gpu/drm/i915/intel_device_info.h | 3 ++- > 3 files changed, 4 insertions(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h > index 2b7a6db4d0d44..1e25cc1e3dba1 100644 > --- a/drivers/gpu/drm/i915/i915_drv.h > +++ b/drivers/gpu/drm/i915/i915_drv.h > @@ -807,4 +807,5 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915, > #define HAS_LMEMBAR_SMEM_STOLEN(i915) (!HAS_LMEM(i915) && \ > GRAPHICS_VER_FULL(i915) >= IP_VER(12, 70)) > > +#define HAS_GUC_TLB_INVALIDATION(i915) (INTEL_INFO(i915)->has_guc_tlb_invalidation) > #endif > diff --git a/drivers/gpu/drm/i915/i915_pci.c b/drivers/gpu/drm/i915/i915_pci.c > index df7c261410f79..c3a5d5efb45d1 100644 > --- a/drivers/gpu/drm/i915/i915_pci.c > +++ b/drivers/gpu/drm/i915/i915_pci.c > @@ -837,6 +837,7 @@ static const struct intel_device_info mtl_info = { > .memory_regions = REGION_SMEM | REGION_STOLEN_LMEM, > .platform_engine_mask = BIT(RCS0) | BIT(BCS0) | BIT(CCS0), > .require_force_probe = 1, > + .has_guc_tlb_invalidation = 1, > MTL_CACHELEVEL, > }; > > diff --git a/drivers/gpu/drm/i915/intel_device_info.h b/drivers/gpu/drm/i915/intel_device_info.h > index 39817490b13fd..ad54db0a22470 100644 > --- a/drivers/gpu/drm/i915/intel_device_info.h > +++ b/drivers/gpu/drm/i915/intel_device_info.h > @@ -173,7 +173,8 @@ enum intel_ppgtt_type { > func(has_coherent_ggtt); \ > func(tuning_thread_rr_after_dep); \ > func(unfenced_needs_alignment); \ > - func(hws_needs_physical); > + func(hws_needs_physical); \ > + func(has_guc_tlb_invalidation); > > struct intel_ip_version { > u8 ver; ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v5,1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt ` (4 preceding siblings ...) 2023-10-04 19:09 ` John Harrison @ 2023-10-05 2:18 ` Patchwork 2023-10-05 2:18 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2023-10-05 2:37 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 7 siblings, 0 replies; 15+ messages in thread From: Patchwork @ 2023-10-05 2:18 UTC (permalink / raw) To: Jonathan Cavitt; +Cc: intel-gfx == Series Details == Series: series starting with [v5,1/4] drm/i915: Add GuC TLB Invalidation pci tags URL : https://patchwork.freedesktop.org/series/124636/ State : warning == Summary == Error: dim checkpatch failed 27e7965d6b55 drm/i915: Add GuC TLB Invalidation pci tags feb663efcadc drm/i915: Define and use GuC and CTB TLB invalidation routines -:342: WARNING:AVOID_BUG: Do not crash the kernel unless it is absolutely unavoidable--use WARN_ON_ONCE() plus recovery code (if feasible) instead of BUG() or variants #342: FILE: drivers/gpu/drm/i915/gt/uc/intel_guc_submission.c:1997: + GEM_BUG_ON(wait->busy); total: 0 errors, 1 warnings, 0 checks, 458 lines checked f59bb1890992 drm/i915: No TLB invalidation on wedged or suspended GT 757482f22624 drm/i915/gt: Increase sleep in gt_tlb selftest sanitycheck ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] ✗ Fi.CI.SPARSE: warning for series starting with [v5,1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt ` (5 preceding siblings ...) 2023-10-05 2:18 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v5,1/4] " Patchwork @ 2023-10-05 2:18 ` Patchwork 2023-10-05 2:37 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork 7 siblings, 0 replies; 15+ messages in thread From: Patchwork @ 2023-10-05 2:18 UTC (permalink / raw) To: Jonathan Cavitt; +Cc: intel-gfx == Series Details == Series: series starting with [v5,1/4] drm/i915: Add GuC TLB Invalidation pci tags URL : https://patchwork.freedesktop.org/series/124636/ State : warning == Summary == Error: dim sparse failed Sparse version: v0.6.2 Fast mode used, each commit won't be checked separately. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [Intel-gfx] ✗ Fi.CI.BAT: failure for series starting with [v5,1/4] drm/i915: Add GuC TLB Invalidation pci tags 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt ` (6 preceding siblings ...) 2023-10-05 2:18 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork @ 2023-10-05 2:37 ` Patchwork 7 siblings, 0 replies; 15+ messages in thread From: Patchwork @ 2023-10-05 2:37 UTC (permalink / raw) To: Jonathan Cavitt; +Cc: intel-gfx [-- Attachment #1: Type: text/plain, Size: 8249 bytes --] == Series Details == Series: series starting with [v5,1/4] drm/i915: Add GuC TLB Invalidation pci tags URL : https://patchwork.freedesktop.org/series/124636/ State : failure == Summary == CI Bug Log - changes from CI_DRM_13713 -> Patchwork_124636v1 ==================================================== Summary ------- **FAILURE** Serious unknown changes coming with Patchwork_124636v1 absolutely need to be verified manually. If you think the reported changes have nothing to do with the changes introduced in Patchwork_124636v1, please notify your bug team (lgci.bug.filing@intel.com) to allow them to document this new failure mode, which will reduce false positives in CI. External URL: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/index.html Participating hosts (40 -> 39) ------------------------------ Missing (1): fi-snb-2520m Possible new issues ------------------- Here are the unknown changes that may have been introduced in Patchwork_124636v1: ### IGT changes ### #### Possible regressions #### * igt@kms_busy@basic@flip: - bat-adln-1: [PASS][1] -> [INCOMPLETE][2] [1]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-adln-1/igt@kms_busy@basic@flip.html [2]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-adln-1/igt@kms_busy@basic@flip.html - bat-rplp-1: [PASS][3] -> [INCOMPLETE][4] [3]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-rplp-1/igt@kms_busy@basic@flip.html [4]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-rplp-1/igt@kms_busy@basic@flip.html - bat-adlp-6: [PASS][5] -> [INCOMPLETE][6] [5]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-adlp-6/igt@kms_busy@basic@flip.html [6]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-adlp-6/igt@kms_busy@basic@flip.html * igt@kms_force_connector_basic@force-connector-state: - bat-rpls-1: [PASS][7] -> [INCOMPLETE][8] [7]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-rpls-1/igt@kms_force_connector_basic@force-connector-state.html [8]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-rpls-1/igt@kms_force_connector_basic@force-connector-state.html Known issues ------------ Here are the changes found in Patchwork_124636v1 that come from known issues: ### CI changes ### #### Possible fixes #### * boot: - fi-bsw-n3050: [FAIL][9] ([i915#8293]) -> [PASS][10] [9]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/fi-bsw-n3050/boot.html [10]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-bsw-n3050/boot.html ### IGT changes ### #### Issues hit #### * igt@gem_exec_suspend@basic-s3@lmem0: - bat-atsm-1: NOTRUN -> [DMESG-WARN][11] ([i915#8841]) +4 other tests dmesg-warn [11]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-atsm-1/igt@gem_exec_suspend@basic-s3@lmem0.html * igt@gem_lmem_swapping@random-engines: - fi-bsw-n3050: NOTRUN -> [SKIP][12] ([fdo#109271]) +18 other tests skip [12]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-bsw-n3050/igt@gem_lmem_swapping@random-engines.html * igt@i915_selftest@live@dmabuf: - fi-bsw-nick: [PASS][13] -> [DMESG-FAIL][14] ([i915#7913]) [13]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/fi-bsw-nick/igt@i915_selftest@live@dmabuf.html [14]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-bsw-nick/igt@i915_selftest@live@dmabuf.html * igt@i915_selftest@live@gtt: - bat-adlp-9: [PASS][15] -> [INCOMPLETE][16] ([i915#7913]) [15]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-adlp-9/igt@i915_selftest@live@gtt.html [16]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-adlp-9/igt@i915_selftest@live@gtt.html - bat-dg2-11: [PASS][17] -> [INCOMPLETE][18] ([i915#7913]) [17]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-dg2-11/igt@i915_selftest@live@gtt.html [18]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-dg2-11/igt@i915_selftest@live@gtt.html * igt@i915_suspend@basic-s3-without-i915: - bat-atsm-1: NOTRUN -> [SKIP][19] ([i915#6645]) [19]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-atsm-1/igt@i915_suspend@basic-s3-without-i915.html - bat-mtlp-8: NOTRUN -> [SKIP][20] ([i915#6645]) [20]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-mtlp-8/igt@i915_suspend@basic-s3-without-i915.html * igt@kms_hdmi_inject@inject-audio: - fi-bsw-n3050: NOTRUN -> [FAIL][21] ([IGT#3]) [21]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-bsw-n3050/igt@kms_hdmi_inject@inject-audio.html * igt@kms_pipe_crc_basic@suspend-read-crc: - bat-atsm-1: NOTRUN -> [SKIP][22] ([i915#1836]) [22]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-atsm-1/igt@kms_pipe_crc_basic@suspend-read-crc.html #### Possible fixes #### * igt@i915_selftest@live@execlists: - fi-bsw-nick: [ABORT][23] ([i915#7911] / [i915#7913]) -> [PASS][24] [23]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/fi-bsw-nick/igt@i915_selftest@live@execlists.html [24]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-bsw-nick/igt@i915_selftest@live@execlists.html * igt@i915_selftest@live@gt_heartbeat: - fi-apl-guc: [DMESG-FAIL][25] ([i915#5334]) -> [PASS][26] [25]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html [26]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-apl-guc/igt@i915_selftest@live@gt_heartbeat.html * igt@i915_selftest@live@requests: - bat-mtlp-8: [ABORT][27] ([i915#9414]) -> [PASS][28] [27]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/bat-mtlp-8/igt@i915_selftest@live@requests.html [28]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/bat-mtlp-8/igt@i915_selftest@live@requests.html #### Warnings #### * igt@i915_suspend@basic-s2idle-without-i915: - fi-ivb-3770: [DMESG-WARN][29] ([i915#1982] / [i915#8841]) -> [DMESG-WARN][30] ([i915#8841]) [29]: https://intel-gfx-ci.01.org/tree/drm-tip/CI_DRM_13713/fi-ivb-3770/igt@i915_suspend@basic-s2idle-without-i915.html [30]: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/fi-ivb-3770/igt@i915_suspend@basic-s2idle-without-i915.html {name}: This element is suppressed. This means it is ignored when computing the status of the difference (SUCCESS, WARNING, or FAILURE). [IGT#3]: https://gitlab.freedesktop.org/drm/igt-gpu-tools/issues/3 [fdo#109271]: https://bugs.freedesktop.org/show_bug.cgi?id=109271 [i915#1836]: https://gitlab.freedesktop.org/drm/intel/issues/1836 [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982 [i915#5334]: https://gitlab.freedesktop.org/drm/intel/issues/5334 [i915#6645]: https://gitlab.freedesktop.org/drm/intel/issues/6645 [i915#7911]: https://gitlab.freedesktop.org/drm/intel/issues/7911 [i915#7913]: https://gitlab.freedesktop.org/drm/intel/issues/7913 [i915#8293]: https://gitlab.freedesktop.org/drm/intel/issues/8293 [i915#8841]: https://gitlab.freedesktop.org/drm/intel/issues/8841 [i915#9414]: https://gitlab.freedesktop.org/drm/intel/issues/9414 Build changes ------------- * Linux: CI_DRM_13713 -> Patchwork_124636v1 CI-20190529: 20190529 CI_DRM_13713: 4540606b1e3d945191858f82b07576e12feb7f26 @ git://anongit.freedesktop.org/gfx-ci/linux IGT_7512: 2eb58faf82d3cd5e2e74154a7319cff56eb40762 @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git Patchwork_124636v1: 4540606b1e3d945191858f82b07576e12feb7f26 @ git://anongit.freedesktop.org/gfx-ci/linux ### Linux commits 26f31d449650 drm/i915/gt: Increase sleep in gt_tlb selftest sanitycheck 8434b48054dc drm/i915: No TLB invalidation on wedged or suspended GT 6fd399123b57 drm/i915: Define and use GuC and CTB TLB invalidation routines e32d37e8465f drm/i915: Add GuC TLB Invalidation pci tags == Logs == For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/Patchwork_124636v1/index.html [-- Attachment #2: Type: text/html, Size: 9724 bytes --] ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2023-10-05 15:09 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2023-10-04 18:36 [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Jonathan Cavitt 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 2/4] drm/i915: Define and use GuC and CTB TLB invalidation routines Jonathan Cavitt 2023-10-05 8:54 ` Tvrtko Ursulin 2023-10-05 15:08 ` Cavitt, Jonathan 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 3/4] drm/i915: No TLB invalidation on wedged or suspended GT Jonathan Cavitt 2023-10-04 18:36 ` [Intel-gfx] [PATCH v5 4/4] drm/i915/gt: Increase sleep in gt_tlb selftest sanitycheck Jonathan Cavitt 2023-10-04 19:03 ` [Intel-gfx] [PATCH v5 1/4] drm/i915: Add GuC TLB Invalidation pci tags Andi Shyti 2023-10-04 19:10 ` John Harrison 2023-10-04 19:24 ` Andi Shyti 2023-10-04 19:44 ` Matt Roper 2023-10-04 19:47 ` Andi Shyti 2023-10-04 19:09 ` John Harrison 2023-10-05 2:18 ` [Intel-gfx] ✗ Fi.CI.CHECKPATCH: warning for series starting with [v5,1/4] " Patchwork 2023-10-05 2:18 ` [Intel-gfx] ✗ Fi.CI.SPARSE: " Patchwork 2023-10-05 2:37 ` [Intel-gfx] ✗ Fi.CI.BAT: failure " Patchwork
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox