diff mbox series

[v2,1/7] drm/i915/huc: only load HuC on GTs that have VCS engines

Message ID 20221013000332.1738078-2-daniele.ceraolospurio@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915: prepare for uC loading on MTL | expand

Commit Message

Daniele Ceraolo Spurio Oct. 13, 2022, 12:03 a.m. UTC 
On MTL the primary GT doesn't have any media capabilities, so no video
engines and no HuC. We must therefore skip the HuC fetch and load on
that specific case. Given that other multi-GT platforms might have HuC
on the primary GT, we can't just check for that and it is easier to
instead check for the lack of VCS engines.

Based on code from Aravind Iddamsetty

v2: clarify which engine_mask is used for each GT and why (Tvrtko)

Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
Cc: John Harrison <john.c.harrison@intel.com>
Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> #v1
---
 drivers/gpu/drm/i915/gt/uc/intel_huc.c | 29 ++++++++++++++++++++++++++
 drivers/gpu/drm/i915/i915_drv.h        |  9 +++++---
 2 files changed, 35 insertions(+), 3 deletions(-)

Comments

Iddamsetty, Aravind Oct. 19, 2022, 9:31 a.m. UTC | #1 
On 13-10-2022 05:33, Daniele Ceraolo Spurio wrote:
> On MTL the primary GT doesn't have any media capabilities, so no video
> engines and no HuC. We must therefore skip the HuC fetch and load on
> that specific case. Given that other multi-GT platforms might have HuC
> on the primary GT, we can't just check for that and it is easier to
> instead check for the lack of VCS engines.
> 
> Based on code from Aravind Iddamsetty
> 
> v2: clarify which engine_mask is used for each GT and why (Tvrtko)
> 
> Signed-off-by: Daniele Ceraolo Spurio <daniele.ceraolospurio@intel.com>
> Cc: Aravind Iddamsetty <aravind.iddamsetty@intel.com>
> Cc: John Harrison <john.c.harrison@intel.com>
> Cc: Alan Previn <alan.previn.teres.alexis@intel.com>
> Cc: Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>
> Reviewed-by: Aravind Iddamsetty <aravind.iddamsetty@intel.com> #v1
> ---
>  drivers/gpu/drm/i915/gt/uc/intel_huc.c | 29 ++++++++++++++++++++++++++
>  drivers/gpu/drm/i915/i915_drv.h        |  9 +++++---
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> index 4d1cc383b681..ca170ea3426c 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
> @@ -203,12 +203,41 @@ void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type *b
>  	huc->delayed_load.nb.notifier_call = NULL;
>  }
>  
> +static bool vcs_supported(struct intel_gt *gt)
> +{
> +	intel_engine_mask_t mask = gt->info.engine_mask;
> +
> +	/*
> +	 * We reach here from i915_driver_early_probe for the primary GT before
> +	 * its engine mask is set, so we use the device info engine mask for it;
> +	 * this means we're not taking VCS fusing into account, but if the
> +	 * primary GT supports VCS engines we expect at least one of them to
> +	 * remain unfused so we're fine.
> +	 * For other GTs we expect the GT-specific mask to be set before we
> +	 * call this function.
> +	 */
Comment sounds good to me. as the rest of the change is same as v1, You
can have my r-b for this.

Thanks,
Aravind.
> +	GEM_BUG_ON(!gt_is_root(gt) && !gt->info.engine_mask);
> +
> +	if (gt_is_root(gt))
> +		mask = RUNTIME_INFO(gt->i915)->platform_engine_mask;
> +	else
> +		mask = gt->info.engine_mask;
> +
> +	return __ENGINE_INSTANCES_MASK(mask, VCS0, I915_MAX_VCS);
> +}
> +
>  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(&huc->fw, INTEL_UC_FW_TYPE_HUC);
>  
> +	if (!vcs_supported(gt)) {
> +		intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_NOT_SUPPORTED);
> +		return;
> +	}
> +
>  	if (GRAPHICS_VER(i915) >= 11) {
>  		huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
>  		huc->status.mask = HUC_LOAD_SUCCESSFUL;
> diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
> index 90ed8e6db2fe..90a347140e90 100644
> --- a/drivers/gpu/drm/i915/i915_drv.h
> +++ b/drivers/gpu/drm/i915/i915_drv.h
> @@ -776,12 +776,15 @@ IS_SUBPLATFORM(const struct drm_i915_private *i915,
>  #define __HAS_ENGINE(engine_mask, id) ((engine_mask) & BIT(id))
>  #define HAS_ENGINE(gt, id) __HAS_ENGINE((gt)->info.engine_mask, id)
>  
> -#define ENGINE_INSTANCES_MASK(gt, first, count) ({		\
> +#define __ENGINE_INSTANCES_MASK(mask, first, count) ({			\
>  	unsigned int first__ = (first);					\
>  	unsigned int count__ = (count);					\
> -	((gt)->info.engine_mask &						\
> -	 GENMASK(first__ + count__ - 1, first__)) >> first__;		\
> +	((mask) & GENMASK(first__ + count__ - 1, first__)) >> first__;	\
>  })
> +
> +#define ENGINE_INSTANCES_MASK(gt, first, count) \
> +	__ENGINE_INSTANCES_MASK((gt)->info.engine_mask, first, count)
> +
>  #define RCS_MASK(gt) \
>  	ENGINE_INSTANCES_MASK(gt, RCS0, I915_MAX_RCS)
>  #define BCS_MASK(gt) \
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/gt/uc/intel_huc.c b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
index 4d1cc383b681..ca170ea3426c 100644
--- a/drivers/gpu/drm/i915/gt/uc/intel_huc.c
+++ b/drivers/gpu/drm/i915/gt/uc/intel_huc.c
@@ -203,12 +203,41 @@  void intel_huc_unregister_gsc_notifier(struct intel_huc *huc, struct bus_type *b
 	huc->delayed_load.nb.notifier_call = NULL;
 }
 
+static bool vcs_supported(struct intel_gt *gt)
+{
+	intel_engine_mask_t mask = gt->info.engine_mask;
+
+	/*
+	 * We reach here from i915_driver_early_probe for the primary GT before
+	 * its engine mask is set, so we use the device info engine mask for it;
+	 * this means we're not taking VCS fusing into account, but if the
+	 * primary GT supports VCS engines we expect at least one of them to
+	 * remain unfused so we're fine.
+	 * For other GTs we expect the GT-specific mask to be set before we
+	 * call this function.
+	 */
+	GEM_BUG_ON(!gt_is_root(gt) && !gt->info.engine_mask);
+
+	if (gt_is_root(gt))
+		mask = RUNTIME_INFO(gt->i915)->platform_engine_mask;
+	else
+		mask = gt->info.engine_mask;
+
+	return __ENGINE_INSTANCES_MASK(mask, VCS0, I915_MAX_VCS);
+}
+
 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(&huc->fw, INTEL_UC_FW_TYPE_HUC);
 
+	if (!vcs_supported(gt)) {
+		intel_uc_fw_change_status(&huc->fw, INTEL_UC_FIRMWARE_NOT_SUPPORTED);
+		return;
+	}
+
 	if (GRAPHICS_VER(i915) >= 11) {
 		huc->status.reg = GEN11_HUC_KERNEL_LOAD_INFO;
 		huc->status.mask = HUC_LOAD_SUCCESSFUL;
diff --git a/drivers/gpu/drm/i915/i915_drv.h b/drivers/gpu/drm/i915/i915_drv.h
index 90ed8e6db2fe..90a347140e90 100644
--- a/drivers/gpu/drm/i915/i915_drv.h
+++ b/drivers/gpu/drm/i915/i915_drv.h
@@ -776,12 +776,15 @@  IS_SUBPLATFORM(const struct drm_i915_private *i915,
 #define __HAS_ENGINE(engine_mask, id) ((engine_mask) & BIT(id))
 #define HAS_ENGINE(gt, id) __HAS_ENGINE((gt)->info.engine_mask, id)
 
-#define ENGINE_INSTANCES_MASK(gt, first, count) ({		\
+#define __ENGINE_INSTANCES_MASK(mask, first, count) ({			\
 	unsigned int first__ = (first);					\
 	unsigned int count__ = (count);					\
-	((gt)->info.engine_mask &						\
-	 GENMASK(first__ + count__ - 1, first__)) >> first__;		\
+	((mask) & GENMASK(first__ + count__ - 1, first__)) >> first__;	\
 })
+
+#define ENGINE_INSTANCES_MASK(gt, first, count) \
+	__ENGINE_INSTANCES_MASK((gt)->info.engine_mask, first, count)
+
 #define RCS_MASK(gt) \
 	ENGINE_INSTANCES_MASK(gt, RCS0, I915_MAX_RCS)
 #define BCS_MASK(gt) \