On Tue, Apr 19, 2022 at 7:06 AM Christian König
<ckoenig.leichtzumerken@xxxxxxxxx> 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@xxxxxxx>
> Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@xxxxxxxxx> (v1)
> Fixes: 461fa7b0ac56 ("drm/amdgpu: remove ctx->lock")
Acked-by: Alex Deucher <alexander.deucher@xxxxxxx>
> ---
> 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
>
