drm/amdgpu: cleanup context structure v2

The comment is misleading and incorrect, remove it.

Printing the id is completely meaningless and this practice
can cause a race conditions on command submission.

The flags and hangs fields are completely unused.

Give all fields a common indentation.

v2: remove fpriv reference and unused flags as well, fix debug message.

Signed-off-by: Christian König <christian.koenig@amd.com>
Reviewed-by: Jammy Zhou <Jammy.Zhou@amd.com>
Reviewed-by: Chunming Zhou <david1.zhou@amd.com>

2 files changed, 29 insertions, 49 deletions

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu.h b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
index 548e2bb72d99..069cc28941ba 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu.h
@@ -179,6 +179,7 @@ struct amdgpu_ring;
 struct amdgpu_semaphore;
 struct amdgpu_cs_parser;
 struct amdgpu_irq_src;
+struct amdgpu_fpriv;
 
 enum amdgpu_cp_irq {
 	AMDGPU_CP_IRQ_GFX_EOP = 0,
@@ -984,27 +985,32 @@ struct amdgpu_vm_manager {
  * context related structures
  */
 
-struct amdgpu_ctx_state {
-	uint64_t flags;
-	uint32_t hangs;
-};
-
 struct amdgpu_ctx {
-	/* call kref_get()before CS start and kref_put() after CS fence signaled */
-	struct kref refcount;
-	struct amdgpu_fpriv *fpriv;
-	struct amdgpu_ctx_state state;
-	uint32_t id;
-	unsigned reset_counter;
+	struct kref		refcount;
+	unsigned		reset_counter;
 };
 
 struct amdgpu_ctx_mgr {
-	struct amdgpu_device *adev;
-	struct idr ctx_handles;
-	/* lock for IDR system */
-	struct mutex lock;
+	struct amdgpu_device	*adev;
+	struct mutex		lock;
+	/* protected by lock */
+	struct idr		ctx_handles;
 };
 
+int amdgpu_ctx_alloc(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv,
+		     uint32_t *id);
+int amdgpu_ctx_free(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv,
+		    uint32_t id);
+
+void amdgpu_ctx_fini(struct amdgpu_fpriv *fpriv);
+
+struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id);
+int amdgpu_ctx_put(struct amdgpu_ctx *ctx);
+
+int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
+		     struct drm_file *filp);
+
+
 /*
  * file private structure
  */
@@ -1013,7 +1019,7 @@ struct amdgpu_fpriv {
 	struct amdgpu_vm	vm;
 	struct mutex		bo_list_lock;
 	struct idr		bo_list_handles;
-	struct amdgpu_ctx_mgr ctx_mgr;
+	struct amdgpu_ctx_mgr	ctx_mgr;
 };
 
 /*
@@ -1850,18 +1856,6 @@ struct amdgpu_atcs {
 	struct amdgpu_atcs_functions functions;
 };
 
-int amdgpu_ctx_alloc(struct amdgpu_device *adev,struct amdgpu_fpriv *fpriv,
-							uint32_t *id,uint32_t flags);
-int amdgpu_ctx_free(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv,
-						  uint32_t id);
-
-void amdgpu_ctx_fini(struct amdgpu_fpriv *fpriv);
-struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id);
-int amdgpu_ctx_put(struct amdgpu_ctx *ctx);
-
-extern int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
-						 struct drm_file *filp);
-
 /*
  * CGS
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 6c66ac8a1891..e63cfb7fa390 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -28,16 +28,13 @@
 static void amdgpu_ctx_do_release(struct kref *ref)
 {
 	struct amdgpu_ctx *ctx;
-	struct amdgpu_ctx_mgr *mgr;
 
 	ctx = container_of(ref, struct amdgpu_ctx, refcount);
-	mgr = &ctx->fpriv->ctx_mgr;
-
-	idr_remove(&mgr->ctx_handles, ctx->id);
 	kfree(ctx);
 }
 
-int amdgpu_ctx_alloc(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv, uint32_t *id, uint32_t flags)
+int amdgpu_ctx_alloc(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv,
+		     uint32_t *id)
 {
 	int r;
 	struct amdgpu_ctx *ctx;
@@ -57,8 +54,6 @@ int amdgpu_ctx_alloc(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv, uin
 	*id = (uint32_t)r;
 
 	memset(ctx, 0, sizeof(*ctx));
-	ctx->id = *id;
-	ctx->fpriv = fpriv;
 	kref_init(&ctx->refcount);
 	mutex_unlock(&mgr->lock);
 
@@ -73,6 +68,7 @@ int amdgpu_ctx_free(struct amdgpu_device *adev, struct amdgpu_fpriv *fpriv, uint
 	mutex_lock(&mgr->lock);
 	ctx = idr_find(&mgr->ctx_handles, id);
 	if (ctx) {
+		idr_remove(&mgr->ctx_handles, id);
 		kref_put(&ctx->refcount, amdgpu_ctx_do_release);
 		mutex_unlock(&mgr->lock);
 		return 0;
@@ -97,8 +93,8 @@ static int amdgpu_ctx_query(struct amdgpu_device *adev,
 	}
 
 	/* TODO: these two are always zero */
-	out->state.flags = ctx->state.flags;
-	out->state.hangs = ctx->state.hangs;
+	out->state.flags = 0x0;
+	out->state.hangs = 0x0;
 
 	/* determine if a GPU reset has occured since the last call */
 	reset_counter = atomic_read(&adev->gpu_reset_counter);
@@ -123,7 +119,7 @@ void amdgpu_ctx_fini(struct amdgpu_fpriv *fpriv)
 
 	idr_for_each_entry(idp,ctx,id) {
 		if (kref_put(&ctx->refcount, amdgpu_ctx_do_release) != 1)
-			DRM_ERROR("ctx (id=%ul) is still alive\n",ctx->id);
+			DRM_ERROR("ctx %p is still alive\n", ctx);
 	}
 
 	mutex_destroy(&mgr->lock);
@@ -134,7 +130,6 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 {
 	int r;
 	uint32_t id;
-	uint32_t flags;
 
 	union drm_amdgpu_ctx *args = data;
 	struct amdgpu_device *adev = dev->dev_private;
@@ -142,11 +137,10 @@ int amdgpu_ctx_ioctl(struct drm_device *dev, void *data,
 
 	r = 0;
 	id = args->in.ctx_id;
-	flags = args->in.flags;
 
 	switch (args->in.op) {
 		case AMDGPU_CTX_OP_ALLOC_CTX:
-			r = amdgpu_ctx_alloc(adev, fpriv, &id, flags);
+			r = amdgpu_ctx_alloc(adev, fpriv, &id);
 			args->out.alloc.ctx_id = id;
 			break;
 		case AMDGPU_CTX_OP_FREE_CTX:
@@ -177,17 +171,9 @@ struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id)
 
 int amdgpu_ctx_put(struct amdgpu_ctx *ctx)
 {
-	struct amdgpu_fpriv *fpriv;
-	struct amdgpu_ctx_mgr *mgr;
-
 	if (ctx == NULL)
 		return -EINVAL;
 
-	fpriv = ctx->fpriv;
-	mgr = &fpriv->ctx_mgr;
-	mutex_lock(&mgr->lock);
 	kref_put(&ctx->refcount, amdgpu_ctx_do_release);
-	mutex_unlock(&mgr->lock);
-
 	return 0;
 }