Re: [Intel-gfx] [PATCH v2 2/8] drm/i915/uc: perma-pin firmwares

2023-05-17 Thread Ceraolo Spurio, Daniele 




On 5/17/2023 1:59 PM, John Harrison wrote:

On 4/28/2023 11:58, Daniele Ceraolo Spurio wrote:

Now that each FW has its own reserved area, we can keep them always
pinned and skip the pin/unpin dance on reset. This will make things
easier for the 2-step HuC authentication, which requires the FW to be
pinned in GGTT after the xfer is completed.
Given that we use dummy vmas for the pinning, we do need to explicitly
re-pin on resume because the automated helper won't cover us.

Signed-off-by: Daniele Ceraolo Spurio
Cc: Alan Previn
---
  drivers/gpu/drm/i915/gt/intel_ggtt.c  |  3 ++
  drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c |  7 -
  drivers/gpu/drm/i915/gt/uc/intel_guc.c    |  2 +-
  drivers/gpu/drm/i915/gt/uc/intel_huc.c    |  2 +-
  drivers/gpu/drm/i915/gt/uc/intel_uc.c |  8 +
  drivers/gpu/drm/i915/gt/uc/intel_uc.h |  2 ++
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 36 ++-
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  5 +++-
  8 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c

index 20915edc8bd9..ab71ed11de79 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -1322,6 +1322,9 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt)
  ggtt->vm.scratch_range(>vm, ggtt->error_capture.start,
 ggtt->error_capture.size);
  +    list_for_each_entry(gt, >gt_list, ggtt_link)
+    intel_uc_resume_mappings(>uc);
+
  ggtt->invalidate(ggtt);
    if (flush)
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c

index 64bff01026e8..af542e3cb3e9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
@@ -80,7 +80,12 @@ void intel_gsc_uc_init_early(struct intel_gsc_uc 
*gsc)

  {
  struct intel_gt *gt = gsc_uc_to_gt(gsc);
  -    intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GSC);
+    /*
+ * GSC FW needs to be copied to a dedicated memory allocations for
+ * loading (see gsc->local), so we don't need to GGTT map the FW 
image

+ * itself into GGTT.
+ */
+    intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GSC, false);
  INIT_WORK(>work, gsc_work);
    /* we can arrive here from i915_driver_early_probe for primary
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c

index c9f20385f6a0..2eb891b270ae 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -164,7 +164,7 @@ void intel_guc_init_early(struct intel_guc *guc)
  struct intel_gt *gt = guc_to_gt(guc);
  struct drm_i915_private *i915 = gt->i915;
  -    intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GUC);
+    intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GUC, true);
  intel_guc_ct_init_early(>ct);
  intel_guc_log_init_early(>log);
  intel_guc_submission_init_early(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c

index aefdaa62da99..9721761373fb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -276,7 +276,7 @@ void intel_huc_init_early(struct intel_huc *huc)
  struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
  struct intel_gt *gt = huc_to_gt(huc);
  -    intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_HUC);
+    intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_HUC, true);
    /*
   * we always init the fence as already completed, even if HuC 
is not
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c

index 996168312340..b6adfda3761e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -697,6 +697,12 @@ void intel_uc_suspend(struct intel_uc *uc)
  }
  }
  +static void __uc_resume_mappings(struct intel_uc *uc)
+{
+    intel_uc_fw_resume_mapping(>guc.fw);
+    intel_uc_fw_resume_mapping(>huc.fw);
+}
+
  static int __uc_resume(struct intel_uc *uc, bool enable_communication)
  {
  struct intel_guc *guc = >guc;
@@ -764,4 +770,6 @@ static const struct intel_uc_ops uc_ops_on = {
    .init_hw = __uc_init_hw,
  .fini_hw = __uc_fini_hw,
+
+    .resume_mappings = __uc_resume_mappings,
  };
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_uc.h

index 5d0f1bcc381e..c2783e6e752b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -24,6 +24,7 @@ struct intel_uc_ops {
  void (*fini)(struct intel_uc *uc);
  int (*init_hw)(struct intel_uc *uc);
  void (*fini_hw)(struct intel_uc *uc);
+    void (*resume_mappings)(struct intel_uc *uc);
  };
    struct intel_uc {
@@ -113,6 +114,7 @@ intel_uc_ops_function(init, init, int, 0);
  intel_uc_ops_function(fini, fini, void, );
  intel_uc_ops_function(init_hw, init_hw, int, 0);
  intel_uc_ops_function(fini_hw,

Re: [Intel-gfx] [PATCH v2 2/8] drm/i915/uc: perma-pin firmwares

2023-05-17 Thread John Harrison 

On 4/28/2023 11:58, Daniele Ceraolo Spurio wrote:

Now that each FW has its own reserved area, we can keep them always
pinned and skip the pin/unpin dance on reset. This will make things
easier for the 2-step HuC authentication, which requires the FW to be
pinned in GGTT after the xfer is completed.
Given that we use dummy vmas for the pinning, we do need to explicitly
re-pin on resume because the automated helper won't cover us.

Signed-off-by: Daniele Ceraolo Spurio
Cc: Alan Previn
---
  drivers/gpu/drm/i915/gt/intel_ggtt.c  |  3 ++
  drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c |  7 -
  drivers/gpu/drm/i915/gt/uc/intel_guc.c|  2 +-
  drivers/gpu/drm/i915/gt/uc/intel_huc.c|  2 +-
  drivers/gpu/drm/i915/gt/uc/intel_uc.c |  8 +
  drivers/gpu/drm/i915/gt/uc/intel_uc.h |  2 ++
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.c  | 36 ++-
  drivers/gpu/drm/i915/gt/uc/intel_uc_fw.h  |  5 +++-
  8 files changed, 53 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/i915/gt/intel_ggtt.c 
b/drivers/gpu/drm/i915/gt/intel_ggtt.c
index 20915edc8bd9..ab71ed11de79 100644
--- a/drivers/gpu/drm/i915/gt/intel_ggtt.c
+++ b/drivers/gpu/drm/i915/gt/intel_ggtt.c
@@ -1322,6 +1322,9 @@ void i915_ggtt_resume(struct i915_ggtt *ggtt)
ggtt->vm.scratch_range(>vm, ggtt->error_capture.start,
   ggtt->error_capture.size);
  
+	list_for_each_entry(gt, >gt_list, ggtt_link)

+   intel_uc_resume_mappings(>uc);
+
ggtt->invalidate(ggtt);
  
  	if (flush)

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
index 64bff01026e8..af542e3cb3e9 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_gsc_uc.c
@@ -80,7 +80,12 @@ void intel_gsc_uc_init_early(struct intel_gsc_uc *gsc)
  {
struct intel_gt *gt = gsc_uc_to_gt(gsc);
  
-	intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GSC);

+   /*
+* GSC FW needs to be copied to a dedicated memory allocations for
+* loading (see gsc->local), so we don't need to GGTT map the FW image
+* itself into GGTT.
+*/
+   intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GSC, false);
INIT_WORK(>work, gsc_work);
  
  	/* we can arrive here from i915_driver_early_probe for primary

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_guc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
index c9f20385f6a0..2eb891b270ae 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_guc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_guc.c
@@ -164,7 +164,7 @@ void intel_guc_init_early(struct intel_guc *guc)
struct intel_gt *gt = guc_to_gt(guc);
struct drm_i915_private *i915 = gt->i915;
  
-	intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GUC);

+   intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_GUC, true);
intel_guc_ct_init_early(>ct);
intel_guc_log_init_early(>log);
intel_guc_submission_init_early(guc);
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index aefdaa62da99..9721761373fb 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -276,7 +276,7 @@ void intel_huc_init_early(struct intel_huc *huc)
struct drm_i915_private *i915 = huc_to_gt(huc)->i915;
struct intel_gt *gt = huc_to_gt(huc);
  
-	intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_HUC);

+   intel_uc_fw_init_early(>fw, INTEL_UC_FW_TYPE_HUC, true);
  
  	/*

 * we always init the fence as already completed, even if HuC is not
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c 
b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
index 996168312340..b6adfda3761e 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
@@ -697,6 +697,12 @@ void intel_uc_suspend(struct intel_uc *uc)
}
  }
  
+static void __uc_resume_mappings(struct intel_uc *uc)

+{
+   intel_uc_fw_resume_mapping(>guc.fw);
+   intel_uc_fw_resume_mapping(>huc.fw);
+}
+
  static int __uc_resume(struct intel_uc *uc, bool enable_communication)
  {
struct intel_guc *guc = >guc;
@@ -764,4 +770,6 @@ static const struct intel_uc_ops uc_ops_on = {
  
  	.init_hw = __uc_init_hw,

.fini_hw = __uc_fini_hw,
+
+   .resume_mappings = __uc_resume_mappings,
  };
diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h 
b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
index 5d0f1bcc381e..c2783e6e752b 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
+++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
@@ -24,6 +24,7 @@ struct intel_uc_ops {
void (*fini)(struct intel_uc *uc);
int (*init_hw)(struct intel_uc *uc);
void (*fini_hw)(struct intel_uc *uc);
+   void (*resume_mappings)(struct intel_uc *uc);
  };
  
  struct intel_uc {

@@ -113,6 +114,7 @@ intel_uc_ops_function(init, init, int, 0);
  intel_uc_ops_function(fini, fini, void, );
  intel_uc_ops_function(init_hw, init_hw, int,