On 03/01/2018 05:14 AM, Michal Wajdeczko wrote:
On Thu, 01 Mar 2018 02:01:38 +0100, Jackie Li <[email protected]> wrote:

On CNL A0 and Gen9, there's a hardware restriction that requires the
available GuC WOPCM size to be larger than or equal to HuC firmware size.

This patch adds new verification code to ensure the available GuC WOPCM
size to be larger than or equal to HuC firmware size on both Gen9 and CNL
A0.

v6:
 - Extended HuC FW size check against GuC WOPCM size to all
   Gen9 and CNL A0 platforms

v7:
 - Fixed patch format issues

v8:
 - Renamed variables and functions to avoid ambiguity (Joonas)
 - Updated commit message and comments to be more comprehensive (Sagar)

v9:
 - Moved code that is not related to restriction check into a separate
   patch and updated the commit message accordingly (Sagar/Michal)
 - Avoided to call uc_get_fw_size for better layer isolation (Michal)

v10:
 - Shorten function names and reorganized size_check code to have clear
   isolation (Joonas)
 - Removed unnecessary comments (Joonas)

v11:
 - Fixed logic error in size check (Michal)

BSpec: 10875

Cc: Michal Wajdeczko <[email protected]>
Cc: Sagar Arun Kamble <[email protected]>
Cc: John Spotswood <[email protected]>
Cc: Jeff McGee <[email protected]>
Cc: Chris Wilson <[email protected]>
Cc: Joonas Lahtinen <[email protected]>
Reviewed-by: Sagar Arun Kamble <[email protected]> (v8)
Signed-off-by: Jackie Li <[email protected]>
---
 drivers/gpu/drm/i915/intel_wopcm.c | 28 +++++++++++++++++++++++++---
 1 file changed, 25 insertions(+), 3 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
index bb78043..b30d7ff 100644
--- a/drivers/gpu/drm/i915/intel_wopcm.c
+++ b/drivers/gpu/drm/i915/intel_wopcm.c
@@ -107,8 +107,26 @@ static inline int gen9_check_dword_gap(u32 guc_wopcm_base, u32 guc_wopcm_size)
     return 0;
 }
+static inline int gen9_check_huc_fw_fits(u32 guc_wopcm_size, u32 huc_fw_size)
+{
+    /*
+     * On Gen9 & CNL A0, hardware requires the total available GuC WOPCM
+     * size to be larger than or equal to HuC firmware size. Otherwise,
+     * firmware uploading would fail.
+     */
+    if (huc_fw_size > guc_wopcm_size - GUC_WOPCM_RESERVED) {
+        DRM_ERROR("HuC fw(%uKiB) won't fit in GuC WOPCM(%uKiB).\n",
+              huc_fw_size / 1024,
+              (guc_wopcm_size - GUC_WOPCM_RESERVED) / 1024);

bikeshed: in earlier patches in similar error messages, you used
"HuC FW (%KiB)" and didn't provide available space. Maybe simplest
way to unify and minimize the code is to add one "failed" tag in
wopcm_init function where you can print all values used for partitioning:

failed:
    DRM_ERROR("Failed to partition %uKiB WOPCM (%d)\n", wopcm->size/1024, err);
    DRM_ERROR("HuC FW size=%uKiB\n", ...);
    DRM_ERROR("GuC FW size=%uKiB\n", ...);
    return err;

I will keep it as it is now to save some time since I was told to keep these message
at the point where the error happened.:-)
+        return -E2BIG;
+    }
+
+    return 0;
+}
+
 static inline int check_hw_restriction(struct drm_i915_private *i915,
-                       u32 guc_wopcm_base, u32 guc_wopcm_size)
+                       u32 guc_wopcm_base, u32 guc_wopcm_size,
+                       u32 huc_fw_size)
 {
     int err = 0;
@@ -117,7 +135,10 @@ static inline int check_hw_restriction(struct drm_i915_private *i915,
     if (err)
         return err;
-    return 0;
+    if (IS_GEN9(i915) || IS_CNL_REVID(i915, CNL_REVID_A0, CNL_REVID_A0))
+        err = gen9_check_huc_fw_fits(guc_wopcm_size, huc_fw_size);
+
+    return err;
 }
/**
@@ -186,7 +207,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
         return -E2BIG;
     }
-    err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size);
+    err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size,
+                   huc_fw_size);
     if (err) {
         DRM_ERROR("Failed to meet HW restriction.\n");
         return err;

but bikeshed is not critical, so

Reviewed-by: Michal Wajdeczko <[email protected]>
Thanks,
-Jackie

_______________________________________________
Intel-gfx mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/intel-gfx

Reply via email to