[PATCH] drm/msm: move submit fence wait out of struct_mutex

[Rob Clark]

On Sat, Feb 25, 2017 at 10:43 AM, Rob Clark <robdclark at gmail.com> wrote:
> Probably a symptom of needing finer grained locking, but if we wait on
> the incoming fence-fd (which could come from a different context) while
> holding struct_mutex, that blocks retire_worker so gpu fences cannot get
> scheduled.

s/scheduled/signalled/

> This causes a problem if userspace manages to get more than a frame
> ahead, leaving the atomic-commit worker blocked waiting on fences that
> cannot be signaled because submit is blocked waiting for a fence
> signalled from vblank (after the atomic commit which is blocked).
>
> If we start having multiple fence ctxs for the gpu, submit_fence_sync()
> would probably need to move outside of struct_mutex as well.
>
> Signed-off-by: Rob Clark <robdclark at gmail.com>
> ---
>  drivers/gpu/drm/msm/msm_gem_submit.c | 39 +++++++++++++++++-------------------
>  1 file changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c
> index 1172fe7..1c545eb 100644
> --- a/drivers/gpu/drm/msm/msm_gem_submit.c
> +++ b/drivers/gpu/drm/msm/msm_gem_submit.c
> @@ -404,6 +404,24 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
>         if (MSM_PIPE_FLAGS(args->flags) & ~MSM_SUBMIT_FLAGS)
>                 return -EINVAL;
>
> +       if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
> +               in_fence = sync_file_get_fence(args->fence_fd);
> +
> +               if (!in_fence)
> +                       return -EINVAL;
> +
> +               /* TODO if we get an array-fence due to userspace merging multiple
> +                * fences, we need a way to determine if all the backing fences
> +                * are from our own context..
> +                */
> +
> +               if (in_fence->context != gpu->fctx->context) {
> +                       ret = dma_fence_wait(in_fence, true);
> +                       if (ret)
> +                               return ret;
> +               }
> +       }
> +
>         ret = mutex_lock_interruptible(&dev->struct_mutex);
>         if (ret)
>                 return ret;
> @@ -431,27 +449,6 @@ int msm_ioctl_gem_submit(struct drm_device *dev, void *data,
>         if (ret)
>                 goto out;
>
> -       if (args->flags & MSM_SUBMIT_FENCE_FD_IN) {
> -               in_fence = sync_file_get_fence(args->fence_fd);
> -
> -               if (!in_fence) {
> -                       ret = -EINVAL;
> -                       goto out;
> -               }
> -
> -               /* TODO if we get an array-fence due to userspace merging multiple
> -                * fences, we need a way to determine if all the backing fences
> -                * are from our own context..
> -                */
> -
> -               if (in_fence->context != gpu->fctx->context) {
> -                       ret = dma_fence_wait(in_fence, true);
> -                       if (ret)
> -                               goto out;
> -               }
> -
> -       }
> -
>         if (!(args->fence & MSM_SUBMIT_NO_IMPLICIT)) {
>                 ret = submit_fence_sync(submit);
>                 if (ret)
> --
> 2.9.3
>