Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Thu, Oct 29, 2020 at 09:59:09AM -0700, Rob Clark wrote:
> On Thu, Oct 29, 2020 at 9:14 AM Daniel Vetter wrote:
> >
> > On Mon, Oct 26, 2020 at 10:34 AM Daniel Vetter wrote:
> > >
> > > On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote:
> > > > On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach
> > > > wrote:
> > > > >
> > > > > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote:
> > > > > > From: Rob Clark
> > > > > >
> > > > > > If there is only a single ring (no-preemption), everything is FIFO
> > > > > > order
> > > > > > and there is no need to implicit-sync.
> > > > > >
> > > > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as
> > > > > > behavior
> > > > > > is undefined when fences are not used to synchronize buffer usage
> > > > > > across
> > > > > > contexts (which is the only case where multiple different priority
> > > > > > rings
> > > > > > could come into play).
> > > > >
> > > > > Really, doesn't this break cross-device implicit sync? Okay, you may
> > > > > not have many peripherals that rely on implicit sync on devices where
> > > > > Adreno is usually found, but it seems rather heavy-handed.
> > > > >
> > > > > Wouldn't it be better to only ignore fences from your own ring context
> > > > > in the implicit sync, like we do in the common DRM scheduler
> > > > > (drm_sched_dependency_optimized)?
> > > >
> > > > we already do this.. as was discussed on an earlier iteration of this
> > > > patchset
> > > >
> > > > But I'm not aware of any other non-gpu related implicit sync use-case
> > > > (even on imx devices where display is decoupled from gpu).. I'll
> > > > revert the patch if someone comes up with one, but otherwise lets let
> > > > the implicit sync baggage die
> > >
> > > The thing is, dma_resv won't die, even if implicit sync is dead. We're
> > > using internally for activity tracking and memory management. If you don't
> > > set these, then we can't share generic code with msm, and I think everyone
> > > inventing their own memory management is a bit a mistake.
> > >
> > > Now you only kill the implicit write sync stuff here, but I'm not sure
> > > that's worth much since you still install all the read fences for
> > > consistency. And if userspace doesn't want to be synced, they can set the
> > > flag and do this on their own: I think you should be able to achieve
> > > exactly the same thing in mesa.
> > >
> > > Aside: If you're worried about overhead, you can do O(1) submit if you
> > > manage your ppgtt like amdgpu does.
> >
> > So just remember a use-case which is maybe a bit yucky, but it is
> > actually possible to implement race-free. If you have implicit sync.
> >
> > There's screen-capture tool in mplayer and obs which capture your
> > compositor by running getfb2 in a loop. It works, and after some
> > initial screaming I realized it does actually work race-free. If you
> > have implicit sync.
> >
> > I really don't think you can sunset this, as much as you want to. And
> > sunsetting it inconsistently is probably the worst.
>
> For the case where you only have a single ring, as long as it is
> importing the fb in to egl to read it (which it would need to do to
> get a linear view), this would still all work
Hm right we still have the implicit sync of the ringbuffer. At least until
you add a submit scheduler to msm ...
> (but I may drop this patch because it is just a micro-optimization and
> seems to cause more confusion)
Yeah I'd say without numbers to justify it it feels a bit on thin ice :-)
-Daniel
>
> BR,
> -R
>
>
> > -Daniel
> >
> > > -Daniel
> > >
> > > >
> > > > BR,
> > > > -R
> > > >
> > > >
> > > >
> > > > >
> > > > > Regards,
> > > > > Lucas
> > > > >
> > > > > > Signed-off-by: Rob Clark
> > > > > > Reviewed-by: Kristian H. Kristensen
> > > > > > ---
> > > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
> > > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > index d04c349d8112..b6babc7f9bb8 100644
> > > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct
> > > > > > msm_gem_submit *submit)
> > > > > > return ret;
> > > > > > }
> > > > > >
> > > > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > > > > no_implicit)
> > > > > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > > > > implicit_sync)
> > > > > > {
> > > > > > int i, ret = 0;
> > > > > >
> > > > > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct
> > > > > > msm_gem_submit *submit, bool no_implicit)
> > > > > > return ret;
> > > > > > }
> > > > > >
> > > > > > - if (no_implicit)
> > > > > > + if (!implicit_sync)
> > > > > >
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Thu, Oct 29, 2020 at 9:14 AM Daniel Vetter wrote:
>
> On Mon, Oct 26, 2020 at 10:34 AM Daniel Vetter wrote:
> >
> > On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote:
> > > On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach
> > > wrote:
> > > >
> > > > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote:
> > > > > From: Rob Clark
> > > > >
> > > > > If there is only a single ring (no-preemption), everything is FIFO
> > > > > order
> > > > > and there is no need to implicit-sync.
> > > > >
> > > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as
> > > > > behavior
> > > > > is undefined when fences are not used to synchronize buffer usage
> > > > > across
> > > > > contexts (which is the only case where multiple different priority
> > > > > rings
> > > > > could come into play).
> > > >
> > > > Really, doesn't this break cross-device implicit sync? Okay, you may
> > > > not have many peripherals that rely on implicit sync on devices where
> > > > Adreno is usually found, but it seems rather heavy-handed.
> > > >
> > > > Wouldn't it be better to only ignore fences from your own ring context
> > > > in the implicit sync, like we do in the common DRM scheduler
> > > > (drm_sched_dependency_optimized)?
> > >
> > > we already do this.. as was discussed on an earlier iteration of this
> > > patchset
> > >
> > > But I'm not aware of any other non-gpu related implicit sync use-case
> > > (even on imx devices where display is decoupled from gpu).. I'll
> > > revert the patch if someone comes up with one, but otherwise lets let
> > > the implicit sync baggage die
> >
> > The thing is, dma_resv won't die, even if implicit sync is dead. We're
> > using internally for activity tracking and memory management. If you don't
> > set these, then we can't share generic code with msm, and I think everyone
> > inventing their own memory management is a bit a mistake.
> >
> > Now you only kill the implicit write sync stuff here, but I'm not sure
> > that's worth much since you still install all the read fences for
> > consistency. And if userspace doesn't want to be synced, they can set the
> > flag and do this on their own: I think you should be able to achieve
> > exactly the same thing in mesa.
> >
> > Aside: If you're worried about overhead, you can do O(1) submit if you
> > manage your ppgtt like amdgpu does.
>
> So just remember a use-case which is maybe a bit yucky, but it is
> actually possible to implement race-free. If you have implicit sync.
>
> There's screen-capture tool in mplayer and obs which capture your
> compositor by running getfb2 in a loop. It works, and after some
> initial screaming I realized it does actually work race-free. If you
> have implicit sync.
>
> I really don't think you can sunset this, as much as you want to. And
> sunsetting it inconsistently is probably the worst.
For the case where you only have a single ring, as long as it is
importing the fb in to egl to read it (which it would need to do to
get a linear view), this would still all work
(but I may drop this patch because it is just a micro-optimization and
seems to cause more confusion)
BR,
-R
> -Daniel
>
> > -Daniel
> >
> > >
> > > BR,
> > > -R
> > >
> > >
> > >
> > > >
> > > > Regards,
> > > > Lucas
> > > >
> > > > > Signed-off-by: Rob Clark
> > > > > Reviewed-by: Kristian H. Kristensen
> > > > > ---
> > > > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
> > > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > > >
> > > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > index d04c349d8112..b6babc7f9bb8 100644
> > > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct
> > > > > msm_gem_submit *submit)
> > > > > return ret;
> > > > > }
> > > > >
> > > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > > > no_implicit)
> > > > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > > > implicit_sync)
> > > > > {
> > > > > int i, ret = 0;
> > > > >
> > > > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct
> > > > > msm_gem_submit *submit, bool no_implicit)
> > > > > return ret;
> > > > > }
> > > > >
> > > > > - if (no_implicit)
> > > > > + if (!implicit_sync)
> > > > > continue;
> > > > >
> > > > > ret = msm_gem_sync_object(_obj->base,
> > > > > submit->ring->fctx,
> > > > > @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> > > > > void *data,
> > > > > if (ret)
> > > > > goto out;
> > > > >
> > > > > - ret = submit_fence_sync(submit, !!(args->flags &
> > > > > MSM_SUBMIT_NO_IMPLICIT));
> > > > > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) &&
> > > > > +
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Mon, Oct 26, 2020 at 10:34 AM Daniel Vetter wrote:
>
> On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote:
> > On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach wrote:
> > >
> > > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote:
> > > > From: Rob Clark
> > > >
> > > > If there is only a single ring (no-preemption), everything is FIFO order
> > > > and there is no need to implicit-sync.
> > > >
> > > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior
> > > > is undefined when fences are not used to synchronize buffer usage across
> > > > contexts (which is the only case where multiple different priority rings
> > > > could come into play).
> > >
> > > Really, doesn't this break cross-device implicit sync? Okay, you may
> > > not have many peripherals that rely on implicit sync on devices where
> > > Adreno is usually found, but it seems rather heavy-handed.
> > >
> > > Wouldn't it be better to only ignore fences from your own ring context
> > > in the implicit sync, like we do in the common DRM scheduler
> > > (drm_sched_dependency_optimized)?
> >
> > we already do this.. as was discussed on an earlier iteration of this
> > patchset
> >
> > But I'm not aware of any other non-gpu related implicit sync use-case
> > (even on imx devices where display is decoupled from gpu).. I'll
> > revert the patch if someone comes up with one, but otherwise lets let
> > the implicit sync baggage die
>
> The thing is, dma_resv won't die, even if implicit sync is dead. We're
> using internally for activity tracking and memory management. If you don't
> set these, then we can't share generic code with msm, and I think everyone
> inventing their own memory management is a bit a mistake.
>
> Now you only kill the implicit write sync stuff here, but I'm not sure
> that's worth much since you still install all the read fences for
> consistency. And if userspace doesn't want to be synced, they can set the
> flag and do this on their own: I think you should be able to achieve
> exactly the same thing in mesa.
>
> Aside: If you're worried about overhead, you can do O(1) submit if you
> manage your ppgtt like amdgpu does.
So just remember a use-case which is maybe a bit yucky, but it is
actually possible to implement race-free. If you have implicit sync.
There's screen-capture tool in mplayer and obs which capture your
compositor by running getfb2 in a loop. It works, and after some
initial screaming I realized it does actually work race-free. If you
have implicit sync.
I really don't think you can sunset this, as much as you want to. And
sunsetting it inconsistently is probably the worst.
-Daniel
> -Daniel
>
> >
> > BR,
> > -R
> >
> >
> >
> > >
> > > Regards,
> > > Lucas
> > >
> > > > Signed-off-by: Rob Clark
> > > > Reviewed-by: Kristian H. Kristensen
> > > > ---
> > > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
> > > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > index d04c349d8112..b6babc7f9bb8 100644
> > > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct
> > > > msm_gem_submit *submit)
> > > > return ret;
> > > > }
> > > >
> > > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > > no_implicit)
> > > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > > implicit_sync)
> > > > {
> > > > int i, ret = 0;
> > > >
> > > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct msm_gem_submit
> > > > *submit, bool no_implicit)
> > > > return ret;
> > > > }
> > > >
> > > > - if (no_implicit)
> > > > + if (!implicit_sync)
> > > > continue;
> > > >
> > > > ret = msm_gem_sync_object(_obj->base,
> > > > submit->ring->fctx,
> > > > @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev,
> > > > void *data,
> > > > if (ret)
> > > > goto out;
> > > >
> > > > - ret = submit_fence_sync(submit, !!(args->flags &
> > > > MSM_SUBMIT_NO_IMPLICIT));
> > > > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) &&
> > > > + !(args->flags & MSM_SUBMIT_NO_IMPLICIT));
> > > > if (ret)
> > > > goto out;
> > > >
> > >
> > ___
> > dri-devel mailing list
> > dri-de...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
>
> --
> Daniel Vetter
> Software Engineer, Intel Corporation
> http://blog.ffwll.ch
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Fri, Oct 23, 2020 at 08:49:14PM -0700, Rob Clark wrote:
> On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach wrote:
> >
> > On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote:
> > > From: Rob Clark
> > >
> > > If there is only a single ring (no-preemption), everything is FIFO order
> > > and there is no need to implicit-sync.
> > >
> > > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior
> > > is undefined when fences are not used to synchronize buffer usage across
> > > contexts (which is the only case where multiple different priority rings
> > > could come into play).
> >
> > Really, doesn't this break cross-device implicit sync? Okay, you may
> > not have many peripherals that rely on implicit sync on devices where
> > Adreno is usually found, but it seems rather heavy-handed.
> >
> > Wouldn't it be better to only ignore fences from your own ring context
> > in the implicit sync, like we do in the common DRM scheduler
> > (drm_sched_dependency_optimized)?
>
> we already do this.. as was discussed on an earlier iteration of this patchset
>
> But I'm not aware of any other non-gpu related implicit sync use-case
> (even on imx devices where display is decoupled from gpu).. I'll
> revert the patch if someone comes up with one, but otherwise lets let
> the implicit sync baggage die
The thing is, dma_resv won't die, even if implicit sync is dead. We're
using internally for activity tracking and memory management. If you don't
set these, then we can't share generic code with msm, and I think everyone
inventing their own memory management is a bit a mistake.
Now you only kill the implicit write sync stuff here, but I'm not sure
that's worth much since you still install all the read fences for
consistency. And if userspace doesn't want to be synced, they can set the
flag and do this on their own: I think you should be able to achieve
exactly the same thing in mesa.
Aside: If you're worried about overhead, you can do O(1) submit if you
manage your ppgtt like amdgpu does.
-Daniel
>
> BR,
> -R
>
>
>
> >
> > Regards,
> > Lucas
> >
> > > Signed-off-by: Rob Clark
> > > Reviewed-by: Kristian H. Kristensen
> > > ---
> > > drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
> > > 1 file changed, 4 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > index d04c349d8112..b6babc7f9bb8 100644
> > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct msm_gem_submit
> > > *submit)
> > > return ret;
> > > }
> > >
> > > -static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > no_implicit)
> > > +static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > > implicit_sync)
> > > {
> > > int i, ret = 0;
> > >
> > > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct msm_gem_submit
> > > *submit, bool no_implicit)
> > > return ret;
> > > }
> > >
> > > - if (no_implicit)
> > > + if (!implicit_sync)
> > > continue;
> > >
> > > ret = msm_gem_sync_object(_obj->base,
> > > submit->ring->fctx,
> > > @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> > > *data,
> > > if (ret)
> > > goto out;
> > >
> > > - ret = submit_fence_sync(submit, !!(args->flags &
> > > MSM_SUBMIT_NO_IMPLICIT));
> > > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) &&
> > > + !(args->flags & MSM_SUBMIT_NO_IMPLICIT));
> > > if (ret)
> > > goto out;
> > >
> >
> ___
> dri-devel mailing list
> dri-de...@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
--
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Fri, Oct 23, 2020 at 11:20 AM Lucas Stach wrote:
>
> On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote:
> > From: Rob Clark
> >
> > If there is only a single ring (no-preemption), everything is FIFO order
> > and there is no need to implicit-sync.
> >
> > Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior
> > is undefined when fences are not used to synchronize buffer usage across
> > contexts (which is the only case where multiple different priority rings
> > could come into play).
>
> Really, doesn't this break cross-device implicit sync? Okay, you may
> not have many peripherals that rely on implicit sync on devices where
> Adreno is usually found, but it seems rather heavy-handed.
>
> Wouldn't it be better to only ignore fences from your own ring context
> in the implicit sync, like we do in the common DRM scheduler
> (drm_sched_dependency_optimized)?
we already do this.. as was discussed on an earlier iteration of this patchset
But I'm not aware of any other non-gpu related implicit sync use-case
(even on imx devices where display is decoupled from gpu).. I'll
revert the patch if someone comes up with one, but otherwise lets let
the implicit sync baggage die
BR,
-R
>
> Regards,
> Lucas
>
> > Signed-off-by: Rob Clark
> > Reviewed-by: Kristian H. Kristensen
> > ---
> > drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
> > 1 file changed, 4 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> > b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index d04c349d8112..b6babc7f9bb8 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -283,7 +283,7 @@ static int submit_lock_objects(struct msm_gem_submit
> > *submit)
> > return ret;
> > }
> >
> > -static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > no_implicit)
> > +static int submit_fence_sync(struct msm_gem_submit *submit, bool
> > implicit_sync)
> > {
> > int i, ret = 0;
> >
> > @@ -303,7 +303,7 @@ static int submit_fence_sync(struct msm_gem_submit
> > *submit, bool no_implicit)
> > return ret;
> > }
> >
> > - if (no_implicit)
> > + if (!implicit_sync)
> > continue;
> >
> > ret = msm_gem_sync_object(_obj->base, submit->ring->fctx,
> > @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> > *data,
> > if (ret)
> > goto out;
> >
> > - ret = submit_fence_sync(submit, !!(args->flags &
> > MSM_SUBMIT_NO_IMPLICIT));
> > + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) &&
> > + !(args->flags & MSM_SUBMIT_NO_IMPLICIT));
> > if (ret)
> > goto out;
> >
>
Re: [PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
On Fr, 2020-10-23 at 09:51 -0700, Rob Clark wrote:
> From: Rob Clark
>
> If there is only a single ring (no-preemption), everything is FIFO order
> and there is no need to implicit-sync.
>
> Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior
> is undefined when fences are not used to synchronize buffer usage across
> contexts (which is the only case where multiple different priority rings
> could come into play).
Really, doesn't this break cross-device implicit sync? Okay, you may
not have many peripherals that rely on implicit sync on devices where
Adreno is usually found, but it seems rather heavy-handed.
Wouldn't it be better to only ignore fences from your own ring context
in the implicit sync, like we do in the common DRM scheduler
(drm_sched_dependency_optimized)?
Regards,
Lucas
> Signed-off-by: Rob Clark
> Reviewed-by: Kristian H. Kristensen
> ---
> drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
> 1 file changed, 4 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
> b/drivers/gpu/drm/msm/msm_gem_submit.c
> index d04c349d8112..b6babc7f9bb8 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -283,7 +283,7 @@ static int submit_lock_objects(struct msm_gem_submit
> *submit)
> return ret;
> }
>
> -static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
> +static int submit_fence_sync(struct msm_gem_submit *submit, bool
> implicit_sync)
> {
> int i, ret = 0;
>
> @@ -303,7 +303,7 @@ static int submit_fence_sync(struct msm_gem_submit
> *submit, bool no_implicit)
> return ret;
> }
>
> - if (no_implicit)
> + if (!implicit_sync)
> continue;
>
> ret = msm_gem_sync_object(_obj->base, submit->ring->fctx,
> @@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void
> *data,
> if (ret)
> goto out;
>
> - ret = submit_fence_sync(submit, !!(args->flags &
> MSM_SUBMIT_NO_IMPLICIT));
> + ret = submit_fence_sync(submit, (gpu->nr_rings > 1) &&
> + !(args->flags & MSM_SUBMIT_NO_IMPLICIT));
> if (ret)
> goto out;
>
[PATCH v4 23/23] drm/msm: Don't implicit-sync if only a single ring
From: Rob Clark
If there is only a single ring (no-preemption), everything is FIFO order
and there is no need to implicit-sync.
Mesa should probably just always use MSM_SUBMIT_NO_IMPLICIT, as behavior
is undefined when fences are not used to synchronize buffer usage across
contexts (which is the only case where multiple different priority rings
could come into play).
Signed-off-by: Rob Clark
Reviewed-by: Kristian H. Kristensen
---
drivers/gpu/drm/msm/msm_gem_submit.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c
b/drivers/gpu/drm/msm/msm_gem_submit.c
index d04c349d8112..b6babc7f9bb8 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -283,7 +283,7 @@ static int submit_lock_objects(struct msm_gem_submit
*submit)
return ret;
}
-static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
+static int submit_fence_sync(struct msm_gem_submit *submit, bool implicit_sync)
{
int i, ret = 0;
@@ -303,7 +303,7 @@ static int submit_fence_sync(struct msm_gem_submit *submit,
bool no_implicit)
return ret;
}
- if (no_implicit)
+ if (!implicit_sync)
continue;
ret = msm_gem_sync_object(_obj->base, submit->ring->fctx,
@@ -774,7 +774,8 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
if (ret)
goto out;
- ret = submit_fence_sync(submit, !!(args->flags &
MSM_SUBMIT_NO_IMPLICIT));
+ ret = submit_fence_sync(submit, (gpu->nr_rings > 1) &&
+ !(args->flags & MSM_SUBMIT_NO_IMPLICIT));
if (ret)
goto out;
--
2.26.2
