[PATCH v2 0/2] drm/xe: flush engine buffers before signalling user fence on all engines

[Thomas Hellström]

On Tue, 2024-06-04 at 16:58 +0200, Andrzej Hajda wrote:
> 
> 
> On 04.06.2024 13:07, Thomas Hellström wrote:
> > On Tue, 2024-06-04 at 11:38 +0200, Andrzej Hajda wrote:
> > > According to the discussion result on my previous patch I have
> > > prepared
> > > new patchset, which reverts previous patch and adds barrier
> > > before
> > > user fence signalling.
> > > Remarks:
> > > - I was not able to test it yet, hopefully CI will do it and me
> > > also
> > > after fixing LNL issue,
> 
> Finally I can confirm that on LNL this patchset the barrier is
> enough, 
> at least on the igt tests I was fixing.
> 
> > > - I am not sure about MI_FLUSH_DW flags, bspec says:
> > >    "After this command is completed with a Store DWord enabled,
> > > CPU
> > > access to graphics memory will be coherent"
> > >    Shouldn't we use "Store DWord" then?
> > It's not impossible that "store dword" needs to be enabled to act
> > as a
> > write barrier. but it also says "The parser pauses on an internal
> > flush
> > until all drawing engines have completed any pending operations."
> > 
> > If it turns out a store dword is indeed needed as a post sync
> > operation, we could perhaps use the "store dword" functionality of
> > this
> > command to store the user-fence value and replace the posted write.
> 
> Yep this seems even more elegant solution for me, but as I stated
> above 
> the current solution works well.

Great.

Perhaps we should add a Fixes: tag to patch 1/2 in case
38007fa96419a9db9719f170b9e8a7877821cdd1 gets backported to stable due
to its Fixes: tag when drm-xe-next gets merged...

Then for the series,
Reviewed-by: Thomas Hellström <thomas.hellstrom at linux.intel.com>

Thanks!
Thomas


> 
> Regards
> Andrzej
> 
> > 
> > /Thomas
> > 
> > > [1]:
> > > https://lore.kernel.org/intel-xe/Zl5DcuZeZiFgxVdJ@DUT025-TGLU.fm.intel.com/T/#m0b4420045908bac70426728d460108c0b2b65dca
> > > 
> > > Signed-off-by: Andrzej Hajda <andrzej.hajda at intel.com>
> > > ---
> > > - Link to v1:
> > > https://lore.kernel.org/r/20240603-fix_user_fence_posted-v1-1-61c76ef69cea@intel.com
> > > 
> > > ---
> > > Andrzej Hajda (2):
> > >        Revert "drm/xe: flush gtt before signalling user fence on
> > > all
> > > engines"
> > >        drm/xe: flush engine buffers before signalling user fence
> > > on
> > > all engines
> > > 
> > >   drivers/gpu/drm/xe/xe_ring_ops.c | 26 ++++++++++++++++++++-----
> > > -
> > >   1 file changed, 20 insertions(+), 6 deletions(-)
> > > ---
> > > base-commit: fe3d637a9c72b22297da0c731fa5e217bd182d2d
> > > change-id: 20240603-fix_user_fence_posted-ca56c79c0662
> > > 
> > > Best regards,
>