Re: [PATCH 2/4] drm/xe: add bind time pat index to xe_bo structure

From: "Ghimiray, Himal Prasad" <himal.prasad.ghimiray@intel.com>
To: <intel-xe@lists.freedesktop.org>
Subject: Re: [PATCH 2/4] drm/xe: add bind time pat index to xe_bo structure
Date: Wed, 31 Jan 2024 11:32:17 +0530	[thread overview]
Message-ID: <2344f0b1-4ec2-49c7-b480-121e0b56e34d@intel.com> (raw)
In-Reply-To: <20240130193652.374270-3-juhapekka.heikkila@gmail.com>

[-- Attachment #1: Type: text/plain, Size: 3180 bytes --]


On 31-01-2024 01:06, Juha-Pekka Heikkila wrote:
> Add BO bind time pat index member to xe_bo structure and store
> pat index from xe_vma to xe_bo.
>
> Signed-off-by: Juha-Pekka Heikkila<juhapekka.heikkila@gmail.com>
> ---
>   drivers/gpu/drm/xe/xe_bo_types.h | 12 ++++++++++++
>   drivers/gpu/drm/xe/xe_pt.c       | 22 ++++++++++++++++++----
>   2 files changed, 30 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/xe/xe_bo_types.h b/drivers/gpu/drm/xe/xe_bo_types.h
> index 14ef13b7b421..6d599f1e846b 100644
> --- a/drivers/gpu/drm/xe/xe_bo_types.h
> +++ b/drivers/gpu/drm/xe/xe_bo_types.h
> @@ -91,6 +91,18 @@ struct xe_bo {
>   
>   	/** @vram_userfault_link: Link into @mem_access.vram_userfault.list */
>   		struct list_head vram_userfault_link;
> +
> +	/**
> +	 * @pat_index: The pat index requested when bind this BO
> +	 */
> +	u16 pat_index;
> +
> +	/**
> +	 * @has_sealed_pat_index: The pat index is sealed because this BO is
> +	 * pinned as framebuffer. This is to prevent flipping compression
> +	 * on/off from framebuffers while in use.
> +	 */
> +	bool has_sealed_pat_index;
>   };
>   
>   #define intel_bo_to_drm_bo(bo) (&(bo)->ttm.base)
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index de1030a47588..c72cb75d993c 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1208,10 +1208,11 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
>   	struct dma_fence *fence;
>   	struct invalidation_fence *ifence = NULL;
>   	struct xe_range_fence *rfence;
> +	struct xe_bo *bo = xe_vma_bo(vma);
>   	int err;
>   
>   	bind_pt_update.locked = false;
> -	xe_bo_assert_held(xe_vma_bo(vma));
> +	xe_bo_assert_held(bo);
>   	xe_vm_assert_held(vm);
>   
>   	vm_dbg(&xe_vma_vm(vma)->xe->drm,
> @@ -1252,8 +1253,21 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
>   		return ERR_PTR(-ENOMEM);
>   	}
>   
> +	/*
> +	 * On Xe2 BO which was pinned as framebuffer before with different
> +	 * PAT index cannot be bound with different PAT index. This is
> +	 * to prevent switching CCS on/off from framebuffers on the fly
> +	 * with Xe2.
> +	 */
> +	if (bo) {
> +		if (bo->has_sealed_pat_index && bo->pat_index != vma->pat_index)
> +			return ERR_PTR(-EINVAL);
> +
> +		bo->pat_index = vma->pat_index;
> +	}
will __xe_pin_fb_vma be always followed after first pt_bind of vma ?
> +
>   	fence = xe_migrate_update_pgtables(tile->migrate,
> -					   vm, xe_vma_bo(vma), q,
> +					   vm, bo, q,
>   					   entries, num_entries,
>   					   syncs, num_syncs,
>   					   &bind_pt_update.base);
> @@ -1287,8 +1301,8 @@ __xe_pt_bind_vma(struct xe_tile *tile, struct xe_vma *vma, struct xe_exec_queue
>   				   DMA_RESV_USAGE_KERNEL :
>   				   DMA_RESV_USAGE_BOOKKEEP);
>   
> -		if (!xe_vma_has_no_bo(vma) && !xe_vma_bo(vma)->vm)
> -			dma_resv_add_fence(xe_vma_bo(vma)->ttm.base.resv, fence,
> +		if (!xe_vma_has_no_bo(vma) && !bo->vm)
> +			dma_resv_add_fence(bo->ttm.base.resv, fence,
>   					   DMA_RESV_USAGE_BOOKKEEP);
>   		xe_pt_commit_bind(vma, entries, num_entries, rebind,
>   				  bind_pt_update.locked ? &deferred : NULL);

[-- Attachment #2: Type: text/html, Size: 3796 bytes --]