[Intel-xe] [PATCH v5 14/23] drm/xe: Add a helper for DRM device-lifetime BO create

Matthew Brost matthew.brost at intel.com
Wed Nov 29 18:27:01 UTC 2023


On Wed, Nov 29, 2023 at 11:34:59PM +0100, Michał Winiarski wrote:
> A helper for managed BO allocations makes it possible to remove specific
> "fini" actions and will simplify the following patches adding ability to
> execute a release action for specific BO directly.
> 
> Signed-off-by: Michał Winiarski <michal.winiarski at intel.com>
> ---
> v4 -> v5:
> - Check for error in managed create from data (Michał Wajdeczko)
> 
>  drivers/gpu/drm/xe/xe_bo.c           | 36 ++++++++++++++++++++++++++++
>  drivers/gpu/drm/xe/xe_bo.h           |  4 ++++
>  drivers/gpu/drm/xe/xe_ggtt.c         |  6 +----
>  drivers/gpu/drm/xe/xe_guc_ads.c      | 20 +++-------------
>  drivers/gpu/drm/xe/xe_guc_ct.c       |  8 +++----
>  drivers/gpu/drm/xe/xe_guc_hwconfig.c | 18 +++-----------
>  drivers/gpu/drm/xe/xe_guc_log.c      | 19 +++------------
>  drivers/gpu/drm/xe/xe_guc_pc.c       |  9 +++----
>  drivers/gpu/drm/xe/xe_hw_engine.c    |  8 +++----
>  drivers/gpu/drm/xe/xe_uc_fw.c        |  9 ++++---
>  10 files changed, 63 insertions(+), 74 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_bo.c b/drivers/gpu/drm/xe/xe_bo.c
> index d59501ffc22ae..1a7fe62de8122 100644
> --- a/drivers/gpu/drm/xe/xe_bo.c
> +++ b/drivers/gpu/drm/xe/xe_bo.c
> @@ -9,6 +9,7 @@
>  
>  #include <drm/drm_drv.h>
>  #include <drm/drm_gem_ttm_helper.h>
> +#include <drm/drm_managed.h>
>  #include <drm/ttm/ttm_device.h>
>  #include <drm/ttm/ttm_placement.h>
>  #include <drm/ttm/ttm_tt.h>
> @@ -1530,6 +1531,41 @@ struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
>  	return bo;
>  }
>  
> +static void __xe_bo_release(struct drm_device *drm, void *arg)

Nit:
s/__xe_bo_release/__xe_bo_unpin_map_no_vm/

Either way:
Reviewed-by: Matthew Brost <matthew.brost at intel.com>

> +{
> +	xe_bo_unpin_map_no_vm(arg);
> +}
> +
> +struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> +					   size_t size, u32 flags)
> +{
> +	struct xe_bo *bo;
> +	int ret;
> +
> +	bo = xe_bo_create_pin_map(xe, tile, NULL, size, ttm_bo_type_kernel, flags);
> +	if (IS_ERR(bo))
> +		return bo;
> +
> +	ret = drmm_add_action_or_reset(&xe->drm, __xe_bo_release, bo);
> +	if (ret)
> +		return ERR_PTR(ret);
> +
> +	return bo;
> +}
> +
> +struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> +					     const void *data, size_t size, u32 flags)
> +{
> +	struct xe_bo *bo = xe_managed_bo_create_pin_map(xe, tile, size, flags);
> +
> +	if (IS_ERR(bo))
> +		return bo;
> +
> +	xe_map_memcpy_to(xe, &bo->vmap, 0, data, size);
> +
> +	return bo;
> +}
> +
>  /*
>   * XXX: This is in the VM bind data path, likely should calculate this once and
>   * store, with a recalculation if the BO is moved.
> diff --git a/drivers/gpu/drm/xe/xe_bo.h b/drivers/gpu/drm/xe/xe_bo.h
> index 77e723dd9102c..098ccab7fa1e8 100644
> --- a/drivers/gpu/drm/xe/xe_bo.h
> +++ b/drivers/gpu/drm/xe/xe_bo.h
> @@ -115,6 +115,10 @@ struct xe_bo *xe_bo_create_pin_map_at(struct xe_device *xe, struct xe_tile *tile
>  struct xe_bo *xe_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
>  				     const void *data, size_t size,
>  				     enum ttm_bo_type type, u32 flags);
> +struct xe_bo *xe_managed_bo_create_pin_map(struct xe_device *xe, struct xe_tile *tile,
> +					   size_t size, u32 flags);
> +struct xe_bo *xe_managed_bo_create_from_data(struct xe_device *xe, struct xe_tile *tile,
> +					     const void *data, size_t size, u32 flags);
>  
>  int xe_bo_placement_for_flags(struct xe_device *xe, struct xe_bo *bo,
>  			      u32 bo_flags);
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index fab3cc04882da..8dc783e9120d2 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -108,7 +108,6 @@ static void ggtt_fini(struct drm_device *drm, void *arg)
>  {
>  	struct xe_ggtt *ggtt = arg;
>  
> -	xe_bo_unpin_map_no_vm(ggtt->scratch);
>  	ggtt->scratch = NULL;
>  }
>  
> @@ -225,10 +224,7 @@ int xe_ggtt_init(struct xe_ggtt *ggtt)
>  	else
>  		flags |= XE_BO_CREATE_VRAM_IF_DGFX(ggtt->tile);
>  
> -	ggtt->scratch = xe_bo_create_pin_map(xe, ggtt->tile, NULL, XE_PAGE_SIZE,
> -					     ttm_bo_type_kernel,
> -					     flags);
> -
> +	ggtt->scratch = xe_managed_bo_create_pin_map(xe, ggtt->tile, XE_PAGE_SIZE, flags);
>  	if (IS_ERR(ggtt->scratch)) {
>  		err = PTR_ERR(ggtt->scratch);
>  		goto err;
> diff --git a/drivers/gpu/drm/xe/xe_guc_ads.c b/drivers/gpu/drm/xe/xe_guc_ads.c
> index 88789826e7817..2f5ff090aa6bd 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ads.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ads.c
> @@ -202,13 +202,6 @@ static size_t guc_ads_size(struct xe_guc_ads *ads)
>  		guc_ads_private_data_size(ads);
>  }
>  
> -static void guc_ads_fini(struct drm_device *drm, void *arg)
> -{
> -	struct xe_guc_ads *ads = arg;
> -
> -	xe_bo_unpin_map_no_vm(ads->bo);
> -}
> -
>  static bool needs_wa_1607983814(struct xe_device *xe)
>  {
>  	return GRAPHICS_VERx100(xe) < 1250;
> @@ -274,25 +267,18 @@ int xe_guc_ads_init(struct xe_guc_ads *ads)
>  	struct xe_gt *gt = ads_to_gt(ads);
>  	struct xe_tile *tile = gt_to_tile(gt);
>  	struct xe_bo *bo;
> -	int err;
>  
>  	ads->golden_lrc_size = calculate_golden_lrc_size(ads);
>  	ads->regset_size = calculate_regset_size(gt);
>  
> -	bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ads_size(ads) +
> -				  MAX_GOLDEN_LRC_SIZE,
> -				  ttm_bo_type_kernel,
> -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -				  XE_BO_CREATE_GGTT_BIT);
> +	bo = xe_managed_bo_create_pin_map(xe, tile, guc_ads_size(ads) + MAX_GOLDEN_LRC_SIZE,
> +					  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +					  XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(bo))
>  		return PTR_ERR(bo);
>  
>  	ads->bo = bo;
>  
> -	err = drmm_add_action_or_reset(&xe->drm, guc_ads_fini, ads);
> -	if (err)
> -		return err;
> -
>  	return 0;
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_ct.c b/drivers/gpu/drm/xe/xe_guc_ct.c
> index 6295d916e39f6..24a33fa36496a 100644
> --- a/drivers/gpu/drm/xe/xe_guc_ct.c
> +++ b/drivers/gpu/drm/xe/xe_guc_ct.c
> @@ -114,7 +114,6 @@ static void guc_ct_fini(struct drm_device *drm, void *arg)
>  	struct xe_guc_ct *ct = arg;
>  
>  	xa_destroy(&ct->fence_lookup);
> -	xe_bo_unpin_map_no_vm(ct->bo);
>  }
>  
>  static void g2h_worker_func(struct work_struct *w);
> @@ -148,10 +147,9 @@ int xe_guc_ct_init(struct xe_guc_ct *ct)
>  
>  	primelockdep(ct);
>  
> -	bo = xe_bo_create_pin_map(xe, tile, NULL, guc_ct_size(),
> -				  ttm_bo_type_kernel,
> -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -				  XE_BO_CREATE_GGTT_BIT);
> +	bo = xe_managed_bo_create_pin_map(xe, tile, guc_ct_size(),
> +					  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +					  XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(bo))
>  		return PTR_ERR(bo);
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_hwconfig.c b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> index 98bb9bb307051..2a13a00917f8c 100644
> --- a/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> +++ b/drivers/gpu/drm/xe/xe_guc_hwconfig.c
> @@ -48,13 +48,6 @@ static int guc_hwconfig_copy(struct xe_guc *guc)
>  	return 0;
>  }
>  
> -static void guc_hwconfig_fini(struct drm_device *drm, void *arg)
> -{
> -	struct xe_guc *guc = arg;
> -
> -	xe_bo_unpin_map_no_vm(guc->hwconfig.bo);
> -}
> -
>  int xe_guc_hwconfig_init(struct xe_guc *guc)
>  {
>  	struct xe_device *xe = guc_to_xe(guc);
> @@ -84,19 +77,14 @@ int xe_guc_hwconfig_init(struct xe_guc *guc)
>  	if (!size)
>  		return -EINVAL;
>  
> -	bo = xe_bo_create_pin_map(xe, tile, NULL, PAGE_ALIGN(size),
> -				  ttm_bo_type_kernel,
> -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -				  XE_BO_CREATE_GGTT_BIT);
> +	bo = xe_managed_bo_create_pin_map(xe, tile, PAGE_ALIGN(size),
> +					  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +					  XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(bo))
>  		return PTR_ERR(bo);
>  	guc->hwconfig.bo = bo;
>  	guc->hwconfig.size = size;
>  
> -	err = drmm_add_action_or_reset(&xe->drm, guc_hwconfig_fini, guc);
> -	if (err)
> -		return err;
> -
>  	return guc_hwconfig_copy(guc);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_guc_log.c b/drivers/gpu/drm/xe/xe_guc_log.c
> index 27c3827bfd054..bcd2f4d34081d 100644
> --- a/drivers/gpu/drm/xe/xe_guc_log.c
> +++ b/drivers/gpu/drm/xe/xe_guc_log.c
> @@ -77,24 +77,15 @@ void xe_guc_log_print(struct xe_guc_log *log, struct drm_printer *p)
>  	}
>  }
>  
> -static void guc_log_fini(struct drm_device *drm, void *arg)
> -{
> -	struct xe_guc_log *log = arg;
> -
> -	xe_bo_unpin_map_no_vm(log->bo);
> -}
> -
>  int xe_guc_log_init(struct xe_guc_log *log)
>  {
>  	struct xe_device *xe = log_to_xe(log);
>  	struct xe_tile *tile = gt_to_tile(log_to_gt(log));
>  	struct xe_bo *bo;
> -	int err;
>  
> -	bo = xe_bo_create_pin_map(xe, tile, NULL, guc_log_size(),
> -				  ttm_bo_type_kernel,
> -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -				  XE_BO_CREATE_GGTT_BIT);
> +	bo = xe_managed_bo_create_pin_map(xe, tile, guc_log_size(),
> +					  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +					  XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(bo))
>  		return PTR_ERR(bo);
>  
> @@ -102,9 +93,5 @@ int xe_guc_log_init(struct xe_guc_log *log)
>  	log->bo = bo;
>  	log->level = xe_modparam.guc_log_level;
>  
> -	err = drmm_add_action_or_reset(&xe->drm, guc_log_fini, log);
> -	if (err)
> -		return err;
> -
>  	return 0;
>  }
> diff --git a/drivers/gpu/drm/xe/xe_guc_pc.c b/drivers/gpu/drm/xe/xe_guc_pc.c
> index 1943893a3fd7d..d2605a684b1c5 100644
> --- a/drivers/gpu/drm/xe/xe_guc_pc.c
> +++ b/drivers/gpu/drm/xe/xe_guc_pc.c
> @@ -964,7 +964,6 @@ void xe_guc_pc_fini(struct xe_guc_pc *pc)
>  	XE_WARN_ON(xe_guc_pc_gucrc_disable(pc));
>  	XE_WARN_ON(xe_guc_pc_stop(pc));
>  	sysfs_remove_files(pc_to_gt(pc)->sysfs, pc_attrs);
> -	xe_bo_unpin_map_no_vm(pc->bo);
>  	mutex_destroy(&pc->freq_lock);
>  }
>  
> @@ -986,11 +985,9 @@ int xe_guc_pc_init(struct xe_guc_pc *pc)
>  
>  	mutex_init(&pc->freq_lock);
>  
> -	bo = xe_bo_create_pin_map(xe, tile, NULL, size,
> -				  ttm_bo_type_kernel,
> -				  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -				  XE_BO_CREATE_GGTT_BIT);
> -
> +	bo = xe_managed_bo_create_pin_map(xe, tile, size,
> +					  XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +					  XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(bo))
>  		return PTR_ERR(bo);
>  
> diff --git a/drivers/gpu/drm/xe/xe_hw_engine.c b/drivers/gpu/drm/xe/xe_hw_engine.c
> index c52c26c395a7d..108ecbfe593ee 100644
> --- a/drivers/gpu/drm/xe/xe_hw_engine.c
> +++ b/drivers/gpu/drm/xe/xe_hw_engine.c
> @@ -239,8 +239,6 @@ static void hw_engine_fini(struct drm_device *drm, void *arg)
>  		xe_execlist_port_destroy(hwe->exl_port);
>  	xe_lrc_finish(&hwe->kernel_lrc);
>  
> -	xe_bo_unpin_map_no_vm(hwe->hwsp);
> -
>  	hwe->gt = NULL;
>  }
>  
> @@ -428,9 +426,9 @@ static int hw_engine_init(struct xe_gt *gt, struct xe_hw_engine *hwe,
>  	xe_reg_sr_apply_mmio(&hwe->reg_sr, gt);
>  	xe_reg_sr_apply_whitelist(hwe);
>  
> -	hwe->hwsp = xe_bo_create_pin_map(xe, tile, NULL, SZ_4K, ttm_bo_type_kernel,
> -					 XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -					 XE_BO_CREATE_GGTT_BIT);
> +	hwe->hwsp = xe_managed_bo_create_pin_map(xe, tile, SZ_4K,
> +						 XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +						 XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(hwe->hwsp)) {
>  		err = PTR_ERR(hwe->hwsp);
>  		goto err_name;
> diff --git a/drivers/gpu/drm/xe/xe_uc_fw.c b/drivers/gpu/drm/xe/xe_uc_fw.c
> index e806e7b6ae424..8ad4bcabb8b5e 100644
> --- a/drivers/gpu/drm/xe/xe_uc_fw.c
> +++ b/drivers/gpu/drm/xe/xe_uc_fw.c
> @@ -292,7 +292,6 @@ static void uc_fw_fini(struct drm_device *drm, void *arg)
>  	if (!xe_uc_fw_is_available(uc_fw))
>  		return;
>  
> -	xe_bo_unpin_map_no_vm(uc_fw->bo);
>  	xe_uc_fw_change_status(uc_fw, XE_UC_FIRMWARE_SELECTED);
>  }
>  
> @@ -692,10 +691,9 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
>  			goto fail;
>  	}
>  
> -	obj = xe_bo_create_from_data(xe, tile, fw->data, fw->size,
> -				     ttm_bo_type_kernel,
> -				     XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> -				     XE_BO_CREATE_GGTT_BIT);
> +	obj = xe_managed_bo_create_from_data(xe, tile, fw->data, fw->size,
> +					     XE_BO_CREATE_VRAM_IF_DGFX(tile) |
> +					     XE_BO_CREATE_GGTT_BIT);
>  	if (IS_ERR(obj)) {
>  		drm_notice(&xe->drm, "%s firmware %s: failed to create / populate bo",
>  			   xe_uc_fw_type_repr(uc_fw->type), uc_fw->path);
> @@ -726,6 +724,7 @@ int xe_uc_fw_init(struct xe_uc_fw *uc_fw)
>  		 xe_uc_fw_type_repr(uc_fw->type), XE_UC_FIRMWARE_URL);
>  
>  	release_firmware(fw);		/* OK even if fw is NULL */
> +
>  	return err;
>  }
>  
> -- 
> 2.43.0
> 


More information about the Intel-xe mailing list