[PATCH 1/9] drm/xe: Remove useless mem_access during probe

[Matthew Auld]

On 04/03/2024 18:21, Rodrigo Vivi wrote:
> xe_pm_init is the very last thing during the xe_pci_probe(),
> hence these protections are useless from the point of view
> of ensuring that the device is awake.
> 
> Let's remove it so we continue towards the goal of killing
> xe_device_mem_access.
> 
> v2: Adding more cases
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi at intel.com>
> ---
>   drivers/gpu/drm/xe/xe_ggtt.c |  2 --
>   drivers/gpu/drm/xe/xe_gt.c   |  9 ---------
>   drivers/gpu/drm/xe/xe_tile.c | 10 +++-------
>   drivers/gpu/drm/xe/xe_uc.c   | 11 -----------
>   4 files changed, 3 insertions(+), 29 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index 717d0e76277a..355e4bb987cb 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -206,14 +206,12 @@ static void xe_ggtt_initial_clear(struct xe_ggtt *ggtt)
>   	u64 start, end;
>   
>   	/* Display may have allocated inside ggtt, so be careful with clearing here */
> -	xe_device_mem_access_get(tile_to_xe(ggtt->tile));
>   	mutex_lock(&ggtt->lock);
>   	drm_mm_for_each_hole(hole, &ggtt->mm, start, end)
>   		xe_ggtt_clear(ggtt, start, end - start);
>   
>   	xe_ggtt_invalidate(ggtt);
>   	mutex_unlock(&ggtt->lock);
> -	xe_device_mem_access_put(tile_to_xe(ggtt->tile));
>   }
>   
>   int xe_ggtt_init(struct xe_ggtt *ggtt)
> diff --git a/drivers/gpu/drm/xe/xe_gt.c b/drivers/gpu/drm/xe/xe_gt.c
> index 85408e7a932b..063b710a8c7b 100644
> --- a/drivers/gpu/drm/xe/xe_gt.c
> +++ b/drivers/gpu/drm/xe/xe_gt.c
> @@ -347,7 +347,6 @@ static int gt_fw_domain_init(struct xe_gt *gt)
>   {
>   	int err, i;
>   
> -	xe_device_mem_access_get(gt_to_xe(gt));
>   	err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
>   	if (err)
>   		goto err_hw_fence_irq;
> @@ -389,7 +388,6 @@ static int gt_fw_domain_init(struct xe_gt *gt)
>   
>   	err = xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>   	XE_WARN_ON(err);
> -	xe_device_mem_access_put(gt_to_xe(gt));
>   
>   	return 0;
>   
> @@ -399,7 +397,6 @@ static int gt_fw_domain_init(struct xe_gt *gt)
>   err_hw_fence_irq:
>   	for (i = 0; i < XE_ENGINE_CLASS_MAX; ++i)
>   		xe_hw_fence_irq_finish(&gt->fence_irq[i]);
> -	xe_device_mem_access_put(gt_to_xe(gt));
>   
>   	return err;
>   }
> @@ -408,7 +405,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
>   {
>   	int err, i;
>   
> -	xe_device_mem_access_get(gt_to_xe(gt));
>   	err = xe_force_wake_get(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>   	if (err)
>   		goto err_hw_fence_irq;
> @@ -474,7 +470,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
>   
>   	err = xe_force_wake_put(gt_to_fw(gt), XE_FORCEWAKE_ALL);
>   	XE_WARN_ON(err);
> -	xe_device_mem_access_put(gt_to_xe(gt));
>   
>   	return 0;
>   
> @@ -483,7 +478,6 @@ static int all_fw_domain_init(struct xe_gt *gt)
>   err_hw_fence_irq:
>   	for (i = 0; i < XE_ENGINE_CLASS_MAX; ++i)
>   		xe_hw_fence_irq_finish(&gt->fence_irq[i]);
> -	xe_device_mem_access_put(gt_to_xe(gt));
>   
>   	return err;
>   }
> @@ -496,7 +490,6 @@ int xe_gt_init_hwconfig(struct xe_gt *gt)
>   {
>   	int err;
>   
> -	xe_device_mem_access_get(gt_to_xe(gt));
>   	err = xe_force_wake_get(gt_to_fw(gt), XE_FW_GT);
>   	if (err)
>   		goto out;
> @@ -519,8 +512,6 @@ int xe_gt_init_hwconfig(struct xe_gt *gt)
>   out_fw:
>   	xe_force_wake_put(gt_to_fw(gt), XE_FW_GT);
>   out:
> -	xe_device_mem_access_put(gt_to_xe(gt));
> -
>   	return err;
>   }
>   
> diff --git a/drivers/gpu/drm/xe/xe_tile.c b/drivers/gpu/drm/xe/xe_tile.c
> index 044c20881de7..74ecb5f39438 100644
> --- a/drivers/gpu/drm/xe/xe_tile.c
> +++ b/drivers/gpu/drm/xe/xe_tile.c
> @@ -160,23 +160,19 @@ int xe_tile_init_noalloc(struct xe_tile *tile)
>   {
>   	int err;
>   
> -	xe_device_mem_access_get(tile_to_xe(tile));
> -
>   	err = tile_ttm_mgr_init(tile);
>   	if (err)
> -		goto err_mem_access;
> +		return err;
>   
>   	tile->mem.kernel_bb_pool = xe_sa_bo_manager_init(tile, SZ_1M, 16);
>   	if (IS_ERR(tile->mem.kernel_bb_pool))
> -		err = PTR_ERR(tile->mem.kernel_bb_pool);
> +		return PTR_ERR(tile->mem.kernel_bb_pool);

I think it was commented on previous version, but this should be split 
into separate change. We now no longer call 
xe_wa_apply_tile_workarounds() etc. Is that a bugfix?

Otherwise,
Reviewed-by: Matthew Auld <mattew.auld at intel.com>

>   
>   	xe_wa_apply_tile_workarounds(tile);
>   
>   	xe_tile_sysfs_init(tile);
>   
> -err_mem_access:
> -	xe_device_mem_access_put(tile_to_xe(tile));
> -	return err;
> +	return 0;
>   }
>   
>   void xe_tile_migrate_wait(struct xe_tile *tile)
> diff --git a/drivers/gpu/drm/xe/xe_uc.c b/drivers/gpu/drm/xe/xe_uc.c
> index 7033f8c1b431..4feb35c95a1c 100644
> --- a/drivers/gpu/drm/xe/xe_uc.c
> +++ b/drivers/gpu/drm/xe/xe_uc.c
> @@ -32,11 +32,8 @@ uc_to_xe(struct xe_uc *uc)
>   /* Should be called once at driver load only */
>   int xe_uc_init(struct xe_uc *uc)
>   {
> -	struct xe_device *xe = uc_to_xe(uc);
>   	int ret;
>   
> -	xe_device_mem_access_get(xe);
> -
>   	/*
>   	 * We call the GuC/HuC/GSC init functions even if GuC submission is off
>   	 * to correctly move our tracking of the FW state to "disabled".
> @@ -65,16 +62,8 @@ int xe_uc_init(struct xe_uc *uc)
>   		goto err;
>   
>   	ret = xe_guc_db_mgr_init(&uc->guc.dbm, ~0);
> -	if (ret)
> -		goto err;
> -
> -	xe_device_mem_access_put(xe);
> -
> -	return 0;
>   
>   err:
> -	xe_device_mem_access_put(xe);
> -
>   	return ret;
>   }
>