From 649f533b7aa2bda13d9ef0a6ef4b0a622b226d2b Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Wed, 16 Oct 2024 16:17:17 +0200 Subject: [PATCH 01/54] drm/xe: Add caller info to xe_gt_reset_async Add caller info to the xe_gt_reset_async() to help debug issues. v2: s/%pS/%ps(Matt) Cc: Matthew Auld Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2874 Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20241016141717.881143-1-nirmoy.das@intel.com Signed-off-by: Nirmoy Das --- drivers/gpu/drm/xe/xe_gt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index 1c79660fb08612..df2fe4005d85aa 100644 --- a/drivers/gpu/drm/xe/xe_gt.c +++ b/drivers/gpu/drm/xe/xe_gt.c @@ -834,7 +834,7 @@ static void gt_reset_worker(struct work_struct *w) void xe_gt_reset_async(struct xe_gt *gt) { - xe_gt_info(gt, "trying reset\n"); + xe_gt_info(gt, "trying reset from %ps\n", __builtin_return_address(0)); /* Don't do a reset while one is already in flight */ if (!xe_fault_inject_gt_reset() && xe_uc_reset_prepare(>->uc)) From f5fc004b332117079613347cfd4e4773066bbf03 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:36 +0530 Subject: [PATCH 02/54] drm/xe: Add member initialized_domains to xe_force_wake() This field serves as a bitmask representing all initialized forcewake domains on the GT. v2 - Move awake_domains datatype change out of this patch (Michal) - Rename domain_init to init_domain (Michal) - optimize alignment (Michal) Cc: Michal Wajdeczko Cc: Badal Nilawar Cc: Rodrigo Vivi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Reviewed-by: Michal Wajdeczko Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-2-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.c | 30 ++++++++++++++---------- drivers/gpu/drm/xe/xe_force_wake_types.h | 2 ++ 2 files changed, 19 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index a64c14757c8420..ac0419da71731e 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -21,15 +21,25 @@ static const char *str_wake_sleep(bool wake) return wake ? "wake" : "sleep"; } -static void domain_init(struct xe_force_wake_domain *domain, +static void mark_domain_initialized(struct xe_force_wake *fw, + enum xe_force_wake_domain_id id) +{ + fw->initialized_domains |= BIT(id); +} + +static void init_domain(struct xe_force_wake *fw, enum xe_force_wake_domain_id id, struct xe_reg reg, struct xe_reg ack) { + struct xe_force_wake_domain *domain = &fw->domains[id]; + domain->id = id; domain->reg_ctl = reg; domain->reg_ack = ack; domain->val = FORCEWAKE_MT(FORCEWAKE_KERNEL); domain->mask = FORCEWAKE_MT_MASK(FORCEWAKE_KERNEL); + + mark_domain_initialized(fw, id); } void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw) @@ -43,13 +53,11 @@ void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw) xe_gt_assert(gt, GRAPHICS_VER(gt_to_xe(gt)) >= 11); if (xe->info.graphics_verx100 >= 1270) { - domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT], - XE_FW_DOMAIN_ID_GT, + init_domain(fw, XE_FW_DOMAIN_ID_GT, FORCEWAKE_GT, FORCEWAKE_ACK_GT_MTL); } else { - domain_init(&fw->domains[XE_FW_DOMAIN_ID_GT], - XE_FW_DOMAIN_ID_GT, + init_domain(fw, XE_FW_DOMAIN_ID_GT, FORCEWAKE_GT, FORCEWAKE_ACK_GT); } @@ -63,8 +71,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw) xe_gt_assert(gt, GRAPHICS_VER(gt_to_xe(gt)) >= 11); if (!xe_gt_is_media_type(gt)) - domain_init(&fw->domains[XE_FW_DOMAIN_ID_RENDER], - XE_FW_DOMAIN_ID_RENDER, + init_domain(fw, XE_FW_DOMAIN_ID_RENDER, FORCEWAKE_RENDER, FORCEWAKE_ACK_RENDER); @@ -72,8 +79,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw) if (!(gt->info.engine_mask & BIT(i))) continue; - domain_init(&fw->domains[XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j], - XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j, + init_domain(fw, XE_FW_DOMAIN_ID_MEDIA_VDBOX0 + j, FORCEWAKE_MEDIA_VDBOX(j), FORCEWAKE_ACK_MEDIA_VDBOX(j)); } @@ -82,15 +88,13 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw) if (!(gt->info.engine_mask & BIT(i))) continue; - domain_init(&fw->domains[XE_FW_DOMAIN_ID_MEDIA_VEBOX0 + j], - XE_FW_DOMAIN_ID_MEDIA_VEBOX0 + j, + init_domain(fw, XE_FW_DOMAIN_ID_MEDIA_VEBOX0 + j, FORCEWAKE_MEDIA_VEBOX(j), FORCEWAKE_ACK_MEDIA_VEBOX(j)); } if (gt->info.engine_mask & BIT(XE_HW_ENGINE_GSCCS0)) - domain_init(&fw->domains[XE_FW_DOMAIN_ID_GSC], - XE_FW_DOMAIN_ID_GSC, + init_domain(fw, XE_FW_DOMAIN_ID_GSC, FORCEWAKE_GSC, FORCEWAKE_ACK_GSC); } diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h index ed0edc2cdf9f84..494240777d0399 100644 --- a/drivers/gpu/drm/xe/xe_force_wake_types.h +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h @@ -79,6 +79,8 @@ struct xe_force_wake { spinlock_t lock; /** @awake_domains: mask of all domains awake */ enum xe_force_wake_domains awake_domains; + /** @initialized_domains: mask of all initialized domains */ + unsigned int initialized_domains; /** @domains: force wake domains */ struct xe_force_wake_domain domains[XE_FW_DOMAIN_ID_COUNT]; }; From 38820e63a3d0557ac8b4c6be47d413bddba798ca Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:37 +0530 Subject: [PATCH 03/54] drm/xe/forcewake: Change awake_domain datatype Change the datatype of awake_domains to unsigned int to accommodate values that differ from the enum xe_force_wake_domains. Cc: Michal Wajdeczko Cc: Badal Nilawar Cc: Rodrigo Vivi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Reviewed-by: Michal Wajdeczko Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-3-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake_types.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h index 494240777d0399..fde17dc3d01e04 100644 --- a/drivers/gpu/drm/xe/xe_force_wake_types.h +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h @@ -78,7 +78,7 @@ struct xe_force_wake { /** @lock: protects everything force wake struct */ spinlock_t lock; /** @awake_domains: mask of all domains awake */ - enum xe_force_wake_domains awake_domains; + unsigned int awake_domains; /** @initialized_domains: mask of all initialized domains */ unsigned int initialized_domains; /** @domains: force wake domains */ From 9d62b07027f0710b7af03d78780d0a6c2425bc1e Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:38 +0530 Subject: [PATCH 04/54] drm/xe/forcewake: Add a helper xe_force_wake_ref_has_domain() The helper xe_force_wake_ref_has_domain() checks if the input domain has been successfully reference-counted and awakened in the reference. v2 - Fix commit message and kernel-doc (Michal) - Remove unnecessary paranthesis (Michal) Cc: Michal Wajdeczko Cc: Badal Nilawar Cc: Rodrigo Vivi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Reviewed-by: Michal Wajdeczko Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-4-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.h | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h index a2577672f4e3e6..1608a55edc846e 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.h +++ b/drivers/gpu/drm/xe/xe_force_wake.h @@ -46,4 +46,20 @@ xe_force_wake_assert_held(struct xe_force_wake *fw, xe_gt_assert(fw->gt, fw->awake_domains & domain); } +/** + * xe_force_wake_ref_has_domain - verifies if the domains are in fw_ref + * @fw_ref : the force_wake reference + * @domain : forcewake domain to verify + * + * This function confirms whether the @fw_ref includes a reference to the + * specified @domain. + * + * Return: true if domain is refcounted. + */ +static inline bool +xe_force_wake_ref_has_domain(unsigned int fw_ref, enum xe_force_wake_domains domain) +{ + return fw_ref & domain; +} + #endif From a7ddcea1f5acba83347ff0d701732abd1c6c7036 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:39 +0530 Subject: [PATCH 05/54] drm/xe: Error handling in xe_force_wake_get() If an acknowledgment timeout occurs for a forcewake domain awake request, do not increment the reference count for the domain. This ensures that subsequent _get calls do not incorrectly assume the domain is awake. The return value is a mask of domains that got refcounted, and these domains need to be provided for subsequent xe_force_wake_put call. While at it, add simple kernel-doc for xe_force_wake_get() v3 - Use explicit type for mask (Michal/Badal) - Improve kernel-doc (Michal) - Use unsigned int instead of abusing enum (Michal) v5 - Use unsigned int for return (MattB/Badal/Rodrigo) - use xe_gt_WARN for domain awake ack failure (Badal/Rodrigo) v6 - Change XE_FORCEWAKE_ALL to single bit, this helps accommodate actually refcounted domains in return. (Michal) - Modify commit message and warn message (Badal) - Remove unnecessary information in kernel-doc (Michal) v7 - Add assert condition for valid input domains (Badal) v9 - Update kernel-doc and simplify conditions (Michal) Cc: Michal Wajdeczko Cc: Badal Nilawar Cc: Rodrigo Vivi Cc: Lucas De Marchi Cc: Nirmoy Das Reviewed-by: Badal Nilawar Reviewed-by: Michal Wajdeczko Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-5-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.c | 52 +++++++++++++++++++----- drivers/gpu/drm/xe/xe_force_wake.h | 4 +- drivers/gpu/drm/xe/xe_force_wake_types.h | 2 +- 3 files changed, 45 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index ac0419da71731e..d36ed4f8bdbe97 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -154,29 +154,61 @@ static int domain_sleep_wait(struct xe_gt *gt, (ffs(tmp__) - 1))) && \ domain__->reg_ctl.addr) -int xe_force_wake_get(struct xe_force_wake *fw, - enum xe_force_wake_domains domains) +/** + * xe_force_wake_get() : Increase the domain refcount + * @fw: struct xe_force_wake + * @domains: forcewake domains to get refcount on + * + * This function wakes up @domains if they are asleep and takes references. + * If requested domain is XE_FORCEWAKE_ALL then only applicable/initialized + * domains will be considered for refcount and it is a caller responsibility + * to check returned ref if it includes any specific domain by using + * xe_force_wake_ref_has_domain() function. Caller must call + * xe_force_wake_put() function to decrease incremented refcounts. + * + * Return: opaque reference to woken domains or zero if none of requested + * domains were awake. + */ +unsigned int xe_force_wake_get(struct xe_force_wake *fw, + enum xe_force_wake_domains domains) { struct xe_gt *gt = fw->gt; struct xe_force_wake_domain *domain; - enum xe_force_wake_domains tmp, woken = 0; + unsigned int ref_incr = 0, awake_rqst = 0, awake_failed = 0; + unsigned int tmp, ref_rqst; unsigned long flags; - int ret = 0; + xe_gt_assert(gt, is_power_of_2(domains)); + xe_gt_assert(gt, domains <= XE_FORCEWAKE_ALL); + xe_gt_assert(gt, domains == XE_FORCEWAKE_ALL || fw->initialized_domains & domains); + + ref_rqst = (domains == XE_FORCEWAKE_ALL) ? fw->initialized_domains : domains; spin_lock_irqsave(&fw->lock, flags); - for_each_fw_domain_masked(domain, domains, fw, tmp) { + for_each_fw_domain_masked(domain, ref_rqst, fw, tmp) { if (!domain->ref++) { - woken |= BIT(domain->id); + awake_rqst |= BIT(domain->id); domain_wake(gt, domain); } + ref_incr |= BIT(domain->id); } - for_each_fw_domain_masked(domain, woken, fw, tmp) { - ret |= domain_wake_wait(gt, domain); + for_each_fw_domain_masked(domain, awake_rqst, fw, tmp) { + if (domain_wake_wait(gt, domain) == 0) { + fw->awake_domains |= BIT(domain->id); + } else { + awake_failed |= BIT(domain->id); + --domain->ref; + } } - fw->awake_domains |= woken; + ref_incr &= ~awake_failed; spin_unlock_irqrestore(&fw->lock, flags); - return ret; + xe_gt_WARN(gt, awake_failed, "Forcewake domain%s %#x failed to acknowledge awake request\n", + str_plural(hweight_long(awake_failed)), awake_failed); + + if (domains == XE_FORCEWAKE_ALL && ref_incr == fw->initialized_domains) + ref_incr |= XE_FORCEWAKE_ALL; + + return ref_incr; } int xe_force_wake_put(struct xe_force_wake *fw, diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h index 1608a55edc846e..75fa1a19797c0c 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.h +++ b/drivers/gpu/drm/xe/xe_force_wake.h @@ -15,8 +15,8 @@ void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw); void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw); -int xe_force_wake_get(struct xe_force_wake *fw, - enum xe_force_wake_domains domains); +unsigned int xe_force_wake_get(struct xe_force_wake *fw, + enum xe_force_wake_domains domains); int xe_force_wake_put(struct xe_force_wake *fw, enum xe_force_wake_domains domains); diff --git a/drivers/gpu/drm/xe/xe_force_wake_types.h b/drivers/gpu/drm/xe/xe_force_wake_types.h index fde17dc3d01e04..899fbbcb3ea9c6 100644 --- a/drivers/gpu/drm/xe/xe_force_wake_types.h +++ b/drivers/gpu/drm/xe/xe_force_wake_types.h @@ -48,7 +48,7 @@ enum xe_force_wake_domains { XE_FW_MEDIA_VEBOX2 = BIT(XE_FW_DOMAIN_ID_MEDIA_VEBOX2), XE_FW_MEDIA_VEBOX3 = BIT(XE_FW_DOMAIN_ID_MEDIA_VEBOX3), XE_FW_GSC = BIT(XE_FW_DOMAIN_ID_GSC), - XE_FORCEWAKE_ALL = BIT(XE_FW_DOMAIN_ID_COUNT) - 1 + XE_FORCEWAKE_ALL = BIT(XE_FW_DOMAIN_ID_COUNT) }; /** From 79f716bbfa2c7c2639d161a4294ed0416a1c6efe Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:40 +0530 Subject: [PATCH 06/54] drm/xe: Modify xe_force_wake_put to handle _get returned mask Instead of calling xe_force_wake_put on all domains that were input to xe_force_wake_get, call _put only on the domains whose reference counts were successfully incremented by the _get call. Since the return value of _get can be a mask that does not match any specific value in the enum xe_force_wake_domains, change the input parameter of _put to unsigned int. v3 - Move WARN to this patch (Badal) - use xe_gt_WARN instead of XE_WARN (Michal) - Stop using xe_force_wake_domains for non enum values. - Remove kernel-doc from this patch (Badal) -v5 - Fix global awake_domain -v6 - put all initialized domains in case of FORCEWAKE_ALL. - Modify ret variable name (Michal) - Modify input var name (Michal) - Modify commit message and warn (Badal) -v9 - Add assert condition. Cc: Michal Wajdeczko Cc: Badal Nilawar Cc: Rodrigo Vivi Cc: Lucas De Marchi Cc: Nirmoy Das Reviewed-by: Badal Nilawar Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Michal Wajdeczko Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-6-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.c | 30 +++++++++++++++++++++++------- drivers/gpu/drm/xe/xe_force_wake.h | 2 +- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index d36ed4f8bdbe97..7f285dbe6e2c4e 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -212,26 +212,42 @@ unsigned int xe_force_wake_get(struct xe_force_wake *fw, } int xe_force_wake_put(struct xe_force_wake *fw, - enum xe_force_wake_domains domains) + unsigned int fw_ref) { struct xe_gt *gt = fw->gt; struct xe_force_wake_domain *domain; - enum xe_force_wake_domains tmp, sleep = 0; + unsigned int tmp, sleep = 0; unsigned long flags; - int ret = 0; + int ack_fail = 0; + + /* + * Avoid unnecessary lock and unlock when the function is called + * in error path of individual domains. + */ + if (!fw_ref) + return 0; + + if (xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) + fw_ref = fw->initialized_domains; spin_lock_irqsave(&fw->lock, flags); - for_each_fw_domain_masked(domain, domains, fw, tmp) { + for_each_fw_domain_masked(domain, fw_ref, fw, tmp) { + xe_gt_assert(gt, domain->ref); + if (!--domain->ref) { sleep |= BIT(domain->id); domain_sleep(gt, domain); } } for_each_fw_domain_masked(domain, sleep, fw, tmp) { - ret |= domain_sleep_wait(gt, domain); + if (domain_sleep_wait(gt, domain) == 0) + fw->awake_domains &= ~BIT(domain->id); + else + ack_fail |= BIT(domain->id); } - fw->awake_domains &= ~sleep; spin_unlock_irqrestore(&fw->lock, flags); - return ret; + xe_gt_WARN(gt, ack_fail, "Forcewake domain%s %#x failed to acknowledge sleep request\n", + str_plural(hweight_long(ack_fail)), ack_fail); + return ack_fail; } diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h index 75fa1a19797c0c..f0b27dbe75811b 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.h +++ b/drivers/gpu/drm/xe/xe_force_wake.h @@ -18,7 +18,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, unsigned int xe_force_wake_get(struct xe_force_wake *fw, enum xe_force_wake_domains domains); int xe_force_wake_put(struct xe_force_wake *fw, - enum xe_force_wake_domains domains); + unsigned int fw_ref); static inline int xe_force_wake_ref(struct xe_force_wake *fw, From 3b41f8882e4b25908043139eb4ea98d031543136 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:41 +0530 Subject: [PATCH 07/54] drm/xe/device: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be escalated/considered as probing error. It internally WARNS on domain ack failure. v5 - return unsigned int xe_force_wake_get() v7 - Fix commit message(Badal) v9 - s/uint/unsigned int (Nikula) Cc: Jani Nikula Cc: Badal Nilawar Cc: Rodrigo Vivi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-7-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_device.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_device.c b/drivers/gpu/drm/xe/xe_device.c index 22b572f0612c91..2da4affe4dfdca 100644 --- a/drivers/gpu/drm/xe/xe_device.c +++ b/drivers/gpu/drm/xe/xe_device.c @@ -604,8 +604,8 @@ int xe_device_probe_early(struct xe_device *xe) static int probe_has_flat_ccs(struct xe_device *xe) { struct xe_gt *gt; + unsigned int fw_ref; u32 reg; - int err; /* Always enabled/disabled, no runtime check to do */ if (GRAPHICS_VER(xe) < 20 || !xe->info.has_flat_ccs) @@ -613,9 +613,9 @@ static int probe_has_flat_ccs(struct xe_device *xe) gt = xe_root_mmio_gt(xe); - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - return err; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return -ETIMEDOUT; reg = xe_gt_mcr_unicast_read_any(gt, XE2_FLAT_CCS_BASE_RANGE_LOWER); xe->info.has_flat_ccs = (reg & XE2_FLAT_CCS_ENABLE); @@ -624,7 +624,8 @@ static int probe_has_flat_ccs(struct xe_device *xe) drm_dbg(&xe->drm, "Flat CCS has been disabled in bios, May lead to performance impact"); - return xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); + return 0; } int xe_device_probe(struct xe_device *xe) @@ -875,6 +876,7 @@ void xe_device_wmb(struct xe_device *xe) void xe_device_td_flush(struct xe_device *xe) { struct xe_gt *gt; + unsigned int fw_ref; u8 id; if (!IS_DGFX(xe) || GRAPHICS_VER(xe) < 20) @@ -889,7 +891,8 @@ void xe_device_td_flush(struct xe_device *xe) if (xe_gt_is_media_type(gt)) continue; - if (xe_force_wake_get(gt_to_fw(gt), XE_FW_GT)) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) return; xe_mmio_write32(>->mmio, XE2_TDF_CTRL, TRANSIENT_FLUSH_REQUEST); @@ -904,22 +907,22 @@ void xe_device_td_flush(struct xe_device *xe) 150, NULL, false)) xe_gt_err_once(gt, "TD flush timeout\n"); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } } void xe_device_l2_flush(struct xe_device *xe) { struct xe_gt *gt; - int err; + unsigned int fw_ref; gt = xe_root_mmio_gt(xe); if (!XE_WA(gt, 16023588340)) return; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) return; spin_lock(>->global_invl_lock); @@ -929,7 +932,7 @@ void xe_device_l2_flush(struct xe_device *xe) xe_gt_err_once(gt, "Global invalidation timeout\n"); spin_unlock(>->global_invl_lock); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } u32 xe_device_ccs_bytes(struct xe_device *xe, u64 size) From 82d9de63cac77f7c923c200ff56a962bddf747c1 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:42 +0530 Subject: [PATCH 08/54] drm/xe/hdcp: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int for xe_force_wake_get() v7 - Fix commit message Cc: Suraj Kandpal Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Suraj Kandpal Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-8-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/display/xe_hdcp_gsc.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c index 6619a40aed1533..3567f474b6cb35 100644 --- a/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c +++ b/drivers/gpu/drm/xe/display/xe_hdcp_gsc.c @@ -41,6 +41,7 @@ bool intel_hdcp_gsc_check_status(struct xe_device *xe) struct xe_gt *gt = tile->media_gt; struct xe_gsc *gsc = >->uc.gsc; bool ret = true; + unsigned int fw_ref; if (!gsc && !xe_uc_fw_is_enabled(&gsc->fw)) { drm_dbg_kms(&xe->drm, @@ -49,7 +50,8 @@ bool intel_hdcp_gsc_check_status(struct xe_device *xe) } xe_pm_runtime_get(xe); - if (xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC)) { + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC); + if (!fw_ref) { drm_dbg_kms(&xe->drm, "failed to get forcewake to check proxy status\n"); ret = false; @@ -59,7 +61,7 @@ bool intel_hdcp_gsc_check_status(struct xe_device *xe) if (!xe_gsc_proxy_init_done(gsc)) ret = false; - xe_force_wake_put(gt_to_fw(gt), XE_FW_GSC); + xe_force_wake_put(gt_to_fw(gt), fw_ref); out: xe_pm_runtime_put(xe); return ret; From 21eb4f178d719ef32b9b1910afb33bc87395ea6d Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:43 +0530 Subject: [PATCH 09/54] drm/xe/gsc: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int for xe_force_wake_get() - No need to WARN from caller in case of forcewake get failure. v7 - Fix commit message Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-9-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_gsc.c | 23 +++++++++++------------ drivers/gpu/drm/xe/xe_gsc_proxy.c | 9 ++++----- 2 files changed, 15 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_gsc.c b/drivers/gpu/drm/xe/xe_gsc.c index 783b09bf368160..1eb791ddc375c7 100644 --- a/drivers/gpu/drm/xe/xe_gsc.c +++ b/drivers/gpu/drm/xe/xe_gsc.c @@ -261,19 +261,17 @@ static int gsc_upload_and_init(struct xe_gsc *gsc) { struct xe_gt *gt = gsc_to_gt(gsc); struct xe_tile *tile = gt_to_tile(gt); + unsigned int fw_ref; int ret; if (XE_WA(tile->primary_gt, 14018094691)) { - ret = xe_force_wake_get(gt_to_fw(tile->primary_gt), XE_FORCEWAKE_ALL); + fw_ref = xe_force_wake_get(gt_to_fw(tile->primary_gt), XE_FORCEWAKE_ALL); /* * If the forcewake fails we want to keep going, because the worst * case outcome in failing to apply the WA is that PXP won't work, - * which is not fatal. We still throw a warning so the issue is - * seen if it happens. + * which is not fatal. Forcewake get warns implicitly in case of failure */ - xe_gt_WARN_ON(tile->primary_gt, ret); - xe_gt_mcr_multicast_write(tile->primary_gt, EU_SYSTOLIC_LIC_THROTTLE_CTL_WITH_LOCK, EU_SYSTOLIC_LIC_THROTTLE_CTL_LOCK_BIT); @@ -282,7 +280,7 @@ static int gsc_upload_and_init(struct xe_gsc *gsc) ret = gsc_upload(gsc); if (XE_WA(tile->primary_gt, 14018094691)) - xe_force_wake_put(gt_to_fw(tile->primary_gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(tile->primary_gt), fw_ref); if (ret) return ret; @@ -352,6 +350,7 @@ static void gsc_work(struct work_struct *work) struct xe_gsc *gsc = container_of(work, typeof(*gsc), work); struct xe_gt *gt = gsc_to_gt(gsc); struct xe_device *xe = gt_to_xe(gt); + unsigned int fw_ref; u32 actions; int ret; @@ -361,7 +360,7 @@ static void gsc_work(struct work_struct *work) spin_unlock_irq(&gsc->lock); xe_pm_runtime_get(xe); - xe_gt_WARN_ON(gt, xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC); if (actions & GSC_ACTION_ER_COMPLETE) { ret = gsc_er_complete(gt); @@ -381,7 +380,7 @@ static void gsc_work(struct work_struct *work) xe_gsc_proxy_request_handler(gsc); out: - xe_force_wake_put(gt_to_fw(gt), XE_FW_GSC); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_pm_runtime_put(xe); } @@ -601,7 +600,7 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p) { struct xe_gt *gt = gsc_to_gt(gsc); struct xe_mmio *mmio = >->mmio; - int err; + unsigned int fw_ref; xe_uc_fw_print(&gsc->fw, p); @@ -610,8 +609,8 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p) if (!xe_uc_fw_is_enabled(&gsc->fw)) return; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC); + if (!fw_ref) return; drm_printf(p, "\nHECI1 FWSTS: 0x%08x 0x%08x 0x%08x 0x%08x 0x%08x 0x%08x\n", @@ -622,5 +621,5 @@ void xe_gsc_print_info(struct xe_gsc *gsc, struct drm_printer *p) xe_mmio_read32(mmio, HECI_FWSTS5(MTL_GSC_HECI1_BASE)), xe_mmio_read32(mmio, HECI_FWSTS6(MTL_GSC_HECI1_BASE))); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GSC); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } diff --git a/drivers/gpu/drm/xe/xe_gsc_proxy.c b/drivers/gpu/drm/xe/xe_gsc_proxy.c index 6d89c22ae8111a..fc64b45d324b9c 100644 --- a/drivers/gpu/drm/xe/xe_gsc_proxy.c +++ b/drivers/gpu/drm/xe/xe_gsc_proxy.c @@ -450,22 +450,21 @@ void xe_gsc_proxy_remove(struct xe_gsc *gsc) { struct xe_gt *gt = gsc_to_gt(gsc); struct xe_device *xe = gt_to_xe(gt); - int err = 0; + unsigned int fw_ref = 0; if (!gsc->proxy.component_added) return; /* disable HECI2 IRQs */ xe_pm_runtime_get(xe); - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GSC); + if (!fw_ref) xe_gt_err(gt, "failed to get forcewake to disable GSC interrupts\n"); /* try do disable irq even if forcewake failed */ gsc_proxy_irq_toggle(gsc, false); - if (!err) - xe_force_wake_put(gt_to_fw(gt), XE_FW_GSC); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_pm_runtime_put(xe); xe_gsc_wait_for_worker_completion(gsc); From 30d105577a3319094f8ae5ff1ceea670f1931487 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:44 +0530 Subject: [PATCH 10/54] drm/xe/gt: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Use helper xe_force_wake_ref_has_domain to verify all domains are initialized or not. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v4 - Rebase fix v5 - return unsigned int for xe_force_wake_get() - remove redundant XE_WARN_ON() v6 - use helper for checking all initialized domains are awake or not. v7 - Fix commit message v9 - Remove redundant WARN_ON (Badal) Cc: Badal Nilawar Cc: Matthew Brost Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-10-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_gt.c | 105 ++++++++++++++++++++----------------- 1 file changed, 58 insertions(+), 47 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index df2fe4005d85aa..89e9d9d4db060b 100644 --- a/drivers/gpu/drm/xe/xe_gt.c +++ b/drivers/gpu/drm/xe/xe_gt.c @@ -97,14 +97,14 @@ void xe_gt_sanitize(struct xe_gt *gt) static void xe_gt_enable_host_l2_vram(struct xe_gt *gt) { + unsigned int fw_ref; u32 reg; - int err; if (!XE_WA(gt, 16023588340)) return; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (WARN_ON(err)) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) return; if (!xe_gt_is_media_type(gt)) { @@ -114,13 +114,13 @@ static void xe_gt_enable_host_l2_vram(struct xe_gt *gt) } xe_gt_mcr_multicast_write(gt, XEHPC_L3CLOS_MASK(3), 0x3); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static void xe_gt_disable_host_l2_vram(struct xe_gt *gt) { + unsigned int fw_ref; u32 reg; - int err; if (!XE_WA(gt, 16023588340)) return; @@ -128,15 +128,15 @@ static void xe_gt_disable_host_l2_vram(struct xe_gt *gt) if (xe_gt_is_media_type(gt)) return; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (WARN_ON(err)) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) return; reg = xe_gt_mcr_unicast_read_any(gt, XE2_GAMREQSTRM_CTRL); reg &= ~CG_DIS_CNTLBUS; xe_gt_mcr_multicast_write(gt, XE2_GAMREQSTRM_CTRL, reg); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } /** @@ -402,11 +402,14 @@ static void dump_pat_on_error(struct xe_gt *gt) static int gt_fw_domain_init(struct xe_gt *gt) { + unsigned int fw_ref; int err, i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) { + err = -ETIMEDOUT; goto err_hw_fence_irq; + } if (!xe_gt_is_media_type(gt)) { err = xe_ggtt_init(gt_to_tile(gt)->mem.ggtt); @@ -441,14 +444,12 @@ static int gt_fw_domain_init(struct xe_gt *gt) */ gt->info.gmdid = xe_mmio_read32(>->mmio, GMD_ID); - err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); - XE_WARN_ON(err); - + xe_force_wake_put(gt_to_fw(gt), fw_ref); return 0; err_force_wake: dump_pat_on_error(gt); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); err_hw_fence_irq: for (i = 0; i < XE_ENGINE_CLASS_MAX; ++i) xe_hw_fence_irq_finish(>->fence_irq[i]); @@ -458,11 +459,14 @@ static int gt_fw_domain_init(struct xe_gt *gt) static int all_fw_domain_init(struct xe_gt *gt) { + unsigned int fw_ref; int err, i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) - goto err_hw_fence_irq; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + err = -ETIMEDOUT; + goto err_force_wake; + } xe_gt_mcr_set_implicit_defaults(gt); xe_reg_sr_apply_mmio(>->reg_sr, gt); @@ -526,14 +530,12 @@ static int all_fw_domain_init(struct xe_gt *gt) if (IS_SRIOV_PF(gt_to_xe(gt))) xe_gt_sriov_pf_init_hw(gt); - err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); - XE_WARN_ON(err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); return 0; err_force_wake: - xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); -err_hw_fence_irq: + xe_force_wake_put(gt_to_fw(gt), fw_ref); for (i = 0; i < XE_ENGINE_CLASS_MAX; ++i) xe_hw_fence_irq_finish(>->fence_irq[i]); @@ -546,11 +548,12 @@ static int all_fw_domain_init(struct xe_gt *gt) */ int xe_gt_init_hwconfig(struct xe_gt *gt) { + unsigned int fw_ref; int err; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - goto out; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return -ETIMEDOUT; xe_gt_mcr_init_early(gt); xe_pat_init(gt); @@ -568,8 +571,7 @@ int xe_gt_init_hwconfig(struct xe_gt *gt) xe_gt_enable_host_l2_vram(gt); out_fw: - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); -out: + xe_force_wake_put(gt_to_fw(gt), fw_ref); return err; } @@ -764,6 +766,7 @@ static int do_gt_restart(struct xe_gt *gt) static int gt_reset(struct xe_gt *gt) { + unsigned int fw_ref; int err; if (xe_device_wedged(gt_to_xe(gt))) @@ -784,9 +787,11 @@ static int gt_reset(struct xe_gt *gt) xe_gt_sanitize(gt); - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) - goto err_msg; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + err = -ETIMEDOUT; + goto err_out; + } xe_uc_gucrc_disable(>->uc); xe_uc_stop_prepare(>->uc); @@ -804,8 +809,7 @@ static int gt_reset(struct xe_gt *gt) if (err) goto err_out; - err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); - XE_WARN_ON(err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_pm_runtime_put(gt_to_xe(gt)); xe_gt_info(gt, "reset done\n"); @@ -813,8 +817,7 @@ static int gt_reset(struct xe_gt *gt) return 0; err_out: - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); -err_msg: + xe_force_wake_put(gt_to_fw(gt), fw_ref); XE_WARN_ON(xe_uc_start(>->uc)); err_fail: xe_gt_err(gt, "reset failed (%pe)\n", ERR_PTR(err)); @@ -846,22 +849,25 @@ void xe_gt_reset_async(struct xe_gt *gt) void xe_gt_suspend_prepare(struct xe_gt *gt) { - XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + unsigned int fw_ref; + + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); xe_uc_stop_prepare(>->uc); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } int xe_gt_suspend(struct xe_gt *gt) { + unsigned int fw_ref; int err; xe_gt_dbg(gt, "suspending\n"); xe_gt_sanitize(gt); - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) goto err_msg; err = xe_uc_suspend(>->uc); @@ -872,14 +878,15 @@ int xe_gt_suspend(struct xe_gt *gt) xe_gt_disable_host_l2_vram(gt); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_gt_dbg(gt, "suspended\n"); return 0; -err_force_wake: - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); err_msg: + err = -ETIMEDOUT; +err_force_wake: + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_gt_err(gt, "suspend failed (%pe)\n", ERR_PTR(err)); return err; @@ -887,9 +894,11 @@ int xe_gt_suspend(struct xe_gt *gt) void xe_gt_shutdown(struct xe_gt *gt) { - xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + unsigned int fw_ref; + + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); do_gt_reset(gt); - xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } /** @@ -914,11 +923,12 @@ int xe_gt_sanitize_freq(struct xe_gt *gt) int xe_gt_resume(struct xe_gt *gt) { + unsigned int fw_ref; int err; xe_gt_dbg(gt, "resuming\n"); - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) goto err_msg; err = do_gt_restart(gt); @@ -927,14 +937,15 @@ int xe_gt_resume(struct xe_gt *gt) xe_gt_idle_enable_pg(gt); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_gt_dbg(gt, "resumed\n"); return 0; -err_force_wake: - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); err_msg: + err = -ETIMEDOUT; +err_force_wake: + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_gt_err(gt, "resume failed (%pe)\n", ERR_PTR(err)); return err; From a66c19895396e66e578e28d9b598959a5406a6cb Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:45 +0530 Subject: [PATCH 11/54] drm/xe/xe_gt_idle: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v4 - Rebase fix v5 - return unsigned int for xe_force_wake_get() - Remove reudandant WARN calls. v7 - Fix commit message Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-11-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_gt_idle.c | 26 +++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_gt_idle.c b/drivers/gpu/drm/xe/xe_gt_idle.c index 746812aee8ff81..fd80afeef56a7c 100644 --- a/drivers/gpu/drm/xe/xe_gt_idle.c +++ b/drivers/gpu/drm/xe/xe_gt_idle.c @@ -101,6 +101,7 @@ void xe_gt_idle_enable_pg(struct xe_gt *gt) struct xe_gt_idle *gtidle = >->gtidle; struct xe_mmio *mmio = >->mmio; u32 vcs_mask, vecs_mask; + unsigned int fw_ref; int i, j; if (IS_SRIOV_VF(xe)) @@ -127,7 +128,7 @@ void xe_gt_idle_enable_pg(struct xe_gt *gt) VDN_MFXVDENC_POWERGATE_ENABLE(j)); } - XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); if (xe->info.skip_guc_pc) { /* * GuC sets the hysteresis value when GuC PC is enabled @@ -138,12 +139,13 @@ void xe_gt_idle_enable_pg(struct xe_gt *gt) } xe_mmio_write32(mmio, POWERGATE_ENABLE, gtidle->powergate_enable); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } void xe_gt_idle_disable_pg(struct xe_gt *gt) { struct xe_gt_idle *gtidle = >->gtidle; + unsigned int fw_ref; if (IS_SRIOV_VF(gt_to_xe(gt))) return; @@ -151,9 +153,9 @@ void xe_gt_idle_disable_pg(struct xe_gt *gt) xe_device_assert_mem_access(gt_to_xe(gt)); gtidle->powergate_enable = 0; - XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); xe_mmio_write32(>->mmio, POWERGATE_ENABLE, gtidle->powergate_enable); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } /** @@ -172,7 +174,8 @@ int xe_gt_idle_pg_print(struct xe_gt *gt, struct drm_printer *p) enum xe_gt_idle_state state; u32 pg_enabled, pg_status = 0; u32 vcs_mask, vecs_mask; - int err, n; + unsigned int fw_ref; + int n; /* * Media Slices * @@ -208,14 +211,14 @@ int xe_gt_idle_pg_print(struct xe_gt *gt, struct drm_printer *p) /* Do not wake the GT to read powergating status */ if (state != GT_IDLE_C6) { - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - return err; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return -ETIMEDOUT; pg_enabled = xe_mmio_read32(>->mmio, POWERGATE_ENABLE); pg_status = xe_mmio_read32(>->mmio, POWERGATE_DOMAIN_STATUS); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FW_GT)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } if (gt->info.engine_mask & XE_HW_ENGINE_RCS_MASK) { @@ -298,13 +301,14 @@ static void gt_idle_fini(void *arg) { struct kobject *kobj = arg; struct xe_gt *gt = kobj_to_gt(kobj->parent); + unsigned int fw_ref; xe_gt_idle_disable_pg(gt); if (gt_to_xe(gt)->info.skip_guc_pc) { - XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FW_GT)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); xe_gt_idle_disable_c6(gt); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } sysfs_remove_files(kobj, gt_idle_attrs); From 9ffd6ec2de08ef4ac5f17f6131d1db57613493f9 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:46 +0530 Subject: [PATCH 12/54] drm/xe/devcoredump: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Use helper xe_force_wake_ref_has_domain to verify all domains are initialized or not. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int for xe_force_wake_get() v6 - use helper xe_force_wake_ref_has_domain() v7 - Fix commit message Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-12-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_devcoredump.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c index 99842a35dbf052..8b0ea77661b2b0 100644 --- a/drivers/gpu/drm/xe/xe_devcoredump.c +++ b/drivers/gpu/drm/xe/xe_devcoredump.c @@ -158,13 +158,15 @@ static void xe_devcoredump_deferred_snap_work(struct work_struct *work) { struct xe_devcoredump_snapshot *ss = container_of(work, typeof(*ss), work); struct xe_devcoredump *coredump = container_of(ss, typeof(*coredump), snapshot); + unsigned int fw_ref; /* keep going if fw fails as we still want to save the memory and SW data */ - if (xe_force_wake_get(gt_to_fw(ss->gt), XE_FORCEWAKE_ALL)) + fw_ref = xe_force_wake_get(gt_to_fw(ss->gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) xe_gt_info(ss->gt, "failed to get forcewake for coredump capture\n"); xe_vm_snapshot_capture_delayed(ss->vm); xe_guc_exec_queue_snapshot_capture_delayed(ss->ge); - xe_force_wake_put(gt_to_fw(ss->gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(ss->gt), fw_ref); /* Calculate devcoredump size */ ss->read.size = __xe_devcoredump_read(NULL, INT_MAX, coredump); @@ -236,8 +238,9 @@ static void devcoredump_snapshot(struct xe_devcoredump *coredump, u32 width_mask = (0x1 << q->width) - 1; const char *process_name = "no process"; - int i; + unsigned int fw_ref; bool cookie; + int i; ss->snapshot_time = ktime_get_real(); ss->boot_time = ktime_get_boottime(); @@ -261,8 +264,7 @@ static void devcoredump_snapshot(struct xe_devcoredump *coredump, } /* keep going if fw fails as we still want to save the memory and SW data */ - if (xe_force_wake_get(gt_to_fw(q->gt), XE_FORCEWAKE_ALL)) - xe_gt_info(ss->gt, "failed to get forcewake for coredump capture\n"); + fw_ref = xe_force_wake_get(gt_to_fw(q->gt), XE_FORCEWAKE_ALL); ss->guc.log = xe_guc_log_snapshot_capture(&guc->log, true); ss->guc.ct = xe_guc_ct_snapshot_capture(&guc->ct, true); @@ -274,7 +276,7 @@ static void devcoredump_snapshot(struct xe_devcoredump *coredump, queue_work(system_unbound_wq, &ss->work); - xe_force_wake_put(gt_to_fw(q->gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(q->gt), fw_ref); dma_fence_end_signalling(cookie); } From 6a966d677d06e96a81d430537abb5db65e2b4fda Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:47 +0530 Subject: [PATCH 13/54] drm/xe/tests/mocs: Update xe_force_wake_get() return handling With xe_force_wake_get() now returning the refcount-incremented domain mask, a return value of 0 indicates failure for single domains. Change assert condition to incorporate this change in return and pass the return value to xe_force_wake_put() v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int for xe_force_wake_get() Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-13-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/tests/xe_mocs.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-) diff --git a/drivers/gpu/drm/xe/tests/xe_mocs.c b/drivers/gpu/drm/xe/tests/xe_mocs.c index ea932c051cc7a6..6f9b7a266b4195 100644 --- a/drivers/gpu/drm/xe/tests/xe_mocs.c +++ b/drivers/gpu/drm/xe/tests/xe_mocs.c @@ -43,12 +43,11 @@ static void read_l3cc_table(struct xe_gt *gt, { struct kunit *test = kunit_get_current_test(); u32 l3cc, l3cc_expected; - unsigned int i; + unsigned int fw_ref, i; u32 reg_val; - u32 ret; - ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - KUNIT_ASSERT_EQ_MSG(test, ret, 0, "Forcewake Failed.\n"); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + KUNIT_ASSERT_NE_MSG(test, fw_ref, 0, "Forcewake Failed.\n"); for (i = 0; i < info->num_mocs_regs; i++) { if (!(i & 1)) { @@ -72,7 +71,7 @@ static void read_l3cc_table(struct xe_gt *gt, KUNIT_EXPECT_EQ_MSG(test, l3cc_expected, l3cc, "l3cc idx=%u has incorrect val.\n", i); } - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static void read_mocs_table(struct xe_gt *gt, @@ -80,15 +79,14 @@ static void read_mocs_table(struct xe_gt *gt, { struct kunit *test = kunit_get_current_test(); u32 mocs, mocs_expected; - unsigned int i; + unsigned int fw_ref, i; u32 reg_val; - u32 ret; KUNIT_EXPECT_TRUE_MSG(test, info->unused_entries_index, "Unused entries index should have been defined\n"); - ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - KUNIT_ASSERT_EQ_MSG(test, ret, 0, "Forcewake Failed.\n"); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + KUNIT_ASSERT_NE_MSG(test, fw_ref, 0, "Forcewake Failed.\n"); for (i = 0; i < info->num_mocs_regs; i++) { if (regs_are_mcr(gt)) @@ -106,7 +104,7 @@ static void read_mocs_table(struct xe_gt *gt, "mocs reg 0x%x has incorrect val.\n", i); } - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static int mocs_kernel_test_run_device(struct xe_device *xe) From a4c48a3fa3cffe4e06502c61034ef23e66ef68a4 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:48 +0530 Subject: [PATCH 14/54] drm/xe/mocs: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - don't use xe_assert() to report HW errors (Michal) v5 - return unsigned int from xe_force_wake_get() - Remove redundant warn v7 - Fix commit message Cc: Michal Wajdeczko Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-14-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_mocs.c | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_mocs.c b/drivers/gpu/drm/xe/xe_mocs.c index 231d0e86ed8359..54d199b5cfb21a 100644 --- a/drivers/gpu/drm/xe/xe_mocs.c +++ b/drivers/gpu/drm/xe/xe_mocs.c @@ -774,25 +774,21 @@ void xe_mocs_init(struct xe_gt *gt) void xe_mocs_dump(struct xe_gt *gt, struct drm_printer *p) { - struct xe_mocs_info table; - unsigned int flags; - u32 ret; struct xe_device *xe = gt_to_xe(gt); + struct xe_mocs_info table; + unsigned int fw_ref, flags; flags = get_mocs_settings(xe, &table); xe_pm_runtime_get_noresume(xe); - ret = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - - if (ret) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) goto err_fw; table.ops->dump(&table, flags, gt, p); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); - + xe_force_wake_put(gt_to_fw(gt), fw_ref); err_fw: - xe_assert(xe, !ret); xe_pm_runtime_put(xe); } From 7fe17fa5ec67e6741af99db9c9f2a666258e9904 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:49 +0530 Subject: [PATCH 15/54] drm/xe/xe_drm_client: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Use helper xe_force_wake_ref_has_domain to verify all domains are initialized or not. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v5 - return unsigned int from xe_force_wake_get() v6 - use xe_force_wake_ref_has_domain() v7 - Fix commit message Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-15-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_drm_client.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_drm_client.c b/drivers/gpu/drm/xe/xe_drm_client.c index fb52a23e28f84e..22f0f1a6dfd55d 100644 --- a/drivers/gpu/drm/xe/xe_drm_client.c +++ b/drivers/gpu/drm/xe/xe_drm_client.c @@ -278,6 +278,7 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file) struct xe_hw_engine *hwe; struct xe_exec_queue *q; u64 gpu_timestamp; + unsigned int fw_ref; xe_pm_runtime_get(xe); @@ -303,13 +304,16 @@ static void show_run_ticks(struct drm_printer *p, struct drm_file *file) continue; fw = xe_hw_engine_to_fw_domain(hwe); - if (xe_force_wake_get(gt_to_fw(gt), fw)) { + + fw_ref = xe_force_wake_get(gt_to_fw(gt), fw); + if (!xe_force_wake_ref_has_domain(fw_ref, fw)) { hwe = NULL; + xe_force_wake_put(gt_to_fw(gt), fw_ref); break; } gpu_timestamp = xe_hw_engine_read_timestamp(hwe); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), fw)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); break; } From 85d547608ef587e7100da2e784e722d6fb968849 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:50 +0530 Subject: [PATCH 16/54] drm/xe/xe_gt_debugfs: Update handling of xe_force_wake_get return With xe_force_wake_get() now returning the refcount-incremented domain mask, a non-zero return value in the case of XE_FORCEWAKE_ALL does not necessarily indicate success. Use xe_force_wake_ref_has_domain() determine the status of the call. Modify the return handling of xe_force_wake_get() accordingly and pass the return value to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v5 - return unsigned int for xe_force_wake_get() v6 - use helper xe_force_wake_ref_has_domain() Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-16-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_gt_debugfs.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_gt_debugfs.c b/drivers/gpu/drm/xe/xe_gt_debugfs.c index cbc43973ff7e12..3e8c351a0eab08 100644 --- a/drivers/gpu/drm/xe/xe_gt_debugfs.c +++ b/drivers/gpu/drm/xe/xe_gt_debugfs.c @@ -90,22 +90,21 @@ static int hw_engines(struct xe_gt *gt, struct drm_printer *p) struct xe_device *xe = gt_to_xe(gt); struct xe_hw_engine *hwe; enum xe_hw_engine_id id; - int err; + unsigned int fw_ref; xe_pm_runtime_get(xe); - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) { + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { xe_pm_runtime_put(xe); - return err; + xe_force_wake_put(gt_to_fw(gt), fw_ref); + return -ETIMEDOUT; } for_each_hw_engine(hwe, gt, id) xe_hw_engine_print(hwe, p); - err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_pm_runtime_put(xe); - if (err) - return err; return 0; } From 31a5dce0a37cbdc6a5a188161d13809aa44862ec Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:51 +0530 Subject: [PATCH 17/54] drm/xe/guc: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Use helper xe_force_wake_ref_has_domain to verify all domains are initialized or not. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v5 - return unsigned int from xe_force_wake_get() - Remove redundant xe_gt_WARN_ON v6 - use helper xe_force_wake_ref_has_domain() v7 - Fix commit message v9 - Rebase Cc: Matthew Brost Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-17-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_guc.c | 13 ++++---- drivers/gpu/drm/xe/xe_guc_log.c | 9 +++--- drivers/gpu/drm/xe/xe_guc_pc.c | 50 ++++++++++++++++++------------ drivers/gpu/drm/xe/xe_guc_submit.c | 6 ++-- 4 files changed, 47 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c index 8570b12182872f..76437d42b8a1eb 100644 --- a/drivers/gpu/drm/xe/xe_guc.c +++ b/drivers/gpu/drm/xe/xe_guc.c @@ -248,10 +248,11 @@ static void guc_fini_hw(void *arg) { struct xe_guc *guc = arg; struct xe_gt *gt = guc_to_gt(guc); + unsigned int fw_ref; - xe_gt_WARN_ON(gt, xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); xe_uc_fini_hw(&guc_to_gt(guc)->uc); - xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } /** @@ -1155,14 +1156,14 @@ int xe_guc_start(struct xe_guc *guc) void xe_guc_print_info(struct xe_guc *guc, struct drm_printer *p) { struct xe_gt *gt = guc_to_gt(guc); + unsigned int fw_ref; u32 status; - int err; int i; xe_uc_fw_print(&guc->fw, p); - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) return; status = xe_mmio_read32(>->mmio, GUC_STATUS); @@ -1183,7 +1184,7 @@ void xe_guc_print_info(struct xe_guc *guc, struct drm_printer *p) i, xe_mmio_read32(>->mmio, SOFT_SCRATCH(i))); } - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_guc_ct_print(&guc->ct, p); xe_guc_submit_print(guc, p); diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c index cc70f448d879c2..fead96216243df 100644 --- a/drivers/gpu/drm/xe/xe_guc_log.c +++ b/drivers/gpu/drm/xe/xe_guc_log.c @@ -145,8 +145,9 @@ struct xe_guc_log_snapshot *xe_guc_log_snapshot_capture(struct xe_guc_log *log, struct xe_device *xe = log_to_xe(log); struct xe_guc *guc = log_to_guc(log); struct xe_gt *gt = log_to_gt(log); + unsigned int fw_ref; size_t remain; - int i, err; + int i; if (!log->bo) { xe_gt_err(gt, "GuC log buffer not allocated\n"); @@ -168,12 +169,12 @@ struct xe_guc_log_snapshot *xe_guc_log_snapshot_capture(struct xe_guc_log *log, remain -= size; } - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) { + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) { snapshot->stamp = ~0; } else { snapshot->stamp = xe_mmio_read32(>->mmio, GUC_PMTIMESTAMP); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } snapshot->ktime = ktime_get_boottime_ns(); snapshot->level = log->level; diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c index 2b654f820ae2c9..e8b9faeaef645f 100644 --- a/drivers/gpu/drm/xe/xe_guc_pc.c +++ b/drivers/gpu/drm/xe/xe_guc_pc.c @@ -415,22 +415,24 @@ u32 xe_guc_pc_get_act_freq(struct xe_guc_pc *pc) int xe_guc_pc_get_cur_freq(struct xe_guc_pc *pc, u32 *freq) { struct xe_gt *gt = pc_to_gt(pc); - int ret; + unsigned int fw_ref; /* * GuC SLPC plays with cur freq request when GuCRC is enabled * Block RC6 for a more reliable read. */ - ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (ret) - return ret; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + xe_force_wake_put(gt_to_fw(gt), fw_ref); + return -ETIMEDOUT; + } *freq = xe_mmio_read32(>->mmio, RPNSWREQ); *freq = REG_FIELD_GET(REQ_RATIO_MASK, *freq); *freq = decode_freq(*freq); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); return 0; } @@ -480,6 +482,7 @@ u32 xe_guc_pc_get_rpn_freq(struct xe_guc_pc *pc) int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq) { struct xe_gt *gt = pc_to_gt(pc); + unsigned int fw_ref; int ret; mutex_lock(&pc->freq_lock); @@ -493,9 +496,11 @@ int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq) * GuC SLPC plays with min freq request when GuCRC is enabled * Block RC6 for a more reliable read. */ - ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (ret) - goto out; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + ret = -ETIMEDOUT; + goto fw; + } ret = pc_action_query_task_state(pc); if (ret) @@ -504,7 +509,7 @@ int xe_guc_pc_get_min_freq(struct xe_guc_pc *pc, u32 *freq) *freq = pc_get_min_freq(pc); fw: - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); out: mutex_unlock(&pc->freq_lock); return ret; @@ -855,6 +860,7 @@ int xe_guc_pc_gucrc_disable(struct xe_guc_pc *pc) { struct xe_device *xe = pc_to_xe(pc); struct xe_gt *gt = pc_to_gt(pc); + unsigned int fw_ref; int ret = 0; if (xe->info.skip_guc_pc) @@ -864,13 +870,15 @@ int xe_guc_pc_gucrc_disable(struct xe_guc_pc *pc) if (ret) return ret; - ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (ret) - return ret; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + xe_force_wake_put(gt_to_fw(gt), fw_ref); + return -ETIMEDOUT; + } xe_gt_idle_disable_c6(gt); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); return 0; } @@ -956,13 +964,16 @@ int xe_guc_pc_start(struct xe_guc_pc *pc) struct xe_device *xe = pc_to_xe(pc); struct xe_gt *gt = pc_to_gt(pc); u32 size = PAGE_ALIGN(sizeof(struct slpc_shared_data)); + unsigned int fw_ref; int ret; xe_gt_assert(gt, xe_device_uc_enabled(xe)); - ret = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (ret) - return ret; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + xe_force_wake_put(gt_to_fw(gt), fw_ref); + return -ETIMEDOUT; + } if (xe->info.skip_guc_pc) { if (xe->info.platform != XE_PVC) @@ -1005,7 +1016,7 @@ int xe_guc_pc_start(struct xe_guc_pc *pc) ret = pc_action_setup_gucrc(pc, GUCRC_FIRMWARE_CONTROL); out: - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); return ret; } @@ -1037,18 +1048,19 @@ static void xe_guc_pc_fini_hw(void *arg) { struct xe_guc_pc *pc = arg; struct xe_device *xe = pc_to_xe(pc); + unsigned int fw_ref; if (xe_device_wedged(xe)) return; - XE_WARN_ON(xe_force_wake_get(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL)); + fw_ref = xe_force_wake_get(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL); xe_guc_pc_gucrc_disable(pc); XE_WARN_ON(xe_guc_pc_stop(pc)); /* Bind requested freq to mert_freq_cap before unload */ pc_set_cur_freq(pc, min(pc_max_freq_cap(pc), pc->rpe_freq)); - xe_force_wake_put(gt_to_fw(pc_to_gt(pc)), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(pc_to_gt(pc)), fw_ref); } /** diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index 0e5649b394b6d8..fc8ababc79fbce 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -1098,6 +1098,7 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job) struct xe_guc *guc = exec_queue_to_guc(q); const char *process_name = "no process"; struct xe_device *xe = guc_to_xe(guc); + unsigned int fw_ref; int err = -ETIME; pid_t pid = -1; int i = 0; @@ -1135,12 +1136,13 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job) if (!exec_queue_killed(q) && !xe->devcoredump.captured && !xe_guc_capture_get_matching_and_lock(job)) { /* take force wake before engine register manual capture */ - if (xe_force_wake_get(gt_to_fw(q->gt), XE_FORCEWAKE_ALL)) + fw_ref = xe_force_wake_get(gt_to_fw(q->gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) xe_gt_info(q->gt, "failed to get forcewake for coredump capture\n"); xe_engine_snapshot_capture_for_job(job); - xe_force_wake_put(gt_to_fw(q->gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(q->gt), fw_ref); } /* From b79ec335e5bf2f9003238c60c615bafae8a27257 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:52 +0530 Subject: [PATCH 18/54] drm/xe/huc: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int from xe_force_wake_get() v7 - Fix commit message Cc: Daniele Ceraolo Spurio Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-18-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_huc.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_huc.c b/drivers/gpu/drm/xe/xe_huc.c index 77c5830309cff8..6a846e4cb2216d 100644 --- a/drivers/gpu/drm/xe/xe_huc.c +++ b/drivers/gpu/drm/xe/xe_huc.c @@ -296,19 +296,19 @@ void xe_huc_sanitize(struct xe_huc *huc) void xe_huc_print_info(struct xe_huc *huc, struct drm_printer *p) { struct xe_gt *gt = huc_to_gt(huc); - int err; + unsigned int fw_ref; xe_uc_fw_print(&huc->fw, p); if (!xe_uc_fw_is_enabled(&huc->fw)) return; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) return; drm_printf(p, "\nHuC status: 0x%08x\n", xe_mmio_read32(>->mmio, HUC_KERNEL_LOAD_INFO)); - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } From 41cd5ce63922180d4206ac097539772125c18d37 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:53 +0530 Subject: [PATCH 19/54] drm/xe/oa: Handle force_wake_get failure in xe_oa_stream_init() With xe_force_wake_get() now returning the refcount-incremented domain mask, a non-zero return value in the case of XE_FORCEWAKE_ALL does not necessarily indicate success. use xe_force_wake_ref_has_domain () to determine the status of the call. Modify the return handling of xe_force_wake_get() accordingly and pass the return value to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v5 - return unsigned int from xe_force_wake_get() v6 - Use helper xe_force_wake_ref_has_domain() Cc: Ashutosh Dixit Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-19-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_oa.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index bbe03db0c4012f..5951ea1755330d 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -837,7 +837,7 @@ static void xe_oa_stream_destroy(struct xe_oa_stream *stream) xe_oa_free_oa_buffer(stream); - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); xe_pm_runtime_put(stream->oa->xe); /* Wa_1509372804:pvc: Unset the override of GUCRC mode to enable rc6 */ @@ -1353,6 +1353,7 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream, { struct xe_oa_unit *u = param->hwe->oa_unit; struct xe_gt *gt = param->hwe->gt; + unsigned int fw_ref; int ret; stream->exec_q = param->exec_q; @@ -1413,7 +1414,11 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream, /* Take runtime pm ref and forcewake to disable RC6 */ xe_pm_runtime_get(stream->oa->xe); - XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + ret = -ETIMEDOUT; + goto err_fw_put; + } ret = xe_oa_alloc_oa_buffer(stream); if (ret) @@ -1455,7 +1460,7 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream, err_free_oa_buf: xe_oa_free_oa_buffer(stream); err_fw_put: - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), fw_ref); xe_pm_runtime_put(stream->oa->xe); if (stream->override_gucrc) xe_gt_WARN_ON(gt, xe_guc_pc_unset_gucrc_mode(>->uc.guc.pc)); From 52f8cd72633ba4588aedd18965527d92294c93a1 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:54 +0530 Subject: [PATCH 20/54] drm/xe/pat: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. - don't use xe_assert() to report HW errors (Michal) v5 - return unsigned int from xe_force_wake_get() - remove redundant warns v7 - Fix commit message - Remove redundant header Cc: Michal Wajdeczko Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-20-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_pat.c | 65 +++++++++++++++++-------------------- 1 file changed, 30 insertions(+), 35 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_pat.c b/drivers/gpu/drm/xe/xe_pat.c index b1647381817396..30fdbdb9341e89 100644 --- a/drivers/gpu/drm/xe/xe_pat.c +++ b/drivers/gpu/drm/xe/xe_pat.c @@ -182,11 +182,12 @@ static void program_pat_mcr(struct xe_gt *gt, const struct xe_pat_table_entry ta static void xelp_dump(struct xe_gt *gt, struct drm_printer *p) { struct xe_device *xe = gt_to_xe(gt); - int i, err; + unsigned int fw_ref; + int i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - goto err_fw; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return; drm_printf(p, "PAT table:\n"); @@ -198,9 +199,7 @@ static void xelp_dump(struct xe_gt *gt, struct drm_printer *p) XELP_MEM_TYPE_STR_MAP[mem_type], pat); } - err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); -err_fw: - xe_assert(xe, !err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static const struct xe_pat_ops xelp_pat_ops = { @@ -211,11 +210,12 @@ static const struct xe_pat_ops xelp_pat_ops = { static void xehp_dump(struct xe_gt *gt, struct drm_printer *p) { struct xe_device *xe = gt_to_xe(gt); - int i, err; + unsigned int fw_ref; + int i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - goto err_fw; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return; drm_printf(p, "PAT table:\n"); @@ -229,9 +229,7 @@ static void xehp_dump(struct xe_gt *gt, struct drm_printer *p) XELP_MEM_TYPE_STR_MAP[mem_type], pat); } - err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); -err_fw: - xe_assert(xe, !err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static const struct xe_pat_ops xehp_pat_ops = { @@ -242,11 +240,12 @@ static const struct xe_pat_ops xehp_pat_ops = { static void xehpc_dump(struct xe_gt *gt, struct drm_printer *p) { struct xe_device *xe = gt_to_xe(gt); - int i, err; + unsigned int fw_ref; + int i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - goto err_fw; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return; drm_printf(p, "PAT table:\n"); @@ -258,9 +257,7 @@ static void xehpc_dump(struct xe_gt *gt, struct drm_printer *p) REG_FIELD_GET(XEHPC_CLOS_LEVEL_MASK, pat), pat); } - err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); -err_fw: - xe_assert(xe, !err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static const struct xe_pat_ops xehpc_pat_ops = { @@ -271,11 +268,12 @@ static const struct xe_pat_ops xehpc_pat_ops = { static void xelpg_dump(struct xe_gt *gt, struct drm_printer *p) { struct xe_device *xe = gt_to_xe(gt); - int i, err; + unsigned int fw_ref; + int i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - goto err_fw; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return; drm_printf(p, "PAT table:\n"); @@ -292,9 +290,7 @@ static void xelpg_dump(struct xe_gt *gt, struct drm_printer *p) REG_FIELD_GET(XELPG_INDEX_COH_MODE_MASK, pat), pat); } - err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); -err_fw: - xe_assert(xe, !err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } /* @@ -330,12 +326,13 @@ static void xe2lpm_program_pat(struct xe_gt *gt, const struct xe_pat_table_entry static void xe2_dump(struct xe_gt *gt, struct drm_printer *p) { struct xe_device *xe = gt_to_xe(gt); - int i, err; + unsigned int fw_ref; u32 pat; + int i; - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - goto err_fw; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return; drm_printf(p, "PAT table:\n"); @@ -374,9 +371,7 @@ static void xe2_dump(struct xe_gt *gt, struct drm_printer *p) REG_FIELD_GET(XE2_COH_MODE, pat), pat); - err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); -err_fw: - xe_assert(xe, !err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } static const struct xe_pat_ops xe2_pat_ops = { From 1d5bf4fd1bff54a773648739a2d72213f0c9facd Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:55 +0530 Subject: [PATCH 21/54] drm/xe/gt_tlb_invalidation_ggtt: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int from xe_force_wake_get() - remove redundant warns v7 - Fix commit message Cc: Matthew Brost Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-21-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c index a530a933eedc07..773de1f08db9bf 100644 --- a/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c +++ b/drivers/gpu/drm/xe/xe_gt_tlb_invalidation.c @@ -268,6 +268,7 @@ static int xe_gt_tlb_invalidation_guc(struct xe_gt *gt, int xe_gt_tlb_invalidation_ggtt(struct xe_gt *gt) { struct xe_device *xe = gt_to_xe(gt); + unsigned int fw_ref; if (xe_guc_ct_enabled(>->uc.guc.ct) && gt->uc.guc.submission_state.enabled) { @@ -286,7 +287,7 @@ int xe_gt_tlb_invalidation_ggtt(struct xe_gt *gt) if (IS_SRIOV_VF(xe)) return 0; - xe_gt_WARN_ON(gt, xe_force_wake_get(gt_to_fw(gt), XE_FW_GT)); + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); if (xe->info.platform == XE_PVC || GRAPHICS_VER(xe) >= 20) { xe_mmio_write32(mmio, PVC_GUC_TLB_INV_DESC1, PVC_GUC_TLB_INV_DESC1_INVALIDATE); @@ -296,7 +297,7 @@ int xe_gt_tlb_invalidation_ggtt(struct xe_gt *gt) xe_mmio_write32(mmio, GUC_TLB_INV_CR, GUC_TLB_INV_CR_INVALIDATE); } - xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); } return 0; From 3bb5d1f05c9c30c8df38c0c3bdecfd193a259751 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:56 +0530 Subject: [PATCH 22/54] drm/xe/xe_reg_sr: Update handling of xe_force_wake_get return With xe_force_wake_get() now returning the refcount-incremented domain mask, a non-zero return value in the case of XE_FORCEWAKE_ALL does not necessarily indicate success. Use xe_force_wake_ref_has_domain() to determine the status of the call. Modify the return handling of xe_force_wake_get() accordingly and pass the return value to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v5 - return unsigned int from xe_force_wake_get() v6 - use helper xe_force_wake_ref_has_domain() Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-22-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_reg_sr.c | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_reg_sr.c b/drivers/gpu/drm/xe/xe_reg_sr.c index 191cb4121acd3b..e1a0e27cda14ca 100644 --- a/drivers/gpu/drm/xe/xe_reg_sr.c +++ b/drivers/gpu/drm/xe/xe_reg_sr.c @@ -188,27 +188,27 @@ void xe_reg_sr_apply_mmio(struct xe_reg_sr *sr, struct xe_gt *gt) { struct xe_reg_sr_entry *entry; unsigned long reg; - int err; + unsigned int fw_ref; if (xa_empty(&sr->xa)) return; xe_gt_dbg(gt, "Applying %s save-restore MMIOs\n", sr->name); - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) goto err_force_wake; xa_for_each(&sr->xa, reg, entry) apply_one_mmio(gt, entry); - err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); - XE_WARN_ON(err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); return; err_force_wake: - xe_gt_err(gt, "Failed to apply, err=%d\n", err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); + xe_gt_err(gt, "Failed to apply, err=-ETIMEDOUT\n"); } void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe) @@ -221,15 +221,15 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe) u32 mmio_base = hwe->mmio_base; unsigned long reg; unsigned int slot = 0; - int err; + unsigned int fw_ref; if (xa_empty(&sr->xa)) return; drm_dbg(&xe->drm, "Whitelisting %s registers\n", sr->name); - err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); - if (err) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) goto err_force_wake; p = drm_dbg_printer(&xe->drm, DRM_UT_DRIVER, NULL); @@ -254,13 +254,13 @@ void xe_reg_sr_apply_whitelist(struct xe_hw_engine *hwe) xe_mmio_write32(>->mmio, RING_FORCE_TO_NONPRIV(mmio_base, slot), addr); } - err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); - XE_WARN_ON(err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); return; err_force_wake: - drm_err(&xe->drm, "Failed to apply, err=%d\n", err); + xe_force_wake_put(gt_to_fw(gt), fw_ref); + drm_err(&xe->drm, "Failed to apply, err=-ETIMEDOUT\n"); } /** From 7b1e9089fe74cc998d6185773df90ed3b3957724 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:57 +0530 Subject: [PATCH 23/54] drm/xe/query: Update handling of xe_force_wake_get return With xe_force_wake_get() now returning the refcount-incremented domain mask, a non-zero return value in the case of XE_FORCEWAKE_ALL does not necessarily indicate success. Use xe_force_wake_ref_has_domain() to determine the status of the call. Modify the return handling of xe_force_wake_get() accordingly and pass the return value to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be checked. It internally WARNS on domain ack failure. v5 - return unsigned int from xe_force_wake_get() v6 - Use helper Use xe_force_wake_ref_has_domain() Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-23-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_query.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c index 5093a243e9fe12..dcd1291da05729 100644 --- a/drivers/gpu/drm/xe/xe_query.c +++ b/drivers/gpu/drm/xe/xe_query.c @@ -117,6 +117,7 @@ query_engine_cycles(struct xe_device *xe, __ktime_func_t cpu_clock; struct xe_hw_engine *hwe; struct xe_gt *gt; + unsigned int fw_ref; if (query->size == 0) { query->size = size; @@ -149,13 +150,16 @@ query_engine_cycles(struct xe_device *xe, if (!hwe) return -EINVAL; - if (xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL)) + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) { + xe_force_wake_put(gt_to_fw(gt), fw_ref); return -EIO; + } hwe_read_timestamp(hwe, &resp.engine_cycles, &resp.cpu_timestamp, &resp.cpu_delta, cpu_clock); - xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); + xe_force_wake_put(gt_to_fw(gt), fw_ref); if (GRAPHICS_VER(xe) >= 20) resp.width = 64; From bd1aad72e05be3f46b3b632199c7ca9f1aa7aa5d Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:58 +0530 Subject: [PATCH 24/54] drm/xe/vram: Update handling of xe_force_wake_get return xe_force_wake_get() now returns the reference count-incremented domain mask. If it fails for individual domains, the return value will always be 0. However, for XE_FORCEWAKE_ALL, it may return a non-zero value even in the event of failure. Update the return handling of xe_force_wake_get() to reflect this behavior, and ensure that the return value is passed as input to xe_force_wake_put(). v3 - return xe_wakeref_t instead of int in xe_force_wake_get() - xe_force_wake_put() error doesn't need to be escalated/considered as probing error. It internally WARNS on domain ack failure. v5 - return unsigned int from xe_force_wake_get() v7 - Fix commit message Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Nirmoy Das Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-24-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_vram.c | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_vram.c b/drivers/gpu/drm/xe/xe_vram.c index 2a623bfcda7eaa..b1f81dca610dc3 100644 --- a/drivers/gpu/drm/xe/xe_vram.c +++ b/drivers/gpu/drm/xe/xe_vram.c @@ -220,8 +220,8 @@ static int tile_vram_size(struct xe_tile *tile, u64 *vram_size, { struct xe_device *xe = tile_to_xe(tile); struct xe_gt *gt = tile->primary_gt; + unsigned int fw_ref; u64 offset; - int err; u32 reg; if (IS_SRIOV_VF(xe)) { @@ -240,9 +240,9 @@ static int tile_vram_size(struct xe_tile *tile, u64 *vram_size, return 0; } - err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); - if (err) - return err; + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); + if (!fw_ref) + return -ETIMEDOUT; /* actual size */ if (unlikely(xe->info.platform == XE_DG1)) { @@ -264,7 +264,9 @@ static int tile_vram_size(struct xe_tile *tile, u64 *vram_size, /* remove the tile offset so we have just the available size */ *vram_size = offset - *tile_offset; - return xe_force_wake_put(gt_to_fw(gt), XE_FW_GT); + xe_force_wake_put(gt_to_fw(gt), fw_ref); + + return 0; } static void vram_fini(void *arg) From 6c0a15e7c734f26facec9a88b798a59282eac6e4 Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:25:59 +0530 Subject: [PATCH 25/54] drm/xe: forcewake debugfs open fails on xe_forcewake_get failure A failure in xe_force_wake_get() no longer increments the domain's refcount. Therefore, if xe_force_wake_get() fails during forcewake debugfs open, return an error. This ensures there are no valid file descriptors to close via forcewake debugfs, preventing refcount mismanagement. v3 - return xe_wakeref_t instead of int in xe_force_wake_get() v5 - return unsigned int from xe_force_wake_get() v6 - Use helper xe_force_wake_ref_has_domain() to determine the status of the call. Cc: Badal Nilawar Cc: Rodrigo Vivi Cc: Lucas De Marchi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-25-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_debugfs.c | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_debugfs.c b/drivers/gpu/drm/xe/xe_debugfs.c index fe4319eb13fdfb..492b4877433f16 100644 --- a/drivers/gpu/drm/xe/xe_debugfs.c +++ b/drivers/gpu/drm/xe/xe_debugfs.c @@ -90,13 +90,32 @@ static int forcewake_open(struct inode *inode, struct file *file) { struct xe_device *xe = inode->i_private; struct xe_gt *gt; - u8 id; + u8 id, last_gt; + unsigned int fw_ref; xe_pm_runtime_get(xe); - for_each_gt(gt, xe, id) - XE_WARN_ON(xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + for_each_gt(gt, xe, id) { + last_gt = id; + + fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL); + if (!xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) + goto err_fw_get; + } return 0; + +err_fw_get: + for_each_gt(gt, xe, id) { + if (id < last_gt) + xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); + else if (id == last_gt) + xe_force_wake_put(gt_to_fw(gt), fw_ref); + else + break; + } + + xe_pm_runtime_put(xe); + return -ETIMEDOUT; } static int forcewake_release(struct inode *inode, struct file *file) @@ -106,7 +125,7 @@ static int forcewake_release(struct inode *inode, struct file *file) u8 id; for_each_gt(gt, xe, id) - XE_WARN_ON(xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL)); + xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL); xe_pm_runtime_put(xe); return 0; From 9ee1780785d1050b59d61cb00fc3354b2f2474ee Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:26:00 +0530 Subject: [PATCH 26/54] drm/xe: Ensure __must_check for xe_force_wake_get() return Add __must_check attribute for xe_force_wake_get(). Cc: Badal Nilawar Cc: Rodrigo Vivi Cc: Lucas De Marchi Cc: Nirmoy Das Reviewed-by: Rodrigo Vivi Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-26-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.c | 4 ++-- drivers/gpu/drm/xe/xe_force_wake.h | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index 7f285dbe6e2c4e..c60db78c44e67a 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -169,8 +169,8 @@ static int domain_sleep_wait(struct xe_gt *gt, * Return: opaque reference to woken domains or zero if none of requested * domains were awake. */ -unsigned int xe_force_wake_get(struct xe_force_wake *fw, - enum xe_force_wake_domains domains) +unsigned int __must_check xe_force_wake_get(struct xe_force_wake *fw, + enum xe_force_wake_domains domains) { struct xe_gt *gt = fw->gt; struct xe_force_wake_domain *domain; diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h index f0b27dbe75811b..70faec9ae2d9e4 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.h +++ b/drivers/gpu/drm/xe/xe_force_wake.h @@ -15,8 +15,8 @@ void xe_force_wake_init_gt(struct xe_gt *gt, struct xe_force_wake *fw); void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw); -unsigned int xe_force_wake_get(struct xe_force_wake *fw, - enum xe_force_wake_domains domains); +unsigned int __must_check xe_force_wake_get(struct xe_force_wake *fw, + enum xe_force_wake_domains domains); int xe_force_wake_put(struct xe_force_wake *fw, unsigned int fw_ref); From 76eb09c8e5e209db63aa02a7754625c31f3a2b0d Mon Sep 17 00:00:00 2001 From: Himal Prasad Ghimiray Date: Mon, 14 Oct 2024 13:26:01 +0530 Subject: [PATCH 27/54] drm/xe: Change return type to void for xe_force_wake_put There is no need to return an error from xe_force_wake_put(), as a failure implicitly indicates that the domain failed to sleep. v3 - Move kernel-doc to this patch (Badal) v5 - change parameter to unsigned int in xe_force_wake_put() v6 - Remove unneccsary wrapping (Michal) - Remove non required header (Michal) - Mention timeout(Michal) v8 - Fix kernel-doc Cc: Michal Wajdeczko Cc: Badal Nilawar Cc: Rodrigo Vivi Cc: Lucas De Marchi Cc: Nirmoy Das Reviewed-by: Badal Nilawar Signed-off-by: Himal Prasad Ghimiray Reviewed-by: Michal Wajdeczko Reviewed-by: Nirmoy Das Link: https://patchwork.freedesktop.org/patch/msgid/20241014075601.2324382-27-himal.prasad.ghimiray@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.c | 16 ++++++++++++---- drivers/gpu/drm/xe/xe_force_wake.h | 3 +-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index c60db78c44e67a..f5067dea59c9f3 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -211,8 +211,17 @@ unsigned int __must_check xe_force_wake_get(struct xe_force_wake *fw, return ref_incr; } -int xe_force_wake_put(struct xe_force_wake *fw, - unsigned int fw_ref) +/** + * xe_force_wake_put - Decrement the refcount and put domain to sleep if refcount becomes 0 + * @fw: Pointer to the force wake structure + * @fw_ref: return of xe_force_wake_get() + * + * This function reduces the reference counts for domains in fw_ref. If + * refcount for any of the specified domain reaches 0, it puts the domain to sleep + * and waits for acknowledgment for domain to sleep within 50 milisec timeout. + * Warns in case of timeout of ack from domain. + */ +void xe_force_wake_put(struct xe_force_wake *fw, unsigned int fw_ref) { struct xe_gt *gt = fw->gt; struct xe_force_wake_domain *domain; @@ -225,7 +234,7 @@ int xe_force_wake_put(struct xe_force_wake *fw, * in error path of individual domains. */ if (!fw_ref) - return 0; + return; if (xe_force_wake_ref_has_domain(fw_ref, XE_FORCEWAKE_ALL)) fw_ref = fw->initialized_domains; @@ -249,5 +258,4 @@ int xe_force_wake_put(struct xe_force_wake *fw, xe_gt_WARN(gt, ack_fail, "Forcewake domain%s %#x failed to acknowledge sleep request\n", str_plural(hweight_long(ack_fail)), ack_fail); - return ack_fail; } diff --git a/drivers/gpu/drm/xe/xe_force_wake.h b/drivers/gpu/drm/xe/xe_force_wake.h index 70faec9ae2d9e4..0e3e84bfa51c38 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.h +++ b/drivers/gpu/drm/xe/xe_force_wake.h @@ -17,8 +17,7 @@ void xe_force_wake_init_engines(struct xe_gt *gt, struct xe_force_wake *fw); unsigned int __must_check xe_force_wake_get(struct xe_force_wake *fw, enum xe_force_wake_domains domains); -int xe_force_wake_put(struct xe_force_wake *fw, - unsigned int fw_ref); +void xe_force_wake_put(struct xe_force_wake *fw, unsigned int fw_ref); static inline int xe_force_wake_ref(struct xe_force_wake *fw, From e5152723380404acb8175e0777b1cea57f319a01 Mon Sep 17 00:00:00 2001 From: Badal Nilawar Date: Thu, 17 Oct 2024 16:44:10 +0530 Subject: [PATCH 28/54] drm/xe/guc/ct: Flush g2h worker in case of g2h response timeout In case if g2h worker doesn't get opportunity to within specified timeout delay then flush the g2h worker explicitly. v2: - Describe change in the comment and add TODO (Matt B/John H) - Add xe_gt_warn on fence done after G2H flush (John H) v3: - Updated the comment with root cause - Clean up xe_gt_warn message (John H) Closes: https://gitlab.freedesktop.org/drm/xe/kernel/issues/1620 Closes: https://gitlab.freedesktop.org/drm/xe/kernel/issues/2902 Signed-off-by: Badal Nilawar Cc: Matthew Brost Cc: Matthew Auld Cc: John Harrison Cc: Himal Prasad Ghimiray Reviewed-by: Himal Prasad Ghimiray Acked-by: Matthew Brost Signed-off-by: Matthew Brost Link: https://patchwork.freedesktop.org/patch/msgid/20241017111410.2553784-2-badal.nilawar@intel.com --- drivers/gpu/drm/xe/xe_guc_ct.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c index c7673f56d41332..c260d884099073 100644 --- a/drivers/gpu/drm/xe/xe_guc_ct.c +++ b/drivers/gpu/drm/xe/xe_guc_ct.c @@ -1018,6 +1018,24 @@ static int guc_ct_send_recv(struct xe_guc_ct *ct, const u32 *action, u32 len, ret = wait_event_timeout(ct->g2h_fence_wq, g2h_fence.done, HZ); + /* + * Occasionally it is seen that the G2H worker starts running after a delay of more than + * a second even after being queued and activated by the Linux workqueue subsystem. This + * leads to G2H timeout error. The root cause of issue lies with scheduling latency of + * Lunarlake Hybrid CPU. Issue dissappears if we disable Lunarlake atom cores from BIOS + * and this is beyond xe kmd. + * + * TODO: Drop this change once workqueue scheduling delay issue is fixed on LNL Hybrid CPU. + */ + if (!ret) { + flush_work(&ct->g2h_worker); + if (g2h_fence.done) { + xe_gt_warn(gt, "G2H fence %u, action %04x, done\n", + g2h_fence.seqno, action[0]); + ret = 1; + } + } + /* * Ensure we serialize with completion side to prevent UAF with fence going out of scope on * the stack, since we have no clue if it will fire after the timeout before we can erase From 61ef737db9f284153546f98d711c4ebf23740d7a Mon Sep 17 00:00:00 2001 From: Vinay Belgaumkar Date: Tue, 15 Oct 2024 16:44:28 -0700 Subject: [PATCH 29/54] drm/xe/ptl: Apply Wa_14022866841 As part of this WA, GuC will hold a forcewake for certain MMIO accesses outside the GT/media domains. Cc: Matt Roper Signed-off-by: Vinay Belgaumkar Reviewed-by: Matt Roper Signed-off-by: John Harrison Link: https://patchwork.freedesktop.org/patch/msgid/20241015234428.2004825-1-vinay.belgaumkar@intel.com --- drivers/gpu/drm/xe/abi/guc_klvs_abi.h | 1 + drivers/gpu/drm/xe/xe_guc_ads.c | 5 +++++ drivers/gpu/drm/xe/xe_wa_oob.rules | 2 ++ 3 files changed, 8 insertions(+) diff --git a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h index 6b30743a2f6cb8..37606cf8cc5e1e 100644 --- a/drivers/gpu/drm/xe/abi/guc_klvs_abi.h +++ b/drivers/gpu/drm/xe/abi/guc_klvs_abi.h @@ -352,6 +352,7 @@ enum xe_guc_klv_ids { GUC_WORKAROUND_KLV_ID_DISABLE_MTP_DURING_ASYNC_COMPUTE = 0x9007, GUC_WA_KLV_NP_RD_WRITE_TO_CLEAR_RCSM_AT_CGP_LATE_RESTORE = 0x9008, GUC_WORKAROUND_KLV_ID_BACK_TO_BACK_RCS_ENGINE_RESET = 0x9009, + GUC_WA_KLV_WAKE_POWER_DOMAINS_FOR_OUTBOUND_MMIO = 0x900a, }; #endif diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index 25292997c7f378..4e746ae98888f5 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -359,6 +359,11 @@ static void guc_waklv_init(struct xe_guc_ads *ads) GUC_WORKAROUND_KLV_ID_DISABLE_MTP_DURING_ASYNC_COMPUTE, &offset, &remain); + if (XE_WA(gt, 14022866841)) + guc_waklv_enable_simple(ads, + GUC_WA_KLV_WAKE_POWER_DOMAINS_FOR_OUTBOUND_MMIO, + &offset, &remain); + /* * On RC6 exit, GuC will write register 0xB04 with the default value provided. As of now, * the default value for this register is determined to be 0xC40. This could change in the diff --git a/drivers/gpu/drm/xe/xe_wa_oob.rules b/drivers/gpu/drm/xe/xe_wa_oob.rules index 264d6e116499ce..bcd04464b85e88 100644 --- a/drivers/gpu/drm/xe/xe_wa_oob.rules +++ b/drivers/gpu/drm/xe/xe_wa_oob.rules @@ -39,3 +39,5 @@ 14019789679 GRAPHICS_VERSION(1255) GRAPHICS_VERSION_RANGE(1270, 2004) no_media_l3 MEDIA_VERSION(3000) +14022866841 GRAPHICS_VERSION(3000), GRAPHICS_STEP(A0, B0) + MEDIA_VERSION(3000), MEDIA_STEP(A0, B0) From a9fbeabe7226a3bf90f82d0e28a02c18e3c67447 Mon Sep 17 00:00:00 2001 From: Shuicheng Lin Date: Thu, 17 Oct 2024 22:15:47 +0000 Subject: [PATCH 30/54] drm/xe: Handle unreliable MMIO reads during forcewake In some cases, when the driver attempts to read an MMIO register, the hardware may return 0xFFFFFFFF. The current force wake path code treats this as a valid response, as it only checks the BIT. However, 0xFFFFFFFF should be considered an invalid value, indicating a potential issue. To address this, we should add a log entry to highlight this condition and return failure. The force wake failure log level is changed from notice to err to match the failure return value. v2 (Matt Brost): - set ret value (-EIO) to kick the error to upper layers v3 (Rodrigo): - add commit message for the log level promotion from notice to err v4: - update reviewed info Suggested-by: Alex Zuo Signed-off-by: Shuicheng Lin Cc: Matthew Brost Cc: Michal Wajdeczko Reviewed-by: Himal Prasad Ghimiray Acked-by: Badal Nilawar Cc: Anshuman Gupta Cc: Matt Roper Cc: Rodrigo Vivi Link: https://patchwork.freedesktop.org/patch/msgid/20241017221547.1564029-1-shuicheng.lin@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_force_wake.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_force_wake.c b/drivers/gpu/drm/xe/xe_force_wake.c index f5067dea59c9f3..4f6784e5abf889 100644 --- a/drivers/gpu/drm/xe/xe_force_wake.c +++ b/drivers/gpu/drm/xe/xe_force_wake.c @@ -119,9 +119,15 @@ static int __domain_wait(struct xe_gt *gt, struct xe_force_wake_domain *domain, XE_FORCE_WAKE_ACK_TIMEOUT_MS * USEC_PER_MSEC, &value, true); if (ret) - xe_gt_notice(gt, "Force wake domain %d failed to ack %s (%pe) reg[%#x] = %#x\n", - domain->id, str_wake_sleep(wake), ERR_PTR(ret), - domain->reg_ack.addr, value); + xe_gt_err(gt, "Force wake domain %d failed to ack %s (%pe) reg[%#x] = %#x\n", + domain->id, str_wake_sleep(wake), ERR_PTR(ret), + domain->reg_ack.addr, value); + if (value == ~0) { + xe_gt_err(gt, + "Force wake domain %d: %s. MMIO unreliable (forcewake register returns 0xFFFFFFFF)!\n", + domain->id, str_wake_sleep(wake)); + ret = -EIO; + } return ret; } From 9408c4508483ffc60811e910a93d6425b8e63928 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Wed, 16 Oct 2024 10:23:03 +0200 Subject: [PATCH 31/54] drm/xe/ufence: Prefetch ufence addr to catch bogus address access_ok() only checks for addr overflow so also try to read the addr to catch invalid addr sent from userspace. Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1630 Cc: Francois Dugast Cc: Maarten Lankhorst Cc: Matthew Auld Cc: Matthew Brost Reviewed-by: Matthew Brost Link: https://patchwork.freedesktop.org/patch/msgid/20241016082304.66009-2-nirmoy.das@intel.com Signed-off-by: Nirmoy Das --- drivers/gpu/drm/xe/xe_sync.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c index c6cf227ead40c0..2e72c06fd40d07 100644 --- a/drivers/gpu/drm/xe/xe_sync.c +++ b/drivers/gpu/drm/xe/xe_sync.c @@ -54,8 +54,9 @@ static struct xe_user_fence *user_fence_create(struct xe_device *xe, u64 addr, { struct xe_user_fence *ufence; u64 __user *ptr = u64_to_user_ptr(addr); + u64 __maybe_unused prefetch_val; - if (!access_ok(ptr, sizeof(*ptr))) + if (get_user(prefetch_val, ptr)) return ERR_PTR(-EFAULT); ufence = kzalloc(sizeof(*ufence), GFP_KERNEL); From 66426bf9e2c930683a883f82d5a471a778282569 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Wed, 16 Oct 2024 10:23:04 +0200 Subject: [PATCH 32/54] drm/xe/ufence: Warn if mmget_not_zero() fails This shouldn't happen but seen this while debugging ufence timeout issue time to time so log it to isolate this particular case. v2: s/XE_WARN_ON/drm_dbg(Maarten) Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1630 Cc: Francois Dugast Cc: Maarten Lankhorst Cc: Matthew Auld Cc: Matthew Brost Reviewed-by: Francois Dugast Link: https://patchwork.freedesktop.org/patch/msgid/20241016082304.66009-3-nirmoy.das@intel.com Signed-off-by: Nirmoy Das --- drivers/gpu/drm/xe/xe_sync.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_sync.c b/drivers/gpu/drm/xe/xe_sync.c index 2e72c06fd40d07..a90480c6aecf56 100644 --- a/drivers/gpu/drm/xe/xe_sync.c +++ b/drivers/gpu/drm/xe/xe_sync.c @@ -83,6 +83,8 @@ static void user_fence_worker(struct work_struct *w) XE_WARN_ON("Copy to user failed"); kthread_unuse_mm(ufence->mm); mmput(ufence->mm); + } else { + drm_dbg(&ufence->xe->drm, "mmget_not_zero() failed, ufence wasn't signaled\n"); } wake_up_all(&ufence->xe->ufence_wq); From 2677520152bc9e732d5e033fe013444db5b4db84 Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Thu, 17 Oct 2024 20:00:39 -0700 Subject: [PATCH 33/54] drm/xe: Use __counted_by for flexible arrays Good practice to use __counted_by in kernel coding for flexible arrays. Signed-off-by: Matthew Brost Reviewed-by: Himal Prasad Ghimiray Link: https://patchwork.freedesktop.org/patch/msgid/20241018030039.1077842-1-matthew.brost@intel.com --- drivers/gpu/drm/xe/xe_exec_queue_types.h | 2 +- drivers/gpu/drm/xe/xe_sched_job_types.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_exec_queue_types.h b/drivers/gpu/drm/xe/xe_exec_queue_types.h index 7deb480e26af0e..1158b6062a6cd1 100644 --- a/drivers/gpu/drm/xe/xe_exec_queue_types.h +++ b/drivers/gpu/drm/xe/xe_exec_queue_types.h @@ -143,7 +143,7 @@ struct xe_exec_queue { /** @hw_engine_group_link: link into exec queues in the same hw engine group */ struct list_head hw_engine_group_link; /** @lrc: logical ring context for this exec queue */ - struct xe_lrc *lrc[]; + struct xe_lrc *lrc[] __counted_by(width); }; /** diff --git a/drivers/gpu/drm/xe/xe_sched_job_types.h b/drivers/gpu/drm/xe/xe_sched_job_types.h index 0d3f76fb05cea2..426d261d7359c6 100644 --- a/drivers/gpu/drm/xe/xe_sched_job_types.h +++ b/drivers/gpu/drm/xe/xe_sched_job_types.h @@ -63,7 +63,7 @@ struct xe_sched_job { struct xe_sched_job_snapshot { u16 batch_addr_len; - u64 batch_addr[]; + u64 batch_addr[] __counted_by(batch_addr_len); }; #endif From 6ef3bb60557d5e7f5af442c8c9ef0a9190bf3d23 Mon Sep 17 00:00:00 2001 From: Fei Yang Date: Thu, 17 Oct 2024 09:27:10 -0700 Subject: [PATCH 34/54] drm/xe: enable lite restore The lite restore is a performance improvement feature which avoids unnecessary context switch (flush, save and restore) if the incoming context has a ContextID matching that of the outgoing context. The scheduling is done by the GuC firmware, so on the driver side it's just a matter of setting corresponding GUC_CTL_FEATURE flag. This is supposed to be enabled by default, thus the flag is set unconditionally. Signed-off-by: Fei Yang Reviewed-by: John Harrison Signed-off-by: John Harrison Link: https://patchwork.freedesktop.org/patch/msgid/20241017162710.942553-2-fei.yang@intel.com --- drivers/gpu/drm/xe/xe_guc.c | 2 +- drivers/gpu/drm/xe/xe_guc_fwif.h | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c index 76437d42b8a1eb..04e50eb6e506e1 100644 --- a/drivers/gpu/drm/xe/xe_guc.c +++ b/drivers/gpu/drm/xe/xe_guc.c @@ -70,7 +70,7 @@ static u32 guc_ctl_debug_flags(struct xe_guc *guc) static u32 guc_ctl_feature_flags(struct xe_guc *guc) { - u32 flags = 0; + u32 flags = GUC_CTL_ENABLE_LITE_RESTORE; if (!guc_to_xe(guc)->info.skip_guc_pc) flags |= GUC_CTL_ENABLE_SLPC; diff --git a/drivers/gpu/drm/xe/xe_guc_fwif.h b/drivers/gpu/drm/xe/xe_guc_fwif.h index 01e3ab590c3aee..08ffe59f22fae7 100644 --- a/drivers/gpu/drm/xe/xe_guc_fwif.h +++ b/drivers/gpu/drm/xe/xe_guc_fwif.h @@ -105,6 +105,7 @@ struct guc_update_exec_queue_policy { #define GUC_CTL_FEATURE 2 #define GUC_CTL_ENABLE_SLPC BIT(2) +#define GUC_CTL_ENABLE_LITE_RESTORE BIT(4) #define GUC_CTL_DISABLE_SCHEDULER BIT(14) #define GUC_CTL_DEBUG 3 From dd1ba621c2951e8ab24711d56dc73ea2828aabd3 Mon Sep 17 00:00:00 2001 From: Zhanjun Dong Date: Mon, 21 Oct 2024 18:01:16 -0700 Subject: [PATCH 35/54] drm/xe/guc: Prevent GuC register capture running on VF GuC based register capture is not supported by VF, thus prevent it running on VF. Signed-off-by: Zhanjun Dong Reviewed-by: Michal Wajdeczko Link: https://patchwork.freedesktop.org/patch/msgid/20241022010116.342240-2-zhanjun.dong@intel.com Signed-off-by: Rodrigo Vivi --- drivers/gpu/drm/xe/xe_guc_capture.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_guc_capture.c b/drivers/gpu/drm/xe/xe_guc_capture.c index 41262bda20ed3d..8b6cb786a2aaf3 100644 --- a/drivers/gpu/drm/xe/xe_guc_capture.c +++ b/drivers/gpu/drm/xe/xe_guc_capture.c @@ -1590,6 +1590,9 @@ xe_engine_manual_capture(struct xe_hw_engine *hwe, struct xe_hw_engine_snapshot u16 guc_id = 0; u32 lrca = 0; + if (IS_SRIOV_VF(xe)) + return; + new = guc_capture_get_prealloc_node(guc); if (!new) return; @@ -1820,7 +1823,7 @@ xe_guc_capture_get_matching_and_lock(struct xe_sched_job *job) return NULL; xe = gt_to_xe(q->gt); - if (xe->wedged.mode >= 2 || !xe_device_uc_enabled(xe)) + if (xe->wedged.mode >= 2 || !xe_device_uc_enabled(xe) || IS_SRIOV_VF(xe)) return NULL; ss = &xe->devcoredump.snapshot; @@ -1876,6 +1879,9 @@ xe_engine_snapshot_capture_for_job(struct xe_sched_job *job) enum xe_hw_engine_id id; u32 adj_logical_mask = q->logical_mask; + if (IS_SRIOV_VF(xe)) + return; + for_each_hw_engine(hwe, q->gt, id) { if (hwe->class != q->hwe->class || !(BIT(hwe->logical_instance) & adj_logical_mask)) { From b982cba5cebd978dc83d3876afa67dbcf3cc2e4c Mon Sep 17 00:00:00 2001 From: Michal Wajdeczko Date: Mon, 21 Oct 2024 22:15:06 +0200 Subject: [PATCH 36/54] drm/xe/pf: Show VFs LMEM provisioning summary over debugfs While we can already view individual VF LMEM provisioning using lmem_quota debugfs attribute, we want to have unified way to show summary across all VFs, like we do for GGTT or GuC doorbells/IDs. Signed-off-by: Michal Wajdeczko Reviewed-by: Marcin Bernatowicz Link: https://patchwork.freedesktop.org/patch/msgid/20241021201506.1771-1-michal.wajdeczko@intel.com --- drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c | 35 +++++++++++++++++++++ drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h | 1 + drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c | 5 +++ 3 files changed, 41 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c index a863e50b756ed3..062a0c2fd2cdf9 100644 --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c @@ -2376,6 +2376,41 @@ int xe_gt_sriov_pf_config_print_dbs(struct xe_gt *gt, struct drm_printer *p) return 0; } +/** + * xe_gt_sriov_pf_config_print_lmem - Print LMEM configurations. + * @gt: the &xe_gt + * @p: the &drm_printer + * + * Print LMEM allocations across all VFs. + * VFs without LMEM allocation are skipped. + * + * This function can only be called on PF. + * Return: 0 on success or a negative error code on failure. + */ +int xe_gt_sriov_pf_config_print_lmem(struct xe_gt *gt, struct drm_printer *p) +{ + unsigned int n, total_vfs = xe_sriov_pf_get_totalvfs(gt_to_xe(gt)); + const struct xe_gt_sriov_config *config; + char buf[10]; + + xe_gt_assert(gt, IS_SRIOV_PF(gt_to_xe(gt))); + mutex_lock(xe_gt_sriov_pf_master_mutex(gt)); + + for (n = 1; n <= total_vfs; n++) { + config = >->sriov.pf.vfs[n].config; + if (!config->lmem_obj) + continue; + + string_get_size(config->lmem_obj->size, 1, STRING_UNITS_2, + buf, sizeof(buf)); + drm_printf(p, "VF%u:\t%zu\t(%s)\n", + n, config->lmem_obj->size, buf); + } + + mutex_unlock(xe_gt_sriov_pf_master_mutex(gt)); + return 0; +} + /** * xe_gt_sriov_pf_config_print_available_ggtt - Print available GGTT ranges. * @gt: the &xe_gt diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h index b74ec38baa182e..0c55aa40a1a7b7 100644 --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.h @@ -65,6 +65,7 @@ void xe_gt_sriov_pf_config_restart(struct xe_gt *gt); int xe_gt_sriov_pf_config_print_ggtt(struct xe_gt *gt, struct drm_printer *p); int xe_gt_sriov_pf_config_print_ctxs(struct xe_gt *gt, struct drm_printer *p); int xe_gt_sriov_pf_config_print_dbs(struct xe_gt *gt, struct drm_printer *p); +int xe_gt_sriov_pf_config_print_lmem(struct xe_gt *gt, struct drm_printer *p); int xe_gt_sriov_pf_config_print_available_ggtt(struct xe_gt *gt, struct drm_printer *p); diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c index 91fc42e386d86a..05df4ab3514bad 100644 --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_debugfs.c @@ -81,6 +81,11 @@ static const struct drm_info_list pf_info[] = { .show = xe_gt_debugfs_simple_show, .data = xe_gt_sriov_pf_config_print_dbs, }, + { + "lmem_provisioned", + .show = xe_gt_debugfs_simple_show, + .data = xe_gt_sriov_pf_config_print_lmem, + }, { "runtime_registers", .show = xe_gt_debugfs_simple_show, From c8b0acd6d8745fd7e6450f5acc38f0227bd253b3 Mon Sep 17 00:00:00 2001 From: Nirmoy Das Date: Tue, 22 Oct 2024 12:35:55 +0200 Subject: [PATCH 37/54] drm/xe: Don't restart parallel queues multiple times on GT reset In case of parallel submissions multiple GuC id will point to the same exec queue and on GT reset such exec queues will get restarted multiple times which is not desirable. v2: don't use exec_queue_enabled() which could race, do the same for xe_guc_submit_stop (Matt B) Link: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2295 Cc: Jonathan Cavitt Cc: Himal Prasad Ghimiray Cc: Matthew Auld Cc: Matthew Brost Cc: Tejas Upadhyay Reviewed-by: Matthew Brost Link: https://patchwork.freedesktop.org/patch/msgid/20241022103555.731557-1-nirmoy.das@intel.com Signed-off-by: Nirmoy Das --- drivers/gpu/drm/xe/xe_guc_submit.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index fc8ababc79fbce..2b6ac1de8ec031 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -1819,8 +1819,13 @@ void xe_guc_submit_stop(struct xe_guc *guc) mutex_lock(&guc->submission_state.lock); - xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) + xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) { + /* Prevent redundant attempts to stop parallel queues */ + if (q->guc->id != index) + continue; + guc_exec_queue_stop(guc, q); + } mutex_unlock(&guc->submission_state.lock); @@ -1858,8 +1863,13 @@ int xe_guc_submit_start(struct xe_guc *guc) mutex_lock(&guc->submission_state.lock); atomic_dec(&guc->submission_state.stopped); - xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) + xa_for_each(&guc->submission_state.exec_queue_lookup, index, q) { + /* Prevent redundant attempts to start parallel queues */ + if (q->guc->id != index) + continue; + guc_exec_queue_start(q); + } mutex_unlock(&guc->submission_state.lock); wake_up_all(&guc->ct.wq); From 059c2a79b0b2bfcc8e65e25ab7444eb8062e1621 Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Mon, 21 Oct 2024 10:35:12 -0700 Subject: [PATCH 38/54] drm/xe: Take ref to job's fence in arm Take ref to job's fence in arm rather than run job. This ref is owned by the drm scheduler so it makes sense to take the ref before handing over the job to the scheduler. Also removes an atomic from the run job path. Suggested-by: Matthew Auld Signed-off-by: Matthew Brost Reviewed-by: Matthew Auld Link: https://patchwork.freedesktop.org/patch/msgid/20241021173512.1584248-1-matthew.brost@intel.com --- drivers/gpu/drm/xe/xe_execlist.c | 2 +- drivers/gpu/drm/xe/xe_guc_submit.c | 9 +++++---- drivers/gpu/drm/xe/xe_sched_job.c | 2 +- drivers/gpu/drm/xe/xe_sched_job_types.h | 1 - 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_execlist.c b/drivers/gpu/drm/xe/xe_execlist.c index f3b71fe7a96dcd..a8c416a4881293 100644 --- a/drivers/gpu/drm/xe/xe_execlist.c +++ b/drivers/gpu/drm/xe/xe_execlist.c @@ -313,7 +313,7 @@ execlist_run_job(struct drm_sched_job *drm_job) q->ring_ops->emit_job(job); xe_execlist_make_active(exl); - return dma_fence_get(job->fence); + return job->fence; } static void execlist_job_free(struct drm_sched_job *drm_job) diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index 2b6ac1de8ec031..d2dcc9afd223d0 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -790,6 +790,7 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_job) struct xe_exec_queue *q = job->q; struct xe_guc *guc = exec_queue_to_guc(q); struct xe_device *xe = guc_to_xe(guc); + struct dma_fence *fence = NULL; bool lr = xe_exec_queue_is_lr(q); xe_assert(xe, !(exec_queue_destroyed(q) || exec_queue_pending_disable(q)) || @@ -807,12 +808,12 @@ guc_exec_queue_run_job(struct drm_sched_job *drm_job) if (lr) { xe_sched_job_set_error(job, -EOPNOTSUPP); - return NULL; - } else if (test_and_set_bit(JOB_FLAG_SUBMIT, &job->fence->flags)) { - return job->fence; + dma_fence_put(job->fence); /* Drop ref from xe_sched_job_arm */ } else { - return dma_fence_get(job->fence); + fence = job->fence; } + + return fence; } static void guc_exec_queue_free_job(struct drm_sched_job *drm_job) diff --git a/drivers/gpu/drm/xe/xe_sched_job.c b/drivers/gpu/drm/xe/xe_sched_job.c index eeccc1c318aef1..1905ca5909658b 100644 --- a/drivers/gpu/drm/xe/xe_sched_job.c +++ b/drivers/gpu/drm/xe/xe_sched_job.c @@ -280,7 +280,7 @@ void xe_sched_job_arm(struct xe_sched_job *job) fence = &chain->base; } - job->fence = fence; + job->fence = dma_fence_get(fence); /* Pairs with put in scheduler */ drm_sched_job_arm(&job->drm); } diff --git a/drivers/gpu/drm/xe/xe_sched_job_types.h b/drivers/gpu/drm/xe/xe_sched_job_types.h index 426d261d7359c6..f13f333f00bebd 100644 --- a/drivers/gpu/drm/xe/xe_sched_job_types.h +++ b/drivers/gpu/drm/xe/xe_sched_job_types.h @@ -40,7 +40,6 @@ struct xe_sched_job { * @fence: dma fence to indicate completion. 1 way relationship - job * can safely reference fence, fence cannot safely reference job. */ -#define JOB_FLAG_SUBMIT DMA_FENCE_FLAG_USER_BITS struct dma_fence *fence; /** @user_fence: write back value when BB is complete */ struct { From 60df57e496e4f92f5efc1610ecf32d30b281b19b Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Mon, 21 Oct 2024 10:57:03 -0700 Subject: [PATCH 39/54] drm/xe: Mark GGTT work queue with WQ_MEM_RECLAIM GGTT work queue is used to free memory thus we should allow this work queue to run during reclaim. Mark with GGTT work queue with WQ_MEM_RECLAIM appropriately. Signed-off-by: Matthew Brost Reviewed-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241021175705.1584521-3-matthew.brost@intel.com --- drivers/gpu/drm/xe/xe_ggtt.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c index 1b31782269871c..0124ad120c0470 100644 --- a/drivers/gpu/drm/xe/xe_ggtt.c +++ b/drivers/gpu/drm/xe/xe_ggtt.c @@ -246,7 +246,7 @@ int xe_ggtt_init_early(struct xe_ggtt *ggtt) else ggtt->pt_ops = &xelp_pt_ops; - ggtt->wq = alloc_workqueue("xe-ggtt-wq", 0, 0); + ggtt->wq = alloc_workqueue("xe-ggtt-wq", 0, WQ_MEM_RECLAIM); drm_mm_init(&ggtt->mm, xe_wopcm_size(xe), ggtt->size - xe_wopcm_size(xe)); From 179e01793ad6f9e4fc69b728bb8073ec566d4583 Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Mon, 21 Oct 2024 10:57:04 -0700 Subject: [PATCH 40/54] drm/xe: Mark G2H work queue with WQ_MEM_RECLAIM G2H work queue can be used to free memory thus we should allow this work queue to run during reclaim. Mark with G2H work queue with WQ_MEM_RECLAIM appropriately. Signed-off-by: Matthew Brost Reviewed-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241021175705.1584521-4-matthew.brost@intel.com --- drivers/gpu/drm/xe/xe_guc_ct.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c index c260d884099073..1b5d8fb1033ad7 100644 --- a/drivers/gpu/drm/xe/xe_guc_ct.c +++ b/drivers/gpu/drm/xe/xe_guc_ct.c @@ -213,7 +213,7 @@ int xe_guc_ct_init(struct xe_guc_ct *ct) xe_gt_assert(gt, !(guc_ct_size() % PAGE_SIZE)); - ct->g2h_wq = alloc_ordered_workqueue("xe-g2h-wq", 0); + ct->g2h_wq = alloc_ordered_workqueue("xe-g2h-wq", WQ_MEM_RECLAIM); if (!ct->g2h_wq) return -ENOMEM; From e2d84e5b22050bb49da19e8ea7943701809bbe88 Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Mon, 21 Oct 2024 10:57:05 -0700 Subject: [PATCH 41/54] drm/xe: Mark GT work queue with WQ_MEM_RECLAIM GT ordered work queue can be used to free memory via resets and fence signaling thus we should allow this work queue to run during reclaim. Mark with GT ordered work queue with WQ_MEM_RECLAIM appropriately. Signed-off-by: Matthew Brost Reviewed-by: Himal Prasad Ghimiray Reviewed-by: Badal Nilawar Link: https://patchwork.freedesktop.org/patch/msgid/20241021175705.1584521-5-matthew.brost@intel.com --- drivers/gpu/drm/xe/xe_gt.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c index 89e9d9d4db060b..d6744be01a687c 100644 --- a/drivers/gpu/drm/xe/xe_gt.c +++ b/drivers/gpu/drm/xe/xe_gt.c @@ -77,7 +77,8 @@ struct xe_gt *xe_gt_alloc(struct xe_tile *tile) return ERR_PTR(-ENOMEM); gt->tile = tile; - gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", 0); + gt->ordered_wq = alloc_ordered_workqueue("gt-ordered-wq", + WQ_MEM_RECLAIM); err = drmm_add_action_or_reset(>_to_xe(gt)->drm, gt_fini, gt); if (err) From dddcb19ad4d4bbe943a72a1fb3266c6e8aa8d541 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:46 -0700 Subject: [PATCH 42/54] drm/xe/oa: Separate batch submission from waiting for completion When we introduce xe_syncs, we don't wait for internal OA programming batches to complete. That is, xe_syncs are signaled asynchronously. In anticipation for this, separate out batch submission from waiting for completion of those batches. v2: Change return type of xe_oa_submit_bb to "struct dma_fence *" (Matt B) v3: Retain init "int err = 0;" in xe_oa_submit_bb (Jose) Reviewed-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-2-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 57 +++++++++++++++++++++++++++++--------- 1 file changed, 44 insertions(+), 13 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 5951ea1755330d..15d952cf0cbd90 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -570,11 +570,10 @@ static __poll_t xe_oa_poll(struct file *file, poll_table *wait) return ret; } -static int xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_bb *bb) +static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_bb *bb) { struct xe_sched_job *job; struct dma_fence *fence; - long timeout; int err = 0; /* Kernel configuration is issued on stream->k_exec_q, not stream->exec_q */ @@ -588,14 +587,9 @@ static int xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_bb *bb) fence = dma_fence_get(&job->drm.s_fence->finished); xe_sched_job_push(job); - timeout = dma_fence_wait_timeout(fence, false, HZ); - dma_fence_put(fence); - if (timeout < 0) - err = timeout; - else if (!timeout) - err = -ETIME; + return fence; exit: - return err; + return ERR_PTR(err); } static void write_cs_mi_lri(struct xe_bb *bb, const struct xe_oa_reg *reg_data, u32 n_regs) @@ -659,6 +653,7 @@ static void xe_oa_store_flex(struct xe_oa_stream *stream, struct xe_lrc *lrc, static int xe_oa_modify_ctx_image(struct xe_oa_stream *stream, struct xe_lrc *lrc, const struct flex *flex, u32 count) { + struct dma_fence *fence; struct xe_bb *bb; int err; @@ -670,7 +665,16 @@ static int xe_oa_modify_ctx_image(struct xe_oa_stream *stream, struct xe_lrc *lr xe_oa_store_flex(stream, lrc, bb, flex, count); - err = xe_oa_submit_bb(stream, bb); + fence = xe_oa_submit_bb(stream, bb); + if (IS_ERR(fence)) { + err = PTR_ERR(fence); + goto free_bb; + } + xe_bb_free(bb, fence); + dma_fence_put(fence); + + return 0; +free_bb: xe_bb_free(bb, NULL); exit: return err; @@ -678,6 +682,7 @@ static int xe_oa_modify_ctx_image(struct xe_oa_stream *stream, struct xe_lrc *lr static int xe_oa_load_with_lri(struct xe_oa_stream *stream, struct xe_oa_reg *reg_lri) { + struct dma_fence *fence; struct xe_bb *bb; int err; @@ -689,7 +694,16 @@ static int xe_oa_load_with_lri(struct xe_oa_stream *stream, struct xe_oa_reg *re write_cs_mi_lri(bb, reg_lri, 1); - err = xe_oa_submit_bb(stream, bb); + fence = xe_oa_submit_bb(stream, bb); + if (IS_ERR(fence)) { + err = PTR_ERR(fence); + goto free_bb; + } + xe_bb_free(bb, fence); + dma_fence_put(fence); + + return 0; +free_bb: xe_bb_free(bb, NULL); exit: return err; @@ -919,15 +933,32 @@ static int xe_oa_emit_oa_config(struct xe_oa_stream *stream, struct xe_oa_config { #define NOA_PROGRAM_ADDITIONAL_DELAY_US 500 struct xe_oa_config_bo *oa_bo; - int err, us = NOA_PROGRAM_ADDITIONAL_DELAY_US; + int err = 0, us = NOA_PROGRAM_ADDITIONAL_DELAY_US; + struct dma_fence *fence; + long timeout; + /* Emit OA configuration batch */ oa_bo = xe_oa_alloc_config_buffer(stream, config); if (IS_ERR(oa_bo)) { err = PTR_ERR(oa_bo); goto exit; } - err = xe_oa_submit_bb(stream, oa_bo->bb); + fence = xe_oa_submit_bb(stream, oa_bo->bb); + if (IS_ERR(fence)) { + err = PTR_ERR(fence); + goto exit; + } + + /* Wait till all previous batches have executed */ + timeout = dma_fence_wait_timeout(fence, false, 5 * HZ); + dma_fence_put(fence); + if (timeout < 0) + err = timeout; + else if (!timeout) + err = -ETIME; + if (err) + drm_dbg(&stream->oa->xe->drm, "dma_fence_wait_timeout err %d\n", err); /* Additional empirical delay needed for NOA programming after registers are written */ usleep_range(us, 2 * us); From c8507a25cebd179db935dd266a33c51bef1b1e80 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:47 -0700 Subject: [PATCH 43/54] drm/xe/oa/uapi: Define and parse OA sync properties MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Now that we have laid the groundwork, introduce OA sync properties in the uapi and parse the input xe_sync array as is done elsewhere in the driver. Also add DRM_XE_OA_CAPS_SYNCS bit in OA capabilities for userspace. v2: Fix and document DRM_XE_SYNC_TYPE_USER_FENCE for OA (Matt B) Add DRM_XE_OA_CAPS_SYNCS bit to OA capabilities (Jose) Acked-by: José Roberto de Souza Reviewed-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-3-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 83 +++++++++++++++++++++++++++++++- drivers/gpu/drm/xe/xe_oa_types.h | 6 +++ drivers/gpu/drm/xe/xe_query.c | 2 +- include/uapi/drm/xe_drm.h | 17 +++++++ 4 files changed, 106 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 15d952cf0cbd90..3f1de88c5f882c 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -36,6 +36,7 @@ #include "xe_pm.h" #include "xe_sched_job.h" #include "xe_sriov.h" +#include "xe_sync.h" #define DEFAULT_POLL_FREQUENCY_HZ 200 #define DEFAULT_POLL_PERIOD_NS (NSEC_PER_SEC / DEFAULT_POLL_FREQUENCY_HZ) @@ -70,6 +71,7 @@ struct flex { }; struct xe_oa_open_param { + struct xe_file *xef; u32 oa_unit_id; bool sample; u32 metric_set; @@ -81,6 +83,9 @@ struct xe_oa_open_param { struct xe_exec_queue *exec_q; struct xe_hw_engine *hwe; bool no_preempt; + struct drm_xe_sync __user *syncs_user; + int num_syncs; + struct xe_sync_entry *syncs; }; struct xe_oa_config_bo { @@ -1398,6 +1403,9 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream, stream->period_exponent = param->period_exponent; stream->no_preempt = param->no_preempt; + stream->num_syncs = param->num_syncs; + stream->syncs = param->syncs; + /* * For Xe2+, when overrun mode is enabled, there are no partial reports at the end * of buffer, making the OA buffer effectively a non-power-of-2 size circular @@ -1752,6 +1760,20 @@ static int xe_oa_set_no_preempt(struct xe_oa *oa, u64 value, return 0; } +static int xe_oa_set_prop_num_syncs(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->num_syncs = value; + return 0; +} + +static int xe_oa_set_prop_syncs_user(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->syncs_user = u64_to_user_ptr(value); + return 0; +} + typedef int (*xe_oa_set_property_fn)(struct xe_oa *oa, u64 value, struct xe_oa_open_param *param); static const xe_oa_set_property_fn xe_oa_set_property_funcs[] = { @@ -1764,6 +1786,8 @@ static const xe_oa_set_property_fn xe_oa_set_property_funcs[] = { [DRM_XE_OA_PROPERTY_EXEC_QUEUE_ID] = xe_oa_set_prop_exec_queue_id, [DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE] = xe_oa_set_prop_engine_instance, [DRM_XE_OA_PROPERTY_NO_PREEMPT] = xe_oa_set_no_preempt, + [DRM_XE_OA_PROPERTY_NUM_SYNCS] = xe_oa_set_prop_num_syncs, + [DRM_XE_OA_PROPERTY_SYNCS] = xe_oa_set_prop_syncs_user, }; static int xe_oa_user_ext_set_property(struct xe_oa *oa, u64 extension, @@ -1823,6 +1847,49 @@ static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number return 0; } +static int xe_oa_parse_syncs(struct xe_oa *oa, struct xe_oa_open_param *param) +{ + int ret, num_syncs, num_ufence = 0; + + if (param->num_syncs && !param->syncs_user) { + drm_dbg(&oa->xe->drm, "num_syncs specified without sync array\n"); + ret = -EINVAL; + goto exit; + } + + if (param->num_syncs) { + param->syncs = kcalloc(param->num_syncs, sizeof(*param->syncs), GFP_KERNEL); + if (!param->syncs) { + ret = -ENOMEM; + goto exit; + } + } + + for (num_syncs = 0; num_syncs < param->num_syncs; num_syncs++) { + ret = xe_sync_entry_parse(oa->xe, param->xef, ¶m->syncs[num_syncs], + ¶m->syncs_user[num_syncs], 0); + if (ret) + goto err_syncs; + + if (xe_sync_is_ufence(¶m->syncs[num_syncs])) + num_ufence++; + } + + if (XE_IOCTL_DBG(oa->xe, num_ufence > 1)) { + ret = -EINVAL; + goto err_syncs; + } + + return 0; + +err_syncs: + while (num_syncs--) + xe_sync_entry_cleanup(¶m->syncs[num_syncs]); + kfree(param->syncs); +exit: + return ret; +} + /** * xe_oa_stream_open_ioctl - Opens an OA stream * @dev: @drm_device @@ -1848,6 +1915,7 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f return -ENODEV; } + param.xef = xef; ret = xe_oa_user_extensions(oa, data, 0, ¶m); if (ret) return ret; @@ -1916,11 +1984,24 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f drm_dbg(&oa->xe->drm, "Using periodic sampling freq %lld Hz\n", oa_freq_hz); } + ret = xe_oa_parse_syncs(oa, ¶m); + if (ret) + goto err_exec_q; + mutex_lock(¶m.hwe->gt->oa.gt_lock); ret = xe_oa_stream_open_ioctl_locked(oa, ¶m); mutex_unlock(¶m.hwe->gt->oa.gt_lock); + if (ret < 0) + goto err_sync_cleanup; + + return ret; + +err_sync_cleanup: + while (param.num_syncs--) + xe_sync_entry_cleanup(¶m.syncs[param.num_syncs]); + kfree(param.syncs); err_exec_q: - if (ret < 0 && param.exec_q) + if (param.exec_q) xe_exec_queue_put(param.exec_q); return ret; } diff --git a/drivers/gpu/drm/xe/xe_oa_types.h b/drivers/gpu/drm/xe/xe_oa_types.h index 8862eca73fbe32..99f4b2d4bdcf6a 100644 --- a/drivers/gpu/drm/xe/xe_oa_types.h +++ b/drivers/gpu/drm/xe/xe_oa_types.h @@ -238,5 +238,11 @@ struct xe_oa_stream { /** @no_preempt: Whether preemption and timeslicing is disabled for stream exec_q */ u32 no_preempt; + + /** @num_syncs: size of @syncs array */ + u32 num_syncs; + + /** @syncs: syncs to wait on and to signal */ + struct xe_sync_entry *syncs; }; #endif diff --git a/drivers/gpu/drm/xe/xe_query.c b/drivers/gpu/drm/xe/xe_query.c index dcd1291da05729..170ae72d1a7bb7 100644 --- a/drivers/gpu/drm/xe/xe_query.c +++ b/drivers/gpu/drm/xe/xe_query.c @@ -670,7 +670,7 @@ static int query_oa_units(struct xe_device *xe, du->oa_unit_id = u->oa_unit_id; du->oa_unit_type = u->type; du->oa_timestamp_freq = xe_oa_timestamp_frequency(gt); - du->capabilities = DRM_XE_OA_CAPS_BASE; + du->capabilities = DRM_XE_OA_CAPS_BASE | DRM_XE_OA_CAPS_SYNCS; j = 0; for_each_hw_engine(hwe, gt, hwe_id) { diff --git a/include/uapi/drm/xe_drm.h b/include/uapi/drm/xe_drm.h index c4182e95a61955..4a8a4a63e99ca8 100644 --- a/include/uapi/drm/xe_drm.h +++ b/include/uapi/drm/xe_drm.h @@ -1485,6 +1485,7 @@ struct drm_xe_oa_unit { /** @capabilities: OA capabilities bit-mask */ __u64 capabilities; #define DRM_XE_OA_CAPS_BASE (1 << 0) +#define DRM_XE_OA_CAPS_SYNCS (1 << 1) /** @oa_timestamp_freq: OA timestamp freq */ __u64 oa_timestamp_freq; @@ -1634,6 +1635,22 @@ enum drm_xe_oa_property_id { * to be disabled for the stream exec queue. */ DRM_XE_OA_PROPERTY_NO_PREEMPT, + + /** + * @DRM_XE_OA_PROPERTY_NUM_SYNCS: Number of syncs in the sync array + * specified in @DRM_XE_OA_PROPERTY_SYNCS + */ + DRM_XE_OA_PROPERTY_NUM_SYNCS, + + /** + * @DRM_XE_OA_PROPERTY_SYNCS: Pointer to struct @drm_xe_sync array + * with array size specified via @DRM_XE_OA_PROPERTY_NUM_SYNCS. OA + * configuration will wait till input fences signal. Output fences + * will signal after the new OA configuration takes effect. For + * @DRM_XE_SYNC_TYPE_USER_FENCE, @addr is a user pointer, similar + * to the VM bind case. + */ + DRM_XE_OA_PROPERTY_SYNCS, }; /** From 2fb4350a283af03a5ee34ba765783a941f942b82 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:48 -0700 Subject: [PATCH 44/54] drm/xe/oa: Add input fence dependencies Add input fence dependencies which will make OA configuration wait till these dependencies are met (till input fences signal). v2: Change add_deps arg to xe_oa_submit_bb from bool to enum (Matt Brost) Reviewed-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-4-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 25 +++++++++++++++++++++---- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 3f1de88c5f882c..380ee1e6325109 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -42,6 +42,11 @@ #define DEFAULT_POLL_PERIOD_NS (NSEC_PER_SEC / DEFAULT_POLL_FREQUENCY_HZ) #define XE_OA_UNIT_INVALID U32_MAX +enum xe_oa_submit_deps { + XE_OA_SUBMIT_NO_DEPS, + XE_OA_SUBMIT_ADD_DEPS, +}; + struct xe_oa_reg { struct xe_reg addr; u32 value; @@ -575,7 +580,8 @@ static __poll_t xe_oa_poll(struct file *file, poll_table *wait) return ret; } -static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_bb *bb) +static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, enum xe_oa_submit_deps deps, + struct xe_bb *bb) { struct xe_sched_job *job; struct dma_fence *fence; @@ -588,11 +594,22 @@ static struct dma_fence *xe_oa_submit_bb(struct xe_oa_stream *stream, struct xe_ goto exit; } + if (deps == XE_OA_SUBMIT_ADD_DEPS) { + for (int i = 0; i < stream->num_syncs && !err; i++) + err = xe_sync_entry_add_deps(&stream->syncs[i], job); + if (err) { + drm_dbg(&stream->oa->xe->drm, "xe_sync_entry_add_deps err %d\n", err); + goto err_put_job; + } + } + xe_sched_job_arm(job); fence = dma_fence_get(&job->drm.s_fence->finished); xe_sched_job_push(job); return fence; +err_put_job: + xe_sched_job_put(job); exit: return ERR_PTR(err); } @@ -670,7 +687,7 @@ static int xe_oa_modify_ctx_image(struct xe_oa_stream *stream, struct xe_lrc *lr xe_oa_store_flex(stream, lrc, bb, flex, count); - fence = xe_oa_submit_bb(stream, bb); + fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_NO_DEPS, bb); if (IS_ERR(fence)) { err = PTR_ERR(fence); goto free_bb; @@ -699,7 +716,7 @@ static int xe_oa_load_with_lri(struct xe_oa_stream *stream, struct xe_oa_reg *re write_cs_mi_lri(bb, reg_lri, 1); - fence = xe_oa_submit_bb(stream, bb); + fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_NO_DEPS, bb); if (IS_ERR(fence)) { err = PTR_ERR(fence); goto free_bb; @@ -949,7 +966,7 @@ static int xe_oa_emit_oa_config(struct xe_oa_stream *stream, struct xe_oa_config goto exit; } - fence = xe_oa_submit_bb(stream, oa_bo->bb); + fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_ADD_DEPS, oa_bo->bb); if (IS_ERR(fence)) { err = PTR_ERR(fence); goto exit; From 343dd246fd9b58e67b395153e8e7298bd250f943 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:49 -0700 Subject: [PATCH 45/54] drm/xe/oa: Signal output fences Introduce 'struct xe_oa_fence' which includes the dma_fence used to signal output fences in the xe_sync array. The fences are signaled asynchronously. When there are no output fences to signal, the OA configuration wait is synchronously re-introduced into the ioctl. v2: Don't wait in the work, use callback + delayed work (Matt B) Use a single, not a per-fence spinlock (Matt Brost) v3: Move ofence alloc before job submission (Matt) Assert, don't fail, from dma_fence_add_callback (Matt) Additional dma_fence_get for dma_fence_wait (Matt) Change dma_fence_wait to non-interruptible (Matt) v4: Introduce last_fence to prevent uaf if stream is closed with pending OA config jobs v5: Remove oa_fence_lock, move spinlock back into xe_oa_fence to prevent uaf Suggested-by: Matthew Brost Reviewed-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Reviewed-by: Matthew Brost Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-5-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 119 ++++++++++++++++++++++++++----- drivers/gpu/drm/xe/xe_oa_types.h | 3 + 2 files changed, 105 insertions(+), 17 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 380ee1e6325109..25558535076a6e 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -100,6 +100,17 @@ struct xe_oa_config_bo { struct xe_bb *bb; }; +struct xe_oa_fence { + /* @base: dma fence base */ + struct dma_fence base; + /* @lock: lock for the fence */ + spinlock_t lock; + /* @work: work to signal @base */ + struct delayed_work work; + /* @cb: callback to schedule @work */ + struct dma_fence_cb cb; +}; + #define DRM_FMT(x) DRM_XE_OA_FMT_TYPE_##x static const struct xe_oa_format oa_formats[] = { @@ -172,10 +183,10 @@ static struct xe_oa_config *xe_oa_get_oa_config(struct xe_oa *oa, int metrics_se return oa_config; } -static void free_oa_config_bo(struct xe_oa_config_bo *oa_bo) +static void free_oa_config_bo(struct xe_oa_config_bo *oa_bo, struct dma_fence *last_fence) { xe_oa_config_put(oa_bo->oa_config); - xe_bb_free(oa_bo->bb, NULL); + xe_bb_free(oa_bo->bb, last_fence); kfree(oa_bo); } @@ -655,7 +666,8 @@ static void xe_oa_free_configs(struct xe_oa_stream *stream) xe_oa_config_put(stream->oa_config); llist_for_each_entry_safe(oa_bo, tmp, stream->oa_config_bos.first, node) - free_oa_config_bo(oa_bo); + free_oa_config_bo(oa_bo, stream->last_fence); + dma_fence_put(stream->last_fence); } static void xe_oa_store_flex(struct xe_oa_stream *stream, struct xe_lrc *lrc, @@ -951,40 +963,113 @@ xe_oa_alloc_config_buffer(struct xe_oa_stream *stream, struct xe_oa_config *oa_c return oa_bo; } +static void xe_oa_update_last_fence(struct xe_oa_stream *stream, struct dma_fence *fence) +{ + dma_fence_put(stream->last_fence); + stream->last_fence = dma_fence_get(fence); +} + +static void xe_oa_fence_work_fn(struct work_struct *w) +{ + struct xe_oa_fence *ofence = container_of(w, typeof(*ofence), work.work); + + /* Signal fence to indicate new OA configuration is active */ + dma_fence_signal(&ofence->base); + dma_fence_put(&ofence->base); +} + +static void xe_oa_config_cb(struct dma_fence *fence, struct dma_fence_cb *cb) +{ + /* Additional empirical delay needed for NOA programming after registers are written */ +#define NOA_PROGRAM_ADDITIONAL_DELAY_US 500 + + struct xe_oa_fence *ofence = container_of(cb, typeof(*ofence), cb); + + INIT_DELAYED_WORK(&ofence->work, xe_oa_fence_work_fn); + queue_delayed_work(system_unbound_wq, &ofence->work, + usecs_to_jiffies(NOA_PROGRAM_ADDITIONAL_DELAY_US)); + dma_fence_put(fence); +} + +static const char *xe_oa_get_driver_name(struct dma_fence *fence) +{ + return "xe_oa"; +} + +static const char *xe_oa_get_timeline_name(struct dma_fence *fence) +{ + return "unbound"; +} + +static const struct dma_fence_ops xe_oa_fence_ops = { + .get_driver_name = xe_oa_get_driver_name, + .get_timeline_name = xe_oa_get_timeline_name, +}; + static int xe_oa_emit_oa_config(struct xe_oa_stream *stream, struct xe_oa_config *config) { #define NOA_PROGRAM_ADDITIONAL_DELAY_US 500 struct xe_oa_config_bo *oa_bo; - int err = 0, us = NOA_PROGRAM_ADDITIONAL_DELAY_US; + struct xe_oa_fence *ofence; + int i, err, num_signal = 0; struct dma_fence *fence; - long timeout; - /* Emit OA configuration batch */ + ofence = kzalloc(sizeof(*ofence), GFP_KERNEL); + if (!ofence) { + err = -ENOMEM; + goto exit; + } + oa_bo = xe_oa_alloc_config_buffer(stream, config); if (IS_ERR(oa_bo)) { err = PTR_ERR(oa_bo); goto exit; } + /* Emit OA configuration batch */ fence = xe_oa_submit_bb(stream, XE_OA_SUBMIT_ADD_DEPS, oa_bo->bb); if (IS_ERR(fence)) { err = PTR_ERR(fence); goto exit; } - /* Wait till all previous batches have executed */ - timeout = dma_fence_wait_timeout(fence, false, 5 * HZ); - dma_fence_put(fence); - if (timeout < 0) - err = timeout; - else if (!timeout) - err = -ETIME; - if (err) - drm_dbg(&stream->oa->xe->drm, "dma_fence_wait_timeout err %d\n", err); + /* Point of no return: initialize and set fence to signal */ + spin_lock_init(&ofence->lock); + dma_fence_init(&ofence->base, &xe_oa_fence_ops, &ofence->lock, 0, 0); - /* Additional empirical delay needed for NOA programming after registers are written */ - usleep_range(us, 2 * us); + for (i = 0; i < stream->num_syncs; i++) { + if (stream->syncs[i].flags & DRM_XE_SYNC_FLAG_SIGNAL) + num_signal++; + xe_sync_entry_signal(&stream->syncs[i], &ofence->base); + } + + /* Additional dma_fence_get in case we dma_fence_wait */ + if (!num_signal) + dma_fence_get(&ofence->base); + + /* Update last fence too before adding callback */ + xe_oa_update_last_fence(stream, fence); + + /* Add job fence callback to schedule work to signal ofence->base */ + err = dma_fence_add_callback(fence, &ofence->cb, xe_oa_config_cb); + xe_gt_assert(stream->gt, !err || err == -ENOENT); + if (err == -ENOENT) + xe_oa_config_cb(fence, &ofence->cb); + + /* If nothing needs to be signaled we wait synchronously */ + if (!num_signal) { + dma_fence_wait(&ofence->base, false); + dma_fence_put(&ofence->base); + } + + /* Done with syncs */ + for (i = 0; i < stream->num_syncs; i++) + xe_sync_entry_cleanup(&stream->syncs[i]); + kfree(stream->syncs); + + return 0; exit: + kfree(ofence); return err; } diff --git a/drivers/gpu/drm/xe/xe_oa_types.h b/drivers/gpu/drm/xe/xe_oa_types.h index 99f4b2d4bdcf6a..c8e0df13faf83a 100644 --- a/drivers/gpu/drm/xe/xe_oa_types.h +++ b/drivers/gpu/drm/xe/xe_oa_types.h @@ -239,6 +239,9 @@ struct xe_oa_stream { /** @no_preempt: Whether preemption and timeslicing is disabled for stream exec_q */ u32 no_preempt; + /** @last_fence: fence to use in stream destroy when needed */ + struct dma_fence *last_fence; + /** @num_syncs: size of @syncs array */ u32 num_syncs; From cc4e6994d5a237ef38363e459ac83cf8ef7626ff Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:50 -0700 Subject: [PATCH 46/54] drm/xe/oa: Move functions up so they can be reused for config ioctl No code changes, only code movement so that functions used during stream open can be reused for the stream reconfiguration ioctl (DRM_XE_OBSERVATION_IOCTL_CONFIG). Reviewed-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-6-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 458 ++++++++++++++++++------------------- 1 file changed, 229 insertions(+), 229 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 25558535076a6e..8684ffca929fd0 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -1141,6 +1141,235 @@ static int xe_oa_enable_metric_set(struct xe_oa_stream *stream) return xe_oa_emit_oa_config(stream, stream->oa_config); } +static int decode_oa_format(struct xe_oa *oa, u64 fmt, enum xe_oa_format_name *name) +{ + u32 counter_size = FIELD_GET(DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE, fmt); + u32 counter_sel = FIELD_GET(DRM_XE_OA_FORMAT_MASK_COUNTER_SEL, fmt); + u32 bc_report = FIELD_GET(DRM_XE_OA_FORMAT_MASK_BC_REPORT, fmt); + u32 type = FIELD_GET(DRM_XE_OA_FORMAT_MASK_FMT_TYPE, fmt); + int idx; + + for_each_set_bit(idx, oa->format_mask, __XE_OA_FORMAT_MAX) { + const struct xe_oa_format *f = &oa->oa_formats[idx]; + + if (counter_size == f->counter_size && bc_report == f->bc_report && + type == f->type && counter_sel == f->counter_select) { + *name = idx; + return 0; + } + } + + return -EINVAL; +} + +static int xe_oa_set_prop_oa_unit_id(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + if (value >= oa->oa_unit_ids) { + drm_dbg(&oa->xe->drm, "OA unit ID out of range %lld\n", value); + return -EINVAL; + } + param->oa_unit_id = value; + return 0; +} + +static int xe_oa_set_prop_sample_oa(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->sample = value; + return 0; +} + +static int xe_oa_set_prop_metric_set(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->metric_set = value; + return 0; +} + +static int xe_oa_set_prop_oa_format(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + int ret = decode_oa_format(oa, value, ¶m->oa_format); + + if (ret) { + drm_dbg(&oa->xe->drm, "Unsupported OA report format %#llx\n", value); + return ret; + } + return 0; +} + +static int xe_oa_set_prop_oa_exponent(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ +#define OA_EXPONENT_MAX 31 + + if (value > OA_EXPONENT_MAX) { + drm_dbg(&oa->xe->drm, "OA timer exponent too high (> %u)\n", OA_EXPONENT_MAX); + return -EINVAL; + } + param->period_exponent = value; + return 0; +} + +static int xe_oa_set_prop_disabled(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->disabled = value; + return 0; +} + +static int xe_oa_set_prop_exec_queue_id(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->exec_queue_id = value; + return 0; +} + +static int xe_oa_set_prop_engine_instance(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->engine_instance = value; + return 0; +} + +static int xe_oa_set_no_preempt(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->no_preempt = value; + return 0; +} + +static int xe_oa_set_prop_num_syncs(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->num_syncs = value; + return 0; +} + +static int xe_oa_set_prop_syncs_user(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + param->syncs_user = u64_to_user_ptr(value); + return 0; +} + +typedef int (*xe_oa_set_property_fn)(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param); +static const xe_oa_set_property_fn xe_oa_set_property_funcs[] = { + [DRM_XE_OA_PROPERTY_OA_UNIT_ID] = xe_oa_set_prop_oa_unit_id, + [DRM_XE_OA_PROPERTY_SAMPLE_OA] = xe_oa_set_prop_sample_oa, + [DRM_XE_OA_PROPERTY_OA_METRIC_SET] = xe_oa_set_prop_metric_set, + [DRM_XE_OA_PROPERTY_OA_FORMAT] = xe_oa_set_prop_oa_format, + [DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT] = xe_oa_set_prop_oa_exponent, + [DRM_XE_OA_PROPERTY_OA_DISABLED] = xe_oa_set_prop_disabled, + [DRM_XE_OA_PROPERTY_EXEC_QUEUE_ID] = xe_oa_set_prop_exec_queue_id, + [DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE] = xe_oa_set_prop_engine_instance, + [DRM_XE_OA_PROPERTY_NO_PREEMPT] = xe_oa_set_no_preempt, + [DRM_XE_OA_PROPERTY_NUM_SYNCS] = xe_oa_set_prop_num_syncs, + [DRM_XE_OA_PROPERTY_SYNCS] = xe_oa_set_prop_syncs_user, +}; + +static int xe_oa_user_ext_set_property(struct xe_oa *oa, u64 extension, + struct xe_oa_open_param *param) +{ + u64 __user *address = u64_to_user_ptr(extension); + struct drm_xe_ext_set_property ext; + int err; + u32 idx; + + err = __copy_from_user(&ext, address, sizeof(ext)); + if (XE_IOCTL_DBG(oa->xe, err)) + return -EFAULT; + + if (XE_IOCTL_DBG(oa->xe, ext.property >= ARRAY_SIZE(xe_oa_set_property_funcs)) || + XE_IOCTL_DBG(oa->xe, ext.pad)) + return -EINVAL; + + idx = array_index_nospec(ext.property, ARRAY_SIZE(xe_oa_set_property_funcs)); + return xe_oa_set_property_funcs[idx](oa, ext.value, param); +} + +typedef int (*xe_oa_user_extension_fn)(struct xe_oa *oa, u64 extension, + struct xe_oa_open_param *param); +static const xe_oa_user_extension_fn xe_oa_user_extension_funcs[] = { + [DRM_XE_OA_EXTENSION_SET_PROPERTY] = xe_oa_user_ext_set_property, +}; + +#define MAX_USER_EXTENSIONS 16 +static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number, + struct xe_oa_open_param *param) +{ + u64 __user *address = u64_to_user_ptr(extension); + struct drm_xe_user_extension ext; + int err; + u32 idx; + + if (XE_IOCTL_DBG(oa->xe, ext_number >= MAX_USER_EXTENSIONS)) + return -E2BIG; + + err = __copy_from_user(&ext, address, sizeof(ext)); + if (XE_IOCTL_DBG(oa->xe, err)) + return -EFAULT; + + if (XE_IOCTL_DBG(oa->xe, ext.pad) || + XE_IOCTL_DBG(oa->xe, ext.name >= ARRAY_SIZE(xe_oa_user_extension_funcs))) + return -EINVAL; + + idx = array_index_nospec(ext.name, ARRAY_SIZE(xe_oa_user_extension_funcs)); + err = xe_oa_user_extension_funcs[idx](oa, extension, param); + if (XE_IOCTL_DBG(oa->xe, err)) + return err; + + if (ext.next_extension) + return xe_oa_user_extensions(oa, ext.next_extension, ++ext_number, param); + + return 0; +} + +static int xe_oa_parse_syncs(struct xe_oa *oa, struct xe_oa_open_param *param) +{ + int ret, num_syncs, num_ufence = 0; + + if (param->num_syncs && !param->syncs_user) { + drm_dbg(&oa->xe->drm, "num_syncs specified without sync array\n"); + ret = -EINVAL; + goto exit; + } + + if (param->num_syncs) { + param->syncs = kcalloc(param->num_syncs, sizeof(*param->syncs), GFP_KERNEL); + if (!param->syncs) { + ret = -ENOMEM; + goto exit; + } + } + + for (num_syncs = 0; num_syncs < param->num_syncs; num_syncs++) { + ret = xe_sync_entry_parse(oa->xe, param->xef, ¶m->syncs[num_syncs], + ¶m->syncs_user[num_syncs], 0); + if (ret) + goto err_syncs; + + if (xe_sync_is_ufence(¶m->syncs[num_syncs])) + num_ufence++; + } + + if (XE_IOCTL_DBG(oa->xe, num_ufence > 1)) { + ret = -EINVAL; + goto err_syncs; + } + + return 0; + +err_syncs: + while (num_syncs--) + xe_sync_entry_cleanup(¶m->syncs[num_syncs]); + kfree(param->syncs); +exit: + return ret; +} + static void xe_oa_stream_enable(struct xe_oa_stream *stream) { stream->pollin = false; @@ -1717,27 +1946,6 @@ static bool engine_supports_oa_format(const struct xe_hw_engine *hwe, int type) } } -static int decode_oa_format(struct xe_oa *oa, u64 fmt, enum xe_oa_format_name *name) -{ - u32 counter_size = FIELD_GET(DRM_XE_OA_FORMAT_MASK_COUNTER_SIZE, fmt); - u32 counter_sel = FIELD_GET(DRM_XE_OA_FORMAT_MASK_COUNTER_SEL, fmt); - u32 bc_report = FIELD_GET(DRM_XE_OA_FORMAT_MASK_BC_REPORT, fmt); - u32 type = FIELD_GET(DRM_XE_OA_FORMAT_MASK_FMT_TYPE, fmt); - int idx; - - for_each_set_bit(idx, oa->format_mask, __XE_OA_FORMAT_MAX) { - const struct xe_oa_format *f = &oa->oa_formats[idx]; - - if (counter_size == f->counter_size && bc_report == f->bc_report && - type == f->type && counter_sel == f->counter_select) { - *name = idx; - return 0; - } - } - - return -EINVAL; -} - /** * xe_oa_unit_id - Return OA unit ID for a hardware engine * @hwe: @xe_hw_engine @@ -1784,214 +1992,6 @@ static int xe_oa_assign_hwe(struct xe_oa *oa, struct xe_oa_open_param *param) return ret; } -static int xe_oa_set_prop_oa_unit_id(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - if (value >= oa->oa_unit_ids) { - drm_dbg(&oa->xe->drm, "OA unit ID out of range %lld\n", value); - return -EINVAL; - } - param->oa_unit_id = value; - return 0; -} - -static int xe_oa_set_prop_sample_oa(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->sample = value; - return 0; -} - -static int xe_oa_set_prop_metric_set(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->metric_set = value; - return 0; -} - -static int xe_oa_set_prop_oa_format(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - int ret = decode_oa_format(oa, value, ¶m->oa_format); - - if (ret) { - drm_dbg(&oa->xe->drm, "Unsupported OA report format %#llx\n", value); - return ret; - } - return 0; -} - -static int xe_oa_set_prop_oa_exponent(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ -#define OA_EXPONENT_MAX 31 - - if (value > OA_EXPONENT_MAX) { - drm_dbg(&oa->xe->drm, "OA timer exponent too high (> %u)\n", OA_EXPONENT_MAX); - return -EINVAL; - } - param->period_exponent = value; - return 0; -} - -static int xe_oa_set_prop_disabled(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->disabled = value; - return 0; -} - -static int xe_oa_set_prop_exec_queue_id(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->exec_queue_id = value; - return 0; -} - -static int xe_oa_set_prop_engine_instance(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->engine_instance = value; - return 0; -} - -static int xe_oa_set_no_preempt(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->no_preempt = value; - return 0; -} - -static int xe_oa_set_prop_num_syncs(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->num_syncs = value; - return 0; -} - -static int xe_oa_set_prop_syncs_user(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param) -{ - param->syncs_user = u64_to_user_ptr(value); - return 0; -} - -typedef int (*xe_oa_set_property_fn)(struct xe_oa *oa, u64 value, - struct xe_oa_open_param *param); -static const xe_oa_set_property_fn xe_oa_set_property_funcs[] = { - [DRM_XE_OA_PROPERTY_OA_UNIT_ID] = xe_oa_set_prop_oa_unit_id, - [DRM_XE_OA_PROPERTY_SAMPLE_OA] = xe_oa_set_prop_sample_oa, - [DRM_XE_OA_PROPERTY_OA_METRIC_SET] = xe_oa_set_prop_metric_set, - [DRM_XE_OA_PROPERTY_OA_FORMAT] = xe_oa_set_prop_oa_format, - [DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT] = xe_oa_set_prop_oa_exponent, - [DRM_XE_OA_PROPERTY_OA_DISABLED] = xe_oa_set_prop_disabled, - [DRM_XE_OA_PROPERTY_EXEC_QUEUE_ID] = xe_oa_set_prop_exec_queue_id, - [DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE] = xe_oa_set_prop_engine_instance, - [DRM_XE_OA_PROPERTY_NO_PREEMPT] = xe_oa_set_no_preempt, - [DRM_XE_OA_PROPERTY_NUM_SYNCS] = xe_oa_set_prop_num_syncs, - [DRM_XE_OA_PROPERTY_SYNCS] = xe_oa_set_prop_syncs_user, -}; - -static int xe_oa_user_ext_set_property(struct xe_oa *oa, u64 extension, - struct xe_oa_open_param *param) -{ - u64 __user *address = u64_to_user_ptr(extension); - struct drm_xe_ext_set_property ext; - int err; - u32 idx; - - err = __copy_from_user(&ext, address, sizeof(ext)); - if (XE_IOCTL_DBG(oa->xe, err)) - return -EFAULT; - - if (XE_IOCTL_DBG(oa->xe, ext.property >= ARRAY_SIZE(xe_oa_set_property_funcs)) || - XE_IOCTL_DBG(oa->xe, ext.pad)) - return -EINVAL; - - idx = array_index_nospec(ext.property, ARRAY_SIZE(xe_oa_set_property_funcs)); - return xe_oa_set_property_funcs[idx](oa, ext.value, param); -} - -typedef int (*xe_oa_user_extension_fn)(struct xe_oa *oa, u64 extension, - struct xe_oa_open_param *param); -static const xe_oa_user_extension_fn xe_oa_user_extension_funcs[] = { - [DRM_XE_OA_EXTENSION_SET_PROPERTY] = xe_oa_user_ext_set_property, -}; - -#define MAX_USER_EXTENSIONS 16 -static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number, - struct xe_oa_open_param *param) -{ - u64 __user *address = u64_to_user_ptr(extension); - struct drm_xe_user_extension ext; - int err; - u32 idx; - - if (XE_IOCTL_DBG(oa->xe, ext_number >= MAX_USER_EXTENSIONS)) - return -E2BIG; - - err = __copy_from_user(&ext, address, sizeof(ext)); - if (XE_IOCTL_DBG(oa->xe, err)) - return -EFAULT; - - if (XE_IOCTL_DBG(oa->xe, ext.pad) || - XE_IOCTL_DBG(oa->xe, ext.name >= ARRAY_SIZE(xe_oa_user_extension_funcs))) - return -EINVAL; - - idx = array_index_nospec(ext.name, ARRAY_SIZE(xe_oa_user_extension_funcs)); - err = xe_oa_user_extension_funcs[idx](oa, extension, param); - if (XE_IOCTL_DBG(oa->xe, err)) - return err; - - if (ext.next_extension) - return xe_oa_user_extensions(oa, ext.next_extension, ++ext_number, param); - - return 0; -} - -static int xe_oa_parse_syncs(struct xe_oa *oa, struct xe_oa_open_param *param) -{ - int ret, num_syncs, num_ufence = 0; - - if (param->num_syncs && !param->syncs_user) { - drm_dbg(&oa->xe->drm, "num_syncs specified without sync array\n"); - ret = -EINVAL; - goto exit; - } - - if (param->num_syncs) { - param->syncs = kcalloc(param->num_syncs, sizeof(*param->syncs), GFP_KERNEL); - if (!param->syncs) { - ret = -ENOMEM; - goto exit; - } - } - - for (num_syncs = 0; num_syncs < param->num_syncs; num_syncs++) { - ret = xe_sync_entry_parse(oa->xe, param->xef, ¶m->syncs[num_syncs], - ¶m->syncs_user[num_syncs], 0); - if (ret) - goto err_syncs; - - if (xe_sync_is_ufence(¶m->syncs[num_syncs])) - num_ufence++; - } - - if (XE_IOCTL_DBG(oa->xe, num_ufence > 1)) { - ret = -EINVAL; - goto err_syncs; - } - - return 0; - -err_syncs: - while (num_syncs--) - xe_sync_entry_cleanup(¶m->syncs[num_syncs]); - kfree(param->syncs); -exit: - return ret; -} - /** * xe_oa_stream_open_ioctl - Opens an OA stream * @dev: @drm_device From 9920c8b88c5cf2e44f4ff508dd3c0c96e4364db0 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:51 -0700 Subject: [PATCH 47/54] drm/xe/oa: Add syncs support to OA config ioctl In addition to stream open, add xe_sync support to the OA config ioctl, where it is even more useful. This allows e.g. Mesa to replay a workload repeatedly on the GPU, each time with a different OA configuration, while precisely controlling (at batch buffer granularity) the workload segment for which a particular OA configuration is active, without introducing stalls in the userspace pipeline. v2: Emit OA config even when config id is same as previous, to ensure consistent sync behavior (Jose) Reviewed-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-7-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 41 ++++++++++++++++++-------------- drivers/gpu/drm/xe/xe_oa_types.h | 3 +++ 2 files changed, 26 insertions(+), 18 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 8684ffca929fd0..9806caf1e530df 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -893,6 +893,7 @@ static void xe_oa_stream_destroy(struct xe_oa_stream *stream) xe_gt_WARN_ON(gt, xe_guc_pc_unset_gucrc_mode(>->uc.guc.pc)); xe_oa_free_configs(stream); + xe_file_put(stream->xef); } static int xe_oa_alloc_oa_buffer(struct xe_oa_stream *stream) @@ -1463,36 +1464,38 @@ static int xe_oa_disable_locked(struct xe_oa_stream *stream) static long xe_oa_config_locked(struct xe_oa_stream *stream, u64 arg) { - struct drm_xe_ext_set_property ext; + struct xe_oa_open_param param = {}; long ret = stream->oa_config->id; struct xe_oa_config *config; int err; - err = __copy_from_user(&ext, u64_to_user_ptr(arg), sizeof(ext)); - if (XE_IOCTL_DBG(stream->oa->xe, err)) - return -EFAULT; - - if (XE_IOCTL_DBG(stream->oa->xe, ext.pad) || - XE_IOCTL_DBG(stream->oa->xe, ext.base.name != DRM_XE_OA_EXTENSION_SET_PROPERTY) || - XE_IOCTL_DBG(stream->oa->xe, ext.base.next_extension) || - XE_IOCTL_DBG(stream->oa->xe, ext.property != DRM_XE_OA_PROPERTY_OA_METRIC_SET)) - return -EINVAL; + err = xe_oa_user_extensions(stream->oa, arg, 0, ¶m); + if (err) + return err; - config = xe_oa_get_oa_config(stream->oa, ext.value); + config = xe_oa_get_oa_config(stream->oa, param.metric_set); if (!config) return -ENODEV; - if (config != stream->oa_config) { - err = xe_oa_emit_oa_config(stream, config); - if (!err) - config = xchg(&stream->oa_config, config); - else - ret = err; + param.xef = stream->xef; + err = xe_oa_parse_syncs(stream->oa, ¶m); + if (err) + goto err_config_put; + + stream->num_syncs = param.num_syncs; + stream->syncs = param.syncs; + + err = xe_oa_emit_oa_config(stream, config); + if (!err) { + config = xchg(&stream->oa_config, config); + drm_dbg(&stream->oa->xe->drm, "changed to oa config uuid=%s\n", + stream->oa_config->uuid); } +err_config_put: xe_oa_config_put(config); - return ret; + return err ?: ret; } static long xe_oa_status_locked(struct xe_oa_stream *stream, unsigned long arg) @@ -1734,6 +1737,7 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream, stream->period_exponent = param->period_exponent; stream->no_preempt = param->no_preempt; + stream->xef = xe_file_get(param->xef); stream->num_syncs = param->num_syncs; stream->syncs = param->syncs; @@ -1837,6 +1841,7 @@ static int xe_oa_stream_init(struct xe_oa_stream *stream, err_free_configs: xe_oa_free_configs(stream); exit: + xe_file_put(stream->xef); return ret; } diff --git a/drivers/gpu/drm/xe/xe_oa_types.h b/drivers/gpu/drm/xe/xe_oa_types.h index c8e0df13faf83a..fea9d981e414fa 100644 --- a/drivers/gpu/drm/xe/xe_oa_types.h +++ b/drivers/gpu/drm/xe/xe_oa_types.h @@ -239,6 +239,9 @@ struct xe_oa_stream { /** @no_preempt: Whether preemption and timeslicing is disabled for stream exec_q */ u32 no_preempt; + /** @xef: xe_file with which the stream was opened */ + struct xe_file *xef; + /** @last_fence: fence to use in stream destroy when needed */ struct dma_fence *last_fence; From 85d3f9e84e0628c412b69aa99b63654dfa08ad68 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 22 Oct 2024 13:03:52 -0700 Subject: [PATCH 48/54] drm/xe/oa: Allow only certain property changes from config Whereas all properties can be specified during OA stream open, when the OA stream is reconfigured only the config_id and syncs can be specified. v2: Use separate function table for reconfig case (Jonathan) Change bool function args to enum (Matt B) v3: s/xe_oa_set_property_funcs/xe_oa_set_property_funcs_open/ (Jonathan) Reviewed-by: Jonathan Cavitt Suggested-by: Jonathan Cavitt Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241022200352.1192560-8-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_oa.c | 60 +++++++++++++++++++++++++++++--------- 1 file changed, 46 insertions(+), 14 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_oa.c b/drivers/gpu/drm/xe/xe_oa.c index 9806caf1e530df..fd2ffe8df15616 100644 --- a/drivers/gpu/drm/xe/xe_oa.c +++ b/drivers/gpu/drm/xe/xe_oa.c @@ -47,6 +47,11 @@ enum xe_oa_submit_deps { XE_OA_SUBMIT_ADD_DEPS, }; +enum xe_oa_user_extn_from { + XE_OA_USER_EXTN_FROM_OPEN, + XE_OA_USER_EXTN_FROM_CONFIG, +}; + struct xe_oa_reg { struct xe_reg addr; u32 value; @@ -1255,9 +1260,15 @@ static int xe_oa_set_prop_syncs_user(struct xe_oa *oa, u64 value, return 0; } +static int xe_oa_set_prop_ret_inval(struct xe_oa *oa, u64 value, + struct xe_oa_open_param *param) +{ + return -EINVAL; +} + typedef int (*xe_oa_set_property_fn)(struct xe_oa *oa, u64 value, struct xe_oa_open_param *param); -static const xe_oa_set_property_fn xe_oa_set_property_funcs[] = { +static const xe_oa_set_property_fn xe_oa_set_property_funcs_open[] = { [DRM_XE_OA_PROPERTY_OA_UNIT_ID] = xe_oa_set_prop_oa_unit_id, [DRM_XE_OA_PROPERTY_SAMPLE_OA] = xe_oa_set_prop_sample_oa, [DRM_XE_OA_PROPERTY_OA_METRIC_SET] = xe_oa_set_prop_metric_set, @@ -1271,8 +1282,22 @@ static const xe_oa_set_property_fn xe_oa_set_property_funcs[] = { [DRM_XE_OA_PROPERTY_SYNCS] = xe_oa_set_prop_syncs_user, }; -static int xe_oa_user_ext_set_property(struct xe_oa *oa, u64 extension, - struct xe_oa_open_param *param) +static const xe_oa_set_property_fn xe_oa_set_property_funcs_config[] = { + [DRM_XE_OA_PROPERTY_OA_UNIT_ID] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_SAMPLE_OA] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_OA_METRIC_SET] = xe_oa_set_prop_metric_set, + [DRM_XE_OA_PROPERTY_OA_FORMAT] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_OA_PERIOD_EXPONENT] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_OA_DISABLED] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_EXEC_QUEUE_ID] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_OA_ENGINE_INSTANCE] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_NO_PREEMPT] = xe_oa_set_prop_ret_inval, + [DRM_XE_OA_PROPERTY_NUM_SYNCS] = xe_oa_set_prop_num_syncs, + [DRM_XE_OA_PROPERTY_SYNCS] = xe_oa_set_prop_syncs_user, +}; + +static int xe_oa_user_ext_set_property(struct xe_oa *oa, enum xe_oa_user_extn_from from, + u64 extension, struct xe_oa_open_param *param) { u64 __user *address = u64_to_user_ptr(extension); struct drm_xe_ext_set_property ext; @@ -1283,23 +1308,30 @@ static int xe_oa_user_ext_set_property(struct xe_oa *oa, u64 extension, if (XE_IOCTL_DBG(oa->xe, err)) return -EFAULT; - if (XE_IOCTL_DBG(oa->xe, ext.property >= ARRAY_SIZE(xe_oa_set_property_funcs)) || + BUILD_BUG_ON(ARRAY_SIZE(xe_oa_set_property_funcs_open) != + ARRAY_SIZE(xe_oa_set_property_funcs_config)); + + if (XE_IOCTL_DBG(oa->xe, ext.property >= ARRAY_SIZE(xe_oa_set_property_funcs_open)) || XE_IOCTL_DBG(oa->xe, ext.pad)) return -EINVAL; - idx = array_index_nospec(ext.property, ARRAY_SIZE(xe_oa_set_property_funcs)); - return xe_oa_set_property_funcs[idx](oa, ext.value, param); + idx = array_index_nospec(ext.property, ARRAY_SIZE(xe_oa_set_property_funcs_open)); + + if (from == XE_OA_USER_EXTN_FROM_CONFIG) + return xe_oa_set_property_funcs_config[idx](oa, ext.value, param); + else + return xe_oa_set_property_funcs_open[idx](oa, ext.value, param); } -typedef int (*xe_oa_user_extension_fn)(struct xe_oa *oa, u64 extension, - struct xe_oa_open_param *param); +typedef int (*xe_oa_user_extension_fn)(struct xe_oa *oa, enum xe_oa_user_extn_from from, + u64 extension, struct xe_oa_open_param *param); static const xe_oa_user_extension_fn xe_oa_user_extension_funcs[] = { [DRM_XE_OA_EXTENSION_SET_PROPERTY] = xe_oa_user_ext_set_property, }; #define MAX_USER_EXTENSIONS 16 -static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number, - struct xe_oa_open_param *param) +static int xe_oa_user_extensions(struct xe_oa *oa, enum xe_oa_user_extn_from from, u64 extension, + int ext_number, struct xe_oa_open_param *param) { u64 __user *address = u64_to_user_ptr(extension); struct drm_xe_user_extension ext; @@ -1318,12 +1350,12 @@ static int xe_oa_user_extensions(struct xe_oa *oa, u64 extension, int ext_number return -EINVAL; idx = array_index_nospec(ext.name, ARRAY_SIZE(xe_oa_user_extension_funcs)); - err = xe_oa_user_extension_funcs[idx](oa, extension, param); + err = xe_oa_user_extension_funcs[idx](oa, from, extension, param); if (XE_IOCTL_DBG(oa->xe, err)) return err; if (ext.next_extension) - return xe_oa_user_extensions(oa, ext.next_extension, ++ext_number, param); + return xe_oa_user_extensions(oa, from, ext.next_extension, ++ext_number, param); return 0; } @@ -1469,7 +1501,7 @@ static long xe_oa_config_locked(struct xe_oa_stream *stream, u64 arg) struct xe_oa_config *config; int err; - err = xe_oa_user_extensions(stream->oa, arg, 0, ¶m); + err = xe_oa_user_extensions(stream->oa, XE_OA_USER_EXTN_FROM_CONFIG, arg, 0, ¶m); if (err) return err; @@ -2023,7 +2055,7 @@ int xe_oa_stream_open_ioctl(struct drm_device *dev, u64 data, struct drm_file *f } param.xef = xef; - ret = xe_oa_user_extensions(oa, data, 0, ¶m); + ret = xe_oa_user_extensions(oa, XE_OA_USER_EXTN_FROM_OPEN, data, 0, ¶m); if (ret) return ret; From 55858fa7eb2f163f7aa34339fd3399ba4ff564c6 Mon Sep 17 00:00:00 2001 From: Jonathan Cavitt Date: Wed, 23 Oct 2024 20:07:15 +0000 Subject: [PATCH 49/54] drm/xe/xe_guc_ads: save/restore OA registers and allowlist regs Several OA registers and allowlist registers were missing from the save/restore list for GuC and could be lost during an engine reset. Add them to the list. v2: - Fix commit message (Umesh) - Add missing closes (Ashutosh) v3: - Add missing fixes (Ashutosh) Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/2249 Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Suggested-by: Umesh Nerlige Ramappa Suggested-by: John Harrison Signed-off-by: Jonathan Cavitt CC: stable@vger.kernel.org # v6.11+ Reviewed-by: Umesh Nerlige Ramappa Reviewed-by: Ashutosh Dixit Signed-off-by: Ashutosh Dixit Link: https://patchwork.freedesktop.org/patch/msgid/20241023200716.82624-1-jonathan.cavitt@intel.com --- drivers/gpu/drm/xe/xe_guc_ads.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index 4e746ae98888f5..a196c4fb90fc9a 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -15,6 +15,7 @@ #include "regs/xe_engine_regs.h" #include "regs/xe_gt_regs.h" #include "regs/xe_guc_regs.h" +#include "regs/xe_oa_regs.h" #include "xe_bo.h" #include "xe_gt.h" #include "xe_gt_ccs_mode.h" @@ -740,6 +741,11 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads, guc_mmio_regset_write_one(ads, regset_map, e->reg, count++); } + for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) + guc_mmio_regset_write_one(ads, regset_map, + RING_FORCE_TO_NONPRIV(hwe->mmio_base, i), + count++); + /* Wa_1607983814 */ if (needs_wa_1607983814(xe) && hwe->class == XE_ENGINE_CLASS_RENDER) { for (i = 0; i < LNCFCMOCS_REG_COUNT; i++) { @@ -748,6 +754,14 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads, } } + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL0, count++); + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL1, count++); + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL2, count++); + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL3, count++); + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL4, count++); + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL5, count++); + guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL6, count++); + return count; } From 833b2ec3bd5d18b85d8a3f416ca590a44bc4f58c Mon Sep 17 00:00:00 2001 From: John Harrison Date: Wed, 23 Oct 2024 17:25:53 -0700 Subject: [PATCH 50/54] drm/xe/guc: Capture all available bits of GuC timestamp The extra bits are not hugely useful because the GuC log only uses 32bit time stamps. But they exist so might as well provide them. Signed-off-by: John Harrison Reviewed-by: Matthew Brost Link: https://patchwork.freedesktop.org/patch/msgid/20241024002554.1983101-2-John.C.Harrison@Intel.com --- drivers/gpu/drm/xe/regs/xe_guc_regs.h | 3 ++- drivers/gpu/drm/xe/xe_guc_log.c | 6 +++--- drivers/gpu/drm/xe/xe_guc_log_types.h | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/drivers/gpu/drm/xe/regs/xe_guc_regs.h b/drivers/gpu/drm/xe/regs/xe_guc_regs.h index b27b73680c124c..2118f7dec287fc 100644 --- a/drivers/gpu/drm/xe/regs/xe_guc_regs.h +++ b/drivers/gpu/drm/xe/regs/xe_guc_regs.h @@ -84,7 +84,8 @@ #define HUC_LOADING_AGENT_GUC REG_BIT(1) #define GUC_WOPCM_OFFSET_VALID REG_BIT(0) #define GUC_MAX_IDLE_COUNT XE_REG(0xc3e4) -#define GUC_PMTIMESTAMP XE_REG(0xc3e8) +#define GUC_PMTIMESTAMP_LO XE_REG(0xc3e8) +#define GUC_PMTIMESTAMP_HI XE_REG(0xc3ec) #define GUC_SEND_INTERRUPT XE_REG(0xc4c8) #define GUC_SEND_TRIGGER REG_BIT(0) diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c index fead96216243df..df4cfb698cdbc7 100644 --- a/drivers/gpu/drm/xe/xe_guc_log.c +++ b/drivers/gpu/drm/xe/xe_guc_log.c @@ -171,9 +171,9 @@ struct xe_guc_log_snapshot *xe_guc_log_snapshot_capture(struct xe_guc_log *log, fw_ref = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT); if (!fw_ref) { - snapshot->stamp = ~0; + snapshot->stamp = ~0ULL; } else { - snapshot->stamp = xe_mmio_read32(>->mmio, GUC_PMTIMESTAMP); + snapshot->stamp = xe_mmio_read64_2x32(>->mmio, GUC_PMTIMESTAMP_LO); xe_force_wake_put(gt_to_fw(gt), fw_ref); } snapshot->ktime = ktime_get_boottime_ns(); @@ -205,7 +205,7 @@ void xe_guc_log_snapshot_print(struct xe_guc_log_snapshot *snapshot, struct drm_ snapshot->ver_found.major, snapshot->ver_found.minor, snapshot->ver_found.patch, snapshot->ver_want.major, snapshot->ver_want.minor, snapshot->ver_want.patch); drm_printf(p, "Kernel timestamp: 0x%08llX [%llu]\n", snapshot->ktime, snapshot->ktime); - drm_printf(p, "GuC timestamp: 0x%08X [%u]\n", snapshot->stamp, snapshot->stamp); + drm_printf(p, "GuC timestamp: 0x%08llX [%llu]\n", snapshot->stamp, snapshot->stamp); drm_printf(p, "Log level: %u\n", snapshot->level); remain = snapshot->size; diff --git a/drivers/gpu/drm/xe/xe_guc_log_types.h b/drivers/gpu/drm/xe/xe_guc_log_types.h index 4d57f8322efc7e..b3d5c72ac75247 100644 --- a/drivers/gpu/drm/xe/xe_guc_log_types.h +++ b/drivers/gpu/drm/xe/xe_guc_log_types.h @@ -27,7 +27,7 @@ struct xe_guc_log_snapshot { /** @ktime: Kernel time the snapshot was taken */ u64 ktime; /** @stamp: GuC timestamp at which the snapshot was taken */ - u32 stamp; + u64 stamp; /** @level: GuC log verbosity level */ u32 level; /** @ver_found: GuC firmware version */ From db38fdb7bf5fe72fbebc3357c8844a5101a16f21 Mon Sep 17 00:00:00 2001 From: John Harrison Date: Wed, 23 Oct 2024 17:25:54 -0700 Subject: [PATCH 51/54] drm/xe/guc: Separate full CTB content from guc_info debugfs The guc_info debugfs file is meant to be a quick view of the current software state of the GuC interface. Including the full CTB contents makes the file as a whole much less human readable and is not partiular useful in the general case. So don't pollute the info dump with the full buffers. Instead, move those into a separate debugfs entry that can be read when that information is actually required. Also, improve the human readability by adding a few extra blank lines to delimt the sections. v2: Hide the internal capture/print params from external callers that don't need to know (review feedback from Matthew Brost). Signed-off-by: John Harrison Reviewed-by: Matthew Brost Link: https://patchwork.freedesktop.org/patch/msgid/20241024002554.1983101-3-John.C.Harrison@Intel.com --- drivers/gpu/drm/xe/xe_devcoredump.c | 2 +- drivers/gpu/drm/xe/xe_guc.c | 5 ++- drivers/gpu/drm/xe/xe_guc_ct.c | 54 +++++++++++++++-------------- drivers/gpu/drm/xe/xe_guc_ct.h | 5 ++- drivers/gpu/drm/xe/xe_guc_debugfs.c | 14 ++++++++ 5 files changed, 49 insertions(+), 31 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_devcoredump.c b/drivers/gpu/drm/xe/xe_devcoredump.c index 8b0ea77661b2b0..d2679c5d976b06 100644 --- a/drivers/gpu/drm/xe/xe_devcoredump.c +++ b/drivers/gpu/drm/xe/xe_devcoredump.c @@ -267,7 +267,7 @@ static void devcoredump_snapshot(struct xe_devcoredump *coredump, fw_ref = xe_force_wake_get(gt_to_fw(q->gt), XE_FORCEWAKE_ALL); ss->guc.log = xe_guc_log_snapshot_capture(&guc->log, true); - ss->guc.ct = xe_guc_ct_snapshot_capture(&guc->ct, true); + ss->guc.ct = xe_guc_ct_snapshot_capture(&guc->ct); ss->ge = xe_guc_exec_queue_snapshot_capture(q); ss->job = xe_sched_job_snapshot_capture(job); ss->vm = xe_vm_snapshot_capture(q->vm); diff --git a/drivers/gpu/drm/xe/xe_guc.c b/drivers/gpu/drm/xe/xe_guc.c index 04e50eb6e506e1..7f704346a8f4d2 100644 --- a/drivers/gpu/drm/xe/xe_guc.c +++ b/drivers/gpu/drm/xe/xe_guc.c @@ -1186,7 +1186,10 @@ void xe_guc_print_info(struct xe_guc *guc, struct drm_printer *p) xe_force_wake_put(gt_to_fw(gt), fw_ref); - xe_guc_ct_print(&guc->ct, p); + drm_puts(p, "\n"); + xe_guc_ct_print(&guc->ct, p, false); + + drm_puts(p, "\n"); xe_guc_submit_print(guc, p); } diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c index 1b5d8fb1033ad7..4870af1c5a9005 100644 --- a/drivers/gpu/drm/xe/xe_guc_ct.c +++ b/drivers/gpu/drm/xe/xe_guc_ct.c @@ -1607,7 +1607,8 @@ static void g2h_worker_func(struct work_struct *w) receive_g2h(ct); } -struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_alloc(struct xe_guc_ct *ct, bool atomic) +static struct xe_guc_ct_snapshot *guc_ct_snapshot_alloc(struct xe_guc_ct *ct, bool atomic, + bool want_ctb) { struct xe_guc_ct_snapshot *snapshot; @@ -1615,7 +1616,7 @@ struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_alloc(struct xe_guc_ct *ct, bool a if (!snapshot) return NULL; - if (ct->bo) { + if (ct->bo && want_ctb) { snapshot->ctb_size = ct->bo->size; snapshot->ctb = kmalloc(snapshot->ctb_size, atomic ? GFP_ATOMIC : GFP_KERNEL); } @@ -1645,25 +1646,13 @@ static void guc_ctb_snapshot_print(struct guc_ctb_snapshot *snapshot, drm_printf(p, "\tstatus (memory): 0x%x\n", snapshot->desc.status); } -/** - * xe_guc_ct_snapshot_capture - Take a quick snapshot of the CT state. - * @ct: GuC CT object. - * @atomic: Boolean to indicate if this is called from atomic context like - * reset or CTB handler or from some regular path like debugfs. - * - * This can be printed out in a later stage like during dev_coredump - * analysis. - * - * Returns: a GuC CT snapshot object that must be freed by the caller - * by using `xe_guc_ct_snapshot_free`. - */ -struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_capture(struct xe_guc_ct *ct, - bool atomic) +static struct xe_guc_ct_snapshot *guc_ct_snapshot_capture(struct xe_guc_ct *ct, bool atomic, + bool want_ctb) { struct xe_device *xe = ct_to_xe(ct); struct xe_guc_ct_snapshot *snapshot; - snapshot = xe_guc_ct_snapshot_alloc(ct, atomic); + snapshot = guc_ct_snapshot_alloc(ct, atomic, want_ctb); if (!snapshot) { xe_gt_err(ct_to_gt(ct), "Skipping CTB snapshot entirely.\n"); return NULL; @@ -1682,6 +1671,21 @@ struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_capture(struct xe_guc_ct *ct, return snapshot; } +/** + * xe_guc_ct_snapshot_capture - Take a quick snapshot of the CT state. + * @ct: GuC CT object. + * + * This can be printed out in a later stage like during dev_coredump + * analysis. This is safe to be called during atomic context. + * + * Returns: a GuC CT snapshot object that must be freed by the caller + * by using `xe_guc_ct_snapshot_free`. + */ +struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_capture(struct xe_guc_ct *ct) +{ + return guc_ct_snapshot_capture(ct, true, true); +} + /** * xe_guc_ct_snapshot_print - Print out a given GuC CT snapshot. * @snapshot: GuC CT snapshot object. @@ -1704,12 +1708,8 @@ void xe_guc_ct_snapshot_print(struct xe_guc_ct_snapshot *snapshot, drm_printf(p, "\tg2h outstanding: %d\n", snapshot->g2h_outstanding); - if (snapshot->ctb) { + if (snapshot->ctb) xe_print_blob_ascii85(p, "CTB data", snapshot->ctb, 0, snapshot->ctb_size); - } else { - drm_printf(p, "CTB snapshot missing!\n"); - return; - } } else { drm_puts(p, "CT disabled\n"); } @@ -1735,14 +1735,16 @@ void xe_guc_ct_snapshot_free(struct xe_guc_ct_snapshot *snapshot) * xe_guc_ct_print - GuC CT Print. * @ct: GuC CT. * @p: drm_printer where it will be printed out. + * @want_ctb: Should the full CTB content be dumped (vs just the headers) * - * This function quickly capture a snapshot and immediately print it out. + * This function will quickly capture a snapshot of the CT state + * and immediately print it out. */ -void xe_guc_ct_print(struct xe_guc_ct *ct, struct drm_printer *p) +void xe_guc_ct_print(struct xe_guc_ct *ct, struct drm_printer *p, bool want_ctb) { struct xe_guc_ct_snapshot *snapshot; - snapshot = xe_guc_ct_snapshot_capture(ct, false); + snapshot = guc_ct_snapshot_capture(ct, false, want_ctb); xe_guc_ct_snapshot_print(snapshot, p); xe_guc_ct_snapshot_free(snapshot); } @@ -1776,7 +1778,7 @@ static void ct_dead_capture(struct xe_guc_ct *ct, struct guc_ctb *ctb, u32 reaso return; snapshot_log = xe_guc_log_snapshot_capture(&guc->log, true); - snapshot_ct = xe_guc_ct_snapshot_capture((ct), true); + snapshot_ct = xe_guc_ct_snapshot_capture((ct)); spin_lock_irqsave(&ct->dead.lock, flags); diff --git a/drivers/gpu/drm/xe/xe_guc_ct.h b/drivers/gpu/drm/xe/xe_guc_ct.h index 338f0b75d29f9c..82c4ae458dda39 100644 --- a/drivers/gpu/drm/xe/xe_guc_ct.h +++ b/drivers/gpu/drm/xe/xe_guc_ct.h @@ -17,11 +17,10 @@ void xe_guc_ct_disable(struct xe_guc_ct *ct); void xe_guc_ct_stop(struct xe_guc_ct *ct); void xe_guc_ct_fast_path(struct xe_guc_ct *ct); -struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_alloc(struct xe_guc_ct *ct, bool atomic); -struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_capture(struct xe_guc_ct *ct, bool atomic); +struct xe_guc_ct_snapshot *xe_guc_ct_snapshot_capture(struct xe_guc_ct *ct); void xe_guc_ct_snapshot_print(struct xe_guc_ct_snapshot *snapshot, struct drm_printer *p); void xe_guc_ct_snapshot_free(struct xe_guc_ct_snapshot *snapshot); -void xe_guc_ct_print(struct xe_guc_ct *ct, struct drm_printer *p); +void xe_guc_ct_print(struct xe_guc_ct *ct, struct drm_printer *p, bool want_ctb); static inline bool xe_guc_ct_enabled(struct xe_guc_ct *ct) { diff --git a/drivers/gpu/drm/xe/xe_guc_debugfs.c b/drivers/gpu/drm/xe/xe_guc_debugfs.c index d3822cbea273a4..995b306aced771 100644 --- a/drivers/gpu/drm/xe/xe_guc_debugfs.c +++ b/drivers/gpu/drm/xe/xe_guc_debugfs.c @@ -47,9 +47,23 @@ static int guc_log(struct seq_file *m, void *data) return 0; } +static int guc_ctb(struct seq_file *m, void *data) +{ + struct xe_guc *guc = node_to_guc(m->private); + struct xe_device *xe = guc_to_xe(guc); + struct drm_printer p = drm_seq_file_printer(m); + + xe_pm_runtime_get(xe); + xe_guc_ct_print(&guc->ct, &p, true); + xe_pm_runtime_put(xe); + + return 0; +} + static const struct drm_info_list debugfs_list[] = { {"guc_info", guc_info, 0}, {"guc_log", guc_log, 0}, + {"guc_ctb", guc_ctb, 0}, }; void xe_guc_debugfs_register(struct xe_guc *guc, struct dentry *parent) From 0191fddf53748cf2b473d78faeabe6dcb47689d2 Mon Sep 17 00:00:00 2001 From: Ashutosh Dixit Date: Tue, 29 Oct 2024 13:01:47 -0700 Subject: [PATCH 52/54] Revert "drm/xe/xe_guc_ads: save/restore OA registers and allowlist regs" This reverts commit 55858fa7eb2f163f7aa34339fd3399ba4ff564c6. '55858fa7eb2f ("drm/xe/xe_guc_ads: save/restore OA registers and allowlist regs")' was not properly reviewed and also causes dmesg asserts in CI. Revert it. Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3295 Fixes: 55858fa7eb2f ("drm/xe/xe_guc_ads: save/restore OA registers and allowlist regs") Signed-off-by: Ashutosh Dixit Reviewed-by: Jonathan Cavitt Signed-off-by: Matt Roper Link: https://patchwork.freedesktop.org/patch/msgid/20241029200147.1476513-1-ashutosh.dixit@intel.com --- drivers/gpu/drm/xe/xe_guc_ads.c | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c index a196c4fb90fc9a..4e746ae98888f5 100644 --- a/drivers/gpu/drm/xe/xe_guc_ads.c +++ b/drivers/gpu/drm/xe/xe_guc_ads.c @@ -15,7 +15,6 @@ #include "regs/xe_engine_regs.h" #include "regs/xe_gt_regs.h" #include "regs/xe_guc_regs.h" -#include "regs/xe_oa_regs.h" #include "xe_bo.h" #include "xe_gt.h" #include "xe_gt_ccs_mode.h" @@ -741,11 +740,6 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads, guc_mmio_regset_write_one(ads, regset_map, e->reg, count++); } - for (i = 0; i < RING_MAX_NONPRIV_SLOTS; i++) - guc_mmio_regset_write_one(ads, regset_map, - RING_FORCE_TO_NONPRIV(hwe->mmio_base, i), - count++); - /* Wa_1607983814 */ if (needs_wa_1607983814(xe) && hwe->class == XE_ENGINE_CLASS_RENDER) { for (i = 0; i < LNCFCMOCS_REG_COUNT; i++) { @@ -754,14 +748,6 @@ static unsigned int guc_mmio_regset_write(struct xe_guc_ads *ads, } } - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL0, count++); - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL1, count++); - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL2, count++); - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL3, count++); - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL4, count++); - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL5, count++); - guc_mmio_regset_write_one(ads, regset_map, EU_PERF_CNTL6, count++); - return count; } From 5a710196883e0ac019ac6df2a6d79c16ad3c32fa Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Wed, 23 Oct 2024 15:12:00 -0700 Subject: [PATCH 53/54] drm/xe: Add mmio read before GGTT invalidate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On LNL without a mmio read before a GGTT invalidate the GuC can incorrectly read the GGTT scratch page upon next access leading to jobs not getting scheduled. A mmio read before a GGTT invalidate seems to fix this. Since a GGTT invalidate is not a hot code path, blindly do a mmio read before each GGTT invalidate. Cc: John Harrison Cc: Daniele Ceraolo Spurio Cc: Thomas Hellström Cc: Lucas De Marchi Cc: stable@vger.kernel.org Fixes: dd08ebf6c352 ("drm/xe: Introduce a new DRM driver for Intel GPUs") Reported-by: Paulo Zanoni Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/3164 Signed-off-by: Matthew Brost Reviewed-by: Lucas De Marchi Link: https://patchwork.freedesktop.org/patch/msgid/20241023221200.1797832-1-matthew.brost@intel.com Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/xe/xe_ggtt.c | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c index 0124ad120c0470..558fac8bb6fb56 100644 --- a/drivers/gpu/drm/xe/xe_ggtt.c +++ b/drivers/gpu/drm/xe/xe_ggtt.c @@ -401,6 +401,16 @@ static void ggtt_invalidate_gt_tlb(struct xe_gt *gt) static void xe_ggtt_invalidate(struct xe_ggtt *ggtt) { + struct xe_device *xe = tile_to_xe(ggtt->tile); + + /* + * XXX: Barrier for GGTT pages. Unsure exactly why this required but + * without this LNL is having issues with the GuC reading scratch page + * vs. correct GGTT page. Not particularly a hot code path so blindly + * do a mmio read here which results in GuC reading correct GGTT page. + */ + xe_mmio_read32(xe_root_tile_mmio(xe), VF_CAP_REG); + /* Each GT in a tile has its own TLB to cache GGTT lookups */ ggtt_invalidate_gt_tlb(ggtt->tile->primary_gt); ggtt_invalidate_gt_tlb(ggtt->tile->media_gt); From 35d25a4a0012e690ef0cc4c5440231176db595cc Mon Sep 17 00:00:00 2001 From: Matthew Brost Date: Fri, 25 Oct 2024 14:43:29 -0700 Subject: [PATCH 54/54] drm/xe: Don't short circuit TDR on jobs not started Short circuiting TDR on jobs not started is an optimization which is not required. On LNL we are facing an issue where jobs do not get scheduled by the GuC if it misses a GGTT page update. When this occurs let the TDR fire, toggle the scheduling which may get the job unstuck, and print a warning message. If the TDR fires twice on job that hasn't started, timeout the job. v2: - Add warning message (Paulo) - Add fixes tag (Paulo) - Timeout job which hasn't started after TDR firing twice v3: - Include local change v4: - Short circuit check_timeout on job not started - use warn level rather than notice (Paulo) Fixes: 7ddb9403dd74 ("drm/xe: Sample ctx timestamp to determine if jobs have timed out") Cc: stable@vger.kernel.org Cc: Paulo Zanoni Signed-off-by: Matthew Brost Reviewed-by: Lucas De Marchi Link: https://patchwork.freedesktop.org/patch/msgid/20241025214330.2010521-2-matthew.brost@intel.com Signed-off-by: Lucas De Marchi --- drivers/gpu/drm/xe/xe_guc_submit.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/drivers/gpu/drm/xe/xe_guc_submit.c b/drivers/gpu/drm/xe/xe_guc_submit.c index d2dcc9afd223d0..ad194fd297dcf5 100644 --- a/drivers/gpu/drm/xe/xe_guc_submit.c +++ b/drivers/gpu/drm/xe/xe_guc_submit.c @@ -991,12 +991,22 @@ static void xe_guc_exec_queue_lr_cleanup(struct work_struct *w) static bool check_timeout(struct xe_exec_queue *q, struct xe_sched_job *job) { struct xe_gt *gt = guc_to_gt(exec_queue_to_guc(q)); - u32 ctx_timestamp = xe_lrc_ctx_timestamp(q->lrc[0]); - u32 ctx_job_timestamp = xe_lrc_ctx_job_timestamp(q->lrc[0]); + u32 ctx_timestamp, ctx_job_timestamp; u32 timeout_ms = q->sched_props.job_timeout_ms; u32 diff; u64 running_time_ms; + if (!xe_sched_job_started(job)) { + xe_gt_warn(gt, "Check job timeout: seqno=%u, lrc_seqno=%u, guc_id=%d, not started", + xe_sched_job_seqno(job), xe_sched_job_lrc_seqno(job), + q->guc->id); + + return xe_sched_invalidate_job(job, 2); + } + + ctx_timestamp = xe_lrc_ctx_timestamp(q->lrc[0]); + ctx_job_timestamp = xe_lrc_ctx_job_timestamp(q->lrc[0]); + /* * Counter wraps at ~223s at the usual 19.2MHz, be paranoid catch * possible overflows with a high timeout. @@ -1126,10 +1136,6 @@ guc_exec_queue_timedout_job(struct drm_sched_job *drm_job) exec_queue_killed_or_banned_or_wedged(q) || exec_queue_destroyed(q); - /* Job hasn't started, can't be timed out */ - if (!skip_timeout_check && !xe_sched_job_started(job)) - goto rearm; - /* * If devcoredump not captured and GuC capture for the job is not ready * do manual capture first and decide later if we need to use it