Re: [PATCH v4 08/12] drm/amdgpu: Consolidate ctx put

[Tvrtko Ursulin]

On 12/01/2026 13:32, Christian König wrote:
On 1/12/26 11:22, Tvrtko Ursulin wrote:
Currently there are two flavours of the context reference count
destructor:

  - amdgpu_ctx_do_release(), used from kref_put from places where the code
    thinks context may have been used, or is in active use, and;
  - amdgpu_ctx_fini(), used when code is sure context entities have already
    been idled.

Since amdgpu_ctx_do_release() calls amdgpu_ctx_fini() after having idled
and destroyed the scheduler entities, we can consolidate the two into a
single function.

Functional difference is that now drm_sched_entity_destroy() is called on
context manager shutdown (file close), where previously it was
drm_sched_entity_fini(). But the former is a superset of the latter, and
during file close the flush method is also called, which calls
drm_sched_entity_flush(), which is also called by
drm_sched_entity_destroy(). And as it is safe to attempt to flush a never
used entity, or flush it twice, there is actually no functional change.

Signed-off-by: Tvrtko Ursulin <tvrtko.ursu...@igalia.com>
Suggested-by: Christian König <christian.koe...@amd.com>

Reviewed-by: Christian König <christian.koe...@amd.com>

Looks like this was the last patch to review. I will pick up the set and try to 
push it to amd-staging-drm-next.

Gentle reminder if we could try to validate the series via 
amd-staging-drm-next.

Also, what could I do on my end to get more confidence some edge case is 
not broken? Run the IGT tests? Is there an useful testlist which can be 
used with the IGT runner or that is not used on the AMD side?

Regards,

Tvrtko

---
v2:
  * Use separate variable for drm_dev_enter for readability.

v3:
  * Keep amdgpu_ctx_fini_entity as a separate function.
---
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 54 ++++---------------------
  drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h |  9 ++++-
  2 files changed, 15 insertions(+), 48 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index 325833ed2571..cc69ad0f03d5 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -284,6 +284,8 @@ static ktime_t amdgpu_ctx_fini_entity(struct amdgpu_device 
*adev,
        if (!entity)
                return res;
  
+	drm_sched_entity_destroy(&entity->entity);
+
        for (i = 0; i < amdgpu_sched_jobs; ++i) {
                res = ktime_add(res, amdgpu_ctx_fence_time(entity->fences[i]));
                dma_fence_put(entity->fences[i]);
@@ -409,7 +411,7 @@ static int amdgpu_ctx_set_stable_pstate(struct amdgpu_ctx 
*ctx,
        return r;
  }
  
-static void amdgpu_ctx_fini(struct kref *ref)
+void amdgpu_ctx_fini(struct kref *ref)
  {
        struct amdgpu_ctx *ctx = container_of(ref, struct amdgpu_ctx, refcount);
        struct amdgpu_ctx_mgr *mgr = ctx->mgr;
@@ -508,24 +510,6 @@ static int amdgpu_ctx_alloc(struct amdgpu_device *adev,
        return r;
  }
  
-static void amdgpu_ctx_do_release(struct kref *ref)
-{
-       struct amdgpu_ctx *ctx;
-       u32 i, j;
-
-       ctx = container_of(ref, struct amdgpu_ctx, refcount);
-       for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
-               for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
-                       if (!ctx->entities[i][j])
-                               continue;
-
-                       drm_sched_entity_destroy(&ctx->entities[i][j]->entity);
-               }
-       }
-
-       amdgpu_ctx_fini(ref);
-}
-
  static int amdgpu_ctx_free(struct amdgpu_fpriv *fpriv, uint32_t id)
  {
        struct amdgpu_ctx_mgr *mgr = &fpriv->ctx_mgr;
@@ -533,8 +517,7 @@ static int amdgpu_ctx_free(struct amdgpu_fpriv *fpriv, 
uint32_t id)
  
  	mutex_lock(&mgr->lock);
        ctx = idr_remove(&mgr->ctx_handles, id);
-       if (ctx)
-               kref_put(&ctx->refcount, amdgpu_ctx_do_release);
+       amdgpu_ctx_put(ctx);
        mutex_unlock(&mgr->lock);
        return ctx ? 0 : -EINVAL;
  }
@@ -750,15 +733,6 @@ struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv 
*fpriv, uint32_t id)
        return ctx;
  }
  
-int amdgpu_ctx_put(struct amdgpu_ctx *ctx)
-{
-       if (ctx == NULL)
-               return -EINVAL;
-
-       kref_put(&ctx->refcount, amdgpu_ctx_do_release);
-       return 0;
-}
-
  uint64_t amdgpu_ctx_add_fence(struct amdgpu_ctx *ctx,
                              struct drm_sched_entity *entity,
                              struct dma_fence *fence)
@@ -927,29 +901,15 @@ long amdgpu_ctx_mgr_entity_flush(struct amdgpu_ctx_mgr 
*mgr, long timeout)
  static void amdgpu_ctx_mgr_entity_fini(struct amdgpu_ctx_mgr *mgr)
  {
        struct amdgpu_ctx *ctx;
-       struct idr *idp;
-       uint32_t id, i, j;
+       uint32_t id;
  
-	idp = &mgr->ctx_handles;
-
-       idr_for_each_entry(idp, ctx, id) {
+       idr_for_each_entry(&mgr->ctx_handles, ctx, id) {
                if (kref_read(&ctx->refcount) != 1) {
                        DRM_ERROR("ctx %p is still alive\n", ctx);
                        continue;
                }
  
-		for (i = 0; i < AMDGPU_HW_IP_NUM; ++i) {
-                       for (j = 0; j < amdgpu_ctx_num_entities[i]; ++j) {
-                               struct drm_sched_entity *entity;
-
-                               if (!ctx->entities[i][j])
-                                       continue;
-
-                               entity = &ctx->entities[i][j]->entity;
-                               drm_sched_entity_fini(entity);
-                       }
-               }
-               kref_put(&ctx->refcount, amdgpu_ctx_fini);
+               amdgpu_ctx_put(ctx);
        }
  }
  
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
index cf8d700a22fe..b1fa7fe74569 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.h
@@ -70,7 +70,14 @@ struct amdgpu_ctx_mgr {
  extern const unsigned int amdgpu_ctx_num_entities[AMDGPU_HW_IP_NUM];
  
  struct amdgpu_ctx *amdgpu_ctx_get(struct amdgpu_fpriv *fpriv, uint32_t id);
-int amdgpu_ctx_put(struct amdgpu_ctx *ctx);
+
+void amdgpu_ctx_fini(struct kref *kref);
+
+static inline void amdgpu_ctx_put(struct amdgpu_ctx *ctx)
+{
+       if (ctx)
+               kref_put(&ctx->refcount, amdgpu_ctx_fini);
+}
  
  int amdgpu_ctx_get_entity(struct amdgpu_ctx *ctx, u32 hw_ip, u32 instance,
                          u32 ring, struct drm_sched_entity **entity);