Re: [PATCH v2] drm/i915/guc: Update to GuC FW v40

John Harrison <John.C.Harrison@xxxxxxxxx> · Tue, 21 Jan 2020 10:40:01 -0800

On 1/21/2020 10:29, Daniele Ceraolo Spurio wrote:
On 1/15/20 12:37 PM, John.C.Harrison@xxxxxxxxx wrote:
From: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>

v1 was from Matt, now it is from Michal?

Grrr. That would be git being annoying. I squashed Michal's update on 
top and it must have somehow decided to use his patch as the base!? I 
can change it back. Or set it to myself? Not sure what is the official 
way to handle patches with multiple authors?



The GuC major version has jumped from 35 to 40. This is because this
FW includes a significant re-write of the API that completely breaks
backwards compatibility for command submission. This patch is
sufficient to enable loading of the GuC and hence authentication of
the HuC. Support of command submission will follow in a much larger
patch series.

The changes required to load v40 FW are:
* An additional data structure and associated 'private_data' pointer
are now required to be setup by the driver. This is basically a
scratch area of memory that the GuC owns. The size is read from the
CSS header.

* A physical to logical engine mapping table is required to be
provided in the GuC additional data structure. This is initialized
with a 1 to 1 mapping.

* GUC_CTL_CTXINFO has been removed from the initialization params.

* The reg_state_buffer has been removed from the GuC ADS.

v2: Folded in removal of reg_state_buffer from Michal.


There are some changes in the suspend/resume protocol, but the impact 
should only be around GuC submission so it should be ok to do those 
later. With the patch ownership sorted out:

Reviewed-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>

Can you resend with enable_guc=2 to trigger a guc-enabled shards run 
on all sharded platforms?

Will do. Just to be pedantic for anyone that does know... Setting 
enable_guc=2 is really 'huc enabled' not 'guc enabled'. Full GuC 
submission is still not supported in upstream.

John.

Daniele

Signed-off-by: Matthew Brost <matthew.brost@xxxxxxxxx>
Signed-off-by: John Harrison <John.C.Harrison@xxxxxxxxx>
Signed-off-by: Michal Wajdeczko <michal.wajdeczko@xxxxxxxxx>
CC: Daniele Ceraolo Spurio <daniele.ceraolospurio@xxxxxxxxx>
---
  drivers/gpu/drm/i915/gt/uc/intel_guc.c       | 18 ------------
  drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c   | 31 ++++++++++++++++++--
  drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h  | 24 +++++++--------
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c     | 21 +++++++------
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h     |  2 ++
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h |  6 +++-
  6 files changed, 58 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index 5d00a3b2d914..e3ef742aac4a 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -213,23 +213,6 @@ static u32 guc_ctl_feature_flags(struct 
intel_guc *guc)
      return flags;
  }
  -static u32 guc_ctl_ctxinfo_flags(struct intel_guc *guc)
-{
-    u32 flags = 0;
-
-    if (intel_guc_is_submission_supported(guc)) {
-        u32 ctxnum, base;
-
-        base = intel_guc_ggtt_offset(guc, guc->stage_desc_pool);
-        ctxnum = GUC_MAX_STAGE_DESCRIPTORS / 16;
-
-        base >>= PAGE_SHIFT;
-        flags |= (base << GUC_CTL_BASE_ADDR_SHIFT) |
-            (ctxnum << GUC_CTL_CTXNUM_IN16_SHIFT);
-    }
-    return flags;
-}
-
  static u32 guc_ctl_log_params_flags(struct intel_guc *guc)
  {
      u32 offset = intel_guc_ggtt_offset(guc, guc->log.vma) >> 
PAGE_SHIFT;
@@ -291,7 +274,6 @@ static void guc_init_params(struct intel_guc *guc)
        BUILD_BUG_ON(sizeof(guc->params) != GUC_CTL_MAX_DWORDS * 
sizeof(u32));
  -    params[GUC_CTL_CTXINFO] = guc_ctl_ctxinfo_flags(guc);
      params[GUC_CTL_LOG_PARAMS] = guc_ctl_log_params_flags(guc);
      params[GUC_CTL_FEATURE] = guc_ctl_feature_flags(guc);
      params[GUC_CTL_DEBUG] = guc_ctl_debug_flags(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
index 101728006ae9..ba4cf89e6338 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_ads.c
@@ -48,6 +48,20 @@ static void guc_ct_pool_entries_init(struct 
guc_ct_pool_entry *pool, u32 num)
      memset(pool, 0, num * sizeof(*pool));
  }
  +static void guc_mapping_table_init(struct guc_gt_system_info 
*system_info)
+{
+    unsigned int i, j;
+
+    /*
+     * Initializing the physical to logical engine mapping table with a
+     * 1 to 1 mapping. This allows the firmware to load on all 
platforms as
+     * all engines are logical exposed to the user.
+     */
+    for (i = 0; i < GUC_MAX_ENGINE_CLASSES; ++i)
+        for (j = 0; j < GUC_MAX_INSTANCES_PER_CLASS; ++j)
+            system_info->mapping_table[i][j] = j;
+}
+
  /*
   * The first 80 dwords of the register state context, containing the
   * execlists and ppgtt registers.
@@ -62,7 +76,6 @@ struct __guc_ads_blob {
      struct guc_gt_system_info system_info;
      struct guc_clients_info clients_info;
      struct guc_ct_pool_entry ct_pool[GUC_CT_POOL_SIZE];
-    u8 reg_state_buffer[GUC_S3_SAVE_SPACE_PAGES * PAGE_SIZE];
  } __packed;
    static void __guc_ads_init(struct intel_guc *guc)
@@ -107,6 +120,8 @@ static void __guc_ads_init(struct intel_guc *guc)
      blob->system_info.vebox_enable_mask = VEBOX_MASK(dev_priv);
      blob->system_info.vdbox_sfc_support_mask = 
RUNTIME_INFO(dev_priv)->vdbox_sfc_access;
  +    guc_mapping_table_init(&blob->system_info);
+
      base = intel_guc_ggtt_offset(guc, guc->ads_vma);
        /* Clients info  */
@@ -118,11 +133,14 @@ static void __guc_ads_init(struct intel_guc *guc)
        /* ADS */
      blob->ads.scheduler_policies = base + ptr_offset(blob, policies);
-    blob->ads.reg_state_buffer = base + ptr_offset(blob, 
reg_state_buffer);
      blob->ads.reg_state_addr = base + ptr_offset(blob, reg_state);
      blob->ads.gt_system_info = base + ptr_offset(blob, system_info);
      blob->ads.clients_info = base + ptr_offset(blob, clients_info);
  +    /* Private Data */
+    blob->ads.private_data = base +
+        PAGE_ALIGN(sizeof(struct __guc_ads_blob));
+
      i915_gem_object_flush_map(guc->ads_vma->obj);
  }
  @@ -135,11 +153,13 @@ static void __guc_ads_init(struct intel_guc 
*guc)
   */
  int intel_guc_ads_create(struct intel_guc *guc)
  {
-    const u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
+    u32 size = PAGE_ALIGN(sizeof(struct __guc_ads_blob));
      int ret;
        GEM_BUG_ON(guc->ads_vma);
  +    size += PAGE_ALIGN(guc->fw.private_data_size);
+
      ret = intel_guc_allocate_and_map_vma(guc, size, &guc->ads_vma,
                           (void **)&guc->ads_blob);
  @@ -169,4 +189,9 @@ void intel_guc_ads_reset(struct intel_guc *guc)
      if (!guc->ads_vma)
          return;
      __guc_ads_init(guc);
+
+    if (guc->fw.private_data_size)
+        memset((void *)guc->ads_blob +
+               PAGE_ALIGN(sizeof(struct __guc_ads_blob)), 0,
+               PAGE_ALIGN(guc->fw.private_data_size));
  }
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h 
b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
index a6b733c146c9..48f77b67de8f 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc_fwif.h
@@ -62,12 +62,7 @@
  #define GUC_STAGE_DESC_ATTR_PCH        BIT(6)
  #define GUC_STAGE_DESC_ATTR_TERMINATED    BIT(7)
  -/* New GuC control data */
-#define GUC_CTL_CTXINFO            0
-#define   GUC_CTL_CTXNUM_IN16_SHIFT    0
-#define   GUC_CTL_BASE_ADDR_SHIFT    12
-
-#define GUC_CTL_LOG_PARAMS        1
+#define GUC_CTL_LOG_PARAMS        0
  #define   GUC_LOG_VALID            (1 << 0)
  #define   GUC_LOG_NOTIFY_ON_HALF_FULL    (1 << 1)
  #define   GUC_LOG_ALLOC_IN_MEGABYTE    (1 << 3)
@@ -79,11 +74,11 @@
  #define   GUC_LOG_ISR_MASK            (0x7 << GUC_LOG_ISR_SHIFT)
  #define   GUC_LOG_BUF_ADDR_SHIFT    12
  -#define GUC_CTL_WA            2
-#define GUC_CTL_FEATURE            3
+#define GUC_CTL_WA            1
+#define GUC_CTL_FEATURE            2
  #define   GUC_CTL_DISABLE_SCHEDULER    (1 << 14)
  -#define GUC_CTL_DEBUG            4
+#define GUC_CTL_DEBUG            3
  #define   GUC_LOG_VERBOSITY_SHIFT    0
  #define   GUC_LOG_VERBOSITY_LOW        (0 << GUC_LOG_VERBOSITY_SHIFT)
  #define   GUC_LOG_VERBOSITY_MED        (1 << GUC_LOG_VERBOSITY_SHIFT)
@@ -97,7 +92,7 @@
  #define   GUC_LOG_DISABLED        (1 << 6)
  #define   GUC_PROFILE_ENABLED        (1 << 7)
  -#define GUC_CTL_ADS            5
+#define GUC_CTL_ADS            4
  #define   GUC_ADS_ADDR_SHIFT        1
  #define   GUC_ADS_ADDR_MASK        (0xFFFFF << GUC_ADS_ADDR_SHIFT)
  @@ -369,7 +364,9 @@ struct guc_gt_system_info {
      u32 vdbox_enable_mask;
      u32 vdbox_sfc_support_mask;
      u32 vebox_enable_mask;
-    u32 reserved[9];
+    u32 reserved1;
+    u8 
mapping_table[GUC_MAX_ENGINE_CLASSES][GUC_MAX_INSTANCES_PER_CLASS];
+    u32 reserved2[8];
  } __packed;
    /* Clients info */
@@ -391,14 +388,15 @@ struct guc_clients_info {
  /* GuC Additional Data Struct */
  struct guc_ads {
      u32 reg_state_addr;
-    u32 reg_state_buffer;
+    u32 reserved0;
      u32 scheduler_policies;
      u32 gt_system_info;
      u32 clients_info;
      u32 control_data;
      u32 golden_context_lrca[GUC_MAX_ENGINE_CLASSES];
      u32 eng_state_size[GUC_MAX_ENGINE_CLASSES];
-    u32 reserved[16];
+    u32 private_data;
+    u32 reserved[15];
  } __packed;
    /* GuC logging structures */
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
index 8ee0a0c7f447..791edd6db6bc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c
@@ -43,15 +43,15 @@ void intel_uc_fw_change_status(struct intel_uc_fw 
*uc_fw,
   * features.
   */
  #define INTEL_UC_FIRMWARE_DEFS(fw_def, guc_def, huc_def) \
-    fw_def(TIGERLAKE,   0, guc_def(tgl, 35, 2, 0), huc_def(tgl,  7, 
0, 3)) \
-    fw_def(ELKHARTLAKE, 0, guc_def(ehl, 33, 0, 4), huc_def(ehl,  9, 
0, 0)) \
-    fw_def(ICELAKE,     0, guc_def(icl, 33, 0, 0), huc_def(icl,  9, 
0, 0)) \
-    fw_def(COFFEELAKE,  5, guc_def(cml, 33, 0, 0), huc_def(cml,  4, 
0, 0)) \
-    fw_def(COFFEELAKE,  0, guc_def(kbl, 33, 0, 0), huc_def(kbl,  4, 
0, 0)) \
-    fw_def(GEMINILAKE,  0, guc_def(glk, 33, 0, 0), huc_def(glk,  4, 
0, 0)) \
-    fw_def(KABYLAKE,    0, guc_def(kbl, 33, 0, 0), huc_def(kbl,  4, 
0, 0)) \
-    fw_def(BROXTON,     0, guc_def(bxt, 33, 0, 0), huc_def(bxt,  2, 
0, 0)) \
-    fw_def(SKYLAKE,     0, guc_def(skl, 33, 0, 0), huc_def(skl,  2, 
0, 0))
+    fw_def(TIGERLAKE,   0, guc_def(tgl, 40, 0, 0), huc_def(tgl,  7, 
0, 3)) \
+    fw_def(ELKHARTLAKE, 0, guc_def(ehl, 40, 0, 0), huc_def(ehl,  9, 
0, 0)) \
+    fw_def(ICELAKE,     0, guc_def(icl, 40, 0, 0), huc_def(icl,  9, 
0, 0)) \
+    fw_def(COFFEELAKE,  5, guc_def(cml, 40, 0, 0), huc_def(cml,  4, 
0, 0)) \
+    fw_def(COFFEELAKE,  0, guc_def(kbl, 40, 0, 0), huc_def(kbl,  4, 
0, 0)) \
+    fw_def(GEMINILAKE,  0, guc_def(glk, 40, 0, 0), huc_def(glk,  4, 
0, 0)) \
+    fw_def(KABYLAKE,    0, guc_def(kbl, 40, 0, 0), huc_def(kbl,  4, 
0, 0)) \
+    fw_def(BROXTON,     0, guc_def(bxt, 40, 0, 0), huc_def(bxt,  2, 
0, 0)) \
+    fw_def(SKYLAKE,     0, guc_def(skl, 40, 0, 0), huc_def(skl,  2, 
0, 0))
    #define __MAKE_UC_FW_PATH(prefix_, name_, major_, minor_, patch_) \
      "i915/" \
@@ -362,6 +362,9 @@ int intel_uc_fw_fetch(struct intel_uc_fw *uc_fw)
          }
      }
  +    if (uc_fw->type == INTEL_UC_FW_TYPE_GUC)
+        uc_fw->private_data_size = css->private_data_size;
+
      obj = i915_gem_object_create_shmem_from_data(i915, fw->data, 
fw->size);
      if (IS_ERR(obj)) {
          err = PTR_ERR(obj);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
index 1f30543d0d2d..7e6a21847660 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h
@@ -84,6 +84,8 @@ struct intel_uc_fw {
        u32 rsa_size;
      u32 ucode_size;
+
+    u32 private_data_size;
  };
    #ifdef CONFIG_DRM_I915_DEBUG_GUC
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h 
b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
index 029214cdedd5..e41ffc7a7fbc 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc_fw_abi.h
@@ -69,7 +69,11 @@ struct uc_css_header {
  #define CSS_SW_VERSION_UC_MAJOR        (0xFF << 16)
  #define CSS_SW_VERSION_UC_MINOR        (0xFF << 8)
  #define CSS_SW_VERSION_UC_PATCH        (0xFF << 0)
-    u32 reserved[14];
+    u32 reserved0[13];
+    union {
+        u32 private_data_size; /* only applies to GuC */
+        u32 reserved1;
+    };
      u32 header_info;
  } __packed;
  static_assert(sizeof(struct uc_css_header) == 128);


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







