[Intel-gfx] [PATCH 2/4] drm/i915/uc: Reserve upper range of GGTT

[Daniele Ceraolo Spurio]

On 4/9/19 2:31 PM, Fernando Pacheco wrote:
> GuC and HuC depend on struct_mutex for device
> reinitialization. Moving away from this dependency
> requires perma-pinning the firmware images in GGTT.
> The upper portion of the GuC address space has
> a sizeable hole (several MB) that is inaccessible
> by GuC. Reserve this range within GGTT as it can
> comfortably hold GuC/HuC firmware images.
> 
> Signed-off-by: Fernando Pacheco <fernando.pacheco at intel.com>
> ---
>   drivers/gpu/drm/i915/i915_gem_gtt.c | 25 ++++++++++++++++++++++---
>   drivers/gpu/drm/i915/i915_gem_gtt.h |  1 +
>   drivers/gpu/drm/i915/intel_guc.h    | 11 +++++++++++
>   3 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.c b/drivers/gpu/drm/i915/i915_gem_gtt.c
> index 736c845eb77f..30f294a07e6d 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.c
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.c
> @@ -2747,6 +2747,18 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
>   	if (ret)
>   		return ret;
>   
> +	/* Reserve a mappable slot for our lockless uC firmware load */
> +	if (USES_GUC(dev_priv)) {

We know the the size of the binaries at this point, so maybe we can do 
something more accurate, like:

	struct drm_mm_node node;
	u32 guc_fw_size = ROUND_UP(guc_fw->size, I915_GTT_PAGE_SIZE);
	u32 huc_fw_size = ROUND_UP(huc_fw->size, I915_GTT_PAGE_SIZE);
	u32 min_reserved_size = guc_fw_size + huc_fw_size;

	ggtt->uc_fw.start =
		min_t(u32, GUC_GGTT_TOP, vm->total - min_reserved_size);
	ggtt->uc_fw.size = ggtt->vm.total - ggtt->uc_fw.start;

	drm_mm_reserve_node(&ggtt->vm.mm, &ggtt->uc_fw);

> +		ret = drm_mm_insert_node_in_range(&ggtt->vm.mm, &ggtt->uc_fw,
> +						  GUC_GGTT_FW_SIZE, 0,
> +						  I915_COLOR_UNEVICTABLE,
> +						  GUC_GGTT_FW_START,
> +						  GUC_GGTT_FW_END,
> +						  DRM_MM_INSERT_LOW);
> +		if (ret)
> +			goto err_node;
> +	}
> +
>   	/* Clear any non-preallocated blocks */
>   	drm_mm_for_each_hole(entry, &ggtt->vm.mm, hole_start, hole_end) {
>   		DRM_DEBUG_KMS("clearing unused GTT space: [%lx, %lx]\n",
> @@ -2761,12 +2773,15 @@ int i915_gem_init_ggtt(struct drm_i915_private *dev_priv)
>   	if (INTEL_PPGTT(dev_priv) == INTEL_PPGTT_ALIASING) {
>   		ret = i915_gem_init_aliasing_ppgtt(dev_priv);
>   		if (ret)
> -			goto err;
> +			goto err_appgtt;
>   	}
>   
>   	return 0;
>   
> -err:
> +err_appgtt:
> +	if (USES_GUC(dev_priv))
> +		drm_mm_remove_node(&ggtt->uc_fw);
> +err_node:
>   	drm_mm_remove_node(&ggtt->error_capture);
>   	return ret;
>   }
> @@ -2792,6 +2807,9 @@ void i915_ggtt_cleanup_hw(struct drm_i915_private *dev_priv)
>   	if (drm_mm_node_allocated(&ggtt->error_capture))
>   		drm_mm_remove_node(&ggtt->error_capture);
>   
> +	if (drm_mm_node_allocated(&ggtt->uc_fw))
> +		drm_mm_remove_node(&ggtt->uc_fw);
> +
>   	if (drm_mm_initialized(&ggtt->vm.mm)) {
>   		intel_vgt_deballoon(dev_priv);
>   		i915_address_space_fini(&ggtt->vm);
> @@ -3370,7 +3388,8 @@ int i915_ggtt_probe_hw(struct drm_i915_private *dev_priv)
>   	 * restriction!
>   	 */
>   	if (USES_GUC(dev_priv)) {
> -		ggtt->vm.total = min_t(u64, ggtt->vm.total, GUC_GGTT_TOP);
> +		ggtt->vm.total = min_t(u64, ggtt->vm.total,
> +				       GUC_GGTT_RESERVED_END);

No need to leave that extra space at the top, the dma engine can access 
all 4GB of the GGTT so we can leave ggtt->vm.total untouched when using 
guc. Just make sure you reserve everything above GUC_GGTT_TOP

Daniele

>   		ggtt->mappable_end =
>   			min_t(u64, ggtt->mappable_end, ggtt->vm.total);
>   	}
> diff --git a/drivers/gpu/drm/i915/i915_gem_gtt.h b/drivers/gpu/drm/i915/i915_gem_gtt.h
> index f597f35b109b..b51e779732c3 100644
> --- a/drivers/gpu/drm/i915/i915_gem_gtt.h
> +++ b/drivers/gpu/drm/i915/i915_gem_gtt.h
> @@ -384,6 +384,7 @@ struct i915_ggtt {
>   	u32 pin_bias;
>   
>   	struct drm_mm_node error_capture;
> +	struct drm_mm_node uc_fw;
>   };
>   
>   struct i915_hw_ppgtt {
> diff --git a/drivers/gpu/drm/i915/intel_guc.h b/drivers/gpu/drm/i915/intel_guc.h
> index 2c59ff8d9f39..b30cc2642fc4 100644
> --- a/drivers/gpu/drm/i915/intel_guc.h
> +++ b/drivers/gpu/drm/i915/intel_guc.h
> @@ -127,6 +127,17 @@ static inline void intel_guc_to_host_event_handler(struct intel_guc *guc)
>   /* GuC addresses above GUC_GGTT_TOP also don't map through the GTT */
>   #define GUC_GGTT_TOP	0xFEE00000
>   
> +/*
> + * This portion of the address space is inaccessible by GuC,
> + * but still accessible by DMA. We can repurpose this portion
> + * to house the uC firmware images and perform the image upload.
> + */
> +#define GUC_GGTT_FW_START	GUC_GGTT_TOP
> +#define GUC_GGTT_FW_END		0xFFE00000
> +#define GUC_GGTT_RESERVED_END	GUC_GGTT_FW_END
> +
> +#define GUC_GGTT_FW_SIZE	(GUC_GGTT_FW_END - GUC_GGTT_FW_START)
> +
>   /**
>    * intel_guc_ggtt_offset() - Get and validate the GGTT offset of @vma
>    * @guc: intel_guc structure.
>