Re: [PATCH] drm/i915/guc: Fix detection of GuC submission in use

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, 05 Sep 2019 13:16:31 +0200, Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx> wrote:

The driver always assumes active GuC submission mode if it is
supported.  That's not true if GuC initialization fails for some
reason.  That may lead to kernel panics, caused e.g. by execlists
fallback submission mode incorrectly detecting GuC submission in use.

Fix it by also checking for GuC enabled status.

Fixes: 356c484822e6 ("drm/i915/uc: Add explicit DISABLED state for firmware")
Signed-off-by: Janusz Krzysztofik <janusz.krzysztofik@xxxxxxxxxxxxxxx>
---
 drivers/gpu/drm/i915/gt/uc/intel_uc.h | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 527995c21196..b28bab64a280 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -51,7 +51,8 @@ static inline bool intel_uc_supports_guc_submission(struct intel_uc *uc)
static inline bool intel_uc_uses_guc_submission(struct intel_uc *uc)
 {
-	return intel_guc_is_submission_supported(&uc->guc);
+	return intel_guc_is_enabled(&uc->guc) &&
+	       intel_guc_is_submission_supported(&uc->guc);

This wont fix your original problem (that btw is not possible to
repro on drm-tip) as after any GuC initialization failure we still
treat GuC as "enabled":

intel_guc_is_supported => H/W support (static)
intel_guc_is_enabled => aka not disabled by the user (config)
intel_guc_is_running => no major fw failure (runtime)

Note that we even s/intel_guc_is_enabled/intel_guc_is_running
won't help as GuC may be running but we may fail to correctly
initialize GuC submission.

Correct fix to original problem must be aligned with new GuC
submission model (coming soon) and it may look as this:

+static inline bool intel_guc_is_submission_active(struct intel_guc *guc)
+{
+	GEM_BUG_ON(guc->submission_active && !intel_guc_is_running(guc));
+	return guc->submission_active;
+}

and then

 static inline bool intel_uc_uses_guc_submission(struct intel_uc *uc)
 {
-	return intel_guc_is_submission_supported(&uc->guc);
+	return intel_guc_is_submission_active(&uc->guc);
 }

We may need to revisit all uses/supports/ macros to better
reflect configuration vs runtime differences.

Thanks,
Michal
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx




[Index of Archives]     [AMD Graphics]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux