[PATCH] drm/amdgpu: partial revert "remove ctx->lock" v2

[Alex Deucher]

On Tue, Apr 19, 2022 at 7:06 AM Christian König
<ckoenig.leichtzumerken at gmail.com> wrote:
>
> This reverts commit 461fa7b0ac565ef25c1da0ced31005dd437883a7.
>
> We are missing some inter dependencies here so re-introduce the lock
> until we have figured out what's missing. Just drop/retake it while
> adding dependencies.
>
> v2: still drop the lock while adding dependencies
>
> Signed-off-by: Christian König <christian.koenig at amd.com>
> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov at gmail.com> (v1)
> Fixes: 461fa7b0ac56 ("drm/amdgpu: remove ctx->lock")

Acked-by: Alex Deucher <alexander.deucher at amd.com>

> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c  | 21 +++++++++++++++------
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c |  2 ++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h |  1 +
>  3 files changed, 18 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 970b065e9a6b..d0d0ea565e3d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -128,6 +128,8 @@ static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, union drm_amdgpu_cs
>                 goto free_chunk;
>         }
>
> +       mutex_lock(&p->ctx->lock);
> +
>         /* skip guilty context job */
>         if (atomic_read(&p->ctx->guilty) == 1) {
>                 ret = -ECANCELED;
> @@ -709,6 +711,7 @@ static void amdgpu_cs_parser_fini(struct amdgpu_cs_parser *parser, int error,
>         dma_fence_put(parser->fence);
>
>         if (parser->ctx) {
> +               mutex_unlock(&parser->ctx->lock);
>                 amdgpu_ctx_put(parser->ctx);
>         }
>         if (parser->bo_list)
> @@ -1157,6 +1160,9 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
>  {
>         int i, r;
>
> +       /* TODO: Investigate why we still need the context lock */
> +       mutex_unlock(&p->ctx->lock);
> +
>         for (i = 0; i < p->nchunks; ++i) {
>                 struct amdgpu_cs_chunk *chunk;
>
> @@ -1167,32 +1173,34 @@ static int amdgpu_cs_dependencies(struct amdgpu_device *adev,
>                 case AMDGPU_CHUNK_ID_SCHEDULED_DEPENDENCIES:
>                         r = amdgpu_cs_process_fence_dep(p, chunk);
>                         if (r)
> -                               return r;
> +                               goto out;
>                         break;
>                 case AMDGPU_CHUNK_ID_SYNCOBJ_IN:
>                         r = amdgpu_cs_process_syncobj_in_dep(p, chunk);
>                         if (r)
> -                               return r;
> +                               goto out;
>                         break;
>                 case AMDGPU_CHUNK_ID_SYNCOBJ_OUT:
>                         r = amdgpu_cs_process_syncobj_out_dep(p, chunk);
>                         if (r)
> -                               return r;
> +                               goto out;
>                         break;
>                 case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_WAIT:
>                         r = amdgpu_cs_process_syncobj_timeline_in_dep(p, chunk);
>                         if (r)
> -                               return r;
> +                               goto out;
>                         break;
>                 case AMDGPU_CHUNK_ID_SYNCOBJ_TIMELINE_SIGNAL:
>                         r = amdgpu_cs_process_syncobj_timeline_out_dep(p, chunk);
>                         if (r)
> -                               return r;
> +                               goto out;
>                         break;
>                 }
>         }
>
> -       return 0;
> +out:
> +       mutex_lock(&p->ctx->lock);
> +       return r;
>  }
>
>  static void amdgpu_cs_post_dependencies(struct amdgpu_cs_parser *p)
> @@ -1368,6 +1376,7 @@ int amdgpu_cs_ioctl(struct drm_device *dev, void *data, struct drm_file *filp)
>                 goto out;
>
>         r = amdgpu_cs_submit(&parser, cs);
> +
>  out:
>         amdgpu_cs_parser_fini(&parser, r, reserved_buffers);
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index 5981c7d9bd48..8f0e6d93bb9c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -237,6 +237,7 @@ static int amdgpu_ctx_init(struct amdgpu_device *adev,
>
>         kref_init(&ctx->refcount);
>         spin_lock_init(&ctx->ring_lock);
> +       mutex_init(&ctx->lock);
>
>         ctx->reset_counter = atomic_read(&adev->gpu_reset_counter);
>         ctx->reset_counter_query = ctx->reset_counter;
> @@ -357,6 +358,7 @@ static void amdgpu_ctx_fini(struct kref *ref)
>                 drm_dev_exit(idx);
>         }
>
> +       mutex_destroy(&ctx->lock);
>         kfree(ctx);
>  }
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> index d0cbfcea90f7..142f2f87d44c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
> @@ -49,6 +49,7 @@ struct amdgpu_ctx {
>         bool                            preamble_presented;
>         int32_t                         init_priority;
>         int32_t                         override_priority;
> +       struct mutex                    lock;
>         atomic_t                        guilty;
>         unsigned long                   ras_counter_ce;
>         unsigned long                   ras_counter_ue;
> --
> 2.25.1
>