Decouple the amdgpu fence from the amdgpu_job structure.
This lets us clean up the separate fence ops for the embedded
fence and other fences.  This also allows us to allocate the
vm fence up front when we allocate the job.

Cc: david....@amd.com
Cc: christian.koe...@amd.com
Signed-off-by: Alex Deucher <alexander.deuc...@amd.com>
---
 drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_device.c  |   2 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c   | 111 +++-----------------
 drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c      |  12 ++-
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.c     |  36 ++++---
 drivers/gpu/drm/amd/amdgpu/amdgpu_job.h     |   3 +-
 drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h    |   3 +
 drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c      |   6 +-
 8 files changed, 58 insertions(+), 117 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
index f81608330a3d0..7ea3cb6491b1b 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
@@ -1902,7 +1902,7 @@ static void amdgpu_ib_preempt_mark_partial_job(struct 
amdgpu_ring *ring)
                        continue;
                }
                job = to_amdgpu_job(s_job);
-               if (preempted && (&job->hw_fence.base) == fence)
+               if (preempted && (&job->hw_fence->base) == fence)
                        /* mark the job as preempted */
                        job->preemption_status |= AMDGPU_IB_PREEMPTED;
        }
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
index e117494e80547..efd86f095323a 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_device.c
@@ -6494,7 +6494,7 @@ int amdgpu_device_gpu_recover(struct amdgpu_device *adev,
         *
         * job->base holds a reference to parent fence
         */
-       if (job && dma_fence_is_signaled(&job->hw_fence.base)) {
+       if (job && dma_fence_is_signaled(&job->hw_fence->base)) {
                job_signaled = true;
                dev_info(adev->dev, "Guilty job already signaled, skipping HW 
reset");
                goto skip_hw_reset;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
index 2d58aefbd68a7..0afe2427c25cc 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c
@@ -45,16 +45,11 @@
  * Cast helper
  */
 static const struct dma_fence_ops amdgpu_fence_ops;
-static const struct dma_fence_ops amdgpu_job_fence_ops;
 static inline struct amdgpu_fence *to_amdgpu_fence(struct dma_fence *f)
 {
        struct amdgpu_fence *__f = container_of(f, struct amdgpu_fence, base);
 
-       if (__f->base.ops == &amdgpu_fence_ops ||
-           __f->base.ops == &amdgpu_job_fence_ops)
-               return __f;
-
-       return NULL;
+       return __f;
 }
 
 /**
@@ -110,35 +105,18 @@ int amdgpu_fence_emit(struct amdgpu_ring *ring, struct 
dma_fence **f,
 {
        struct amdgpu_device *adev = ring->adev;
        struct dma_fence *fence;
-       struct amdgpu_fence *am_fence;
        struct dma_fence __rcu **ptr;
        uint32_t seq;
        int r;
 
-       if (!af) {
-               /* create a separate hw fence */
-               am_fence = kzalloc(sizeof(*am_fence), GFP_KERNEL);
-               if (!am_fence)
-                       return -ENOMEM;
-       } else {
-               am_fence = af;
-       }
-       fence = &am_fence->base;
-       am_fence->ring = ring;
+       fence = &af->base;
+       af->ring = ring;
 
        seq = ++ring->fence_drv.sync_seq;
-       am_fence->seq = seq;
-       if (af) {
-               dma_fence_init(fence, &amdgpu_job_fence_ops,
-                              &ring->fence_drv.lock,
-                              adev->fence_context + ring->idx, seq);
-               /* Against remove in amdgpu_job_{free, free_cb} */
-               dma_fence_get(fence);
-       } else {
-               dma_fence_init(fence, &amdgpu_fence_ops,
-                              &ring->fence_drv.lock,
-                              adev->fence_context + ring->idx, seq);
-       }
+       af->seq = seq;
+       dma_fence_init(fence, &amdgpu_fence_ops,
+                      &ring->fence_drv.lock,
+                      adev->fence_context + ring->idx, seq);
 
        amdgpu_ring_emit_fence(ring, ring->fence_drv.gpu_addr,
                               seq, flags | AMDGPU_FENCE_FLAG_INT);
@@ -683,18 +661,22 @@ void amdgpu_fence_driver_clear_job_fences(struct 
amdgpu_ring *ring)
        for (i = 0; i <= ring->fence_drv.num_fences_mask; i++) {
                ptr = &ring->fence_drv.fences[i];
                old = rcu_dereference_protected(*ptr, 1);
-               if (old && old->ops == &amdgpu_job_fence_ops) {
+               if (old) {
+                       struct amdgpu_fence *af;
                        struct amdgpu_job *job;
 
                        /* For non-scheduler bad job, i.e. failed ib test, we 
need to signal
                         * it right here or we won't be able to track them in 
fence_drv
                         * and they will remain unsignaled during sa_bo free.
                         */
-                       job = container_of(old, struct amdgpu_job, 
hw_fence.base);
-                       if (!job->base.s_fence && !dma_fence_is_signaled(old))
-                               dma_fence_signal(old);
-                       RCU_INIT_POINTER(*ptr, NULL);
-                       dma_fence_put(old);
+                       af = container_of(old, struct amdgpu_fence, base);
+                       job = af->job;
+                       if (job) {
+                               if (!job->base.s_fence && 
!dma_fence_is_signaled(old))
+                                       dma_fence_signal(old);
+                               RCU_INIT_POINTER(*ptr, NULL);
+                               dma_fence_put(old);
+                       }
                }
        }
 }
@@ -830,13 +812,6 @@ static const char *amdgpu_fence_get_timeline_name(struct 
dma_fence *f)
        return (const char *)to_amdgpu_fence(f)->ring->name;
 }
 
-static const char *amdgpu_job_fence_get_timeline_name(struct dma_fence *f)
-{
-       struct amdgpu_job *job = container_of(f, struct amdgpu_job, 
hw_fence.base);
-
-       return (const char *)to_amdgpu_ring(job->base.sched)->name;
-}
-
 /**
  * amdgpu_fence_enable_signaling - enable signalling on fence
  * @f: fence
@@ -853,23 +828,6 @@ static bool amdgpu_fence_enable_signaling(struct dma_fence 
*f)
        return true;
 }
 
-/**
- * amdgpu_job_fence_enable_signaling - enable signalling on job fence
- * @f: fence
- *
- * This is the simliar function with amdgpu_fence_enable_signaling above, it
- * only handles the job embedded fence.
- */
-static bool amdgpu_job_fence_enable_signaling(struct dma_fence *f)
-{
-       struct amdgpu_job *job = container_of(f, struct amdgpu_job, 
hw_fence.base);
-
-       if 
(!timer_pending(&to_amdgpu_ring(job->base.sched)->fence_drv.fallback_timer))
-               amdgpu_fence_schedule_fallback(to_amdgpu_ring(job->base.sched));
-
-       return true;
-}
-
 /**
  * amdgpu_fence_free - free up the fence memory
  *
@@ -885,21 +843,6 @@ static void amdgpu_fence_free(struct rcu_head *rcu)
        kfree(to_amdgpu_fence(f));
 }
 
-/**
- * amdgpu_job_fence_free - free up the job with embedded fence
- *
- * @rcu: RCU callback head
- *
- * Free up the job with embedded fence after the RCU grace period.
- */
-static void amdgpu_job_fence_free(struct rcu_head *rcu)
-{
-       struct dma_fence *f = container_of(rcu, struct dma_fence, rcu);
-
-       /* free job if fence has a parent job */
-       kfree(container_of(f, struct amdgpu_job, hw_fence.base));
-}
-
 /**
  * amdgpu_fence_release - callback that fence can be freed
  *
@@ -913,19 +856,6 @@ static void amdgpu_fence_release(struct dma_fence *f)
        call_rcu(&f->rcu, amdgpu_fence_free);
 }
 
-/**
- * amdgpu_job_fence_release - callback that job embedded fence can be freed
- *
- * @f: fence
- *
- * This is the simliar function with amdgpu_fence_release above, it
- * only handles the job embedded fence.
- */
-static void amdgpu_job_fence_release(struct dma_fence *f)
-{
-       call_rcu(&f->rcu, amdgpu_job_fence_free);
-}
-
 static const struct dma_fence_ops amdgpu_fence_ops = {
        .get_driver_name = amdgpu_fence_get_driver_name,
        .get_timeline_name = amdgpu_fence_get_timeline_name,
@@ -933,13 +863,6 @@ static const struct dma_fence_ops amdgpu_fence_ops = {
        .release = amdgpu_fence_release,
 };
 
-static const struct dma_fence_ops amdgpu_job_fence_ops = {
-       .get_driver_name = amdgpu_fence_get_driver_name,
-       .get_timeline_name = amdgpu_job_fence_get_timeline_name,
-       .enable_signaling = amdgpu_job_fence_enable_signaling,
-       .release = amdgpu_job_fence_release,
-};
-
 /*
  * Fence debugfs
  */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
index 7d9bcb72e8dd3..192ed1bd2ec63 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
@@ -128,7 +128,7 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
int num_ibs,
        struct amdgpu_device *adev = ring->adev;
        struct amdgpu_ib *ib = &ibs[0];
        struct dma_fence *tmp = NULL;
-       struct amdgpu_fence *af;
+       struct amdgpu_fence *af, *vm_af;
        bool need_ctx_switch;
        struct amdgpu_vm *vm;
        uint64_t fence_ctx;
@@ -154,12 +154,15 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
int num_ibs,
                csa_va = job->csa_va;
                gds_va = job->gds_va;
                init_shadow = job->init_shadow;
-               af = &job->hw_fence;
+               af = job->hw_fence;
                /* Save the context of the job for reset handling.
                 * The driver needs this so it can skip the ring
                 * contents for guilty contexts.
                 */
                af->context = job->base.s_fence ? 
job->base.s_fence->finished.context : 0;
+               vm_af = job->hw_vm_fence;
+               /* the vm fence is also part of the job's context */
+               vm_af->context = job->base.s_fence ? 
job->base.s_fence->finished.context : 0;
        } else {
                vm = NULL;
                fence_ctx = 0;
@@ -167,7 +170,10 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned 
int num_ibs,
                csa_va = 0;
                gds_va = 0;
                init_shadow = false;
-               af = NULL;
+               af = kzalloc(sizeof(*af), GFP_NOWAIT);
+               if (!af)
+                       return -ENOMEM;
+               vm_af = NULL;
        }
 
        if (!ring->sched.ready) {
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
index 311e97c96c4e0..4189a2b8cb674 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.c
@@ -138,7 +138,7 @@ static enum drm_gpu_sched_stat amdgpu_job_timedout(struct 
drm_sched_job *s_job)
                   ring->funcs->reset) {
                dev_err(adev->dev, "Starting %s ring reset\n",
                        s_job->sched->name);
-               r = amdgpu_ring_reset(ring, job->vmid, &job->hw_fence);
+               r = amdgpu_ring_reset(ring, job->vmid, job->hw_fence);
                if (!r) {
                        atomic_inc(&ring->adev->gpu_reset_counter);
                        dev_err(adev->dev, "Ring %s reset succeeded\n",
@@ -185,6 +185,8 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
                     struct drm_sched_entity *entity, void *owner,
                     unsigned int num_ibs, struct amdgpu_job **job)
 {
+       struct amdgpu_fence *af;
+
        if (num_ibs == 0)
                return -EINVAL;
 
@@ -192,6 +194,23 @@ int amdgpu_job_alloc(struct amdgpu_device *adev, struct 
amdgpu_vm *vm,
        if (!*job)
                return -ENOMEM;
 
+       af = kzalloc(sizeof(struct amdgpu_fence), GFP_KERNEL);
+       if (!af) {
+               kfree(*job);
+               return -ENOMEM;
+       }
+       /* assign the job so we can clean up in 
amdgpu_fence_driver_clear_job_fences() */
+       af->job = *job;
+       (*job)->hw_fence = af;
+
+       af = kzalloc(sizeof(struct amdgpu_fence), GFP_KERNEL);
+       if (!af) {
+               kfree((*job)->hw_fence);
+               kfree(*job);
+               return -ENOMEM;
+       }
+       (*job)->hw_vm_fence = af;
+
        (*job)->vm = vm;
 
        amdgpu_sync_create(&(*job)->explicit_sync);
@@ -251,8 +270,8 @@ void amdgpu_job_free_resources(struct amdgpu_job *job)
        /* Check if any fences where initialized */
        if (job->base.s_fence && job->base.s_fence->finished.ops)
                f = &job->base.s_fence->finished;
-       else if (job->hw_fence.base.ops)
-               f = &job->hw_fence.base;
+       else if (job->hw_fence)
+               f = &job->hw_fence->base;
        else
                f = NULL;
 
@@ -268,11 +287,7 @@ static void amdgpu_job_free_cb(struct drm_sched_job *s_job)
 
        amdgpu_sync_free(&job->explicit_sync);
 
-       /* only put the hw fence if has embedded fence */
-       if (!job->hw_fence.base.ops)
-               kfree(job);
-       else
-               dma_fence_put(&job->hw_fence.base);
+       kfree(job);
 }
 
 void amdgpu_job_set_gang_leader(struct amdgpu_job *job,
@@ -301,10 +316,7 @@ void amdgpu_job_free(struct amdgpu_job *job)
        if (job->gang_submit != &job->base.s_fence->scheduled)
                dma_fence_put(job->gang_submit);
 
-       if (!job->hw_fence.base.ops)
-               kfree(job);
-       else
-               dma_fence_put(&job->hw_fence.base);
+       kfree(job);
 }
 
 struct dma_fence *amdgpu_job_submit(struct amdgpu_job *job)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
index 931fed8892cc1..077b2414a24b7 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_job.h
@@ -48,7 +48,8 @@ struct amdgpu_job {
        struct drm_sched_job    base;
        struct amdgpu_vm        *vm;
        struct amdgpu_sync      explicit_sync;
-       struct amdgpu_fence     hw_fence;
+       struct amdgpu_fence     *hw_fence;
+       struct amdgpu_fence     *hw_vm_fence;
        struct dma_fence        *gang_submit;
        uint32_t                preamble_status;
        uint32_t                preemption_status;
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
index 7670f5d82b9e4..ceadd7e4bdb58 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ring.h
@@ -143,6 +143,9 @@ struct amdgpu_fence {
        struct amdgpu_ring              *ring;
        ktime_t                         start_timestamp;
 
+       /* store the job for cleanup */
+       struct amdgpu_job *job;
+
        /* wptr for the fence for resets */
        u64                             wptr;
        /* fence context for resets */
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c 
b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
index bf42246a3db2f..a287718506aa6 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_vm.c
@@ -772,7 +772,6 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
amdgpu_job *job,
        bool cleaner_shader_needed = false;
        bool pasid_mapping_needed = false;
        struct dma_fence *fence = NULL;
-       struct amdgpu_fence *af;
        unsigned int patch;
        int r;
 
@@ -835,12 +834,9 @@ int amdgpu_vm_flush(struct amdgpu_ring *ring, struct 
amdgpu_job *job,
        }
 
        if (vm_flush_needed || pasid_mapping_needed || cleaner_shader_needed) {
-               r = amdgpu_fence_emit(ring, &fence, NULL, 0);
+               r = amdgpu_fence_emit(ring, &fence, job->hw_vm_fence, 0);
                if (r)
                        return r;
-               /* this is part of the job's context */
-               af = container_of(fence, struct amdgpu_fence, base);
-               af->context = job->base.s_fence ? 
job->base.s_fence->finished.context : 0;
        }
 
        if (vm_flush_needed) {
-- 
2.51.0

Reply via email to