Re: [PATCH 1/3] drm/i915/guc: fix GuC loading/submission check

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

 



On 09/06/16 12:04, Tvrtko Ursulin wrote:

On 07/06/16 09:41, Tvrtko Ursulin wrote:

On 07/06/16 09:14, Dave Gordon wrote:
The last stage of the GuC loader also sanitises the GuC submission
settings, so should be called unconditionally (even on platforms
without a GuC) to ensure consistent settings; in particular, this
prevents any attempt to use GuC submission on GuCless platforms!

Also fix error path handling and clarify DRM_INFO fallback message.

Signed-off-by: Dave Gordon <david.s.gordon@xxxxxxxxx>
---
  drivers/gpu/drm/i915/i915_gem.c         |  8 +++-----
  drivers/gpu/drm/i915/intel_guc_loader.c | 12 ++++++++----
  2 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/i915/i915_gem.c
b/drivers/gpu/drm/i915/i915_gem.c
index 1bfc260..eae8d7a 100644
--- a/drivers/gpu/drm/i915/i915_gem.c
+++ b/drivers/gpu/drm/i915/i915_gem.c
@@ -4930,11 +4930,9 @@ int i915_gem_init_engines(struct drm_device *dev)
      intel_mocs_init_l3cc_table(dev);

      /* We can't enable contexts until all firmware is loaded */
-    if (HAS_GUC(dev)) {
-        ret = intel_guc_setup(dev);
-        if (ret)
-            goto out;
-    }
+    ret = intel_guc_setup(dev);
+    if (ret)
+        goto out;

      /*
       * Increment the next seqno by 0x100 so we have a visible break
diff --git a/drivers/gpu/drm/i915/intel_guc_loader.c
b/drivers/gpu/drm/i915/intel_guc_loader.c
index f2b88c7..4e34c2e 100644
--- a/drivers/gpu/drm/i915/intel_guc_loader.c
+++ b/drivers/gpu/drm/i915/intel_guc_loader.c
@@ -425,9 +425,13 @@ int intel_guc_setup(struct drm_device *dev)
      if (!i915.enable_guc_loading) {
          err = 0;
          goto fail;
-    } else if (fw_path == NULL || *fw_path == '\0') {
-        if (*fw_path == '\0')

Ops. I can only assume I meant !fw_path.

-            DRM_INFO("No GuC firmware known for this platform\n");
+    } else if (fw_path == NULL) {
+        /* Device is known to have no uCode (e.g. no GuC) */
+        err = -ENXIO;
+        goto fail;
+    } else if (*fw_path == '\0') {
+        /* Device has a GuC but we don't know what f/w to load? */
+        DRM_INFO("No GuC firmware known for this platform\n");
          err = -ENODEV;
          goto fail;
      }
@@ -535,7 +539,7 @@ int intel_guc_setup(struct drm_device *dev)
          if (fw_path == NULL)
              DRM_INFO("GuC submission without firmware not
supported\n");
          if (ret == 0)
-            DRM_INFO("Falling back to execlist mode\n");
+            DRM_INFO("Falling back from GuC submission to execlist
mode\n");
          else
              DRM_ERROR("GuC init failed: %d\n", ret);
      }


Reviewed-by: Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxx>

Bah now on BDW we get on boot:

[drm:gen8_init_common_ring] Execlists enabled for render ring
[drm:gen8_init_common_ring] Execlists enabled for blitter ring
[drm:gen8_init_common_ring] Execlists enabled for bsd ring
[drm:gen8_init_common_ring] Execlists enabled for video enhancement ring
[drm:intel_guc_setup] GuC fw status: path (null), fetch NONE, load NONE
[drm] GuC firmware load skipped
[drm] GuC submission without firmware not supported
[drm] Falling back from GuC submission to execlist mode

Which is a bit untidy. :(

Regards,
Tvrtko

You shouldn't get the second or third [drm] message, if you haven't overridden the default values. The default for enable_guc_submission is (-1) which gets mapped to HAS_GUC_SCHED() which is 0 on BDW; you only get the extra messages if you've set it to a non-default value.

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




[Index of Archives]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]
  Powered by Linux