Re: [PATCH 07/23] drm/vmwgfx: stop using dma_resv_excl_fence

[Zack Rusin <zackr@vmware.com>]

On Tue, 2022-03-22 at 08:13 +0100, Christian König wrote:
> Am 21.03.22 um 16:11 schrieb Zack Rusin:
> > On Mon, 2022-03-21 at 15:12 +0100, Christian König wrote:
> > > Am 21.03.22 um 15:02 schrieb Zack Rusin:
> > > > On Mon, 2022-03-21 at 14:58 +0100, Christian König wrote:
> > > > > ⚠ External Email: This email originated from outside of the
> > > > > organization. Do not click links or open attachments unless
> > > > > you
> > > > > recognize the sender.
> > > > > 
> > > > > Instead use the new dma_resv_get_singleton function.
> > > > > 
> > > > > Signed-off-by: Christian König <christian.koenig@amd.com>
> > > > > Reviewed-by: Daniel Vetter <daniel.vetter@ffwll.ch>
> > > > > Cc: VMware Graphics <linux-graphics-maintainer@vmware.com>
> > > > > Cc: Zack Rusin <zackr@vmware.com>
> > > > > ---
> > > > >    drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 6 ++++--
> > > > >    1 file changed, 4 insertions(+), 2 deletions(-)
> > > > > 
> > > > > diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> > > > > b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> > > > > index 708899ba2102..36c3b5db7e69 100644
> > > > > --- a/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> > > > > +++ b/drivers/gpu/drm/vmwgfx/vmwgfx_resource.c
> > > > > @@ -1165,8 +1165,10 @@ int vmw_resources_clean(struct
> > > > > vmw_buffer_object *vbo, pgoff_t start,
> > > > >                   vmw_bo_fence_single(bo, NULL);
> > > > >                   if (bo->moving)
> > > > >                           dma_fence_put(bo->moving);
> > > > > -               bo->moving = dma_fence_get
> > > > > -                       (dma_resv_excl_fence(bo->base.resv));
> > > > > +
> > > > > +               /* TODO: This is actually a memory management
> > > > > dependency */
> > > > > +               return dma_resv_get_singleton(bo->base.resv,
> > > > > false,
> > > > > +                                             &bo->moving);
> > > > >           }
> > > > > 
> > > > >           return 0;
> > > > > --
> > > > > 2.25.1
> > > > > 
> > > > Sorry, I haven't had the time to go over the entire series, the
> > > > patch
> > > > looks good, but what's the memory management dependency the
> > > > todo
> > > > mentions?
> > > Previously the function installed only the exclusive fence as
> > > moving
> > > fence into the BO.
> > > Now it grabs all fences and installs them as moving fence into
> > > the
> > > BO.
> > > 
> > > But what we really need is tracking if a fence in the reservation
> > > object
> > > is a kernel memory management dependency or not.
> > > 
> > > Patch #19 adds that and patch #23 then finally gets rid of the
> > > whole
> > > bo->moving handling here because it becomes completely
> > > unnecessary.
> > > 
> > > I can drop the comment if you want or just note that it is only
> > > temporary until the follow up patches are merged.
> > Ah, yes, if you could remove it that'd be great. The patch will
> > never
> > be backported anywhere without the rest of the series, so it
> > shouldn't
> > be a problem.
> 
> Sure, can I then have your rb for this one?

Yes, of course. Thanks!
Reviewed-by: Zack Rusin <zackr@vmware.com>