Re: [PATCH] drm/amdgpu: Fix pipelining jobs with timeline syncobj dependencies

[Christian König]

On 17.09.25 11:59, David Rosca wrote:
> drm_syncobj_find_fence returns fence chain for timeline syncobjs.
> Scheduler expects normal fences as job dependencies to be able to
> determine whether the fences come from the same entity or sched
> and skip waiting on them.
> With fence chain as job dependency, the fence will always be
> waited on forcing CPU round-trip before starting the job.
> 
> Signed-off-by: David Rosca <david.ro...@amd.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c 
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 2e93d570153c..779c11227a53 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -29,6 +29,7 @@
>  #include <linux/pagemap.h>
>  #include <linux/sync_file.h>
>  #include <linux/dma-buf.h>
> +#include <linux/dma-fence-unwrap.h>
>  
>  #include <drm/amdgpu_drm.h>
>  #include <drm/drm_syncobj.h>
> @@ -450,7 +451,8 @@ static int amdgpu_syncobj_lookup_and_add(struct 
> amdgpu_cs_parser *p,
>                                        uint32_t handle, u64 point,
>                                        u64 flags)
>  {
> -     struct dma_fence *fence;
> +     struct dma_fence *fence, *f;
> +     struct dma_fence_unwrap iter;
>       int r;
>  
>       r = drm_syncobj_find_fence(p->filp, handle, point, flags, &fence);
> @@ -460,7 +462,11 @@ static int amdgpu_syncobj_lookup_and_add(struct 
> amdgpu_cs_parser *p,
>               return r;
>       }
>  
> -     r = amdgpu_sync_fence(&p->sync, fence, GFP_KERNEL);
> +     dma_fence_unwrap_for_each(f, &iter, fence) {
> +             if (!r)
> +                     r = amdgpu_sync_fence(&p->sync, f, GFP_KERNEL);

You should probably add an dma_fence_unwrap_cleanup() function which drops 
iter->chain and sets it to NULL and then code this like:

r = ...
if (r) {
        dma_fence_unwrap_cleanup(..);
        return r;
}

> +     }
> +
>       dma_fence_put(fence);

That is superfluous now, fence was consumed by dma_fence_unwrap_for_each().

Regards,
Christian.

>       return r;
>  }