[PATCH v4 14/18] drm/msm: Don't break exclusive fence ordering

[Rob Clark]

On Tue, Jul 13, 2021 at 9:58 AM Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
>
> On Tue, Jul 13, 2021 at 6:51 PM Rob Clark <robdclark at gmail.com> wrote:
> >
> > On Mon, Jul 12, 2021 at 1:02 PM Daniel Vetter <daniel.vetter at ffwll.ch> wrote:
> > >
> > > There's only one exclusive slot, and we must not break the ordering.
> > >
> > > Adding a new exclusive fence drops all previous fences from the
> > > dma_resv. To avoid violating the signalling order we err on the side of
> > > over-synchronizing by waiting for the existing fences, even if
> > > userspace asked us to ignore them.
> > >
> > > A better fix would be to us a dma_fence_chain or _array like e.g.
> > > amdgpu now uses, but
> > > - msm has a synchronous dma_fence_wait for anything from another
> > >   context, so doesn't seem to care much,
> > > - and it probably makes sense to lift this into dma-resv.c code as a
> > >   proper concept, so that drivers don't have to hack up their own
> > >   solution each on their own.
> > >
> > > v2: Improve commit message per Lucas' suggestion.
> > >
> > > Cc: Lucas Stach <l.stach at pengutronix.de>
> > > Signed-off-by: Daniel Vetter <daniel.vetter at intel.com>
> > > Cc: Rob Clark <robdclark at gmail.com>
> > > Cc: Sean Paul <sean at poorly.run>
> > > Cc: linux-arm-msm at vger.kernel.org
> > > Cc: freedreno at lists.freedesktop.org
> > > ---
> > >  drivers/gpu/drm/msm/msm_gem_submit.c | 3 ++-
> > >  1 file changed, 2 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > index b71da71a3dd8..edd0051d849f 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > @@ -306,7 +306,8 @@ static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
> > >                                 return ret;
> > >                 }
> > >
> > > -               if (no_implicit)
> > > +               /* exclusive fences must be ordered */
> > > +               if (no_implicit && !write)
> > >                         continue;
> >
> > In practice, modern userspace (the kind that is more likely to set the
> > no-implicit flag on every submit) also sets MSM_SUBMIT_BO_WRITE on
> > every bo, to shave some cpu overhead so I suppose this would not
> > really hurt anything
> >
> > Do you know if this is covered in any piglit/etc test?
>
> You need some command submission, plus buffer sharing with vgem
> setting it's own exclusive fences, plus checking with dma_buf poll()
> whether it signals all in the right order. That's pretty low-level, so
> maybe something in igt, but I haven't typed that. Maybe I need to do
> that for i915 at least.

ok, you lost me at vgem ;-)

(the vgem vs cache situation on arm is kinda hopeless)

BR,
-R

> -Daniel
>
> > BR,
> > -R
> >
> > >
> > >                 ret = msm_gem_sync_object(&msm_obj->base, submit->ring->fctx,
> > > --
> > > 2.32.0
> > >
>
>
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch