Re: [PATCH] drm/lima: use drm_sched_fault for error task handling
Thanks, applied to drm-misc-next.
Regards,
Qiang
On Tue, Jan 7, 2020 at 4:16 PM Andreas Baierl wrote:
>
> Am 03.01.2020 um 06:46 schrieb Vasily Khoruzhick:
> > On Wed, Jan 1, 2020 at 2:39 AM Qiang Yu 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
> > LGTM in general.
> >
> > Reviewed-by: Vasily Khoruzhick
> >
> > 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
> >> ---
> >> 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
> >>
> > ___
Re: [PATCH] drm/lima: use drm_sched_fault for error task handling
Am 03.01.2020 um 06:46 schrieb Vasily Khoruzhick:
On Wed, Jan 1, 2020 at 2:39 AM Qiang Yu 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
LGTM in general.
Reviewed-by: Vasily Khoruzhick
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
---
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
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
Re: [PATCH] drm/lima: use drm_sched_fault for error task handling
On Wed, Jan 1, 2020 at 2:39 AM Qiang Yu 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
LGTM in general.
Reviewed-by: Vasily Khoruzhick
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
>
___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel
[PATCH] drm/lima: use drm_sched_fault for error task handling
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
---
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
