[PATCH] drm/amdgpu: unpack dma_fence_chain containers during sync

Pierre-Loup A. Griffais pgriffais at valvesoftware.com
Tue Nov 24 21:58:00 UTC 2020


I can test some more tonight. I'll also try to prepare a standalone 
trace so you can observe the exact pattern being used on your end. 
Vulkan traces tend to be GPU and driver-specific. We'll use Mesa as the 
driver, but what GPU would be most convenient on your side for 
replaying? On our end I would guess Navi10 would be most practical.

On 11/23/20 11:56 PM, Christian König wrote:
> Mhm, then I don't know what's going wrong here.
>
> Could be that the fence somehow ends up in a BO dependency.
>
> Pierre do you have some time for testing today? Or could you provide 
> me some way to test this?
>
> Christian.
>
> Am 24.11.20 um 03:48 schrieb Pierre-Loup A. Griffais:
>>
>> I just built my kernel with it and tested Horizon Zero Dawn on stock 
>> Proton 5.13, and it doesn't seem to change things there.
>>
>> This pattern looks identical as with before the kernel patch, as far 
>> as I can tell:
>>
>> https://imgur.com/a/1fZWgNG
>>
>> The last purple block is a piece of GPU work on the gfx ring. It's 
>> been queued by software 0.7ms ago, but doesn't get put on the HW ring 
>> until right after the previous piece of GPU work completes. The 
>> orange chunk below is the 'gfx' kernel task executing, to queue it.
>>
>> Thanks,
>>
>>  - Pierre-Loup
>>
>> On 2020-11-23 18:09, Marek Olšák wrote:
>>> Pierre-Loup, does this do what you requested?
>>>
>>> Thanks,
>>> Marek
>>>
>>> On Mon, Nov 23, 2020 at 3:17 PM Christian König 
>>> <ckoenig.leichtzumerken at gmail.com 
>>> <mailto:ckoenig.leichtzumerken at gmail.com>> wrote:
>>>
>>>     That the CPU round trip is gone now.
>>>
>>>     Christian.
>>>
>>>     Am 23.11.20 um 20:49 schrieb Marek Olšák:
>>>>     What is the behavior we should expect?
>>>>
>>>>     Marek
>>>>
>>>>     On Mon, Nov 23, 2020 at 7:31 AM Christian König
>>>>     <ckoenig.leichtzumerken at gmail.com
>>>>     <mailto:ckoenig.leichtzumerken at gmail.com>> wrote:
>>>>
>>>>         Ping, Pierre/Marek does this change works as expected?
>>>>
>>>>         Regards,
>>>>         Christian.
>>>>
>>>>         Am 18.11.20 um 14:20 schrieb Christian König:
>>>>         > This allows for optimizing the CPU round trip away.
>>>>         >
>>>>         > Signed-off-by: Christian König <christian.koenig at amd.com
>>>>         <mailto:christian.koenig at amd.com>>
>>>>         > ---
>>>>         >   drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c   | 2 +-
>>>>         >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c | 27
>>>>         ++++++++++++++++++++++++
>>>>         >   drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h | 1 +
>>>>         >   3 files changed, 29 insertions(+), 1 deletion(-)
>>>>         >
>>>>         > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>         > index 79342976fa76..68f9a4adf5d2 100644
>>>>         > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>         > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
>>>>         > @@ -1014,7 +1014,7 @@ static int
>>>>         amdgpu_syncobj_lookup_and_add_to_sync(struct
>>>>         amdgpu_cs_parser *p,
>>>>         >               return r;
>>>>         >       }
>>>>         >
>>>>         > -     r = amdgpu_sync_fence(&p->job->sync, fence);
>>>>         > +     r = amdgpu_sync_fence_chain(&p->job->sync, fence);
>>>>         >       dma_fence_put(fence);
>>>>         >
>>>>         >       return r;
>>>>         > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>>>>         > index 8ea6c49529e7..d0d64af06f54 100644
>>>>         > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>>>>         > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.c
>>>>         > @@ -28,6 +28,8 @@
>>>>         >    *    Christian König <christian.koenig at amd.com
>>>>         <mailto:christian.koenig at amd.com>>
>>>>         >    */
>>>>         >
>>>>         > +#include <linux/dma-fence-chain.h>
>>>>         > +
>>>>         >   #include "amdgpu.h"
>>>>         >   #include "amdgpu_trace.h"
>>>>         >   #include "amdgpu_amdkfd.h"
>>>>         > @@ -169,6 +171,31 @@ int amdgpu_sync_fence(struct
>>>>         amdgpu_sync *sync, struct dma_fence *f)
>>>>         >       return 0;
>>>>         >   }
>>>>         >
>>>>         > +/**
>>>>         > + * amdgpu_sync_fence_chain - unpack dma_fence_chain and sync
>>>>         > + *
>>>>         > + * @sync: sync object to add fence to
>>>>         > + * @f: potential dma_fence_chain to sync to.
>>>>         > + *
>>>>         > + * Add the fences inside the chain to the sync object.
>>>>         > + */
>>>>         > +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync,
>>>>         struct dma_fence *f)
>>>>         > +{
>>>>         > +     int r;
>>>>         > +
>>>>         > +     dma_fence_chain_for_each(f, f) {
>>>>         > +             if (dma_fence_is_signaled(f))
>>>>         > +                     continue;
>>>>         > +
>>>>         > +             r = amdgpu_sync_fence(sync, f);
>>>>         > +             if (r) {
>>>>         > +                     dma_fence_put(f);
>>>>         > +                     return r;
>>>>         > +             }
>>>>         > +     }
>>>>         > +     return 0;
>>>>         > +}
>>>>         > +
>>>>         >   /**
>>>>         >    * amdgpu_sync_vm_fence - remember to sync to this VM fence
>>>>         >    *
>>>>         > diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>>>>         b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>>>>         > index 7c0fe20c470d..b142175b65b6 100644
>>>>         > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>>>>         > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_sync.h
>>>>         > @@ -48,6 +48,7 @@ struct amdgpu_sync {
>>>>         >
>>>>         >   void amdgpu_sync_create(struct amdgpu_sync *sync);
>>>>         >   int amdgpu_sync_fence(struct amdgpu_sync *sync, struct
>>>>         dma_fence *f);
>>>>         > +int amdgpu_sync_fence_chain(struct amdgpu_sync *sync,
>>>>         struct dma_fence *f);
>>>>         >   int amdgpu_sync_vm_fence(struct amdgpu_sync *sync,
>>>>         struct dma_fence *fence);
>>>>         >   int amdgpu_sync_resv(struct amdgpu_device *adev, struct
>>>>         amdgpu_sync *sync,
>>>>         >                    struct dma_resv *resv, enum
>>>>         amdgpu_sync_mode mode,
>>>>
>>>>         _______________________________________________
>>>>         amd-gfx mailing list
>>>>         amd-gfx at lists.freedesktop.org
>>>>         <mailto:amd-gfx at lists.freedesktop.org>
>>>>         https://lists.freedesktop.org/mailman/listinfo/amd-gfx
>>>>         <https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Flists.freedesktop.org%2Fmailman%2Flistinfo%2Famd-gfx&data=04%7C01%7Cchristian.koenig%40amd.com%7C4b90eb41edd04592bd4f08d89023653b%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637417828990103282%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&sdata=wHx40e8opSOOIndSVMBaPuMarMpA%2FnDRxl%2BI5BV210s%3D&reserved=0>
>>>>
>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/amd-gfx/attachments/20201124/d1a2db9d/attachment-0001.htm>


More information about the amd-gfx mailing list