[PATCH 1/8] drm/etnaviv: move down etnaviv_gpu_recover_hang() in file

[PATCH 1/8] drm/etnaviv: move down etnaviv_gpu_recover_hang() in file

[Lucas Stach]

So it can use the event_free function without adding another
forward declaration. No functional change.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 88 +++++++++++++--------------
 1 file changed, 44 insertions(+), 44 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index de8c9894967c..6d4df9f1aeff 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1059,50 +1059,6 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m)
 }
 #endif
 
-void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
-{
-	struct etnaviv_gpu *gpu = submit->gpu;
-	char *comm = NULL, *cmd = NULL;
-	struct task_struct *task;
-	unsigned int i;
-
-	dev_err(gpu->dev, "recover hung GPU!\n");
-
-	task = get_pid_task(submit->pid, PIDTYPE_PID);
-	if (task) {
-		comm = kstrdup(task->comm, GFP_KERNEL);
-		cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
-		put_task_struct(task);
-	}
-
-	if (comm && cmd)
-		dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
-
-	kfree(cmd);
-	kfree(comm);
-
-	if (pm_runtime_get_sync(gpu->dev) < 0)
-		goto pm_put;
-
-	mutex_lock(&gpu->lock);
-
-	etnaviv_hw_reset(gpu);
-
-	/* complete all events, the GPU won't do it after the reset */
-	spin_lock(&gpu->event_spinlock);
-	for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
-		complete(&gpu->event_free);
-	bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
-	spin_unlock(&gpu->event_spinlock);
-
-	etnaviv_gpu_hw_init(gpu);
-
-	mutex_unlock(&gpu->lock);
-	pm_runtime_mark_last_busy(gpu->dev);
-pm_put:
-	pm_runtime_put_autosuspend(gpu->dev);
-}
-
 /* fence object management */
 struct etnaviv_fence {
 	struct etnaviv_gpu *gpu;
@@ -1454,6 +1410,50 @@ static void sync_point_worker(struct work_struct *work)
 	etnaviv_gpu_start_fe(gpu, addr + 2, 2);
 }
 
+void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
+{
+	struct etnaviv_gpu *gpu = submit->gpu;
+	char *comm = NULL, *cmd = NULL;
+	struct task_struct *task;
+	unsigned int i;
+
+	dev_err(gpu->dev, "recover hung GPU!\n");
+
+	task = get_pid_task(submit->pid, PIDTYPE_PID);
+	if (task) {
+		comm = kstrdup(task->comm, GFP_KERNEL);
+		cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
+		put_task_struct(task);
+	}
+
+	if (comm && cmd)
+		dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
+
+	kfree(cmd);
+	kfree(comm);
+
+	if (pm_runtime_get_sync(gpu->dev) < 0)
+		goto pm_put;
+
+	mutex_lock(&gpu->lock);
+
+	etnaviv_hw_reset(gpu);
+
+	/* complete all events, the GPU won't do it after the reset */
+	spin_lock(&gpu->event_spinlock);
+	for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
+		complete(&gpu->event_free);
+	bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
+	spin_unlock(&gpu->event_spinlock);
+
+	etnaviv_gpu_hw_init(gpu);
+
+	mutex_unlock(&gpu->lock);
+	pm_runtime_mark_last_busy(gpu->dev);
+pm_put:
+	pm_runtime_put_autosuspend(gpu->dev);
+}
+
 static void dump_mmu_fault(struct etnaviv_gpu *gpu)
 {
 	static const char *fault_reasons[] = {
-- 
2.39.2

[PATCH 2/8] drm/etnaviv: free events the usual way in recover worker

[Lucas Stach]

Clearing the whole bitmap at once is only a minor optimization in
a path that should be extremely cold. Free the events by calling
event_free() instead of directly manipulating the completion count
and event bitmap.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 6d4df9f1aeff..4e18aa8566c6 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1442,8 +1442,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
 	/* complete all events, the GPU won't do it after the reset */
 	spin_lock(&gpu->event_spinlock);
 	for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
-		complete(&gpu->event_free);
-	bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
+		event_free(gpu, i);
 	spin_unlock(&gpu->event_spinlock);
 
 	etnaviv_gpu_hw_init(gpu);
-- 
2.39.2

Re: [PATCH 2/8] drm/etnaviv: free events the usual way in recover worker

[Christian Gmeiner]

Hi Lucas

>
> Clearing the whole bitmap at once is only a minor optimization in
> a path that should be extremely cold. Free the events by calling
> event_free() instead of directly manipulating the completion count
> and event bitmap.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 6d4df9f1aeff..4e18aa8566c6 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1442,8 +1442,7 @@ void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
>         /* complete all events, the GPU won't do it after the reset */
>         spin_lock(&gpu->event_spinlock);
>         for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
> -               complete(&gpu->event_free);
> -       bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
> +               event_free(gpu, i);
>         spin_unlock(&gpu->event_spinlock);
>
>         etnaviv_gpu_hw_init(gpu);
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

[PATCH 3/8] drm/etnaviv: move runtime PM handling to events

[Lucas Stach]

Conceptually events are the right abstraction to handle the GPU
runtime PM state: as long as any event is pending the GPU can not
be idle. Events are also properly freed and reallocated when the
GPU has been reset after a hang.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  1 -
 drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  3 ---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c        | 27 ++++++++++++--------
 3 files changed, 16 insertions(+), 15 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
index baa81cbf701a..a42d260cac2c 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
@@ -97,7 +97,6 @@ struct etnaviv_gem_submit {
 	struct list_head node; /* GPU active submit list */
 	struct etnaviv_cmdbuf cmdbuf;
 	struct pid *pid;       /* submitting process */
-	bool runtime_resumed;
 	u32 exec_state;
 	u32 flags;
 	unsigned int nr_pmrs;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
index 45403ea38906..2416c526f9b0 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
@@ -362,9 +362,6 @@ static void submit_cleanup(struct kref *kref)
 			container_of(kref, struct etnaviv_gem_submit, refcount);
 	unsigned i;
 
-	if (submit->runtime_resumed)
-		pm_runtime_put_autosuspend(submit->gpu->dev);
-
 	if (submit->cmdbuf.suballoc)
 		etnaviv_cmdbuf_free(&submit->cmdbuf);
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 4e18aa8566c6..54a1249c5bca 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1139,7 +1139,8 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
 	unsigned int *events)
 {
 	unsigned long timeout = msecs_to_jiffies(10 * 10000);
-	unsigned i, acquired = 0;
+	unsigned i, acquired = 0, rpm_count = 0;
+	int ret;
 
 	for (i = 0; i < nr_events; i++) {
 		unsigned long ret;
@@ -1148,6 +1149,7 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
 
 		if (!ret) {
 			dev_err(gpu->dev, "wait_for_completion_timeout failed");
+			ret = -EBUSY;
 			goto out;
 		}
 
@@ -1167,13 +1169,23 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
 
 	spin_unlock(&gpu->event_spinlock);
 
+	for (i = 0; i < nr_events; i++) {
+		ret = pm_runtime_resume_and_get(gpu->dev);
+		if (ret)
+			goto out_rpm;
+		rpm_count++;
+	}
+
 	return 0;
 
+out_rpm:
+	for (i = 0; i < rpm_count; i++)
+		pm_runtime_put_autosuspend(gpu->dev);
 out:
 	for (i = 0; i < acquired; i++)
 		complete(&gpu->event_free);
 
-	return -EBUSY;
+	return ret;
 }
 
 static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
@@ -1185,6 +1197,8 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
 		clear_bit(event, gpu->event_bitmap);
 		complete(&gpu->event_free);
 	}
+
+	pm_runtime_put_autosuspend(gpu->dev);
 }
 
 /*
@@ -1327,15 +1341,6 @@ struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit)
 	unsigned int i, nr_events = 1, event[3];
 	int ret;
 
-	if (!submit->runtime_resumed) {
-		ret = pm_runtime_get_sync(gpu->dev);
-		if (ret < 0) {
-			pm_runtime_put_noidle(gpu->dev);
-			return NULL;
-		}
-		submit->runtime_resumed = true;
-	}
-
 	/*
 	 * if there are performance monitor requests we need to have
 	 * - a sync point to re-configure gpu and process ETNA_PM_PROCESS_PRE
-- 
2.39.2

Re: [PATCH 3/8] drm/etnaviv: move runtime PM handling to events

[Christian Gmeiner]

Hi Lucas

>
> Conceptually events are the right abstraction to handle the GPU
> runtime PM state: as long as any event is pending the GPU can not
> be idle. Events are also properly freed and reallocated when the
> GPU has been reset after a hang.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  1 -
>  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  3 ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c        | 27 ++++++++++++--------
>  3 files changed, 16 insertions(+), 15 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> index baa81cbf701a..a42d260cac2c 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> @@ -97,7 +97,6 @@ struct etnaviv_gem_submit {
>         struct list_head node; /* GPU active submit list */
>         struct etnaviv_cmdbuf cmdbuf;
>         struct pid *pid;       /* submitting process */
> -       bool runtime_resumed;
>         u32 exec_state;
>         u32 flags;
>         unsigned int nr_pmrs;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> index 45403ea38906..2416c526f9b0 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> @@ -362,9 +362,6 @@ static void submit_cleanup(struct kref *kref)
>                         container_of(kref, struct etnaviv_gem_submit, refcount);
>         unsigned i;
>
> -       if (submit->runtime_resumed)
> -               pm_runtime_put_autosuspend(submit->gpu->dev);
> -
>         if (submit->cmdbuf.suballoc)
>                 etnaviv_cmdbuf_free(&submit->cmdbuf);
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 4e18aa8566c6..54a1249c5bca 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1139,7 +1139,8 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
>         unsigned int *events)
>  {
>         unsigned long timeout = msecs_to_jiffies(10 * 10000);
> -       unsigned i, acquired = 0;
> +       unsigned i, acquired = 0, rpm_count = 0;

rpm is the short form of runtime power management?

> +       int ret;
>
>         for (i = 0; i < nr_events; i++) {
>                 unsigned long ret;
> @@ -1148,6 +1149,7 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
>
>                 if (!ret) {
>                         dev_err(gpu->dev, "wait_for_completion_timeout failed");
> +                       ret = -EBUSY;
>                         goto out;
>                 }
>
> @@ -1167,13 +1169,23 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
>
>         spin_unlock(&gpu->event_spinlock);
>
> +       for (i = 0; i < nr_events; i++) {
> +               ret = pm_runtime_resume_and_get(gpu->dev);
> +               if (ret)
> +                       goto out_rpm;
> +               rpm_count++;
> +       }
> +
>         return 0;
>
> +out_rpm:
> +       for (i = 0; i < rpm_count; i++)
> +               pm_runtime_put_autosuspend(gpu->dev);
>  out:
>         for (i = 0; i < acquired; i++)
>                 complete(&gpu->event_free);
>
> -       return -EBUSY;
> +       return ret;
>  }
>
>  static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> @@ -1185,6 +1197,8 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
>                 clear_bit(event, gpu->event_bitmap);
>                 complete(&gpu->event_free);
>         }
> +
> +       pm_runtime_put_autosuspend(gpu->dev);
>  }
>
>  /*
> @@ -1327,15 +1341,6 @@ struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit)
>         unsigned int i, nr_events = 1, event[3];
>         int ret;
>
> -       if (!submit->runtime_resumed) {
> -               ret = pm_runtime_get_sync(gpu->dev);
> -               if (ret < 0) {
> -                       pm_runtime_put_noidle(gpu->dev);
> -                       return NULL;
> -               }
> -               submit->runtime_resumed = true;
> -       }
> -
>         /*
>          * if there are performance monitor requests we need to have
>          * - a sync point to re-configure gpu and process ETNA_PM_PROCESS_PRE
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

Re: [PATCH 3/8] drm/etnaviv: move runtime PM handling to events

[Lucas Stach]

Hi Christian,

Am Mittwoch, dem 14.06.2023 um 20:41 +0200 schrieb Christian Gmeiner:
> Hi Lucas
> 
> > 
> > Conceptually events are the right abstraction to handle the GPU
> > runtime PM state: as long as any event is pending the GPU can not
> > be idle. Events are also properly freed and reallocated when the
> > GPU has been reset after a hang.
> > 
> > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > ---
> >  drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  1 -
> >  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  3 ---
> >  drivers/gpu/drm/etnaviv/etnaviv_gpu.c        | 27 ++++++++++++--------
> >  3 files changed, 16 insertions(+), 15 deletions(-)
> > 
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > index baa81cbf701a..a42d260cac2c 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > @@ -97,7 +97,6 @@ struct etnaviv_gem_submit {
> >         struct list_head node; /* GPU active submit list */
> >         struct etnaviv_cmdbuf cmdbuf;
> >         struct pid *pid;       /* submitting process */
> > -       bool runtime_resumed;
> >         u32 exec_state;
> >         u32 flags;
> >         unsigned int nr_pmrs;
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > index 45403ea38906..2416c526f9b0 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > @@ -362,9 +362,6 @@ static void submit_cleanup(struct kref *kref)
> >                         container_of(kref, struct etnaviv_gem_submit, refcount);
> >         unsigned i;
> > 
> > -       if (submit->runtime_resumed)
> > -               pm_runtime_put_autosuspend(submit->gpu->dev);
> > -
> >         if (submit->cmdbuf.suballoc)
> >                 etnaviv_cmdbuf_free(&submit->cmdbuf);
> > 
> > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > index 4e18aa8566c6..54a1249c5bca 100644
> > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > @@ -1139,7 +1139,8 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> >         unsigned int *events)
> >  {
> >         unsigned long timeout = msecs_to_jiffies(10 * 10000);
> > -       unsigned i, acquired = 0;
> > +       unsigned i, acquired = 0, rpm_count = 0;
> 
> rpm is the short form of runtime power management?
> 
Yes, it's used in this way in multiple places in the kernel. Do you
think it's clear enough from the code what's going on to keep it that
way or should I change it to a longer name?

Regards,
Lucas

> > +       int ret;
> > 
> >         for (i = 0; i < nr_events; i++) {
> >                 unsigned long ret;
> > @@ -1148,6 +1149,7 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> > 
> >                 if (!ret) {
> >                         dev_err(gpu->dev, "wait_for_completion_timeout failed");
> > +                       ret = -EBUSY;
> >                         goto out;
> >                 }
> > 
> > @@ -1167,13 +1169,23 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> > 
> >         spin_unlock(&gpu->event_spinlock);
> > 
> > +       for (i = 0; i < nr_events; i++) {
> > +               ret = pm_runtime_resume_and_get(gpu->dev);
> > +               if (ret)
> > +                       goto out_rpm;
> > +               rpm_count++;
> > +       }
> > +
> >         return 0;
> > 
> > +out_rpm:
> > +       for (i = 0; i < rpm_count; i++)
> > +               pm_runtime_put_autosuspend(gpu->dev);
> >  out:
> >         for (i = 0; i < acquired; i++)
> >                 complete(&gpu->event_free);
> > 
> > -       return -EBUSY;
> > +       return ret;
> >  }
> > 
> >  static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> > @@ -1185,6 +1197,8 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> >                 clear_bit(event, gpu->event_bitmap);
> >                 complete(&gpu->event_free);
> >         }
> > +
> > +       pm_runtime_put_autosuspend(gpu->dev);
> >  }
> > 
> >  /*
> > @@ -1327,15 +1341,6 @@ struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit)
> >         unsigned int i, nr_events = 1, event[3];
> >         int ret;
> > 
> > -       if (!submit->runtime_resumed) {
> > -               ret = pm_runtime_get_sync(gpu->dev);
> > -               if (ret < 0) {
> > -                       pm_runtime_put_noidle(gpu->dev);
> > -                       return NULL;
> > -               }
> > -               submit->runtime_resumed = true;
> > -       }
> > -
> >         /*
> >          * if there are performance monitor requests we need to have
> >          * - a sync point to re-configure gpu and process ETNA_PM_PROCESS_PRE
> > --
> > 2.39.2
> > 
> 
> 

Re: [PATCH 3/8] drm/etnaviv: move runtime PM handling to events

[Christian Gmeiner]

Hi Lucas,

Am Do., 15. Juni 2023 um 11:37 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>:
>
> Hi Christian,
>
> Am Mittwoch, dem 14.06.2023 um 20:41 +0200 schrieb Christian Gmeiner:
> > Hi Lucas
> >
> > >
> > > Conceptually events are the right abstraction to handle the GPU
> > > runtime PM state: as long as any event is pending the GPU can not
> > > be idle. Events are also properly freed and reallocated when the
> > > GPU has been reset after a hang.
> > >
> > > Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
> > > ---
> > >  drivers/gpu/drm/etnaviv/etnaviv_gem.h        |  1 -
> > >  drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c |  3 ---
> > >  drivers/gpu/drm/etnaviv/etnaviv_gpu.c        | 27 ++++++++++++--------
> > >  3 files changed, 16 insertions(+), 15 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem.h b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > index baa81cbf701a..a42d260cac2c 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem.h
> > > @@ -97,7 +97,6 @@ struct etnaviv_gem_submit {
> > >         struct list_head node; /* GPU active submit list */
> > >         struct etnaviv_cmdbuf cmdbuf;
> > >         struct pid *pid;       /* submitting process */
> > > -       bool runtime_resumed;
> > >         u32 exec_state;
> > >         u32 flags;
> > >         unsigned int nr_pmrs;
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > index 45403ea38906..2416c526f9b0 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gem_submit.c
> > > @@ -362,9 +362,6 @@ static void submit_cleanup(struct kref *kref)
> > >                         container_of(kref, struct etnaviv_gem_submit, refcount);
> > >         unsigned i;
> > >
> > > -       if (submit->runtime_resumed)
> > > -               pm_runtime_put_autosuspend(submit->gpu->dev);
> > > -
> > >         if (submit->cmdbuf.suballoc)
> > >                 etnaviv_cmdbuf_free(&submit->cmdbuf);
> > >
> > > diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > index 4e18aa8566c6..54a1249c5bca 100644
> > > --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> > > @@ -1139,7 +1139,8 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> > >         unsigned int *events)
> > >  {
> > >         unsigned long timeout = msecs_to_jiffies(10 * 10000);
> > > -       unsigned i, acquired = 0;
> > > +       unsigned i, acquired = 0, rpm_count = 0;
> >
> > rpm is the short form of runtime power management?
> >
> Yes, it's used in this way in multiple places in the kernel. Do you
> think it's clear enough from the code what's going on to keep it that
> way or should I change it to a longer name?
>

Yes it is clear what is going on - even with short variable name :)

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> Regards,
> Lucas
>
> > > +       int ret;
> > >
> > >         for (i = 0; i < nr_events; i++) {
> > >                 unsigned long ret;
> > > @@ -1148,6 +1149,7 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> > >
> > >                 if (!ret) {
> > >                         dev_err(gpu->dev, "wait_for_completion_timeout failed");
> > > +                       ret = -EBUSY;
> > >                         goto out;
> > >                 }
> > >
> > > @@ -1167,13 +1169,23 @@ static int event_alloc(struct etnaviv_gpu *gpu, unsigned nr_events,
> > >
> > >         spin_unlock(&gpu->event_spinlock);
> > >
> > > +       for (i = 0; i < nr_events; i++) {
> > > +               ret = pm_runtime_resume_and_get(gpu->dev);
> > > +               if (ret)
> > > +                       goto out_rpm;
> > > +               rpm_count++;
> > > +       }
> > > +
> > >         return 0;
> > >
> > > +out_rpm:
> > > +       for (i = 0; i < rpm_count; i++)
> > > +               pm_runtime_put_autosuspend(gpu->dev);
> > >  out:
> > >         for (i = 0; i < acquired; i++)
> > >                 complete(&gpu->event_free);
> > >
> > > -       return -EBUSY;
> > > +       return ret;
> > >  }
> > >
> > >  static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> > > @@ -1185,6 +1197,8 @@ static void event_free(struct etnaviv_gpu *gpu, unsigned int event)
> > >                 clear_bit(event, gpu->event_bitmap);
> > >                 complete(&gpu->event_free);
> > >         }
> > > +
> > > +       pm_runtime_put_autosuspend(gpu->dev);
> > >  }
> > >
> > >  /*
> > > @@ -1327,15 +1341,6 @@ struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit)
> > >         unsigned int i, nr_events = 1, event[3];
> > >         int ret;
> > >
> > > -       if (!submit->runtime_resumed) {
> > > -               ret = pm_runtime_get_sync(gpu->dev);
> > > -               if (ret < 0) {
> > > -                       pm_runtime_put_noidle(gpu->dev);
> > > -                       return NULL;
> > > -               }
> > > -               submit->runtime_resumed = true;
> > > -       }
> > > -
> > >         /*
> > >          * if there are performance monitor requests we need to have
> > >          * - a sync point to re-configure gpu and process ETNA_PM_PROCESS_PRE
> > > --
> > > 2.39.2
> > >
> >
> >
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

[PATCH 4/8] drm/etnaviv: make clock handling symetric between runtime resume and suspend

[Lucas Stach]

Currently the clock is enabled in the runtime resume function, but are
disabled a level further down in the callstack in the suspend function.
Move the clock disable into the suspend function to make handling
symmetrical between resume and suspend.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 54a1249c5bca..57cf77ed2fcf 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1632,7 +1632,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms)
 	} while (1);
 }
 
-static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
+static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
 {
 	if (gpu->initialized && gpu->fe_running) {
 		/* Replace the last WAIT with END */
@@ -1651,8 +1651,6 @@ static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
 	}
 
 	gpu->exec_state = -1;
-
-	return etnaviv_gpu_clk_disable(gpu);
 }
 
 static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu)
@@ -1789,6 +1787,7 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
 		pm_runtime_put_sync_suspend(gpu->dev);
 	} else {
 		etnaviv_gpu_hw_suspend(gpu);
+		etnaviv_gpu_clk_disable(gpu);
 	}
 
 	if (gpu->mmu_context)
@@ -1922,7 +1921,9 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
 		return -EBUSY;
 	}
 
-	return etnaviv_gpu_hw_suspend(gpu);
+	etnaviv_gpu_hw_suspend(gpu);
+
+	return etnaviv_gpu_clk_disable(gpu);
 }
 
 static int etnaviv_gpu_rpm_resume(struct device *dev)
-- 
2.39.2

Re: [PATCH 4/8] drm/etnaviv: make clock handling symetric between runtime resume and suspend

[Christian Gmeiner]

Hi Lucas
>
> Currently the clock is enabled in the runtime resume function, but are
> disabled a level further down in the callstack in the suspend function.
> Move the clock disable into the suspend function to make handling
> symmetrical between resume and suspend.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 54a1249c5bca..57cf77ed2fcf 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1632,7 +1632,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms)
>         } while (1);
>  }
>
> -static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
> +static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
>  {
>         if (gpu->initialized && gpu->fe_running) {
>                 /* Replace the last WAIT with END */
> @@ -1651,8 +1651,6 @@ static int etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
>         }
>
>         gpu->exec_state = -1;
> -
> -       return etnaviv_gpu_clk_disable(gpu);
>  }
>
>  static int etnaviv_gpu_hw_resume(struct etnaviv_gpu *gpu)
> @@ -1789,6 +1787,7 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
>                 pm_runtime_put_sync_suspend(gpu->dev);
>         } else {
>                 etnaviv_gpu_hw_suspend(gpu);
> +               etnaviv_gpu_clk_disable(gpu);
>         }
>
>         if (gpu->mmu_context)
> @@ -1922,7 +1921,9 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
>                 return -EBUSY;
>         }
>
> -       return etnaviv_gpu_hw_suspend(gpu);
> +       etnaviv_gpu_hw_suspend(gpu);
> +
> +       return etnaviv_gpu_clk_disable(gpu);
>  }
>
>  static int etnaviv_gpu_rpm_resume(struct device *dev)
> --
> 2.39.2
>


--
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

[PATCH 5/8] drm/etnaviv: avoid runtime PM usage in etnaviv_gpu_bind

[Lucas Stach]

Nothing in this callpath actually touches the GPU, so there is no reason
to get it out of suspend state here. Only if runtime PM isn't enabled at
all we must make sure to enable the clocks, so the GPU init routine can
access the GPU later on.

This also removes the need to guard against the state where the driver
isn't fully initialized yet in the runtime PM resume handler.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 57cf77ed2fcf..fb07d0e73802 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1735,13 +1735,11 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
 	if (ret)
 		goto out_workqueue;
 
-	if (IS_ENABLED(CONFIG_PM))
-		ret = pm_runtime_get_sync(gpu->dev);
-	else
+	if (!IS_ENABLED(CONFIG_PM)) {
 		ret = etnaviv_gpu_clk_enable(gpu);
-	if (ret < 0)
-		goto out_sched;
-
+		if (ret < 0)
+			goto out_sched;
+	}
 
 	gpu->drm = drm;
 	gpu->fence_context = dma_fence_context_alloc(1);
@@ -1753,9 +1751,6 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
 
 	priv->gpu[priv->num_gpus++] = gpu;
 
-	pm_runtime_mark_last_busy(gpu->dev);
-	pm_runtime_put_autosuspend(gpu->dev);
-
 	return 0;
 
 out_sched:
@@ -1936,7 +1931,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
 		return ret;
 
 	/* Re-initialise the basic hardware state */
-	if (gpu->drm && gpu->initialized) {
+	if (gpu->initialized) {
 		ret = etnaviv_gpu_hw_resume(gpu);
 		if (ret) {
 			etnaviv_gpu_clk_disable(gpu);
-- 
2.39.2

Re: [PATCH 5/8] drm/etnaviv: avoid runtime PM usage in etnaviv_gpu_bind

[Christian Gmeiner]

Hi Lucas

>
> Nothing in this callpath actually touches the GPU, so there is no reason
> to get it out of suspend state here. Only if runtime PM isn't enabled at
> all we must make sure to enable the clocks, so the GPU init routine can
> access the GPU later on.
>
> This also removes the need to guard against the state where the driver
> isn't fully initialized yet in the runtime PM resume handler.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 15 +++++----------
>  1 file changed, 5 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 57cf77ed2fcf..fb07d0e73802 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1735,13 +1735,11 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
>         if (ret)
>                 goto out_workqueue;
>
> -       if (IS_ENABLED(CONFIG_PM))
> -               ret = pm_runtime_get_sync(gpu->dev);
> -       else
> +       if (!IS_ENABLED(CONFIG_PM)) {
>                 ret = etnaviv_gpu_clk_enable(gpu);
> -       if (ret < 0)
> -               goto out_sched;
> -
> +               if (ret < 0)
> +                       goto out_sched;
> +       }
>
>         gpu->drm = drm;
>         gpu->fence_context = dma_fence_context_alloc(1);
> @@ -1753,9 +1751,6 @@ static int etnaviv_gpu_bind(struct device *dev, struct device *master,
>
>         priv->gpu[priv->num_gpus++] = gpu;
>
> -       pm_runtime_mark_last_busy(gpu->dev);
> -       pm_runtime_put_autosuspend(gpu->dev);
> -
>         return 0;
>
>  out_sched:
> @@ -1936,7 +1931,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
>                 return ret;
>
>         /* Re-initialise the basic hardware state */
> -       if (gpu->drm && gpu->initialized) {
> +       if (gpu->initialized) {
>                 ret = etnaviv_gpu_hw_resume(gpu);
>                 if (ret) {
>                         etnaviv_gpu_clk_disable(gpu);
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

[PATCH 6/8] drm/etnaviv: better track GPU state

[Lucas Stach]

Instead of only tracking if the FE is running, use a enum to better
describe the various states the GPU can be in. This allows some
additional validation to make sure that functions that expect a
certain GPU state are only called when the GPU is actually in that
state.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 25 ++++++++++++++++++-------
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 10 +++++++++-
 2 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index fb07d0e73802..96cbb290b869 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -576,7 +576,7 @@ static int etnaviv_hw_reset(struct etnaviv_gpu *gpu)
 	/* We rely on the GPU running, so program the clock */
 	etnaviv_gpu_update_clock(gpu);
 
-	gpu->fe_running = false;
+	gpu->state = ETNA_GPU_STATE_RESET;
 	gpu->exec_state = -1;
 	if (gpu->mmu_context)
 		etnaviv_iommu_context_put(gpu->mmu_context);
@@ -651,8 +651,6 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch)
 			  VIVS_MMUv2_SEC_COMMAND_CONTROL_ENABLE |
 			  VIVS_MMUv2_SEC_COMMAND_CONTROL_PREFETCH(prefetch));
 	}
-
-	gpu->fe_running = true;
 }
 
 static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
@@ -661,6 +659,8 @@ static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
 	u16 prefetch;
 	u32 address;
 
+	WARN_ON(gpu->state != ETNA_GPU_STATE_INITIALIZED);
+
 	/* setup the MMU */
 	etnaviv_iommu_restore(gpu, context);
 
@@ -670,6 +670,8 @@ static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
 					&gpu->mmu_context->cmdbuf_mapping);
 
 	etnaviv_gpu_start_fe(gpu, address, prefetch);
+
+	gpu->state = ETNA_GPU_STATE_RUNNING;
 }
 
 static void etnaviv_gpu_setup_pulse_eater(struct etnaviv_gpu *gpu)
@@ -705,6 +707,9 @@ static void etnaviv_gpu_setup_pulse_eater(struct etnaviv_gpu *gpu)
 
 static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
 {
+	WARN_ON(!(gpu->state == ETNA_GPU_STATE_IDENTIFIED ||
+		  gpu->state == ETNA_GPU_STATE_RESET));
+
 	if ((etnaviv_is_model_rev(gpu, GC320, 0x5007) ||
 	     etnaviv_is_model_rev(gpu, GC320, 0x5220)) &&
 	    gpu_read(gpu, VIVS_HI_CHIP_TIME) != 0x2062400) {
@@ -751,6 +756,8 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
 	etnaviv_gpu_setup_pulse_eater(gpu);
 
 	gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
+
+	gpu->state = ETNA_GPU_STATE_INITIALIZED;
 }
 
 int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
@@ -793,6 +800,8 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
 	    (gpu->identity.minor_features10 & chipMinorFeatures10_SECURITY_AHB))
 		gpu->sec_mode = ETNA_SEC_KERNEL;
 
+	gpu->state = ETNA_GPU_STATE_IDENTIFIED;
+
 	ret = etnaviv_hw_reset(gpu);
 	if (ret) {
 		dev_err(gpu->dev, "GPU reset failed\n");
@@ -1368,7 +1377,7 @@ struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit)
 		goto out_unlock;
 	}
 
-	if (!gpu->fe_running)
+	if (gpu->state == ETNA_GPU_STATE_INITIALIZED)
 		etnaviv_gpu_start_fe_idleloop(gpu, submit->mmu_context);
 
 	if (submit->prev_mmu_context)
@@ -1634,7 +1643,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms)
 
 static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
 {
-	if (gpu->initialized && gpu->fe_running) {
+	if (gpu->state == ETNA_GPU_STATE_RUNNING) {
 		/* Replace the last WAIT with END */
 		mutex_lock(&gpu->lock);
 		etnaviv_buffer_end(gpu);
@@ -1647,7 +1656,7 @@ static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
 		 */
 		etnaviv_gpu_wait_idle(gpu, 100);
 
-		gpu->fe_running = false;
+		gpu->state = ETNA_GPU_STATE_INITIALIZED;
 	}
 
 	gpu->exec_state = -1;
@@ -1918,6 +1927,8 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
 
 	etnaviv_gpu_hw_suspend(gpu);
 
+	gpu->state = ETNA_GPU_STATE_IDENTIFIED;
+
 	return etnaviv_gpu_clk_disable(gpu);
 }
 
@@ -1931,7 +1942,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
 		return ret;
 
 	/* Re-initialise the basic hardware state */
-	if (gpu->initialized) {
+	if (gpu->state == ETNA_GPU_STATE_IDENTIFIED) {
 		ret = etnaviv_gpu_hw_resume(gpu);
 		if (ret) {
 			etnaviv_gpu_clk_disable(gpu);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 98c6f9c320fc..33ecc1bf84b1 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -95,6 +95,14 @@ struct clk;
 
 #define ETNA_NR_EVENTS 30
 
+enum etnaviv_gpu_state {
+	ETNA_GPU_STATE_UNKNOWN = 0,
+	ETNA_GPU_STATE_IDENTIFIED,
+	ETNA_GPU_STATE_RESET,
+	ETNA_GPU_STATE_INITIALIZED,
+	ETNA_GPU_STATE_RUNNING,
+};
+
 struct etnaviv_gpu {
 	struct drm_device *drm;
 	struct thermal_cooling_device *cooling;
@@ -105,8 +113,8 @@ struct etnaviv_gpu {
 	struct workqueue_struct *wq;
 	struct mutex sched_lock;
 	struct drm_gpu_scheduler sched;
+	enum etnaviv_gpu_state state;
 	bool initialized;
-	bool fe_running;
 
 	/* 'ring'-buffer: */
 	struct etnaviv_cmdbuf buffer;
-- 
2.39.2

Re: [PATCH 6/8] drm/etnaviv: better track GPU state

[Christian Gmeiner]

Hi Lucas

>
> Instead of only tracking if the FE is running, use a enum to better
> describe the various states the GPU can be in. This allows some
> additional validation to make sure that functions that expect a
> certain GPU state are only called when the GPU is actually in that
> state.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 25 ++++++++++++++++++-------
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h | 10 +++++++++-
>  2 files changed, 27 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index fb07d0e73802..96cbb290b869 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -576,7 +576,7 @@ static int etnaviv_hw_reset(struct etnaviv_gpu *gpu)
>         /* We rely on the GPU running, so program the clock */
>         etnaviv_gpu_update_clock(gpu);
>
> -       gpu->fe_running = false;
> +       gpu->state = ETNA_GPU_STATE_RESET;
>         gpu->exec_state = -1;
>         if (gpu->mmu_context)
>                 etnaviv_iommu_context_put(gpu->mmu_context);
> @@ -651,8 +651,6 @@ void etnaviv_gpu_start_fe(struct etnaviv_gpu *gpu, u32 address, u16 prefetch)
>                           VIVS_MMUv2_SEC_COMMAND_CONTROL_ENABLE |
>                           VIVS_MMUv2_SEC_COMMAND_CONTROL_PREFETCH(prefetch));
>         }
> -
> -       gpu->fe_running = true;
>  }
>
>  static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
> @@ -661,6 +659,8 @@ static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
>         u16 prefetch;
>         u32 address;
>
> +       WARN_ON(gpu->state != ETNA_GPU_STATE_INITIALIZED);
> +
>         /* setup the MMU */
>         etnaviv_iommu_restore(gpu, context);
>
> @@ -670,6 +670,8 @@ static void etnaviv_gpu_start_fe_idleloop(struct etnaviv_gpu *gpu,
>                                         &gpu->mmu_context->cmdbuf_mapping);
>
>         etnaviv_gpu_start_fe(gpu, address, prefetch);
> +
> +       gpu->state = ETNA_GPU_STATE_RUNNING;
>  }
>
>  static void etnaviv_gpu_setup_pulse_eater(struct etnaviv_gpu *gpu)
> @@ -705,6 +707,9 @@ static void etnaviv_gpu_setup_pulse_eater(struct etnaviv_gpu *gpu)
>
>  static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
>  {
> +       WARN_ON(!(gpu->state == ETNA_GPU_STATE_IDENTIFIED ||
> +                 gpu->state == ETNA_GPU_STATE_RESET));
> +
>         if ((etnaviv_is_model_rev(gpu, GC320, 0x5007) ||
>              etnaviv_is_model_rev(gpu, GC320, 0x5220)) &&
>             gpu_read(gpu, VIVS_HI_CHIP_TIME) != 0x2062400) {
> @@ -751,6 +756,8 @@ static void etnaviv_gpu_hw_init(struct etnaviv_gpu *gpu)
>         etnaviv_gpu_setup_pulse_eater(gpu);
>
>         gpu_write(gpu, VIVS_HI_INTR_ENBL, ~0U);
> +
> +       gpu->state = ETNA_GPU_STATE_INITIALIZED;
>  }
>
>  int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
> @@ -793,6 +800,8 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>             (gpu->identity.minor_features10 & chipMinorFeatures10_SECURITY_AHB))
>                 gpu->sec_mode = ETNA_SEC_KERNEL;
>
> +       gpu->state = ETNA_GPU_STATE_IDENTIFIED;
> +
>         ret = etnaviv_hw_reset(gpu);
>         if (ret) {
>                 dev_err(gpu->dev, "GPU reset failed\n");
> @@ -1368,7 +1377,7 @@ struct dma_fence *etnaviv_gpu_submit(struct etnaviv_gem_submit *submit)
>                 goto out_unlock;
>         }
>
> -       if (!gpu->fe_running)
> +       if (gpu->state == ETNA_GPU_STATE_INITIALIZED)
>                 etnaviv_gpu_start_fe_idleloop(gpu, submit->mmu_context);
>
>         if (submit->prev_mmu_context)
> @@ -1634,7 +1643,7 @@ int etnaviv_gpu_wait_idle(struct etnaviv_gpu *gpu, unsigned int timeout_ms)
>
>  static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
>  {
> -       if (gpu->initialized && gpu->fe_running) {
> +       if (gpu->state == ETNA_GPU_STATE_RUNNING) {
>                 /* Replace the last WAIT with END */
>                 mutex_lock(&gpu->lock);
>                 etnaviv_buffer_end(gpu);
> @@ -1647,7 +1656,7 @@ static void etnaviv_gpu_hw_suspend(struct etnaviv_gpu *gpu)
>                  */
>                 etnaviv_gpu_wait_idle(gpu, 100);
>
> -               gpu->fe_running = false;
> +               gpu->state = ETNA_GPU_STATE_INITIALIZED;
>         }
>
>         gpu->exec_state = -1;
> @@ -1918,6 +1927,8 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
>
>         etnaviv_gpu_hw_suspend(gpu);
>
> +       gpu->state = ETNA_GPU_STATE_IDENTIFIED;
> +
>         return etnaviv_gpu_clk_disable(gpu);
>  }
>
> @@ -1931,7 +1942,7 @@ static int etnaviv_gpu_rpm_resume(struct device *dev)
>                 return ret;
>
>         /* Re-initialise the basic hardware state */
> -       if (gpu->initialized) {
> +       if (gpu->state == ETNA_GPU_STATE_IDENTIFIED) {
>                 ret = etnaviv_gpu_hw_resume(gpu);
>                 if (ret) {
>                         etnaviv_gpu_clk_disable(gpu);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 98c6f9c320fc..33ecc1bf84b1 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -95,6 +95,14 @@ struct clk;
>
>  #define ETNA_NR_EVENTS 30
>
> +enum etnaviv_gpu_state {
> +       ETNA_GPU_STATE_UNKNOWN = 0,
> +       ETNA_GPU_STATE_IDENTIFIED,
> +       ETNA_GPU_STATE_RESET,
> +       ETNA_GPU_STATE_INITIALIZED,
> +       ETNA_GPU_STATE_RUNNING,
> +};
> +
>  struct etnaviv_gpu {
>         struct drm_device *drm;
>         struct thermal_cooling_device *cooling;
> @@ -105,8 +113,8 @@ struct etnaviv_gpu {
>         struct workqueue_struct *wq;
>         struct mutex sched_lock;
>         struct drm_gpu_scheduler sched;
> +       enum etnaviv_gpu_state state;
>         bool initialized;
> -       bool fe_running;
>
>         /* 'ring'-buffer: */
>         struct etnaviv_cmdbuf buffer;
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

[PATCH 7/8] drm/etnaviv: drop GPU initialized property

[Lucas Stach]

Now that it is only used to track the driver internal state of
the MMU global and cmdbuf objects, we can get rid of this property
by making the free/finit functions of those objects safe to call
on an uninitialized object.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 +++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c    | 9 ++-------
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h    | 1 -
 drivers/gpu/drm/etnaviv/etnaviv_mmu.c    | 3 +++
 4 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
index 9dc20d892c15..721d633aece9 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
@@ -121,6 +121,9 @@ void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf)
 	int order = order_base_2(ALIGN(cmdbuf->size, SUBALLOC_GRANULE) /
 				 SUBALLOC_GRANULE);
 
+	if (!suballoc)
+		return;
+
 	mutex_lock(&suballoc->lock);
 	bitmap_release_region(suballoc->granule_map,
 			      cmdbuf->suballoc_offset / SUBALLOC_GRANULE,
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index 96cbb290b869..e62761032afe 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -868,8 +868,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
 	pm_runtime_mark_last_busy(gpu->dev);
 	pm_runtime_put_autosuspend(gpu->dev);
 
-	gpu->initialized = true;
-
 	return 0;
 
 fail:
@@ -1797,11 +1795,8 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
 	if (gpu->mmu_context)
 		etnaviv_iommu_context_put(gpu->mmu_context);
 
-	if (gpu->initialized) {
-		etnaviv_cmdbuf_free(&gpu->buffer);
-		etnaviv_iommu_global_fini(gpu);
-		gpu->initialized = false;
-	}
+	etnaviv_cmdbuf_free(&gpu->buffer);
+	etnaviv_iommu_global_fini(gpu);
 
 	gpu->drm = NULL;
 	xa_destroy(&gpu->user_fences);
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index 33ecc1bf84b1..a4a9253f0d52 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -114,7 +114,6 @@ struct etnaviv_gpu {
 	struct mutex sched_lock;
 	struct drm_gpu_scheduler sched;
 	enum etnaviv_gpu_state state;
-	bool initialized;
 
 	/* 'ring'-buffer: */
 	struct etnaviv_cmdbuf buffer;
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
index 67bdce5326c6..4fa72567183a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
@@ -553,6 +553,9 @@ void etnaviv_iommu_global_fini(struct etnaviv_gpu *gpu)
 	struct etnaviv_drm_private *priv = gpu->drm->dev_private;
 	struct etnaviv_iommu_global *global = priv->mmu_global;
 
+	if (!global)
+		return;
+
 	if (--global->use > 0)
 		return;
 
-- 
2.39.2

Re: [PATCH 7/8] drm/etnaviv: drop GPU initialized property

[Christian Gmeiner]

Hi Lucas

Am Mi., 7. Juni 2023 um 15:02 Uhr schrieb Lucas Stach <l.stach@pengutronix.de>:
>
> Now that it is only used to track the driver internal state of
> the MMU global and cmdbuf objects, we can get rid of this property
> by making the free/finit functions of those objects safe to call
> on an uninitialized object.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c | 3 +++
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c    | 9 ++-------
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h    | 1 -
>  drivers/gpu/drm/etnaviv/etnaviv_mmu.c    | 3 +++
>  4 files changed, 8 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> index 9dc20d892c15..721d633aece9 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_cmdbuf.c
> @@ -121,6 +121,9 @@ void etnaviv_cmdbuf_free(struct etnaviv_cmdbuf *cmdbuf)
>         int order = order_base_2(ALIGN(cmdbuf->size, SUBALLOC_GRANULE) /
>                                  SUBALLOC_GRANULE);
>
> +       if (!suballoc)
> +               return;
> +
>         mutex_lock(&suballoc->lock);
>         bitmap_release_region(suballoc->granule_map,
>                               cmdbuf->suballoc_offset / SUBALLOC_GRANULE,
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index 96cbb290b869..e62761032afe 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -868,8 +868,6 @@ int etnaviv_gpu_init(struct etnaviv_gpu *gpu)
>         pm_runtime_mark_last_busy(gpu->dev);
>         pm_runtime_put_autosuspend(gpu->dev);
>
> -       gpu->initialized = true;
> -
>         return 0;
>
>  fail:
> @@ -1797,11 +1795,8 @@ static void etnaviv_gpu_unbind(struct device *dev, struct device *master,
>         if (gpu->mmu_context)
>                 etnaviv_iommu_context_put(gpu->mmu_context);
>
> -       if (gpu->initialized) {
> -               etnaviv_cmdbuf_free(&gpu->buffer);
> -               etnaviv_iommu_global_fini(gpu);
> -               gpu->initialized = false;
> -       }
> +       etnaviv_cmdbuf_free(&gpu->buffer);
> +       etnaviv_iommu_global_fini(gpu);
>
>         gpu->drm = NULL;
>         xa_destroy(&gpu->user_fences);
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index 33ecc1bf84b1..a4a9253f0d52 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -114,7 +114,6 @@ struct etnaviv_gpu {
>         struct mutex sched_lock;
>         struct drm_gpu_scheduler sched;
>         enum etnaviv_gpu_state state;
> -       bool initialized;
>
>         /* 'ring'-buffer: */
>         struct etnaviv_cmdbuf buffer;
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> index 67bdce5326c6..4fa72567183a 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_mmu.c
> @@ -553,6 +553,9 @@ void etnaviv_iommu_global_fini(struct etnaviv_gpu *gpu)
>         struct etnaviv_drm_private *priv = gpu->drm->dev_private;
>         struct etnaviv_iommu_global *global = priv->mmu_global;
>
> +       if (!global)
> +               return;
> +
>         if (--global->use > 0)
>                 return;
>
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

[PATCH 8/8] drm/etnaviv: expedited MMU fault handling

[Lucas Stach]

The GPU is halted when it hits a MMU exception, so there is no point in
waiting for the job timeout to expire or try to work out if the GPU is
still making progress in the timeout handler, as we know that the GPU
won't make any more progress.

Signed-off-by: Lucas Stach <l.stach@pengutronix.de>
---
 drivers/gpu/drm/etnaviv/etnaviv_gpu.c   | 2 ++
 drivers/gpu/drm/etnaviv/etnaviv_gpu.h   | 1 +
 drivers/gpu/drm/etnaviv/etnaviv_sched.c | 5 +++--
 3 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index e62761032afe..74fdcaf52fc5 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1531,6 +1531,8 @@ static irqreturn_t irq_handler(int irq, void *data)
 
 		if (intr & VIVS_HI_INTR_ACKNOWLEDGE_MMU_EXCEPTION) {
 			dump_mmu_fault(gpu);
+			gpu->state = ETNA_GPU_STATE_FAULT;
+			drm_sched_fault(&gpu->sched);
 			intr &= ~VIVS_HI_INTR_ACKNOWLEDGE_MMU_EXCEPTION;
 		}
 
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
index a4a9253f0d52..d4b9a97f2c72 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
@@ -101,6 +101,7 @@ enum etnaviv_gpu_state {
 	ETNA_GPU_STATE_RESET,
 	ETNA_GPU_STATE_INITIALIZED,
 	ETNA_GPU_STATE_RUNNING,
+	ETNA_GPU_STATE_FAULT,
 };
 
 struct etnaviv_gpu {
diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
index 1ae87dfd19c4..345fec6cb1a4 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
@@ -55,8 +55,9 @@ static enum drm_gpu_sched_stat etnaviv_sched_timedout_job(struct drm_sched_job
 	 */
 	dma_addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS);
 	change = dma_addr - gpu->hangcheck_dma_addr;
-	if (gpu->completed_fence != gpu->hangcheck_fence ||
-	    change < 0 || change > 16) {
+	if (gpu->state == ETNA_GPU_STATE_RUNNING &&
+	    (gpu->completed_fence != gpu->hangcheck_fence ||
+	     change < 0 || change > 16)) {
 		gpu->hangcheck_dma_addr = dma_addr;
 		gpu->hangcheck_fence = gpu->completed_fence;
 		goto out_no_timeout;
-- 
2.39.2

Re: [PATCH 8/8] drm/etnaviv: expedited MMU fault handling

[Christian Gmeiner]

Hi Lucas

>
> The GPU is halted when it hits a MMU exception, so there is no point in
> waiting for the job timeout to expire or try to work out if the GPU is
> still making progress in the timeout handler, as we know that the GPU
> won't make any more progress.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c   | 2 ++
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.h   | 1 +
>  drivers/gpu/drm/etnaviv/etnaviv_sched.c | 5 +++--
>  3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index e62761032afe..74fdcaf52fc5 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1531,6 +1531,8 @@ static irqreturn_t irq_handler(int irq, void *data)
>
>                 if (intr & VIVS_HI_INTR_ACKNOWLEDGE_MMU_EXCEPTION) {
>                         dump_mmu_fault(gpu);
> +                       gpu->state = ETNA_GPU_STATE_FAULT;
> +                       drm_sched_fault(&gpu->sched);
>                         intr &= ~VIVS_HI_INTR_ACKNOWLEDGE_MMU_EXCEPTION;
>                 }
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> index a4a9253f0d52..d4b9a97f2c72 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.h
> @@ -101,6 +101,7 @@ enum etnaviv_gpu_state {
>         ETNA_GPU_STATE_RESET,
>         ETNA_GPU_STATE_INITIALIZED,
>         ETNA_GPU_STATE_RUNNING,
> +       ETNA_GPU_STATE_FAULT,
>  };
>
>  struct etnaviv_gpu {
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_sched.c b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> index 1ae87dfd19c4..345fec6cb1a4 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_sched.c
> @@ -55,8 +55,9 @@ static enum drm_gpu_sched_stat etnaviv_sched_timedout_job(struct drm_sched_job
>          */
>         dma_addr = gpu_read(gpu, VIVS_FE_DMA_ADDRESS);
>         change = dma_addr - gpu->hangcheck_dma_addr;
> -       if (gpu->completed_fence != gpu->hangcheck_fence ||
> -           change < 0 || change > 16) {
> +       if (gpu->state == ETNA_GPU_STATE_RUNNING &&
> +           (gpu->completed_fence != gpu->hangcheck_fence ||
> +            change < 0 || change > 16)) {
>                 gpu->hangcheck_dma_addr = dma_addr;
>                 gpu->hangcheck_fence = gpu->completed_fence;
>                 goto out_no_timeout;
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy

Re: [PATCH 1/8] drm/etnaviv: move down etnaviv_gpu_recover_hang() in file

[Christian Gmeiner]

Hi Lucas

>
> So it can use the event_free function without adding another
> forward declaration. No functional change.
>
> Signed-off-by: Lucas Stach <l.stach@pengutronix.de>

Reviewed-by: Christian Gmeiner <cgmeiner@igalia.com>

> ---
>  drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 88 +++++++++++++--------------
>  1 file changed, 44 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> index de8c9894967c..6d4df9f1aeff 100644
> --- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> +++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
> @@ -1059,50 +1059,6 @@ int etnaviv_gpu_debugfs(struct etnaviv_gpu *gpu, struct seq_file *m)
>  }
>  #endif
>
> -void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
> -{
> -       struct etnaviv_gpu *gpu = submit->gpu;
> -       char *comm = NULL, *cmd = NULL;
> -       struct task_struct *task;
> -       unsigned int i;
> -
> -       dev_err(gpu->dev, "recover hung GPU!\n");
> -
> -       task = get_pid_task(submit->pid, PIDTYPE_PID);
> -       if (task) {
> -               comm = kstrdup(task->comm, GFP_KERNEL);
> -               cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
> -               put_task_struct(task);
> -       }
> -
> -       if (comm && cmd)
> -               dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
> -
> -       kfree(cmd);
> -       kfree(comm);
> -
> -       if (pm_runtime_get_sync(gpu->dev) < 0)
> -               goto pm_put;
> -
> -       mutex_lock(&gpu->lock);
> -
> -       etnaviv_hw_reset(gpu);
> -
> -       /* complete all events, the GPU won't do it after the reset */
> -       spin_lock(&gpu->event_spinlock);
> -       for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
> -               complete(&gpu->event_free);
> -       bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
> -       spin_unlock(&gpu->event_spinlock);
> -
> -       etnaviv_gpu_hw_init(gpu);
> -
> -       mutex_unlock(&gpu->lock);
> -       pm_runtime_mark_last_busy(gpu->dev);
> -pm_put:
> -       pm_runtime_put_autosuspend(gpu->dev);
> -}
> -
>  /* fence object management */
>  struct etnaviv_fence {
>         struct etnaviv_gpu *gpu;
> @@ -1454,6 +1410,50 @@ static void sync_point_worker(struct work_struct *work)
>         etnaviv_gpu_start_fe(gpu, addr + 2, 2);
>  }
>
> +void etnaviv_gpu_recover_hang(struct etnaviv_gem_submit *submit)
> +{
> +       struct etnaviv_gpu *gpu = submit->gpu;
> +       char *comm = NULL, *cmd = NULL;
> +       struct task_struct *task;
> +       unsigned int i;
> +
> +       dev_err(gpu->dev, "recover hung GPU!\n");
> +
> +       task = get_pid_task(submit->pid, PIDTYPE_PID);
> +       if (task) {
> +               comm = kstrdup(task->comm, GFP_KERNEL);
> +               cmd = kstrdup_quotable_cmdline(task, GFP_KERNEL);
> +               put_task_struct(task);
> +       }
> +
> +       if (comm && cmd)
> +               dev_err(gpu->dev, "offending task: %s (%s)\n", comm, cmd);
> +
> +       kfree(cmd);
> +       kfree(comm);
> +
> +       if (pm_runtime_get_sync(gpu->dev) < 0)
> +               goto pm_put;
> +
> +       mutex_lock(&gpu->lock);
> +
> +       etnaviv_hw_reset(gpu);
> +
> +       /* complete all events, the GPU won't do it after the reset */
> +       spin_lock(&gpu->event_spinlock);
> +       for_each_set_bit(i, gpu->event_bitmap, ETNA_NR_EVENTS)
> +               complete(&gpu->event_free);
> +       bitmap_zero(gpu->event_bitmap, ETNA_NR_EVENTS);
> +       spin_unlock(&gpu->event_spinlock);
> +
> +       etnaviv_gpu_hw_init(gpu);
> +
> +       mutex_unlock(&gpu->lock);
> +       pm_runtime_mark_last_busy(gpu->dev);
> +pm_put:
> +       pm_runtime_put_autosuspend(gpu->dev);
> +}
> +
>  static void dump_mmu_fault(struct etnaviv_gpu *gpu)
>  {
>         static const char *fault_reasons[] = {
> --
> 2.39.2
>


-- 
greets
--
Christian Gmeiner, MSc

https://christian-gmeiner.info/privacypolicy