Re: [PATCH 06/12] drm/xe: Rename xe_ggtt_node related functions

[Matthew Brost <matthew.brost@intel.com>]

On Thu, Jul 11, 2024 at 01:11:49PM -0400, Rodrigo Vivi wrote:
> Bring some consistency and prepare for more xe_ggtt_node related
> functions to be introduced.
> 
> Signed-off-by: Rodrigo Vivi <rodrigo.vivi@intel.com>

Reviewed-by: Matthew Brost <matthew.brost@intel.com>

> ---
>  drivers/gpu/drm/xe/display/xe_fb_pin.c     |  8 +-
>  drivers/gpu/drm/xe/xe_ggtt.c               | 86 +++++++++++-----------
>  drivers/gpu/drm/xe/xe_ggtt.h               | 12 +--
>  drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c |  4 +-
>  4 files changed, 54 insertions(+), 56 deletions(-)
> 
> diff --git a/drivers/gpu/drm/xe/display/xe_fb_pin.c b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> index a93923fb8721..db74c3395ef8 100644
> --- a/drivers/gpu/drm/xe/display/xe_fb_pin.c
> +++ b/drivers/gpu/drm/xe/display/xe_fb_pin.c
> @@ -209,8 +209,7 @@ static int __xe_pin_fb_vma_ggtt(const struct intel_framebuffer *fb,
>  	} else if (view->type == I915_GTT_VIEW_NORMAL) {
>  		u32 x, size = bo->ttm.base.size;
>  
> -		ret = xe_ggtt_insert_special_node_locked(ggtt, &vma->node, size,
> -							 align, 0);
> +		ret = xe_ggtt_node_insert_locked(ggtt, &vma->node, size, align, 0);
>  		if (ret)
>  			goto out_unlock;
>  
> @@ -227,8 +226,7 @@ static int __xe_pin_fb_vma_ggtt(const struct intel_framebuffer *fb,
>  		/* display seems to use tiles instead of bytes here, so convert it back.. */
>  		u32 size = intel_rotation_info_size(rot_info) * XE_PAGE_SIZE;
>  
> -		ret = xe_ggtt_insert_special_node_locked(ggtt, &vma->node, size,
> -							 align, 0);
> +		ret = xe_ggtt_node_insert_locked(ggtt, &vma->node, size, align, 0);
>  		if (ret)
>  			goto out_unlock;
>  
> @@ -327,7 +325,7 @@ static void __xe_unpin_fb_vma(struct i915_vma *vma)
>  		xe_bo_unpin_map_no_vm(vma->dpt);
>  	else if (!drm_mm_node_allocated(&vma->bo->ggtt_node.base) ||
>  		 vma->bo->ggtt_node.base.start != vma->node.base.start)
> -		xe_ggtt_remove_node(ggtt, &vma->node, false);
> +		xe_ggtt_node_remove(ggtt, &vma->node, false);
>  
>  	ttm_bo_reserve(&vma->bo->ttm, false, false, NULL);
>  	ttm_bo_unpin(&vma->bo->ttm);
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.c b/drivers/gpu/drm/xe/xe_ggtt.c
> index ea55c7eabee4..3ab29588a068 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.c
> +++ b/drivers/gpu/drm/xe/xe_ggtt.c
> @@ -403,7 +403,7 @@ void xe_ggtt_deballoon(struct xe_ggtt *ggtt, struct xe_ggtt_node *node)
>  }
>  
>  /**
> - * xe_ggtt_insert_special_node_locked - Locked version to insert a &xe_ggtt_node into the GGTT
> + * xe_ggtt_node_insert_locked - Locked version to insert a &xe_ggtt_node into the GGTT
>   * @ggtt: the &xe_ggtt where node will be inserted
>   * @node: the &xe_ggtt_node to be inserted
>   * @size: size of the node
> @@ -414,15 +414,15 @@ void xe_ggtt_deballoon(struct xe_ggtt *ggtt, struct xe_ggtt_node *node)
>   *
>   * Return: 0 on success or a negative error code on failure.
>   */
> -int xe_ggtt_insert_special_node_locked(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> -				       u32 size, u32 align, u32 mm_flags)
> +int xe_ggtt_node_insert_locked(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> +			       u32 size, u32 align, u32 mm_flags)
>  {
>  	return drm_mm_insert_node_generic(&ggtt->mm, &node->base, size, align, 0,
>  					  mm_flags);
>  }
>  
>  /**
> - * xe_ggtt_insert_special_node - Insert a &xe_ggtt_node into the GGTT
> + * xe_ggtt_node_insert - Insert a &xe_ggtt_node into the GGTT
>   * @ggtt: the &xe_ggtt where node will be inserted
>   * @node: the &xe_ggtt_node to be inserted
>   * @size: size of the node
> @@ -430,19 +430,53 @@ int xe_ggtt_insert_special_node_locked(struct xe_ggtt *ggtt, struct xe_ggtt_node
>   *
>   * Return: 0 on success or a negative error code on failure.
>   */
> -int xe_ggtt_insert_special_node(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> -				u32 size, u32 align)
> +int xe_ggtt_node_insert(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> +			u32 size, u32 align)
>  {
>  	int ret;
>  
>  	mutex_lock(&ggtt->lock);
> -	ret = xe_ggtt_insert_special_node_locked(ggtt, node, size,
> -						 align, DRM_MM_INSERT_HIGH);
> +	ret = xe_ggtt_node_insert_locked(ggtt, node, size,
> +					 align, DRM_MM_INSERT_HIGH);
>  	mutex_unlock(&ggtt->lock);
>  
>  	return ret;
>  }
>  
> +/**
> + * xe_ggtt_node_remove - Remove a &xe_ggtt_node from the GGTT
> + * @ggtt: the &xe_ggtt where node will be removed
> + * @node: the &xe_ggtt_node to be removed
> + * @invalidate: if node needs invalidation upon removal
> + */
> +void xe_ggtt_node_remove(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> +			 bool invalidate)
> +{
> +	struct xe_device *xe = tile_to_xe(ggtt->tile);
> +	bool bound;
> +	int idx;
> +
> +	bound = drm_dev_enter(&xe->drm, &idx);
> +	if (bound)
> +		xe_pm_runtime_get_noresume(xe);
> +
> +	mutex_lock(&ggtt->lock);
> +	if (bound)
> +		xe_ggtt_clear(ggtt, node->base.start, node->base.size);
> +	drm_mm_remove_node(&node->base);
> +	node->base.size = 0;
> +	mutex_unlock(&ggtt->lock);
> +
> +	if (!bound)
> +		return;
> +
> +	if (invalidate)
> +		xe_ggtt_invalidate(ggtt);
> +
> +	xe_pm_runtime_put(xe);
> +	drm_dev_exit(idx);
> +}
> +
>  /**
>   * xe_ggtt_map_bo - Map the BO into GGTT
>   * @ggtt: the &xe_ggtt where node will be mapped
> @@ -522,40 +556,6 @@ int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>  	return __xe_ggtt_insert_bo_at(ggtt, bo, 0, U64_MAX);
>  }
>  
> -/**
> - * xe_ggtt_remove_node - Remove a &xe_ggtt_node from the GGTT
> - * @ggtt: the &xe_ggtt where node will be removed
> - * @node: the &xe_ggtt_node to be removed
> - * @invalidate: if node needs invalidation upon removal
> - */
> -void xe_ggtt_remove_node(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> -			 bool invalidate)
> -{
> -	struct xe_device *xe = tile_to_xe(ggtt->tile);
> -	bool bound;
> -	int idx;
> -
> -	bound = drm_dev_enter(&xe->drm, &idx);
> -	if (bound)
> -		xe_pm_runtime_get_noresume(xe);
> -
> -	mutex_lock(&ggtt->lock);
> -	if (bound)
> -		xe_ggtt_clear(ggtt, node->base.start, node->base.size);
> -	drm_mm_remove_node(&node->base);
> -	node->base.size = 0;
> -	mutex_unlock(&ggtt->lock);
> -
> -	if (!bound)
> -		return;
> -
> -	if (invalidate)
> -		xe_ggtt_invalidate(ggtt);
> -
> -	xe_pm_runtime_put(xe);
> -	drm_dev_exit(idx);
> -}
> -
>  /**
>   * xe_ggtt_remove_bo - Remove a BO from the GGTT
>   * @ggtt: the &xe_ggtt where node will be removed
> @@ -569,7 +569,7 @@ void xe_ggtt_remove_bo(struct xe_ggtt *ggtt, struct xe_bo *bo)
>  	/* This BO is not currently in the GGTT */
>  	xe_tile_assert(ggtt->tile, bo->ggtt_node.base.size == bo->size);
>  
> -	xe_ggtt_remove_node(ggtt, &bo->ggtt_node,
> +	xe_ggtt_node_remove(ggtt, &bo->ggtt_node,
>  			    bo->flags & XE_BO_FLAG_GGTT_INVALIDATE);
>  }
>  
> diff --git a/drivers/gpu/drm/xe/xe_ggtt.h b/drivers/gpu/drm/xe/xe_ggtt.h
> index 30a521f7b075..5147930357de 100644
> --- a/drivers/gpu/drm/xe/xe_ggtt.h
> +++ b/drivers/gpu/drm/xe/xe_ggtt.h
> @@ -16,12 +16,12 @@ int xe_ggtt_init(struct xe_ggtt *ggtt);
>  int xe_ggtt_balloon(struct xe_ggtt *ggtt, u64 start, u64 size, struct xe_ggtt_node *node);
>  void xe_ggtt_deballoon(struct xe_ggtt *ggtt, struct xe_ggtt_node *node);
>  
> -int xe_ggtt_insert_special_node(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> -				u32 size, u32 align);
> -int xe_ggtt_insert_special_node_locked(struct xe_ggtt *ggtt,
> -				       struct xe_ggtt_node *node,
> -				       u32 size, u32 align, u32 mm_flags);
> -void xe_ggtt_remove_node(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> +int xe_ggtt_node_insert(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
> +			u32 size, u32 align);
> +int xe_ggtt_node_insert_locked(struct xe_ggtt *ggtt,
> +			       struct xe_ggtt_node *node,
> +			       u32 size, u32 align, u32 mm_flags);
> +void xe_ggtt_node_remove(struct xe_ggtt *ggtt, struct xe_ggtt_node *node,
>  			 bool invalidate);
>  void xe_ggtt_map_bo(struct xe_ggtt *ggtt, struct xe_bo *bo);
>  int xe_ggtt_insert_bo(struct xe_ggtt *ggtt, struct xe_bo *bo);
> diff --git a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> index 3600468da013..dfa71bcd0f56 100644
> --- a/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> +++ b/drivers/gpu/drm/xe/xe_gt_sriov_pf_config.c
> @@ -382,7 +382,7 @@ static void pf_release_ggtt(struct xe_tile *tile, struct xe_ggtt_node *node)
>  		 * is redundant, as PTE will be implicitly re-assigned to PF by
>  		 * the xe_ggtt_clear() called by below xe_ggtt_remove_node().
>  		 */
> -		xe_ggtt_remove_node(ggtt, node, false);
> +		xe_ggtt_node_remove(ggtt, node, false);
>  	}
>  }
>  
> @@ -418,7 +418,7 @@ static int pf_provision_vf_ggtt(struct xe_gt *gt, unsigned int vfid, u64 size)
>  	if (!size)
>  		return 0;
>  
> -	err = xe_ggtt_insert_special_node(ggtt, node, size, alignment);
> +	err = xe_ggtt_node_insert(ggtt, node, size, alignment);
>  	if (unlikely(err))
>  		return err;
>  
> -- 
> 2.45.2
>