[Intel-gfx] [PATCH 1/2] drm/i915/wopcm: Try to use already locked WOPCM layout

[Daniele Ceraolo Spurio]

On 8/14/19 4:38 AM, Michal Wajdeczko wrote:
> If WOPCM layout is already locked in HW we shouldn't continue
> with our own partitioning as it could be likely different and
> we will be unable to enforce it and fail. Instead we should try
> to reuse what is already programmed, maybe there will be a fit.
> 
> This should enable us to reload driver with slightly different
> HuC firmware (or even without HuC) without need to reboot.
> 
> Signed-off-by: Michal Wajdeczko <michal.wajdeczko at intel.com>
> Cc: Daniele Ceraolo Spurio <daniele.ceraolospurio at intel.com>
> Cc: Chris Wilson <chris at chris-wilson.co.uk>
> Cc: Michal Winiarski <michal.winiarski at intel.com>
> ---
>   drivers/gpu/drm/i915/intel_wopcm.c | 29 +++++++++++++++++++++++++++--
>   1 file changed, 27 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_wopcm.c b/drivers/gpu/drm/i915/intel_wopcm.c
> index 2bda24200498..e5bc7b8a433e 100644
> --- a/drivers/gpu/drm/i915/intel_wopcm.c
> +++ b/drivers/gpu/drm/i915/intel_wopcm.c
> @@ -154,6 +154,21 @@ static inline int check_hw_restriction(struct drm_i915_private *i915,
>   	return err;
>   }
>   
> +static bool __wopcm_regs_locked(struct intel_uncore *uncore,
> +				u32 *guc_wopcm_base, u32 *guc_wopcm_size)
> +{
> +	u32 reg_base = intel_uncore_read(uncore, DMA_GUC_WOPCM_OFFSET);
> +	u32 reg_size = intel_uncore_read(uncore, GUC_WOPCM_SIZE);
> +
> +	if (!(reg_size & GUC_WOPCM_SIZE_LOCKED) ||
> +	    !(reg_base & GUC_WOPCM_OFFSET_VALID))
> +		return false;

Should we at least WARN in the unlikely case where only one of the 2 
regs is locked?

> +
> +	*guc_wopcm_base = reg_base & GUC_WOPCM_OFFSET_MASK;
> +	*guc_wopcm_size = reg_size & GUC_WOPCM_SIZE_MASK;
> +	return true;
> +}
> +
>   /**
>    * intel_wopcm_init() - Initialize the WOPCM structure.
>    * @wopcm: pointer to intel_wopcm.
> @@ -167,8 +182,9 @@ static inline int check_hw_restriction(struct drm_i915_private *i915,
>   void intel_wopcm_init(struct intel_wopcm *wopcm)
>   {
>   	struct drm_i915_private *i915 = wopcm_to_i915(wopcm);
> -	u32 guc_fw_size = intel_uc_fw_get_upload_size(&i915->gt.uc.guc.fw);
> -	u32 huc_fw_size = intel_uc_fw_get_upload_size(&i915->gt.uc.huc.fw);
> +	struct intel_gt *gt = &i915->gt;
> +	u32 guc_fw_size = intel_uc_fw_get_upload_size(&gt->uc.guc.fw);
> +	u32 huc_fw_size = intel_uc_fw_get_upload_size(&gt->uc.huc.fw);
>   	u32 ctx_rsvd = context_reserved_size(i915);
>   	u32 guc_wopcm_base;
>   	u32 guc_wopcm_size;
> @@ -185,6 +201,14 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>   	if (i915_inject_probe_failure(i915))
>   		return;
>   
> +	if (__wopcm_regs_locked(gt->uncore, &guc_wopcm_base, &guc_wopcm_size)) {
> +		DRM_DEV_DEBUG_DRIVER(i915->drm.dev,
> +				     "GuC WOPCM is already locked [%uK, %uK)\n",
> +				     guc_wopcm_base / SZ_1K,
> +				     guc_wopcm_size / SZ_1K);
> +		goto check;
> +	}
> +
>   	if (guc_fw_size >= wopcm->size) {
>   		DRM_ERROR("GuC FW (%uKiB) is too big to fit in WOPCM.",
>   			  guc_fw_size / 1024);
> @@ -211,6 +235,7 @@ void intel_wopcm_init(struct intel_wopcm *wopcm)
>   	DRM_DEBUG_DRIVER("Calculated GuC WOPCM Region: [%uKiB, %uKiB)\n",
>   			 guc_wopcm_base / 1024, guc_wopcm_size / 1024);
>   
> +check:

The checks here don't really verify that the sizes we're locked with are 
enough to fit the binaries. We need to at least print an error in that 
case so we can debug why GuC/HuC fails to load later if the sizes are 
not ok.

Daniele

>   	guc_wopcm_rsvd = GUC_WOPCM_RESERVED + GUC_WOPCM_STACK_RESERVED;
>   	if ((guc_fw_size + guc_wopcm_rsvd) > guc_wopcm_size) {
>   		DRM_ERROR("Need %uKiB WOPCM for GuC, %uKiB available.\n",
>