[Intel-gfx] [PATCH v11 4/6] drm/i915: Add HuC firmware size related restriction for Gen9 and CNL A0

[Michal Wajdeczko]

On Thu, 01 Mar 2018 02:01:38 +0100, Jackie Li <yaodong.li at intel.com> wrote:

> On CNL A0 and Gen9, there's a hardware restriction that requires the
> available GuC WOPCM size to be larger than or equal to HuC firmware size.
>
> This patch adds new verification code to ensure the available GuC WOPCM
> size to be larger than or equal to HuC firmware size on both Gen9 and CNL
> A0.
>
> v6:
>  - Extended HuC FW size check against GuC WOPCM size to all
>    Gen9 and CNL A0 platforms
>
> v7:
>  - Fixed patch format issues
>
> v8:
>  - Renamed variables and functions to avoid ambiguity (Joonas)
>  - Updated commit message and comments to be more comprehensive (Sagar)
>
> v9:
>  - Moved code that is not related to restriction check into a separate
>    patch and updated the commit message accordingly (Sagar/Michal)
>  - Avoided to call uc_get_fw_size for better layer isolation (Michal)
>
> v10:
>  - Shorten function names and reorganized size_check code to have clear
>    isolation (Joonas)
>  - Removed unnecessary comments (Joonas)
>
> v11:
>  - Fixed logic error in size check (Michal)
>
> BSpec: 10875
>
> Cc: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Sagar Arun Kamble <sagar.a.kamble at intel.com>
> Cc: John Spotswood <john.a.spotswood at intel.com>
> Cc: Jeff McGee <jeff.mcgee at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Joonas Lahtinen <joonas.lahtinen at linux.intel.com>
> Reviewed-by: Sagar Arun Kamble <sagar.a.kamble at intel.com> (v8)
> Signed-off-by: Jackie Li <yaodong.li at intel.com>
> ---
>  drivers/gpu/drm/i915/intel_wopcm.c | 28 +++++++++++++++++++++++++---
>  1 file changed, 25 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c  
> b/drivers/gpu/drm/i915/intel_wopcm.c
> index bb78043..b30d7ff 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -107,8 +107,26 @@ static inline int gen9_check_dword_gap(u32  
> guc_wopcm_base, u32 guc_wopcm_size)
>  	return 0;
>  }
> +static inline int gen9_check_huc_fw_fits(u32 guc_wopcm_size, u32  
> huc_fw_size)
> +{
> +	/*
> +	 * On Gen9 & CNL A0, hardware requires the total available GuC WOPCM
> +	 * size to be larger than or equal to HuC firmware size. Otherwise,
> +	 * firmware uploading would fail.
> +	 */
> +	if (huc_fw_size > guc_wopcm_size - GUC_WOPCM_RESERVED) {
> +		DRM_ERROR("HuC fw(%uKiB) won't fit in GuC WOPCM(%uKiB).\n",
> +			  huc_fw_size / 1024,
> +			  (guc_wopcm_size - GUC_WOPCM_RESERVED) / 1024);

bikeshed: in earlier patches in similar error messages, you used
"HuC FW (%KiB)" and didn't provide available space. Maybe simplest
way to unify and minimize the code is to add one "failed" tag in
wopcm_init function where you can print all values used for partitioning:

failed:
	DRM_ERROR("Failed to partition %uKiB WOPCM (%d)\n", wopcm->size/1024,  
err);
	DRM_ERROR("HuC FW size=%uKiB\n", ...);
	DRM_ERROR("GuC FW size=%uKiB\n", ...);
	return err;

> +		return -E2BIG;
> +	}
> +
> +	return 0;
> +}
> +
>  static inline int check_hw_restriction(struct drm_i915_private *i915,
> -				       u32 guc_wopcm_base, u32 guc_wopcm_size)
> +				       u32 guc_wopcm_base, u32 guc_wopcm_size,
> +				       u32 huc_fw_size)
>  {
>  	int err = 0;
> @@ -117,7 +135,10 @@ static inline int check_hw_restriction(struct  
> drm_i915_private *i915,
>  	if (err)
>  		return err;
> -	return 0;
> +	if (IS_GEN9(i915) || IS_CNL_REVID(i915, CNL_REVID_A0, CNL_REVID_A0))
> +		err = gen9_check_huc_fw_fits(guc_wopcm_size, huc_fw_size);
> +
> +	return err;
>  }
> /**
> @@ -186,7 +207,8 @@ int intel_wopcm_init(struct intel_wopcm *wopcm)
>  		return -E2BIG;
>  	}
> -	err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size);
> +	err = check_hw_restriction(i915, guc_wopcm_base, guc_wopcm_size,
> +				   huc_fw_size);
>  	if (err) {
>  		DRM_ERROR("Failed to meet HW restriction.\n");
>  		return err;

but bikeshed is not critical, so

Reviewed-by: Michal Wajdeczko <michal.wajdeczko at intel.com>