Re: [Intel-gfx] [PATCH v5 16/20] drm/msm: Don't break exclusive fence ordering
On Thu, Aug 26, 2021 at 09:16:25AM -0700, Rob Clark wrote: > On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter 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 > > Signed-off-by: Daniel Vetter > > Cc: Rob Clark > > Cc: Sean Paul > > Cc: linux-arm-...@vger.kernel.org > > Cc: freedr...@lists.freedesktop.org > > a-b Also pushed to drm-misc-next, thanks for review&testing. -Daniel > > > --- > > 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 fb5a2eab27a2..66633dfd58a2 100644 > > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > > @@ -330,7 +330,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; > > > > ret = drm_sched_job_add_implicit_dependencies(&submit->base, > > -- > > 2.32.0 > > -- Daniel Vetter Software Engineer, Intel Corporation http://blog.ffwll.ch
Re: [Intel-gfx] [PATCH v5 16/20] drm/msm: Don't break exclusive fence ordering
On Thu, Aug 5, 2021 at 3:47 AM Daniel Vetter 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 > Signed-off-by: Daniel Vetter > Cc: Rob Clark > Cc: Sean Paul > Cc: linux-arm-...@vger.kernel.org > Cc: freedr...@lists.freedesktop.org a-b > --- > 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 fb5a2eab27a2..66633dfd58a2 100644 > --- a/drivers/gpu/drm/msm/msm_gem_submit.c > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c > @@ -330,7 +330,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; > > ret = drm_sched_job_add_implicit_dependencies(&submit->base, > -- > 2.32.0 >
[Intel-gfx] [PATCH v5 16/20] drm/msm: Don't break exclusive fence ordering
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 Signed-off-by: Daniel Vetter Cc: Rob Clark Cc: Sean Paul Cc: linux-arm-...@vger.kernel.org Cc: freedr...@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 fb5a2eab27a2..66633dfd58a2 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -330,7 +330,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; ret = drm_sched_job_add_implicit_dependencies(&submit->base, -- 2.32.0
