Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Update to GuC FW v40
On 1/21/2020 10:29, Daniele Ceraolo Spurio wrote:
On 1/15/20 12:37 PM, john.c.harri...@intel.com wrote:
From: Michal Wajdeczko
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
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
Signed-off-by: John Harrison
Signed-off-by: Michal Wajdeczko
CC: Daniele Ceraolo Spurio
---
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
Re: [Intel-gfx] [PATCH v2] drm/i915/guc: Update to GuC FW v40
On 1/15/20 12:37 PM, john.c.harri...@intel.com wrote:
From: Michal Wajdeczko
v1 was from Matt, now it is from Michal?
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
Can you resend with enable_guc=2 to trigger a guc-enabled shards run on
all sharded platforms?
Daniele
Signed-off-by: Matthew Brost
Signed-off-by: John Harrison
Signed-off-by: Michal Wajdeczko
CC: Daniele Ceraolo Spurio
---
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(>system_info);
+
base
[Intel-gfx] [PATCH v2] drm/i915/guc: Update to GuC FW v40
From: Michal Wajdeczko
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.
Signed-off-by: Matthew Brost
Signed-off-by: John Harrison
Signed-off-by: Michal Wajdeczko
CC: Daniele Ceraolo Spurio
---
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(>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 +
