| Message ID | 20200101103831.22429-1-yuq825@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/lima: use drm_sched_fault for error task handling | expand |
On Wed, Jan 1, 2020 at 2:39 AM Qiang Yu <yuq825@gmail.com> wrote: > > drm_sched_job_timedout works with drm_sched_stop as a pair, > so we'd better use the drm_sched_fault helper to make the > error and timeout handling go the same path. > > This also fixes application hang when task error. > > Signed-off-by: Qiang Yu <yuq825@gmail.com> LGTM in general. Reviewed-by: Vasily Khoruzhick <anarsoul@gmail.com> Erico, Andreas, could you test this patch on actual hardware? I'll have pretty limited access to the hardware for next few weeks, so I won't be able to test it myself. > --- > drivers/gpu/drm/lima/lima_sched.c | 35 ++++++++----------------------- > drivers/gpu/drm/lima/lima_sched.h | 2 -- > 2 files changed, 9 insertions(+), 28 deletions(-) > > diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c > index f522c5f99729..a31a90c380b6 100644 > --- a/drivers/gpu/drm/lima/lima_sched.c > +++ b/drivers/gpu/drm/lima/lima_sched.c > @@ -255,13 +255,17 @@ static struct dma_fence *lima_sched_run_job(struct drm_sched_job *job) > return task->fence; > } > > -static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, > - struct lima_sched_task *task) > +static void lima_sched_timedout_job(struct drm_sched_job *job) > { > + struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); > + struct lima_sched_task *task = to_lima_task(job); > + > + if (!pipe->error) > + DRM_ERROR("lima job timeout\n"); > + > drm_sched_stop(&pipe->base, &task->base); > > - if (task) > - drm_sched_increase_karma(&task->base); > + drm_sched_increase_karma(&task->base); > > pipe->task_error(pipe); > > @@ -284,16 +288,6 @@ static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, > drm_sched_start(&pipe->base, true); > } > > -static void lima_sched_timedout_job(struct drm_sched_job *job) > -{ > - struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); > - struct lima_sched_task *task = to_lima_task(job); > - > - DRM_ERROR("lima job timeout\n"); > - > - lima_sched_handle_error_task(pipe, task); > -} > - > static void lima_sched_free_job(struct drm_sched_job *job) > { > struct lima_sched_task *task = to_lima_task(job); > @@ -318,15 +312,6 @@ static const struct drm_sched_backend_ops lima_sched_ops = { > .free_job = lima_sched_free_job, > }; > > -static void lima_sched_error_work(struct work_struct *work) > -{ > - struct lima_sched_pipe *pipe = > - container_of(work, struct lima_sched_pipe, error_work); > - struct lima_sched_task *task = pipe->current_task; > - > - lima_sched_handle_error_task(pipe, task); > -} > - > int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) > { > unsigned int timeout = lima_sched_timeout_ms > 0 ? > @@ -335,8 +320,6 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) > pipe->fence_context = dma_fence_context_alloc(1); > spin_lock_init(&pipe->fence_lock); > > - INIT_WORK(&pipe->error_work, lima_sched_error_work); > - > return drm_sched_init(&pipe->base, &lima_sched_ops, 1, 0, > msecs_to_jiffies(timeout), name); > } > @@ -349,7 +332,7 @@ void lima_sched_pipe_fini(struct lima_sched_pipe *pipe) > void lima_sched_pipe_task_done(struct lima_sched_pipe *pipe) > { > if (pipe->error) > - schedule_work(&pipe->error_work); > + drm_sched_fault(&pipe->base); > else { > struct lima_sched_task *task = pipe->current_task; > > diff --git a/drivers/gpu/drm/lima/lima_sched.h b/drivers/gpu/drm/lima/lima_sched.h > index 928af91c1118..1d814fecbcc0 100644 > --- a/drivers/gpu/drm/lima/lima_sched.h > +++ b/drivers/gpu/drm/lima/lima_sched.h > @@ -68,8 +68,6 @@ struct lima_sched_pipe { > void (*task_fini)(struct lima_sched_pipe *pipe); > void (*task_error)(struct lima_sched_pipe *pipe); > void (*task_mmu_error)(struct lima_sched_pipe *pipe); > - > - struct work_struct error_work; > }; > > int lima_sched_task_init(struct lima_sched_task *task, > -- > 2.17.1 >
Am 03.01.2020 um 06:46 schrieb Vasily Khoruzhick: > On Wed, Jan 1, 2020 at 2:39 AM Qiang Yu <yuq825@gmail.com> wrote: >> drm_sched_job_timedout works with drm_sched_stop as a pair, >> so we'd better use the drm_sched_fault helper to make the >> error and timeout handling go the same path. >> >> This also fixes application hang when task error. >> >> Signed-off-by: Qiang Yu <yuq825@gmail.com> > LGTM in general. > > Reviewed-by: Vasily Khoruzhick <anarsoul@gmail.com> > > Erico, Andreas, could you test this patch on actual hardware? I'll > have pretty limited access to the hardware for next few weeks, so I > won't be able to test it myself. I've tested that one on top of a recent kernel (3a562aee727a) on a Mali450/ Allwinner H5 device with deqp and got no regressions and kernel issues. So... Tested-by: Andreas Baierl <ichgeh@imkreisrum.de> >> --- >> drivers/gpu/drm/lima/lima_sched.c | 35 ++++++++----------------------- >> drivers/gpu/drm/lima/lima_sched.h | 2 -- >> 2 files changed, 9 insertions(+), 28 deletions(-) >> >> diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c >> index f522c5f99729..a31a90c380b6 100644 >> --- a/drivers/gpu/drm/lima/lima_sched.c >> +++ b/drivers/gpu/drm/lima/lima_sched.c >> @@ -255,13 +255,17 @@ static struct dma_fence *lima_sched_run_job(struct drm_sched_job *job) >> return task->fence; >> } >> >> -static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, >> - struct lima_sched_task *task) >> +static void lima_sched_timedout_job(struct drm_sched_job *job) >> { >> + struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); >> + struct lima_sched_task *task = to_lima_task(job); >> + >> + if (!pipe->error) >> + DRM_ERROR("lima job timeout\n"); >> + >> drm_sched_stop(&pipe->base, &task->base); >> >> - if (task) >> - drm_sched_increase_karma(&task->base); >> + drm_sched_increase_karma(&task->base); >> >> pipe->task_error(pipe); >> >> @@ -284,16 +288,6 @@ static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, >> drm_sched_start(&pipe->base, true); >> } >> >> -static void lima_sched_timedout_job(struct drm_sched_job *job) >> -{ >> - struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); >> - struct lima_sched_task *task = to_lima_task(job); >> - >> - DRM_ERROR("lima job timeout\n"); >> - >> - lima_sched_handle_error_task(pipe, task); >> -} >> - >> static void lima_sched_free_job(struct drm_sched_job *job) >> { >> struct lima_sched_task *task = to_lima_task(job); >> @@ -318,15 +312,6 @@ static const struct drm_sched_backend_ops lima_sched_ops = { >> .free_job = lima_sched_free_job, >> }; >> >> -static void lima_sched_error_work(struct work_struct *work) >> -{ >> - struct lima_sched_pipe *pipe = >> - container_of(work, struct lima_sched_pipe, error_work); >> - struct lima_sched_task *task = pipe->current_task; >> - >> - lima_sched_handle_error_task(pipe, task); >> -} >> - >> int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) >> { >> unsigned int timeout = lima_sched_timeout_ms > 0 ? >> @@ -335,8 +320,6 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) >> pipe->fence_context = dma_fence_context_alloc(1); >> spin_lock_init(&pipe->fence_lock); >> >> - INIT_WORK(&pipe->error_work, lima_sched_error_work); >> - >> return drm_sched_init(&pipe->base, &lima_sched_ops, 1, 0, >> msecs_to_jiffies(timeout), name); >> } >> @@ -349,7 +332,7 @@ void lima_sched_pipe_fini(struct lima_sched_pipe *pipe) >> void lima_sched_pipe_task_done(struct lima_sched_pipe *pipe) >> { >> if (pipe->error) >> - schedule_work(&pipe->error_work); >> + drm_sched_fault(&pipe->base); >> else { >> struct lima_sched_task *task = pipe->current_task; >> >> diff --git a/drivers/gpu/drm/lima/lima_sched.h b/drivers/gpu/drm/lima/lima_sched.h >> index 928af91c1118..1d814fecbcc0 100644 >> --- a/drivers/gpu/drm/lima/lima_sched.h >> +++ b/drivers/gpu/drm/lima/lima_sched.h >> @@ -68,8 +68,6 @@ struct lima_sched_pipe { >> void (*task_fini)(struct lima_sched_pipe *pipe); >> void (*task_error)(struct lima_sched_pipe *pipe); >> void (*task_mmu_error)(struct lima_sched_pipe *pipe); >> - >> - struct work_struct error_work; >> }; >> >> int lima_sched_task_init(struct lima_sched_task *task, >> -- >> 2.17.1 >> > _______________________________________________ > dri-devel mailing list > dri-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Thanks, applied to drm-misc-next. Regards, Qiang On Tue, Jan 7, 2020 at 4:16 PM Andreas Baierl <list@imkreisrum.de> wrote: > > Am 03.01.2020 um 06:46 schrieb Vasily Khoruzhick: > > On Wed, Jan 1, 2020 at 2:39 AM Qiang Yu <yuq825@gmail.com> wrote: > >> drm_sched_job_timedout works with drm_sched_stop as a pair, > >> so we'd better use the drm_sched_fault helper to make the > >> error and timeout handling go the same path. > >> > >> This also fixes application hang when task error. > >> > >> Signed-off-by: Qiang Yu <yuq825@gmail.com> > > LGTM in general. > > > > Reviewed-by: Vasily Khoruzhick <anarsoul@gmail.com> > > > > Erico, Andreas, could you test this patch on actual hardware? I'll > > have pretty limited access to the hardware for next few weeks, so I > > won't be able to test it myself. > I've tested that one on top of a recent kernel (3a562aee727a) on a > Mali450/ Allwinner H5 device with deqp and got no regressions and kernel > issues. > So... > > Tested-by: Andreas Baierl <ichgeh@imkreisrum.de> > >> --- > >> drivers/gpu/drm/lima/lima_sched.c | 35 ++++++++----------------------- > >> drivers/gpu/drm/lima/lima_sched.h | 2 -- > >> 2 files changed, 9 insertions(+), 28 deletions(-) > >> > >> diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c > >> index f522c5f99729..a31a90c380b6 100644 > >> --- a/drivers/gpu/drm/lima/lima_sched.c > >> +++ b/drivers/gpu/drm/lima/lima_sched.c > >> @@ -255,13 +255,17 @@ static struct dma_fence *lima_sched_run_job(struct drm_sched_job *job) > >> return task->fence; > >> } > >> > >> -static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, > >> - struct lima_sched_task *task) > >> +static void lima_sched_timedout_job(struct drm_sched_job *job) > >> { > >> + struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); > >> + struct lima_sched_task *task = to_lima_task(job); > >> + > >> + if (!pipe->error) > >> + DRM_ERROR("lima job timeout\n"); > >> + > >> drm_sched_stop(&pipe->base, &task->base); > >> > >> - if (task) > >> - drm_sched_increase_karma(&task->base); > >> + drm_sched_increase_karma(&task->base); > >> > >> pipe->task_error(pipe); > >> > >> @@ -284,16 +288,6 @@ static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, > >> drm_sched_start(&pipe->base, true); > >> } > >> > >> -static void lima_sched_timedout_job(struct drm_sched_job *job) > >> -{ > >> - struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); > >> - struct lima_sched_task *task = to_lima_task(job); > >> - > >> - DRM_ERROR("lima job timeout\n"); > >> - > >> - lima_sched_handle_error_task(pipe, task); > >> -} > >> - > >> static void lima_sched_free_job(struct drm_sched_job *job) > >> { > >> struct lima_sched_task *task = to_lima_task(job); > >> @@ -318,15 +312,6 @@ static const struct drm_sched_backend_ops lima_sched_ops = { > >> .free_job = lima_sched_free_job, > >> }; > >> > >> -static void lima_sched_error_work(struct work_struct *work) > >> -{ > >> - struct lima_sched_pipe *pipe = > >> - container_of(work, struct lima_sched_pipe, error_work); > >> - struct lima_sched_task *task = pipe->current_task; > >> - > >> - lima_sched_handle_error_task(pipe, task); > >> -} > >> - > >> int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) > >> { > >> unsigned int timeout = lima_sched_timeout_ms > 0 ? > >> @@ -335,8 +320,6 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) > >> pipe->fence_context = dma_fence_context_alloc(1); > >> spin_lock_init(&pipe->fence_lock); > >> > >> - INIT_WORK(&pipe->error_work, lima_sched_error_work); > >> - > >> return drm_sched_init(&pipe->base, &lima_sched_ops, 1, 0, > >> msecs_to_jiffies(timeout), name); > >> } > >> @@ -349,7 +332,7 @@ void lima_sched_pipe_fini(struct lima_sched_pipe *pipe) > >> void lima_sched_pipe_task_done(struct lima_sched_pipe *pipe) > >> { > >> if (pipe->error) > >> - schedule_work(&pipe->error_work); > >> + drm_sched_fault(&pipe->base); > >> else { > >> struct lima_sched_task *task = pipe->current_task; > >> > >> diff --git a/drivers/gpu/drm/lima/lima_sched.h b/drivers/gpu/drm/lima/lima_sched.h > >> index 928af91c1118..1d814fecbcc0 100644 > >> --- a/drivers/gpu/drm/lima/lima_sched.h > >> +++ b/drivers/gpu/drm/lima/lima_sched.h > >> @@ -68,8 +68,6 @@ struct lima_sched_pipe { > >> void (*task_fini)(struct lima_sched_pipe *pipe); > >> void (*task_error)(struct lima_sched_pipe *pipe); > >> void (*task_mmu_error)(struct lima_sched_pipe *pipe); > >> - > >> - struct work_struct error_work; > >> }; > >> > >> int lima_sched_task_init(struct lima_sched_task *task, > >> -- > >> 2.17.1 > >> > > _______________________________________________ > > dri-devel mailing list > > dri-devel@lists.freedesktop.org > > https://lists.freedesktop.org/mailman/listinfo/dri-devel >
diff --git a/drivers/gpu/drm/lima/lima_sched.c b/drivers/gpu/drm/lima/lima_sched.c index f522c5f99729..a31a90c380b6 100644 --- a/drivers/gpu/drm/lima/lima_sched.c +++ b/drivers/gpu/drm/lima/lima_sched.c @@ -255,13 +255,17 @@ static struct dma_fence *lima_sched_run_job(struct drm_sched_job *job) return task->fence; } -static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, - struct lima_sched_task *task) +static void lima_sched_timedout_job(struct drm_sched_job *job) { + struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); + struct lima_sched_task *task = to_lima_task(job); + + if (!pipe->error) + DRM_ERROR("lima job timeout\n"); + drm_sched_stop(&pipe->base, &task->base); - if (task) - drm_sched_increase_karma(&task->base); + drm_sched_increase_karma(&task->base); pipe->task_error(pipe); @@ -284,16 +288,6 @@ static void lima_sched_handle_error_task(struct lima_sched_pipe *pipe, drm_sched_start(&pipe->base, true); } -static void lima_sched_timedout_job(struct drm_sched_job *job) -{ - struct lima_sched_pipe *pipe = to_lima_pipe(job->sched); - struct lima_sched_task *task = to_lima_task(job); - - DRM_ERROR("lima job timeout\n"); - - lima_sched_handle_error_task(pipe, task); -} - static void lima_sched_free_job(struct drm_sched_job *job) { struct lima_sched_task *task = to_lima_task(job); @@ -318,15 +312,6 @@ static const struct drm_sched_backend_ops lima_sched_ops = { .free_job = lima_sched_free_job, }; -static void lima_sched_error_work(struct work_struct *work) -{ - struct lima_sched_pipe *pipe = - container_of(work, struct lima_sched_pipe, error_work); - struct lima_sched_task *task = pipe->current_task; - - lima_sched_handle_error_task(pipe, task); -} - int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) { unsigned int timeout = lima_sched_timeout_ms > 0 ? @@ -335,8 +320,6 @@ int lima_sched_pipe_init(struct lima_sched_pipe *pipe, const char *name) pipe->fence_context = dma_fence_context_alloc(1); spin_lock_init(&pipe->fence_lock); - INIT_WORK(&pipe->error_work, lima_sched_error_work); - return drm_sched_init(&pipe->base, &lima_sched_ops, 1, 0, msecs_to_jiffies(timeout), name); } @@ -349,7 +332,7 @@ void lima_sched_pipe_fini(struct lima_sched_pipe *pipe) void lima_sched_pipe_task_done(struct lima_sched_pipe *pipe) { if (pipe->error) - schedule_work(&pipe->error_work); + drm_sched_fault(&pipe->base); else { struct lima_sched_task *task = pipe->current_task; diff --git a/drivers/gpu/drm/lima/lima_sched.h b/drivers/gpu/drm/lima/lima_sched.h index 928af91c1118..1d814fecbcc0 100644 --- a/drivers/gpu/drm/lima/lima_sched.h +++ b/drivers/gpu/drm/lima/lima_sched.h @@ -68,8 +68,6 @@ struct lima_sched_pipe { void (*task_fini)(struct lima_sched_pipe *pipe); void (*task_error)(struct lima_sched_pipe *pipe); void (*task_mmu_error)(struct lima_sched_pipe *pipe); - - struct work_struct error_work; }; int lima_sched_task_init(struct lima_sched_task *task,
drm_sched_job_timedout works with drm_sched_stop as a pair, so we'd better use the drm_sched_fault helper to make the error and timeout handling go the same path. This also fixes application hang when task error. Signed-off-by: Qiang Yu <yuq825@gmail.com> --- drivers/gpu/drm/lima/lima_sched.c | 35 ++++++++----------------------- drivers/gpu/drm/lima/lima_sched.h | 2 -- 2 files changed, 9 insertions(+), 28 deletions(-)