Am Montag, dem 28.06.2021 um 09:41 +0200 schrieb Boris Brezillon:

Mali Midgard/Bifrost GPUs have 3 hardware queues but only a global GPU reset. This leads to extra complexity when we need to synchronize timeout works with the reset work. One solution to address that is to have an ordered workqueue at the driver level that will be used by the different schedulers to queue their timeout work. Thanks to the serialization provided by the ordered workqueue we are guaranteed that timeout handlers are executed sequentially, and can thus easily reset the GPU from the timeout handler without extra synchronization.

v3:

• New patch

v4:

• Actually use the timeout_wq to queue the timeout work

Signed-off-by: Boris Brezillon boris.brezillon@collabora.com

Reviewed-by: Lucas Stach l.stach@pengutronix.de

---

drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c | 2 +- drivers/gpu/drm/etnaviv/etnaviv_sched.c | 3 ++- drivers/gpu/drm/lima/lima_sched.c | 3 ++- drivers/gpu/drm/panfrost/panfrost_job.c | 3 ++- drivers/gpu/drm/scheduler/sched_main.c | 14 +++++++++----- drivers/gpu/drm/v3d/v3d_sched.c | 10 +++++----- include/drm/gpu_scheduler.h | 5 ++++- 7 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c index 47ea46859618..532636ea20bc 100644 --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_fence.c @@ -488,7 +488,7 @@ int amdgpu_fence_driver_init_ring(struct amdgpu_ring *ring,

r = drm_sched_init(&ring->sched, &amdgpu_sched_ops, num_hw_submission, amdgpu_job_hang_limit,

• 	   timeout, sched_score, ring->name);
  

• 	   timeout, NULL, sched_score, ring->name);
  

  if (r) { DRM_ERROR("Failed to create scheduler on ring %s.\n", ring->name);

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c index 19826e504efc..feb6da1b6ceb 100644 --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c @@ -190,7 +190,8 @@ int etnaviv_sched_init(struct etnaviv_gpu *gpu)

ret = drm_sched_init(&gpu->sched, &etnaviv_sched_ops, etnaviv_hw_jobs_limit, etnaviv_job_hang_limit,

• 	     msecs_to_jiffies(500), NULL, dev_name(gpu->dev));
  

• 	     msecs_to_jiffies(500), NULL, NULL,
  

• 	     dev_name(gpu->dev));
  

  if (ret) return ret;

diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index ecf3267334ff..dba8329937a3 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -508,7 +508,8 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) INIT_WORK(&pipe->recover_work, lima_sched_recover_work);

return drm_sched_init(&pipe->base, &lima_sched_ops, 1,

• 	      lima_job_hang_limit, msecs_to_jiffies(timeout),
  

• 	      lima_job_hang_limit,
  

• 	      msecs_to_jiffies(timeout), NULL,
        NULL, name);
  

}

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 682f2161b999..8ff79fd49577 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -626,7 +626,8 @@ int panfrost_job_init(struct panfrost_device *pfdev)

ret = drm_sched_init(&js->queue[j].sched,
		     &panfrost_sched_ops,

• 		     1, 0, msecs_to_jiffies(JOB_TIMEOUT_MS),
  

• 		     1, 0,
  

• 		     msecs_to_jiffies(JOB_TIMEOUT_MS), NULL,
  	     NULL, "pan_js");
  

  if (ret) { dev_err(pfdev->dev, "Failed to create scheduler: %d.", ret);

diff --git a/drivers/gpu/drm/scheduler/sched_main.c b/drivers/gpu/drm/scheduler/sched_main.c index c0a2f8f8d472..3e180f0d4305 100644 --- a/drivers/gpu/drm/scheduler/sched_main.c +++ b/drivers/gpu/drm/scheduler/sched_main.c @@ -232,7 +232,7 @@ static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched) { if (sched->timeout != MAX_SCHEDULE_TIMEOUT && !list_empty(&sched->pending_list))

• schedule_delayed_work(&sched->work_tdr, sched->timeout);
  

• queue_delayed_work(sched->timeout_wq, &sched->work_tdr, sched->timeout);
  

}

/** @@ -244,7 +244,7 @@ static void drm_sched_start_timeout(struct drm_gpu_scheduler *sched) */ void drm_sched_fault(struct drm_gpu_scheduler *sched) {

• mod_delayed_work(system_wq, &sched->work_tdr, 0);

• mod_delayed_work(sched->timeout_wq, &sched->work_tdr, 0);

} EXPORT_SYMBOL(drm_sched_fault);

@@ -270,7 +270,7 @@ unsigned long drm_sched_suspend_timeout(struct drm_gpu_scheduler *sched) * Modify the timeout to an arbitrarily large value. This also prevents * the timeout to be restarted when new submissions arrive */

• if (mod_delayed_work(system_wq, &sched->work_tdr, MAX_SCHEDULE_TIMEOUT)

• if (mod_delayed_work(sched->timeout_wq, &sched->work_tdr, MAX_SCHEDULE_TIMEOUT) && time_after(sched_timeout, now)) return sched_timeout - now; else

@@ -294,7 +294,7 @@ void drm_sched_resume_timeout(struct drm_gpu_scheduler *sched, if (list_empty(&sched->pending_list)) cancel_delayed_work(&sched->work_tdr); else

• mod_delayed_work(system_wq, &sched->work_tdr, remaining);
  

• mod_delayed_work(sched->timeout_wq, &sched->work_tdr, remaining);
  
  

  spin_unlock(&sched->job_list_lock);

} @@ -837,6 +837,8 @@ static int drm_sched_main(void *param)

• @hw_submission: number of hw submissions that can be in flight
• @hang_limit: number of times to allow a job to hang before dropping it
• @timeout: timeout value in jiffies for the scheduler

• • @timeout_wq: workqueue to use for timeout work. If NULL, the system_wq is

• • used
    

  • @score: optional score atomic shared with other schedulers
  • @name: name used for debugging

@@ -844,7 +846,8 @@ static int drm_sched_main(void *param) */ int drm_sched_init(struct drm_gpu_scheduler *sched, const struct drm_sched_backend_ops *ops,

•    unsigned hw_submission, unsigned hang_limit, long timeout,
  

•    unsigned hw_submission, unsigned hang_limit,
  

•    long timeout, struct workqueue_struct *timeout_wq,
  

  atomic_t *score, const char *name)

{ int i, ret; @@ -852,6 +855,7 @@ int drm_sched_init(struct drm_gpu_scheduler *sched, sched->hw_submission_limit = hw_submission; sched->name = name; sched->timeout = timeout;

• sched->timeout_wq = timeout_wq ? : system_wq; sched->hang_limit = hang_limit; sched->score = score ? score : &sched->_score; for (i = DRM_SCHED_PRIORITY_MIN; i < DRM_SCHED_PRIORITY_COUNT; i++)

diff --git a/drivers/gpu/drm/v3d/v3d_sched.c b/drivers/gpu/drm/v3d/v3d_sched.c index 8992480c88fa..a39bdd5cfc4f 100644 --- a/drivers/gpu/drm/v3d/v3d_sched.c +++ b/drivers/gpu/drm/v3d/v3d_sched.c @@ -402,7 +402,7 @@ v3d_sched_init(struct v3d_dev *v3d) ret = drm_sched_init(&v3d->queue[V3D_BIN].sched, &v3d_bin_sched_ops, hw_jobs_limit, job_hang_limit,

• 	     msecs_to_jiffies(hang_limit_ms),
  

• 	     msecs_to_jiffies(hang_limit_ms), NULL,
       NULL, "v3d_bin");
  

  if (ret) { dev_err(v3d->drm.dev, "Failed to create bin scheduler: %d.", ret);

@@ -412,7 +412,7 @@ v3d_sched_init(struct v3d_dev *v3d) ret = drm_sched_init(&v3d->queue[V3D_RENDER].sched, &v3d_render_sched_ops, hw_jobs_limit, job_hang_limit,

• 	     msecs_to_jiffies(hang_limit_ms),
  

• 	     msecs_to_jiffies(hang_limit_ms), NULL,
       NULL, "v3d_render");
  

  if (ret) { dev_err(v3d->drm.dev, "Failed to create render scheduler: %d.",

@@ -424,7 +424,7 @@ v3d_sched_init(struct v3d_dev *v3d) ret = drm_sched_init(&v3d->queue[V3D_TFU].sched, &v3d_tfu_sched_ops, hw_jobs_limit, job_hang_limit,

• 	     msecs_to_jiffies(hang_limit_ms),
  

• 	     msecs_to_jiffies(hang_limit_ms), NULL,
       NULL, "v3d_tfu");
  

  if (ret) { dev_err(v3d->drm.dev, "Failed to create TFU scheduler: %d.",

@@ -437,7 +437,7 @@ v3d_sched_init(struct v3d_dev *v3d) ret = drm_sched_init(&v3d->queue[V3D_CSD].sched, &v3d_csd_sched_ops, hw_jobs_limit, job_hang_limit,

• 		     msecs_to_jiffies(hang_limit_ms),
  

• 		     msecs_to_jiffies(hang_limit_ms), NULL,
  	     NULL, "v3d_csd");
  

  if (ret) { dev_err(v3d->drm.dev, "Failed to create CSD scheduler: %d.",

@@ -449,7 +449,7 @@ v3d_sched_init(struct v3d_dev *v3d) ret = drm_sched_init(&v3d->queue[V3D_CACHE_CLEAN].sched, &v3d_cache_clean_sched_ops, hw_jobs_limit, job_hang_limit,

• 		     msecs_to_jiffies(hang_limit_ms),
  

• 		     msecs_to_jiffies(hang_limit_ms), NULL,
  	     NULL, "v3d_cache_clean");
  

  if (ret) { dev_err(v3d->drm.dev, "Failed to create CACHE_CLEAN scheduler: %d.",

diff --git a/include/drm/gpu_scheduler.h b/include/drm/gpu_scheduler.h index 10225a0a35d0..d4cdc906709e 100644 --- a/include/drm/gpu_scheduler.h +++ b/include/drm/gpu_scheduler.h @@ -269,6 +269,7 @@ struct drm_sched_backend_ops {

•             finished.
  

• @hw_rq_count: the number of jobs currently in the hardware queue.
• @job_id_count: used to assign unique id to the each job.

• • @timeout_wq: workqueue used to queue @work_tdr
  • @work_tdr: schedules a delayed call to @drm_sched_job_timedout after the

  •        timeout interval is over.
    

  • @thread: the kthread on which the scheduler which run.

@@ -293,6 +294,7 @@ struct drm_gpu_scheduler { wait_queue_head_t job_scheduled; atomic_t hw_rq_count; atomic64_t job_id_count;

• struct workqueue_struct *timeout_wq; struct delayed_work work_tdr; struct task_struct *thread; struct list_head pending_list;

@@ -306,7 +308,8 @@ struct drm_gpu_scheduler {

int drm_sched_init(struct drm_gpu_scheduler *sched, const struct drm_sched_backend_ops *ops,

•    uint32_t hw_submission, unsigned hang_limit, long timeout,
  

•    uint32_t hw_submission, unsigned hang_limit,
  

•    long timeout, struct workqueue_struct *timeout_wq,
  

  atomic_t *score, const char *name);

void drm_sched_fini(struct drm_gpu_scheduler *sched);

On 28/06/2021 08:42, Boris Brezillon wrote:

Now that we can pass our own workqueue to drm_sched_init(), we can use an ordered workqueue on for both the scheduler timeout tdr and our own reset work (which we use when the reset is not caused by a fault/timeout on a specific job, like when we have AS_ACTIVE bit stuck). This guarantees that the timeout handlers and reset handler can't run concurrently which drastically simplifies the locking.

v4:

• Actually pass the reset workqueue to drm_sched_init()
• Don't call cancel_work_sync() in panfrost_reset(). It will deadlock since it might be called from the reset work, which is executing and cancel_work_sync() will wait for the handler to return. Checking the reset pending status should avoid spurious resets

v3:

• New patch

Suggested-by: Daniel Vetter daniel.vetter@ffwll.ch Signed-off-by: Boris Brezillon boris.brezillon@collabora.com

While I'm very much not keen on the hack to drop out of the dma_fence_signalling section - I don't believe this is introducing the bug just making it more visible. What we really need is someone to find time to actually fix up the lock ordering. But until then:

Reviewed-by: Steven Price steven.price@arm.com

---

drivers/gpu/drm/panfrost/panfrost_device.h | 6 +- drivers/gpu/drm/panfrost/panfrost_job.c | 187 ++++++++------------- 2 files changed, 72 insertions(+), 121 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index f2190f90be75..59a487e8aba3 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -108,6 +108,7 @@ struct panfrost_device { struct mutex sched_lock;

struct {

• struct workqueue_struct *wq;
  

  struct work_struct work; atomic_t pending; } reset;

@@ -246,9 +247,8 @@ const char *panfrost_exception_name(u32 exception_code); static inline void panfrost_device_schedule_reset(struct panfrost_device *pfdev) {

• /* Schedule a reset if there's no reset in progress. */
• if (!atomic_xchg(&pfdev->reset.pending, 1))

• schedule_work(&pfdev->reset.work);
  

• atomic_set(&pfdev->reset.pending, 1);
• queue_work(pfdev->reset.wq, &pfdev->reset.work);

}

#endif diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index e0c479e67304..98193a557a2d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -25,17 +25,8 @@ #define job_write(dev, reg, data) writel(data, dev->iomem + (reg)) #define job_read(dev, reg) readl(dev->iomem + (reg))

-enum panfrost_queue_status {

• PANFROST_QUEUE_STATUS_ACTIVE,
• PANFROST_QUEUE_STATUS_STOPPED,
• PANFROST_QUEUE_STATUS_STARTING,
• PANFROST_QUEUE_STATUS_FAULT_PENDING,

-};

struct panfrost_queue_state { struct drm_gpu_scheduler sched;

• atomic_t status;
• struct mutex lock; u64 fence_context; u64 emit_seqno;

}; @@ -379,57 +370,72 @@ void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) job_write(pfdev, JOB_INT_MASK, irq_mask); }

-static bool panfrost_scheduler_stop(struct panfrost_queue_state *queue,

• 		    struct drm_sched_job *bad)
  

+static void panfrost_reset(struct panfrost_device *pfdev,

• 	   struct drm_sched_job *bad)
  

{

• enum panfrost_queue_status old_status;
• bool stopped = false;

• unsigned int i;
• bool cookie;

• mutex_lock(&queue->lock);
• old_status = atomic_xchg(&queue->status,

• 		 PANFROST_QUEUE_STATUS_STOPPED);
  

• if (old_status == PANFROST_QUEUE_STATUS_STOPPED)

• goto out;
  

• if (!atomic_read(&pfdev->reset.pending))

• return;
  

• /* Stop the schedulers.

• *
  

• * FIXME: We temporarily get out of the dma_fence_signalling section
  

• * because the cleanup path generate lockdep splats when taking locks
  

• * to release job resources. We should rework the code to follow this
  

• * pattern:
  

• *
  

• *	try_lock
  

• *	if (locked)
  

• *		release
  

• *	else
  

• *		schedule_work_to_release_later
  

• */
  

• for (i = 0; i < NUM_JOB_SLOTS; i++)

• drm_sched_stop(&pfdev->js->queue[i].sched, bad);
  

• cookie = dma_fence_begin_signalling();

• WARN_ON(old_status != PANFROST_QUEUE_STATUS_ACTIVE);

• drm_sched_stop(&queue->sched, bad); if (bad) drm_sched_increase_karma(bad);

• stopped = true;

• spin_lock(&pfdev->js->job_lock);
• for (i = 0; i < NUM_JOB_SLOTS; i++) {

• if (pfdev->jobs[i]) {
  

• 	pm_runtime_put_noidle(pfdev->dev);
  

• 	panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
  

• 	pfdev->jobs[i] = NULL;
  

• }
  

• }
• spin_unlock(&pfdev->js->job_lock);

• /*

• * Set the timeout to max so the timer doesn't get started
  

• * when we return from the timeout handler (restored in
  

• * panfrost_scheduler_start()).
  

• panfrost_device_reset(pfdev);
• /* GPU has been reset, we can cancel timeout/fault work that may have

• * been queued in the meantime and clear the reset pending bit.
  

  */

• queue->sched.timeout = MAX_SCHEDULE_TIMEOUT;

• atomic_set(&pfdev->reset.pending, 0);
• for (i = 0; i < NUM_JOB_SLOTS; i++)

• cancel_delayed_work(&pfdev->js->queue[i].sched.work_tdr);
  
  

-out:

• mutex_unlock(&queue->lock);

• return stopped;

-}

• /* Now resubmit jobs that were previously queued but didn't have a

• * chance to finish.
  

• * FIXME: We temporarily get out of the DMA fence signalling section
  

• * while resubmitting jobs because the job submission logic will
  

• * allocate memory with the GFP_KERNEL flag which can trigger memory
  

• * reclaim and exposes a lock ordering issue.
  

• */
  

• dma_fence_end_signalling(cookie);
• for (i = 0; i < NUM_JOB_SLOTS; i++)

• drm_sched_resubmit_jobs(&pfdev->js->queue[i].sched);
  

• cookie = dma_fence_begin_signalling();

-static void panfrost_scheduler_start(struct panfrost_queue_state *queue) -{

• enum panfrost_queue_status old_status;

• for (i = 0; i < NUM_JOB_SLOTS; i++)

• drm_sched_start(&pfdev->js->queue[i].sched, true);
  
  

• mutex_lock(&queue->lock);
• old_status = atomic_xchg(&queue->status,

• 		 PANFROST_QUEUE_STATUS_STARTING);
  

• WARN_ON(old_status != PANFROST_QUEUE_STATUS_STOPPED);
• /* Restore the original timeout before starting the scheduler. */
• queue->sched.timeout = msecs_to_jiffies(JOB_TIMEOUT_MS);
• drm_sched_resubmit_jobs(&queue->sched);
• drm_sched_start(&queue->sched, true);
• old_status = atomic_xchg(&queue->status,

• 		 PANFROST_QUEUE_STATUS_ACTIVE);
  

• if (old_status == PANFROST_QUEUE_STATUS_FAULT_PENDING)

• drm_sched_fault(&queue->sched);
  

• mutex_unlock(&queue->lock);

• dma_fence_end_signalling(cookie);

}

static enum drm_gpu_sched_stat panfrost_job_timedout(struct drm_sched_job @@ -454,11 +460,8 @@ static enum drm_gpu_sched_stat panfrost_job_timedout(struct drm_sched_job job_read(pfdev, JS_TAIL_LO(js)), sched_job);

• /* Scheduler is already stopped, nothing to do. */
• if (!panfrost_scheduler_stop(&pfdev->js->queue[js], sched_job))

• return DRM_GPU_SCHED_STAT_NOMINAL;
  

• panfrost_device_schedule_reset(pfdev);

• atomic_set(&pfdev->reset.pending, 1);

• panfrost_reset(pfdev, sched_job);

  return DRM_GPU_SCHED_STAT_NOMINAL;

} @@ -485,8 +488,6 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) job_write(pfdev, JOB_INT_CLEAR, mask);

if (status & JOB_INT_MASK_ERR(j)) {

• 	enum panfrost_queue_status old_status;
  

• job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_NOP);
  
  dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x",
  

@@ -494,19 +495,7 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) panfrost_exception_name(job_read(pfdev, JS_STATUS(j))), job_read(pfdev, JS_HEAD_LO(j)), job_read(pfdev, JS_TAIL_LO(j)));

• 	/*
  

• 	 * When the queue is being restarted we don't report
  

• 	 * faults directly to avoid races between the timeout
  

• 	 * and reset handlers. panfrost_scheduler_start() will
  

• 	 * call drm_sched_fault() after the queue has been
  

• 	 * started if status == FAULT_PENDING.
  

• 	 */
  

• 	old_status = atomic_cmpxchg(&pfdev->js->queue[j].status,
  

• 				    PANFROST_QUEUE_STATUS_STARTING,
  

• 				    PANFROST_QUEUE_STATUS_FAULT_PENDING);
  

• 	if (old_status == PANFROST_QUEUE_STATUS_ACTIVE)
  

• 		drm_sched_fault(&pfdev->js->queue[j].sched);
  

• 	drm_sched_fault(&pfdev->js->queue[j].sched);
  

  }

  if (status & JOB_INT_MASK_DONE(j)) {

@@ -561,56 +550,13 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) return IRQ_WAKE_THREAD; }

-static void panfrost_reset(struct work_struct *work) +static void panfrost_reset_work(struct work_struct *work) { struct panfrost_device *pfdev = container_of(work, struct panfrost_device, reset.work);

• unsigned int i;

• bool cookie;

• cookie = dma_fence_begin_signalling();

• for (i = 0; i < NUM_JOB_SLOTS; i++) {

• /*
  

•  * We want pending timeouts to be handled before we attempt
  

•  * to stop the scheduler. If we don't do that and the timeout
  

•  * handler is in flight, it might have removed the bad job
  

•  * from the list, and we'll lose this job if the reset handler
  

•  * enters the critical section in panfrost_scheduler_stop()
  

•  * before the timeout handler.
  

•  *
  

•  * Timeout is set to MAX_SCHEDULE_TIMEOUT - 1 because we need
  

•  * something big enough to make sure the timer will not expire
  

•  * before we manage to stop the scheduler, but we can't use
  

•  * MAX_SCHEDULE_TIMEOUT because drm_sched_get_cleanup_job()
  

•  * considers that as 'timer is not running' and will dequeue
  

•  * the job without making sure the timeout handler is not
  

•  * running.
  

•  */
  

• pfdev->js->queue[i].sched.timeout = MAX_SCHEDULE_TIMEOUT - 1;
  

• cancel_delayed_work_sync(&pfdev->js->queue[i].sched.work_tdr);
  

• panfrost_scheduler_stop(&pfdev->js->queue[i], NULL);
  

• }

• /* All timers have been stopped, we can safely reset the pending state. */

• atomic_set(&pfdev->reset.pending, 0);

• spin_lock(&pfdev->js->job_lock);

• for (i = 0; i < NUM_JOB_SLOTS; i++) {

• if (pfdev->jobs[i]) {
  

• 	pm_runtime_put_noidle(pfdev->dev);
  

• 	panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
  

• 	pfdev->jobs[i] = NULL;
  

• }
  

• }

• spin_unlock(&pfdev->js->job_lock);

• panfrost_device_reset(pfdev);

• for (i = 0; i < NUM_JOB_SLOTS; i++)

• panfrost_scheduler_start(&pfdev->js->queue[i]);
  

• dma_fence_end_signalling(cookie);

• panfrost_reset(pfdev, NULL);

}

int panfrost_job_init(struct panfrost_device *pfdev) @@ -618,7 +564,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) struct panfrost_job_slot *js; int ret, j, irq;

• INIT_WORK(&pfdev->reset.work, panfrost_reset);

• INIT_WORK(&pfdev->reset.work, panfrost_reset_work);

  pfdev->js = js = devm_kzalloc(pfdev->dev, sizeof(*js), GFP_KERNEL); if (!js)

@@ -640,15 +586,18 @@ int panfrost_job_init(struct panfrost_device *pfdev) return ret; }

• for (j = 0; j < NUM_JOB_SLOTS; j++) {

• mutex_init(&js->queue[j].lock);
  

• pfdev->reset.wq = alloc_ordered_workqueue("panfrost-reset", 0);

• if (!pfdev->reset.wq)

• return -ENOMEM;
  
  

• for (j = 0; j < NUM_JOB_SLOTS; j++) { js->queue[j].fence_context = dma_fence_context_alloc(1);

  ret = drm_sched_init(&js->queue[j].sched, &panfrost_sched_ops, 1, 0,

• 		     msecs_to_jiffies(JOB_TIMEOUT_MS), NULL,
  

• 		     msecs_to_jiffies(JOB_TIMEOUT_MS),
  

• 		     pfdev->reset.wq,
  	     NULL, "pan_js");
  

  if (ret) { dev_err(pfdev->dev, "Failed to create scheduler: %d.", ret);

@@ -664,6 +613,7 @@ int panfrost_job_init(struct panfrost_device *pfdev) for (j--; j >= 0; j--) drm_sched_fini(&js->queue[j].sched);

• destroy_workqueue(pfdev->reset.wq); return ret;

}

@@ -676,9 +626,10 @@ void panfrost_job_fini(struct panfrost_device *pfdev)

for (j = 0; j < NUM_JOB_SLOTS; j++) { drm_sched_fini(&js->queue[j].sched);

• mutex_destroy(&js->queue[j].lock);
  

  }

• cancel_work_sync(&pfdev->reset.work);
• destroy_workqueue(pfdev->reset.wq);

}

int panfrost_job_open(struct panfrost_file_priv *panfrost_priv)

On 28/06/2021 08:42, Boris Brezillon wrote:

This is not yet needed because we let active jobs be killed during by the reset and we don't really bother making sure they can be restarted. But once we start adding soft-stop support, controlling when we deal with the remaining interrrupts and making sure those are handled before the reset is issued gets tricky if we keep job interrupts active.

Let's prepare for that and mask+flush job IRQs before issuing a reset.

v4:

• Add a comment explaining why we WARN_ON(!job) in the irq handler
• Keep taking the job_lock when evicting stalled jobs

v3:

• New patch

Signed-off-by: Boris Brezillon boris.brezillon@collabora.com

Reviewed-by: Steven Price steven.price@arm.com

---

drivers/gpu/drm/panfrost/panfrost_job.c | 27 ++++++++++++++++++++----- 1 file changed, 22 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 98193a557a2d..4bd4d11377b7 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -34,6 +34,7 @@ struct panfrost_queue_state { struct panfrost_job_slot { struct panfrost_queue_state queue[NUM_JOB_SLOTS]; spinlock_t job_lock;

• int irq;

};

static struct panfrost_job * @@ -400,6 +401,15 @@ static void panfrost_reset(struct panfrost_device *pfdev, if (bad) drm_sched_increase_karma(bad);

• /* Mask job interrupts and synchronize to make sure we won't be

• * interrupted during our reset.
  

• */
  

• job_write(pfdev, JOB_INT_MASK, 0);
• synchronize_irq(pfdev->js->irq);
• /* Schedulers are stopped and interrupts are masked+flushed, we don't

• * need to protect the 'evict unfinished jobs' lock with the job_lock.
  

• */
  

  spin_lock(&pfdev->js->job_lock); for (i = 0; i < NUM_JOB_SLOTS; i++) { if (pfdev->jobs[i]) {

@@ -502,7 +512,14 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) struct panfrost_job *job;

	job = pfdev->jobs[j];

• 	/* Only NULL if job timeout occurred */
  

• 	/* The only reason this job could be NULL is if the
  

• 	 * job IRQ handler is called just after the
  

• 	 * in-flight job eviction in the reset path, and
  

• 	 * this shouldn't happen because the job IRQ has
  

• 	 * been masked and synchronized when this eviction
  

• 	 * happens.
  

• 	 */
  

• 	WARN_ON(!job);
  if (job) {
  	pfdev->jobs[j] = NULL;
  
  

@@ -562,7 +579,7 @@ static void panfrost_reset_work(struct work_struct *work) int panfrost_job_init(struct panfrost_device *pfdev) { struct panfrost_job_slot *js;

• int ret, j, irq;

• int ret, j;

  INIT_WORK(&pfdev->reset.work, panfrost_reset_work);

@@ -572,11 +589,11 @@ int panfrost_job_init(struct panfrost_device *pfdev)

spin_lock_init(&js->job_lock);

• irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "job");
• if (irq <= 0)

• js->irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "job");
• if (js->irq <= 0) return -ENODEV;

• ret = devm_request_threaded_irq(pfdev->dev, irq,

• ret = devm_request_threaded_irq(pfdev->dev, js->irq, panfrost_job_irq_handler, panfrost_job_irq_handler_thread, IRQF_SHARED, KBUILD_MODNAME "-job",

On 28/06/2021 08:42, Boris Brezillon wrote:

If we can recover from a fault without a reset there's no reason to issue one.

v3:

• Drop the mention of Valhall requiring a reset on JOB_BUS_FAULT
• Set the fence error to -EINVAL instead of having per-exception error codes

Signed-off-by: Boris Brezillon boris.brezillon@collabora.com

Reviewed-by: Steven Price steven.price@arm.com

---

drivers/gpu/drm/panfrost/panfrost_device.c | 9 +++++++++ drivers/gpu/drm/panfrost/panfrost_device.h | 2 ++ drivers/gpu/drm/panfrost/panfrost_job.c | 16 ++++++++++++++-- 3 files changed, 25 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.c b/drivers/gpu/drm/panfrost/panfrost_device.c index 736854542b05..f4e42009526d 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.c +++ b/drivers/gpu/drm/panfrost/panfrost_device.c @@ -379,6 +379,15 @@ const char *panfrost_exception_name(u32 exception_code) return panfrost_exception_infos[exception_code].name; }

+bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,

• 		    u32 exception_code)
  

+{

• /* Right now, none of the GPU we support need a reset, but this

• * might change.
  

• */
  

• return false;

+}

void panfrost_device_reset(struct panfrost_device *pfdev) { panfrost_gpu_soft_reset(pfdev); diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index 2dc8c0d1d987..d91f71366214 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -244,6 +244,8 @@ enum drm_panfrost_exception_type { };

const char *panfrost_exception_name(u32 exception_code); +bool panfrost_exception_needs_reset(const struct panfrost_device *pfdev,

• 		    u32 exception_code);
  
  

static inline void panfrost_device_schedule_reset(struct panfrost_device *pfdev) diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 4bd4d11377b7..b0f4857ca084 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -498,14 +498,26 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) job_write(pfdev, JOB_INT_CLEAR, mask);

if (status & JOB_INT_MASK_ERR(j)) {

• 	u32 js_status = job_read(pfdev, JS_STATUS(j));
  

• job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_NOP);
  
  dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x",
  	j,
  

• 		panfrost_exception_name(job_read(pfdev, JS_STATUS(j))),
  

• 		panfrost_exception_name(js_status),
  	job_read(pfdev, JS_HEAD_LO(j)),
  	job_read(pfdev, JS_TAIL_LO(j)));
  

• 	drm_sched_fault(&pfdev->js->queue[j].sched);
  

• 	/* If we need a reset, signal it to the timeout
  

• 	 * handler, otherwise, update the fence error field and
  

• 	 * signal the job fence.
  

• 	 */
  

• 	if (panfrost_exception_needs_reset(pfdev, js_status)) {
  

• 		drm_sched_fault(&pfdev->js->queue[j].sched);
  

• 	} else {
  

• 		dma_fence_set_error(pfdev->jobs[j]->done_fence, -EINVAL);
  

• 		status |= JOB_INT_MASK_DONE(j);
  

• 	}
  

  }

  if (status & JOB_INT_MASK_DONE(j)) {

On 28/06/2021 08:42, Boris Brezillon wrote:

If the process who submitted these jobs decided to close the FD before the jobs are done it probably means it doesn't care about the result.

v4:

• Don't disable/restore irqs when taking the job_lock (not needed since this lock is never taken from an interrupt context)

v3:

• Set fence error to ECANCELED when a TERMINATED exception is received

Signed-off-by: Boris Brezillon boris.brezillon@collabora.com

drivers/gpu/drm/panfrost/panfrost_job.c | 42 +++++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index b0f4857ca084..979108dbc323 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -499,14 +499,21 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status)

if (status & JOB_INT_MASK_ERR(j)) {
	u32 js_status = job_read(pfdev, JS_STATUS(j));

• 	const char *exception_name = panfrost_exception_name(js_status);
  
  job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_NOP);
  
  

• 	dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 		j,
  

• 		panfrost_exception_name(js_status),
  

• 		job_read(pfdev, JS_HEAD_LO(j)),
  

• 		job_read(pfdev, JS_TAIL_LO(j)));
  

• 	if (js_status < DRM_PANFROST_EXCEPTION_JOB_CONFIG_FAULT) {
  

I can see what your trying to do here, but the code isn't very readable (it's not clear what JOB_CONFIG_FAULT has to do with the decision).

I think there's two options here:

1. (In Midgard) Bits 7:6 are the "exception class" and are 0 for "non-fault status codes". So we could rewrite it as ((js_status & 0xC0) == 0) - or even better with appropriate macros.

2. Provide a macro definition for DRM_PANFROST_MAX_NON_FAULT_CODE which (at least currently) just happens to equal JOB_CONFIG_FAULT - 1 and use that instead.

(1) is nice, but sadly Bifrost doesn't define things in terms of exception class any more and the exception type is described as just an 8-bit enumeration. Of course we're entirely relying on any new non-fault status codes being ordered nicely, and option 1 and 2 are actually exactly the same check.

• 		dev_dbg(pfdev->dev, "js interrupt, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 			j, exception_name,
  

• 			job_read(pfdev, JS_HEAD_LO(j)),
  

• 			job_read(pfdev, JS_TAIL_LO(j)));
  

• 	} else {
  

• 		dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 			j, exception_name,
  

• 			job_read(pfdev, JS_HEAD_LO(j)),
  

• 			job_read(pfdev, JS_TAIL_LO(j)));
  

• 	}
  
  /* If we need a reset, signal it to the timeout
   * handler, otherwise, update the fence error field and
  

@@ -515,7 +522,16 @@ static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) if (panfrost_exception_needs_reset(pfdev, js_status)) { drm_sched_fault(&pfdev->js->queue[j].sched); } else {

• 		dma_fence_set_error(pfdev->jobs[j]->done_fence, -EINVAL);
  

• 		int error = 0;
  

• 		if (js_status == DRM_PANFROST_EXCEPTION_TERMINATED)
  

• 			error = -ECANCELED;
  

• 		else if (js_status >= DRM_PANFROST_EXCEPTION_JOB_CONFIG_FAULT)
  

As above.

• 			error = -EINVAL;
  

• 		if (error)
  

• 			dma_fence_set_error(pfdev->jobs[j]->done_fence, error);
  

• 	status |= JOB_INT_MASK_DONE(j);
  }
  

  }

@@ -681,10 +697,24 @@ int panfrost_job_open(struct panfrost_file_priv *panfrost_priv)

void panfrost_job_close(struct panfrost_file_priv *panfrost_priv) {

• struct panfrost_device *pfdev = panfrost_priv->pfdev; int i;

  for (i = 0; i < NUM_JOB_SLOTS; i++) drm_sched_entity_destroy(&panfrost_priv->sched_entity[i]);

• /* Kill in-flight jobs */

• spin_lock(&pfdev->js->job_lock);

• for (i = 0; i < NUM_JOB_SLOTS; i++) {

• struct drm_sched_entity *entity = &panfrost_priv->sched_entity[i];
  

• struct panfrost_job *job = pfdev->jobs[i];
  

• if (!job || job->base.entity != entity)
  

• 	continue;
  

• job_write(pfdev, JS_COMMAND(i), JS_COMMAND_HARD_STOP);
  

• }

• spin_unlock(&pfdev->js->job_lock);

}

int panfrost_job_is_idle(struct panfrost_device *pfdev)

On 28/06/2021 08:42, Boris Brezillon wrote:

From: Steven Price steven.price@arm.com

The hardware has a set of '_NEXT' registers that can hold a second job while the first is executing. Make use of these registers to enqueue a second job per slot.

v3:

• Fix the done/err job dequeuing logic to get a valid active state
• Only enable the second slot on GPUs supporting jobchain disambiguation
• Split interrupt handling in sub-functions

Signed-off-by: Steven Price steven.price@arm.com Signed-off-by: Boris Brezillon boris.brezillon@collabora.com

One nit below...

---

drivers/gpu/drm/panfrost/panfrost_device.h | 2 +- drivers/gpu/drm/panfrost/panfrost_job.c | 468 +++++++++++++++------ 2 files changed, 351 insertions(+), 119 deletions(-)

diff --git a/drivers/gpu/drm/panfrost/panfrost_device.h b/drivers/gpu/drm/panfrost/panfrost_device.h index d91f71366214..81f81fc8650e 100644 --- a/drivers/gpu/drm/panfrost/panfrost_device.h +++ b/drivers/gpu/drm/panfrost/panfrost_device.h @@ -101,7 +101,7 @@ struct panfrost_device {

struct panfrost_job_slot *js;

• struct panfrost_job *jobs[NUM_JOB_SLOTS];

• struct panfrost_job *jobs[NUM_JOB_SLOTS][2]; struct list_head scheduled_jobs;

  struct panfrost_perfcnt *perfcnt;

diff --git a/drivers/gpu/drm/panfrost/panfrost_job.c b/drivers/gpu/drm/panfrost/panfrost_job.c index 979108dbc323..b965669a1fed 100644 --- a/drivers/gpu/drm/panfrost/panfrost_job.c +++ b/drivers/gpu/drm/panfrost/panfrost_job.c @@ -4,6 +4,7 @@ #include <linux/delay.h> #include <linux/interrupt.h> #include <linux/io.h> +#include <linux/iopoll.h> #include <linux/platform_device.h> #include <linux/pm_runtime.h> #include <linux/dma-resv.h> @@ -140,9 +141,52 @@ static void panfrost_job_write_affinity(struct panfrost_device *pfdev, job_write(pfdev, JS_AFFINITY_NEXT_HI(js), affinity >> 32); }

+static u32 +panfrost_get_job_chain_flag(const struct panfrost_job *job) +{

• struct panfrost_fence *f = to_panfrost_fence(job->done_fence);
• if (!panfrost_has_hw_feature(job->pfdev, HW_FEATURE_JOBCHAIN_DISAMBIGUATION))

• return 0;
  

• return (f->seqno & 1) ? JS_CONFIG_JOB_CHAIN_FLAG : 0;

+}

+static struct panfrost_job * +panfrost_dequeue_job(struct panfrost_device *pfdev, int slot) +{

• struct panfrost_job *job = pfdev->jobs[slot][0];
• WARN_ON(!job);
• pfdev->jobs[slot][0] = pfdev->jobs[slot][1];
• pfdev->jobs[slot][1] = NULL;
• return job;

+}

+static unsigned int +panfrost_enqueue_job(struct panfrost_device *pfdev, int slot,

•      struct panfrost_job *job)
  

+{

• if (WARN_ON(!job))

• return 0;
  

• if (!pfdev->jobs[slot][0]) {

• pfdev->jobs[slot][0] = job;
  

• return 0;
  

• }
• WARN_ON(pfdev->jobs[slot][1]);
• pfdev->jobs[slot][1] = job;
• WARN_ON(panfrost_get_job_chain_flag(job) ==

• panfrost_get_job_chain_flag(pfdev->jobs[slot][0]));
  

• return 1;

+}

static void panfrost_job_hw_submit(struct panfrost_job *job, int js) { struct panfrost_device *pfdev = job->pfdev;

• unsigned int subslot; u32 cfg; u64 jc_head = job->jc; int ret;

@@ -168,7 +212,8 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) * start */ cfg |= JS_CONFIG_THREAD_PRI(8) | JS_CONFIG_START_FLUSH_CLEAN_INVALIDATE |

• JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE;
  

• JS_CONFIG_END_FLUSH_CLEAN_INVALIDATE |
  

• panfrost_get_job_chain_flag(job);
  
  

  if (panfrost_has_hw_feature(pfdev, HW_FEATURE_FLUSH_REDUCTION)) cfg |= JS_CONFIG_ENABLE_FLUSH_REDUCTION;

@@ -182,10 +227,17 @@ static void panfrost_job_hw_submit(struct panfrost_job *job, int js) job_write(pfdev, JS_FLUSH_ID_NEXT(js), job->flush_id);

/* GO ! */

• dev_dbg(pfdev->dev, "JS: Submitting atom %p to js[%d] with head=0x%llx",

• 		job, js, jc_head);
  
  

• job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START);

• spin_lock(&pfdev->js->job_lock);
• subslot = panfrost_enqueue_job(pfdev, js, job);
• /* Don't queue the job if a reset is in progress */
• if (!atomic_read(&pfdev->reset.pending)) {

• job_write(pfdev, JS_COMMAND_NEXT(js), JS_COMMAND_START);
  

• dev_dbg(pfdev->dev,
  

• 	"JS: Submitting atom %p to js[%d][%d] with head=0x%llx AS %d",
  

• 	job, js, subslot, jc_head, cfg & 0xf);
  

• }
• spin_unlock(&pfdev->js->job_lock);

}

static void panfrost_acquire_object_fences(struct drm_gem_object **bos, @@ -343,7 +395,11 @@ static struct dma_fence *panfrost_job_run(struct drm_sched_job *sched_job) if (unlikely(job->base.s_fence->finished.error)) return NULL;

• pfdev->jobs[slot] = job;

• /* Nothing to execute: can happen if the job has finished while

• * we were resetting the GPU.
  

• */
  

• if (!job->jc)

• return NULL;
  
  

  fence = panfrost_fence_create(pfdev, slot); if (IS_ERR(fence))

@@ -371,11 +427,218 @@ void panfrost_job_enable_interrupts(struct panfrost_device *pfdev) job_write(pfdev, JOB_INT_MASK, irq_mask); }

-static void panfrost_reset(struct panfrost_device *pfdev,

• 	   struct drm_sched_job *bad)
  

+static void panfrost_job_handle_err(struct panfrost_device *pfdev,

• 		    struct panfrost_job *job,
  

• 		    unsigned int js)
  

{

• unsigned int i;

• u32 js_status = job_read(pfdev, JS_STATUS(js));
• const char *exception_name = panfrost_exception_name(js_status);
• bool signal_fence = true;
• if (js_status < DRM_PANFROST_EXCEPTION_JOB_CONFIG_FAULT) {

• dev_dbg(pfdev->dev, "js event, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 	js, exception_name,
  

• 	job_read(pfdev, JS_HEAD_LO(js)),
  

• 	job_read(pfdev, JS_TAIL_LO(js)));
  

• } else {

• dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 	js, exception_name,
  

• 	job_read(pfdev, JS_HEAD_LO(js)),
  

• 	job_read(pfdev, JS_TAIL_LO(js)));
  

• }
• if (js_status == DRM_PANFROST_EXCEPTION_STOPPED) {

• /* Update the job head so we can resume */
  

• job->jc = job_read(pfdev, JS_TAIL_LO(js)) |
  

• 	  ((u64)job_read(pfdev, JS_TAIL_HI(js)) << 32);
  

• /* The job will be resumed, don't signal the fence */
  

• signal_fence = false;
  

• } else if (js_status == DRM_PANFROST_EXCEPTION_TERMINATED) {

• /* Job has been hard-stopped, flag it as canceled */
  

• dma_fence_set_error(job->done_fence, -ECANCELED);
  

• job->jc = 0;
  

• } else if (js_status >= DRM_PANFROST_EXCEPTION_JOB_CONFIG_FAULT) {

• /* We might want to provide finer-grained error code based on
  

•  * the exception type, but unconditionally setting to EINVAL
  

•  * is good enough for now.
  

•  */
  

• dma_fence_set_error(job->done_fence, -EINVAL);
  

• job->jc = 0;
  

• }
• panfrost_mmu_as_put(pfdev, job->file_priv->mmu);
• panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
• if (signal_fence)

• dma_fence_signal_locked(job->done_fence);
  

• pm_runtime_put_autosuspend(pfdev->dev);
• if (panfrost_exception_needs_reset(pfdev, js_status)) {

• atomic_set(&pfdev->reset.pending, 1);
  

• drm_sched_fault(&pfdev->js->queue[js].sched);
  

• }

+}

+static void panfrost_job_handle_done(struct panfrost_device *pfdev,

• 		     struct panfrost_job *job)
  

+{

• /* Set ->jc to 0 to avoid re-submitting an already finished job (can

• * happen when we receive the DONE interrupt while doing a GPU reset).
  

• */
  

• job->jc = 0;
• panfrost_mmu_as_put(pfdev, job->file_priv->mmu);
• panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
• dma_fence_signal_locked(job->done_fence);
• pm_runtime_put_autosuspend(pfdev->dev);

+}

+static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) +{

• struct panfrost_job *done[NUM_JOB_SLOTS][2] = {};
• struct panfrost_job *failed[NUM_JOB_SLOTS] = {};
• u32 js_state = 0, js_events = 0;
• unsigned int i, j;
• /* First we collect all failed/done jobs. */
• while (status) {

• u32 js_state_mask = 0;
  

• for (j = 0; j < NUM_JOB_SLOTS; j++) {
  

• 	if (status & MK_JS_MASK(j))
  

• 		js_state_mask |= MK_JS_MASK(j);
  

• 	if (status & JOB_INT_MASK_DONE(j)) {
  

• 		if (done[j][0])
  

• 			done[j][1] = panfrost_dequeue_job(pfdev, j);
  

• 		else
  

• 			done[j][0] = panfrost_dequeue_job(pfdev, j);
  

• 	}
  

• 	if (status & JOB_INT_MASK_ERR(j)) {
  

• 		/* Cancel the next submission. Will be submitted
  

• 		 * after we're done handling this failure if
  

• 		 * there's no reset pending.
  

• 		 */
  

• 		job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_NOP);
  

• 		failed[j] = panfrost_dequeue_job(pfdev, j);
  

• 	}
  

• }
  

• /* JS_STATE is sampled when JOB_INT_CLEAR is written.
  

•  * For each BIT(slot) or BIT(slot + 16) bit written to
  

•  * JOB_INT_CLEAR, the corresponding bits in JS_STATE
  

•  * (BIT(slot) and BIT(slot + 16)) are updated, but this
  

•  * is racy. If we only have one job done at the time we
  

•  * read JOB_INT_RAWSTAT but the second job fails before we
  

•  * clear the status, we end up with a status containing
  

•  * only the DONE bit and consider both jobs as DONE since
  

•  * JS_STATE reports both NEXT and CURRENT as inactive.
  

•  * To prevent that, let's repeat this clear+read steps
  

•  * until status is 0.
  

•  */
  

• job_write(pfdev, JOB_INT_CLEAR, status);
  

• js_state &= ~js_state_mask;
  

• js_state |= job_read(pfdev, JOB_INT_JS_STATE) & js_state_mask;
  

• js_events |= status;
  

• status = job_read(pfdev, JOB_INT_RAWSTAT);
  

• }
• /* Then we handle the dequeued jobs. */
• for (j = 0; j < NUM_JOB_SLOTS; j++) {

• if (!(js_events & MK_JS_MASK(j)))
  

• 	continue;
  

• if (failed[j]) {
  

• 	panfrost_job_handle_err(pfdev, failed[j], j);
  

• } else if (pfdev->jobs[j][0] && !(js_state & MK_JS_MASK(j))) {
  

• 	/* When the current job doesn't fail, the JM dequeues
  

• 	 * the next job without waiting for an ACK, this means
  

• 	 * we can have 2 jobs dequeued and only catch the
  

• 	 * interrupt when the second one is done. If both slots
  

• 	 * are inactive, but one job remains in pfdev->jobs[j],
  

• 	 * consider it done. Of course that doesn't apply if a
  

• 	 * failure happened since we cancelled execution of the
  

• 	 * job in _NEXT (see above).
  

• 	 */
  

• 	if (WARN_ON(!done[j][0]))
  

• 		done[j][0] = panfrost_dequeue_job(pfdev, j);
  

• 	else
  

• 		done[j][1] = panfrost_dequeue_job(pfdev, j);
  

• }
  

• for (i = 0; i < ARRAY_SIZE(done[0]) && done[j][i]; i++)
  

• 	panfrost_job_handle_done(pfdev, done[j][i]);
  

• }
• /* And finally we requeue jobs that were waiting in the second slot

• * and have been stopped if we detected a failure on the first slot.
  

• */
  

• for (j = 0; j < NUM_JOB_SLOTS; j++) {

• if (!(js_events & MK_JS_MASK(j)))
  

• 	continue;
  

• if (!failed[j] || !pfdev->jobs[j][0])
  

• 	continue;
  

• if (pfdev->jobs[j][0]->jc == 0) {
  

• 	/* The job was cancelled, signal the fence now */
  

• 	struct panfrost_job *canceled = panfrost_dequeue_job(pfdev, j);
  

• 	dma_fence_set_error(canceled->done_fence, -ECANCELED);
  

• 	panfrost_job_handle_done(pfdev, canceled);
  

• } else if (!atomic_read(&pfdev->reset.pending)) {
  

• 	/* Requeue the job we removed if no reset is pending */
  

• 	job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_START);
  

• }
  

• }

+}

+static void panfrost_job_handle_irqs(struct panfrost_device *pfdev) +{

• u32 status = job_read(pfdev, JOB_INT_RAWSTAT);
• while (status) {

• pm_runtime_mark_last_busy(pfdev->dev);
  

• spin_lock(&pfdev->js->job_lock);
  

• panfrost_job_handle_irq(pfdev, status);
  

• spin_unlock(&pfdev->js->job_lock);
  

• status = job_read(pfdev, JOB_INT_RAWSTAT);
  

• }

+}

+static u32 panfrost_active_slots(struct panfrost_device *pfdev,

• 		 u32 *js_state_mask, u32 js_state)
  

+{

• u32 rawstat;
• if (!(js_state & *js_state_mask))

• return 0;
  

• rawstat = job_read(pfdev, JOB_INT_RAWSTAT);
• if (rawstat) {

• unsigned int i;
  

• for (i = 0; i < NUM_JOB_SLOTS; i++) {
  

• 	if (rawstat & MK_JS_MASK(i))
  

• 		*js_state_mask &= ~MK_JS_MASK(i);
  

• }
  

• }
• return js_state & *js_state_mask;

+}

+static void +panfrost_reset(struct panfrost_device *pfdev,

•       struct drm_sched_job *bad)
  

+{

• u32 js_state, js_state_mask = 0xffffffff;

• unsigned int i, j; bool cookie;

• int ret;

  if (!atomic_read(&pfdev->reset.pending)) return;

@@ -407,21 +670,46 @@ static void panfrost_reset(struct panfrost_device *pfdev, job_write(pfdev, JOB_INT_MASK, 0); synchronize_irq(pfdev->js->irq);

• /* Schedulers are stopped and interrupts are masked+flushed, we don't

• * need to protect the 'evict unfinished jobs' lock with the job_lock.
  

• for (i = 0; i < NUM_JOB_SLOTS; i++) {

• /* Cancel the next job and soft-stop the running job. */
  

• job_write(pfdev, JS_COMMAND_NEXT(i), JS_COMMAND_NOP);
  

• job_write(pfdev, JS_COMMAND(i), JS_COMMAND_SOFT_STOP);
  

• }
• /* Wait at most 10ms for soft-stops to complete */
• ret = readl_poll_timeout(pfdev->iomem + JOB_INT_JS_STATE, js_state,

• 		 !panfrost_active_slots(pfdev, &js_state_mask, js_state),
  

• 		 10, 10000);
  

• if (ret)

• dev_err(pfdev->dev, "Soft-stop failed\n");
  

• /* Handle the remaining interrupts before we reset. */
• panfrost_job_handle_irqs(pfdev);
• /* Remaining interrupts have been handled, but we might still have

• * stuck jobs. Let's make sure the PM counters stay balanced by
  

• * manually calling pm_runtime_put_noidle() and
  

• * panfrost_devfreq_record_idle() for each stuck job.
  

  */ spin_lock(&pfdev->js->job_lock); for (i = 0; i < NUM_JOB_SLOTS; i++) {

• if (pfdev->jobs[i]) {
  

• for (j = 0; j < ARRAY_SIZE(pfdev->jobs[0]) && pfdev->jobs[i][j]; j++) {
  pm_runtime_put_noidle(pfdev->dev);
  panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
  

• 	pfdev->jobs[i] = NULL;
  

  } }

• memset(pfdev->jobs, 0, sizeof(pfdev->jobs)); spin_unlock(&pfdev->js->job_lock);

• /* Proceed with reset now. */ panfrost_device_reset(pfdev);

• /* panfrost_device_reset() unmasks job interrupts, but we want to

• * keep them masked a bit longer.
  

• */
  

• job_write(pfdev, JOB_INT_MASK, 0);

• /* GPU has been reset, we can cancel timeout/fault work that may have

  • been queued in the meantime and clear the reset pending bit.

  */

@@ -429,7 +717,6 @@ static void panfrost_reset(struct panfrost_device *pfdev, for (i = 0; i < NUM_JOB_SLOTS; i++) cancel_delayed_work(&pfdev->js->queue[i].sched.work_tdr);

• /* Now resubmit jobs that were previously queued but didn't have a
  • chance to finish.
  • FIXME: We temporarily get out of the DMA fence signalling section

@@ -442,9 +729,15 @@ static void panfrost_reset(struct panfrost_device *pfdev, drm_sched_resubmit_jobs(&pfdev->js->queue[i].sched); cookie = dma_fence_begin_signalling();

• /* Restart the schedulers */ for (i = 0; i < NUM_JOB_SLOTS; i++) drm_sched_start(&pfdev->js->queue[i].sched, true);

• /* Re-enable job interrupts now that everything has been restarted. */

• job_write(pfdev, JOB_INT_MASK,

•   GENMASK(16 + NUM_JOB_SLOTS - 1, 16) |
  

•   GENMASK(NUM_JOB_SLOTS - 1, 0));
  

• dma_fence_end_signalling(cookie);

}

@@ -476,6 +769,14 @@ static enum drm_gpu_sched_stat panfrost_job_timedout(struct drm_sched_job return DRM_GPU_SCHED_STAT_NOMINAL; }

+static void panfrost_reset_work(struct work_struct *work) +{

• struct panfrost_device *pfdev;
• pfdev = container_of(work, struct panfrost_device, reset.work);
• panfrost_reset(pfdev, NULL);

+}

static const struct drm_sched_backend_ops panfrost_sched_ops = { .dependency = panfrost_job_dependency, .run_job = panfrost_job_run, @@ -483,100 +784,11 @@ static const struct drm_sched_backend_ops panfrost_sched_ops = { .free_job = panfrost_job_free };

-static void panfrost_job_handle_irq(struct panfrost_device *pfdev, u32 status) -{

• int j;
• dev_dbg(pfdev->dev, "jobslot irq status=%x\n", status);
• for (j = 0; status; j++) {

• u32 mask = MK_JS_MASK(j);
  

• if (!(status & mask))
  

• 	continue;
  

• job_write(pfdev, JOB_INT_CLEAR, mask);
  

• if (status & JOB_INT_MASK_ERR(j)) {
  

• 	u32 js_status = job_read(pfdev, JS_STATUS(j));
  

• 	const char *exception_name = panfrost_exception_name(js_status);
  

• 	job_write(pfdev, JS_COMMAND_NEXT(j), JS_COMMAND_NOP);
  

• 	if (js_status < DRM_PANFROST_EXCEPTION_JOB_CONFIG_FAULT) {
  

• 		dev_dbg(pfdev->dev, "js interrupt, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 			j, exception_name,
  

• 			job_read(pfdev, JS_HEAD_LO(j)),
  

• 			job_read(pfdev, JS_TAIL_LO(j)));
  

• 	} else {
  

• 		dev_err(pfdev->dev, "js fault, js=%d, status=%s, head=0x%x, tail=0x%x",
  

• 			j, exception_name,
  

• 			job_read(pfdev, JS_HEAD_LO(j)),
  

• 			job_read(pfdev, JS_TAIL_LO(j)));
  

• 	}
  

• 	/* If we need a reset, signal it to the timeout
  

• 	 * handler, otherwise, update the fence error field and
  

• 	 * signal the job fence.
  

• 	 */
  

• 	if (panfrost_exception_needs_reset(pfdev, js_status)) {
  

• 		drm_sched_fault(&pfdev->js->queue[j].sched);
  

• 	} else {
  

• 		int error = 0;
  

• 		if (js_status == DRM_PANFROST_EXCEPTION_TERMINATED)
  

• 			error = -ECANCELED;
  

• 		else if (js_status >= DRM_PANFROST_EXCEPTION_JOB_CONFIG_FAULT)
  

• 			error = -EINVAL;
  

• 		if (error)
  

• 			dma_fence_set_error(pfdev->jobs[j]->done_fence, error);
  

• 		status |= JOB_INT_MASK_DONE(j);
  

• 	}
  

• }
  

• if (status & JOB_INT_MASK_DONE(j)) {
  

• 	struct panfrost_job *job;
  

• 	job = pfdev->jobs[j];
  

• 	/* The only reason this job could be NULL is if the
  

• 	 * job IRQ handler is called just after the
  

• 	 * in-flight job eviction in the reset path, and
  

• 	 * this shouldn't happen because the job IRQ has
  

• 	 * been masked and synchronized when this eviction
  

• 	 * happens.
  

• 	 */
  

• 	WARN_ON(!job);
  

• 	if (job) {
  

• 		pfdev->jobs[j] = NULL;
  

• 		panfrost_mmu_as_put(pfdev, job->file_priv->mmu);
  

• 		panfrost_devfreq_record_idle(&pfdev->pfdevfreq);
  

• 		dma_fence_signal_locked(job->done_fence);
  

• 		pm_runtime_put_autosuspend(pfdev->dev);
  

• 	}
  

• }
  

• status &= ~mask;
  

• }

-}

static irqreturn_t panfrost_job_irq_handler_thread(int irq, void *data) { struct panfrost_device *pfdev = data;

• u32 status = job_read(pfdev, JOB_INT_RAWSTAT);
• while (status) {

• pm_runtime_mark_last_busy(pfdev->dev);
  

• spin_lock(&pfdev->js->job_lock);
  

• panfrost_job_handle_irq(pfdev, status);
  

• spin_unlock(&pfdev->js->job_lock);
  

• status = job_read(pfdev, JOB_INT_RAWSTAT);
  

• }

• panfrost_job_handle_irqs(pfdev); job_write(pfdev, JOB_INT_MASK, GENMASK(16 + NUM_JOB_SLOTS - 1, 16) | GENMASK(NUM_JOB_SLOTS - 1, 0));

@@ -595,26 +807,24 @@ static irqreturn_t panfrost_job_irq_handler(int irq, void *data) return IRQ_WAKE_THREAD; }

-static void panfrost_reset_work(struct work_struct *work) -{

• struct panfrost_device *pfdev = container_of(work,

• 				     struct panfrost_device,
  

• 				     reset.work);
  

• panfrost_reset(pfdev, NULL);

-}

int panfrost_job_init(struct panfrost_device *pfdev) { struct panfrost_job_slot *js;

• unsigned int nslots = 2; int ret, j;

• INIT_WORK(&pfdev->reset.work, panfrost_reset_work);

• /* All GPUs have twnslotsueue, but without jobchain

^^^^^^^^^^^^ It's a made up word ;)

Also the use of "slot" here is confusing because it's referring to hw submission entries in the call to drm_sched_init() and not the "job slots" that the HW has (Mali GPUs have 3 slots).

Thanks,

Steve

• * disambiguation stopping the right job in the close path is tricky,
  

• * so let's just advertize one slot in that case.
  

• */
  

• if (!panfrost_has_hw_feature(pfdev, HW_FEATURE_JOBCHAIN_DISAMBIGUATION))

• nslots = 1;
  
  

  pfdev->js = js = devm_kzalloc(pfdev->dev, sizeof(*js), GFP_KERNEL); if (!js) return -ENOMEM;

• INIT_WORK(&pfdev->reset.work, panfrost_reset_work); spin_lock_init(&js->job_lock);

  js->irq = platform_get_irq_byname(to_platform_device(pfdev->dev), "job");

@@ -640,7 +850,7 @@ int panfrost_job_init(struct panfrost_device *pfdev)

ret = drm_sched_init(&js->queue[j].sched,
		     &panfrost_sched_ops,

• 		     1, 0,
  

• 		     nslots, 0,
  	     msecs_to_jiffies(JOB_TIMEOUT_MS),
  	     pfdev->reset.wq,
  	     NULL, "pan_js");
  

@@ -707,12 +917,34 @@ void panfrost_job_close(struct panfrost_file_priv *panfrost_priv) spin_lock(&pfdev->js->job_lock); for (i = 0; i < NUM_JOB_SLOTS; i++) { struct drm_sched_entity *entity = &panfrost_priv->sched_entity[i];

• struct panfrost_job *job = pfdev->jobs[i];
  

• int j;
  
  

• if (!job || job->base.entity != entity)
  

• 	continue;
  

• for (j = ARRAY_SIZE(pfdev->jobs[0]) - 1; j >= 0; j--) {
  

• 	struct panfrost_job *job = pfdev->jobs[i][j];
  

• 	u32 cmd;
  
  

• job_write(pfdev, JS_COMMAND(i), JS_COMMAND_HARD_STOP);
  

• 	if (!job || job->base.entity != entity)
  

• 		continue;
  

• 	if (j == 1) {
  

• 		/* Try to cancel the job before it starts */
  

• 		job_write(pfdev, JS_COMMAND_NEXT(i), JS_COMMAND_NOP);
  

• 		/* Reset the job head so it doesn't get restarted if
  

• 		 * the job in the first slot failed.
  

• 		 */
  

• 		job->jc = 0;
  

• 	}
  

• 	if (panfrost_has_hw_feature(pfdev, HW_FEATURE_JOBCHAIN_DISAMBIGUATION)) {
  

• 		cmd = panfrost_get_job_chain_flag(job) ?
  

• 		      JS_COMMAND_HARD_STOP_1 :
  

• 		      JS_COMMAND_HARD_STOP_0;
  

• 	} else {
  

• 		cmd = JS_COMMAND_HARD_STOP;
  

• 	}
  

• 	job_write(pfdev, JS_COMMAND(i), cmd);
  

• }
  

  } spin_unlock(&pfdev->js->job_lock);

}