Re: [Intel-gfx] [PATCH] drm/i915/dg2: make GuC FW a requirement for Gen12 and beyond devices

[Tvrtko Ursulin <tvrtko.ursulin@xxxxxxxxxxxxxxx>]

On 07/12/2021 17:53, Adrian Larumbe wrote:
> Beginning with DG2, all successive devices will require GuC FW to be
> present and loaded at probe() time. This change alters error handling in
> the FW init and load functions so that the driver's probe() function will
> fail if GuC could not be loaded.
>
> Signed-off-by: Adrian Larumbe <adrian.larumbe@xxxxxxxxxxxxx>
> ---
>   drivers/gpu/drm/i915/gt/uc/intel_uc.c | 20 ++++++++++++++++----
>   drivers/gpu/drm/i915/gt/uc/intel_uc.h |  4 ++--
>   drivers/gpu/drm/i915/i915_gem.c       |  7 ++++++-
>   3 files changed, 24 insertions(+), 7 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.c b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> index 7660eba893fa..8b0778b6d9ab 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.c
> @@ -277,14 +277,19 @@ static void guc_disable_communication(struct intel_guc *guc)
>   	drm_dbg(&i915->drm, "GuC communication disabled\n");
>   }
>   
> -static void __uc_fetch_firmwares(struct intel_uc *uc)
> +static int __uc_fetch_firmwares(struct intel_uc *uc)
>   {
> +	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
>   	int err;
>   
>   	GEM_BUG_ON(!intel_uc_wants_guc(uc));
>   
>   	err = intel_uc_fw_fetch(&uc->guc.fw);
>   	if (err) {
> +		/* GuC is mandatory on Gen12 and beyond */
> +		if (GRAPHICS_VER(i915) >= 12)
> +			return err;
> +

Is it DG2 or Gen12, latter starts from Tigerlake?

Regards,

Tvrtko

>   		/* Make sure we transition out of transient "SELECTED" state */
>   		if (intel_uc_wants_huc(uc)) {
>   			drm_dbg(&uc_to_gt(uc)->i915->drm,
> @@ -293,11 +298,13 @@ static void __uc_fetch_firmwares(struct intel_uc *uc)
>   						  INTEL_UC_FIRMWARE_ERROR);
>   		}
>   
> -		return;
> +		return 0;
>   	}
>   
>   	if (intel_uc_wants_huc(uc))
>   		intel_uc_fw_fetch(&uc->huc.fw);
> +
> +	return 0;
>   }
>   
>   static void __uc_cleanup_firmwares(struct intel_uc *uc)
> @@ -308,14 +315,19 @@ static void __uc_cleanup_firmwares(struct intel_uc *uc)
>   
>   static int __uc_init(struct intel_uc *uc)
>   {
> +	struct drm_i915_private *i915 = uc_to_gt(uc)->i915;
>   	struct intel_guc *guc = &uc->guc;
>   	struct intel_huc *huc = &uc->huc;
>   	int ret;
>   
>   	GEM_BUG_ON(!intel_uc_wants_guc(uc));
>   
> -	if (!intel_uc_uses_guc(uc))
> -		return 0;
> +	if (!intel_uc_uses_guc(uc)) {
> +		if (GRAPHICS_VER(i915) >= 12)
> +			return -EINVAL;
> +		else
> +			return 0;
> +	}
>   
>   	if (i915_inject_probe_failure(uc_to_gt(uc)->i915))
>   		return -ENOMEM;
> diff --git a/drivers/gpu/drm/i915/gt/uc/intel_uc.h b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> index 866b462821c0..3bcd781447bc 100644
> --- a/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> +++ b/drivers/gpu/drm/i915/gt/uc/intel_uc.h
> @@ -17,7 +17,7 @@ struct intel_uc;
>   
>   struct intel_uc_ops {
>   	int (*sanitize)(struct intel_uc *uc);
> -	void (*init_fw)(struct intel_uc *uc);
> +	int (*init_fw)(struct intel_uc *uc);
>   	void (*fini_fw)(struct intel_uc *uc);
>   	int (*init)(struct intel_uc *uc);
>   	void (*fini)(struct intel_uc *uc);
> @@ -104,7 +104,7 @@ static inline _TYPE intel_uc_##_NAME(struct intel_uc *uc) \
>   	return _RET; \
>   }
>   intel_uc_ops_function(sanitize, sanitize, int, 0);
> -intel_uc_ops_function(fetch_firmwares, init_fw, void, );
> +intel_uc_ops_function(fetch_firmwares, init_fw, int, 0);
>   intel_uc_ops_function(cleanup_firmwares, fini_fw, void, );
>   intel_uc_ops_function(init, init, int, 0);
>   intel_uc_ops_function(fini, fini, void, );
> diff --git a/drivers/gpu/drm/i915/i915_gem.c b/drivers/gpu/drm/i915/i915_gem.c
> index 527228d4da7e..7f8204af6826 100644
> --- a/drivers/gpu/drm/i915/i915_gem.c
> +++ b/drivers/gpu/drm/i915/i915_gem.c
> @@ -1049,7 +1049,12 @@ int i915_gem_init(struct drm_i915_private *dev_priv)
>   	if (ret)
>   		return ret;
>   
> -	intel_uc_fetch_firmwares(&dev_priv->gt.uc);
> +	ret = intel_uc_fetch_firmwares(&dev_priv->gt.uc);
> +	if (ret) {
> +		i915_probe_error(dev_priv, "Failed to fetch firmware\n");
> +		return ret;
> +	}
> +
>   	intel_wopcm_init(&dev_priv->wopcm);
>   
>   	ret = i915_init_ggtt(dev_priv);