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.brezil...@collabora.com>

Reviewed-by: Lucas Stach <l.st...@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);


Reply via email to