On 4/11/19 1:44 AM, Michal Wajdeczko wrote:
Gen11 defines new register for checking HuC authentication status.
Look into the right register and bit.

BSpec: 19686

Signed-off-by: Michal Wajdeczko <michal.wajdec...@intel.com>
Cc: Joonas Lahtinen <joonas.lahti...@linux.intel.com>
Cc: Rodrigo Vivi <rodrigo.v...@intel.com>
Cc: Tony Ye <tony...@intel.com>
Cc: Vinay Belgaumkar <vinay.belgaum...@intel.com>
Cc: John Spotswood <john.a.spotsw...@intel.com>
Cc: Anusha Srivatsa <anusha.sriva...@intel.com>
---
  drivers/gpu/drm/i915/intel_guc_reg.h |  3 ++
  drivers/gpu/drm/i915/intel_huc.c     | 56 ++++++++++++++++++++++++----
  2 files changed, 51 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_guc_reg.h 
b/drivers/gpu/drm/i915/intel_guc_reg.h
index d26de5193568..7eba65795b58 100644
--- a/drivers/gpu/drm/i915/intel_guc_reg.h
+++ b/drivers/gpu/drm/i915/intel_guc_reg.h
@@ -79,6 +79,9 @@
  #define HUC_STATUS2             _MMIO(0xD3B0)
  #define   HUC_FW_VERIFIED       (1<<7)
+#define GEN11_HUC_KERNEL_LOAD_INFO _MMIO(0xC1DC)
+#define   HUC_LOAD_SUCCESSFUL            (1 << 0)
+
  #define GUC_WOPCM_SIZE                        _MMIO(0xc050)
  #define   GUC_WOPCM_SIZE_LOCKED                 (1<<0)
  #define   GUC_WOPCM_SIZE_SHIFT                12
diff --git a/drivers/gpu/drm/i915/intel_huc.c b/drivers/gpu/drm/i915/intel_huc.c
index 94c04f16a2ad..708a4b387259 100644
--- a/drivers/gpu/drm/i915/intel_huc.c
+++ b/drivers/gpu/drm/i915/intel_huc.c
@@ -40,6 +40,47 @@ int intel_huc_init_misc(struct intel_huc *huc)
        return 0;
  }
+static int gen8_huc_wait_verified(struct intel_huc *huc)

why gen8?

+{
+       struct drm_i915_private *i915 = huc_to_i915(huc);
+       u32 status;
+       int ret;
+
+       ret = __intel_wait_for_register(&i915->uncore,
+                                       HUC_STATUS2,
+                                       HUC_FW_VERIFIED,
+                                       HUC_FW_VERIFIED,
+                                       2, 50, &status);
+       if (ret)
+               DRM_ERROR("HuC: status %#x\n", status);
+       return ret;
+}
+
+static int gen11_huc_wait_verified(struct intel_huc *huc)
+{
+       struct drm_i915_private *i915 = huc_to_i915(huc);
+       int ret;
+
+       ret = __intel_wait_for_register(&i915->uncore,
+                                       GEN11_HUC_KERNEL_LOAD_INFO,
+                                       HUC_LOAD_SUCCESSFUL,
+                                       HUC_LOAD_SUCCESSFUL,
+                                       2, 50, NULL);
+       return ret;
+}
+
+static int huc_wait_verified(struct intel_huc *huc)

We do call this only once, so maybe we can just avoid having a separate function and just have it directly in intel_huc_auth? the code is simple enough. Otherwise, to avoid 2 identical functions which diff only in the register details, we could save the register and the expected value in the huc struct during init_early and just wait on (huc->auth.reg & huc->auth.mask), which we could also use in intel_huc_check_status().

Apart from this, register values do match the FW and the specs.

Daniele

+{
+       struct drm_i915_private *i915 = huc_to_i915(huc);
+       int ret;
+
+       if (INTEL_GEN(i915) >= 11)
+               ret = gen11_huc_wait_verified(huc);
+       else
+               ret = gen8_huc_wait_verified(huc);
+       return ret;
+}
+
  /**
   * intel_huc_auth() - Authenticate HuC uCode
   * @huc: intel_huc structure
@@ -56,7 +97,6 @@ int intel_huc_auth(struct intel_huc *huc)
        struct drm_i915_private *i915 = huc_to_i915(huc);
        struct intel_guc *guc = &i915->guc;
        struct i915_vma *vma;
-       u32 status;
        int ret;
if (huc->fw.load_status != INTEL_UC_FIRMWARE_SUCCESS)
@@ -79,13 +119,9 @@ int intel_huc_auth(struct intel_huc *huc)
        }
/* Check authentication status, it should be done by now */
-       ret = __intel_wait_for_register(&i915->uncore,
-                                       HUC_STATUS2,
-                                       HUC_FW_VERIFIED,
-                                       HUC_FW_VERIFIED,
-                                       2, 50, &status);
+       ret = huc_wait_verified(huc);
        if (ret) {
-               DRM_ERROR("HuC: Firmware not verified %#x\n", status);
+               DRM_ERROR("HuC: Firmware not verified %d\n", ret);
                goto fail_unpin;
        }
@@ -122,7 +158,11 @@ int intel_huc_check_status(struct intel_huc *huc)
                return -ENODEV;
with_intel_runtime_pm(dev_priv, wakeref)
-               status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
+               if (INTEL_GEN(dev_priv) >= 11)
+                       status = I915_READ(GEN11_HUC_KERNEL_LOAD_INFO) &
+                               HUC_LOAD_SUCCESSFUL;
+               else
+                       status = I915_READ(HUC_STATUS2) & HUC_FW_VERIFIED;
return status;
  }

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

Reply via email to