drm/msm: Remove exclusive-fence hack

Message ID 20221101214051.159988-1-robdclark@gmail.com
State New
Headers
Series drm/msm: Remove exclusive-fence hack |

Commit Message

Rob Clark Nov. 1, 2022, 9:40 p.m. UTC
  From: Rob Clark <robdclark@chromium.org>

The workaround was initially necessary due to dma_resv having only a
single exclusive fence slot, yet whe don't necessarily know what order
the gpu scheduler will schedule jobs.  Unfortunately this workaround
also has the result of forcing implicit sync, even when userspace does
not want it.

However, since commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove
dma_resv workaround") the workaround is no longer needed.  So remove
it.  This effectively reverts commit f1b3f696a084 ("drm/msm: Don't
break exclusive fence ordering")

Signed-off-by: Rob Clark <robdclark@chromium.org>
---
 drivers/gpu/drm/msm/msm_gem_submit.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)
  

Comments

Christian König Nov. 2, 2022, 10:46 a.m. UTC | #1
Am 01.11.22 um 22:40 schrieb Rob Clark:
> From: Rob Clark <robdclark@chromium.org>
>
> The workaround was initially necessary due to dma_resv having only a
> single exclusive fence slot, yet whe don't necessarily know what order
> the gpu scheduler will schedule jobs.  Unfortunately this workaround
> also has the result of forcing implicit sync, even when userspace does
> not want it.
>
> However, since commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove
> dma_resv workaround") the workaround is no longer needed.  So remove
> it.  This effectively reverts commit f1b3f696a084 ("drm/msm: Don't
> break exclusive fence ordering")
>
> Signed-off-by: Rob Clark <robdclark@chromium.org>

Oh, yes please. I had that on my todo list for after the initial patch 
had landed, but couldn't find the time to look into it once more.

There was another case with one of the other ARM drivers which could be 
cleaned up now, but I can't find it any more of hand.

Anyway this patch here is Acked-by: Christian König 
<christian.koenig@amd.com>.

Regards,
Christian.

> ---
>   drivers/gpu/drm/msm/msm_gem_submit.c | 3 +--
>   1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 5599d93ec0d2..cc48f73adadf 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -334,8 +334,7 @@ static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
>   		if (ret)
>   			return ret;
>   
> -		/* exclusive fences must be ordered */
> -		if (no_implicit && !write)
> +		if (no_implicit)
>   			continue;
>   
>   		ret = drm_sched_job_add_implicit_dependencies(&submit->base,
  
Rob Clark Nov. 2, 2022, 4 p.m. UTC | #2
On Wed, Nov 2, 2022 at 3:46 AM Christian König <christian.koenig@amd.com> wrote:
>
> Am 01.11.22 um 22:40 schrieb Rob Clark:
> > From: Rob Clark <robdclark@chromium.org>
> >
> > The workaround was initially necessary due to dma_resv having only a
> > single exclusive fence slot, yet whe don't necessarily know what order
> > the gpu scheduler will schedule jobs.  Unfortunately this workaround
> > also has the result of forcing implicit sync, even when userspace does
> > not want it.
> >
> > However, since commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove
> > dma_resv workaround") the workaround is no longer needed.  So remove
> > it.  This effectively reverts commit f1b3f696a084 ("drm/msm: Don't
> > break exclusive fence ordering")
> >
> > Signed-off-by: Rob Clark <robdclark@chromium.org>
>
> Oh, yes please. I had that on my todo list for after the initial patch
> had landed, but couldn't find the time to look into it once more.
>
> There was another case with one of the other ARM drivers which could be
> cleaned up now, but I can't find it any more of hand.
>
> Anyway this patch here is Acked-by: Christian König
> <christian.koenig@amd.com>.

Thanks.. I had a quick look for the other driver but couldn't spot
anything, so perhaps it has already been fixed?

BR,
-R

>
> Regards,
> Christian.
>
> > ---
> >   drivers/gpu/drm/msm/msm_gem_submit.c | 3 +--
> >   1 file changed, 1 insertion(+), 2 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> > index 5599d93ec0d2..cc48f73adadf 100644
> > --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> > +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> > @@ -334,8 +334,7 @@ static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
> >               if (ret)
> >                       return ret;
> >
> > -             /* exclusive fences must be ordered */
> > -             if (no_implicit && !write)
> > +             if (no_implicit)
> >                       continue;
> >
> >               ret = drm_sched_job_add_implicit_dependencies(&submit->base,
>
  

Patch

diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
index 5599d93ec0d2..cc48f73adadf 100644
--- a/drivers/gpu/drm/msm/msm_gem_submit.c
+++ b/drivers/gpu/drm/msm/msm_gem_submit.c
@@ -334,8 +334,7 @@  static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit)
 		if (ret)
 			return ret;
 
-		/* exclusive fences must be ordered */
-		if (no_implicit && !write)
+		if (no_implicit)
 			continue;
 
 		ret = drm_sched_job_add_implicit_dependencies(&submit->base,