[PATCH v4 16/30] drm/xe: Use ordered WQ for TLB invalidation fences

[Zeng, Oak]

> -----Original Message-----
> From: Intel-xe <intel-xe-bounces at lists.freedesktop.org> On Behalf Of Matthew
> Brost
> Sent: Friday, March 8, 2024 12:08 AM
> To: intel-xe at lists.freedesktop.org
> Cc: Brost, Matthew <matthew.brost at intel.com>
> Subject: [PATCH v4 16/30] drm/xe: Use ordered WQ for TLB invalidation fences
> 
> TLB invalidation fences need to be ordered within an exec queue and if
> an unordered WQ is used TLB invalidation fences could be reordered. Use
> an ordered WQ to fix this.


The code change below makes sense to me. But I think we should improve the description. I think we need to explain what need to be in order and why. As I understand it, we need things to be in below order (just an example):
1)Gpu page table update (such as invalidate certain page table)
2)Invalidate tlb entries for above page table entries
3)Notify/call back to user fence to let user know above vm_unbind is complete.

Obviously 1) and 2) can't be re-ordered, because 2) should be triggered by 1). But 2) and 3) can be reordered if we use unordered WQ - why we want to use ordered wq here.

Simply saying TLB invalidation fences need to be ordered is too simple, at least it is hard for me to understand. Better to improve it.

Thanks,
Oak

> 
> Signed-off-by: Matthew Brost <matthew.brost at intel.com>
> ---
>  drivers/gpu/drm/xe/xe_pt.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/xe/xe_pt.c b/drivers/gpu/drm/xe/xe_pt.c
> index 110d6917089b..a878e2217c7f 100644
> --- a/drivers/gpu/drm/xe/xe_pt.c
> +++ b/drivers/gpu/drm/xe/xe_pt.c
> @@ -1107,7 +1107,7 @@ static void invalidation_fence_cb(struct dma_fence
> *fence,
> 
>  	trace_xe_gt_tlb_invalidation_fence_cb(&ifence->base);
>  	if (!ifence->fence->error) {
> -		queue_work(system_wq, &ifence->work);
> +		queue_work(ifence->gt->ordered_wq, &ifence->work);
>  	} else {
>  		ifence->base.base.error = ifence->fence->error;
>  		dma_fence_signal(&ifence->base.base);
> --
> 2.34.1