2020-03-02 19:15:44

by Guido Günther

[permalink] [raw]
Subject: [PATCH 0/5] drm/etnaviv: Ignore MC bit when checking for runtime suspend

At least GC7000 fails to enter runtime suspend for long periods of time since
the MC becomes busy again even when the FE is idle. The rest of the series
makes detecting similar issues easier to debug in the future by checking
all known bits in debugfs and also warning in the EBUSY case.

Tested on GC7000 with a reduced runtime delay of 50ms. Patches are
against next-20200226.

Thanks to Lucas Stach for pointing me in the right direction.

Guido Günther (5):
drm/etnaviv: Fix typo in comment
drm/etnaviv: Update idle bits
drm/etnaviv: Consider all kwnown idle bits in debugfs
drm/etnaviv: Ignore MC when checking runtime suspend idleness
drm/etnaviv: Warn when GPU doesn't idle fast enough

drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 26 ++++++++++++++++++++++----
drivers/gpu/drm/etnaviv/state_hi.xml.h | 7 +++++++
2 files changed, 29 insertions(+), 4 deletions(-)

--
2.23.0


2020-03-02 19:16:01

by Guido Günther

[permalink] [raw]
Subject: [PATCH 5/5] drm/etnaviv: Warn when GPU doesn't idle fast enough

If the GPU isn't idle after signalling pm_runtime_mark_last_busy() plus
waiting for the autosuspend delay there's likely something wrong with
the way we check idleness so warn about that.

Signed-off-by: Guido Günther <[email protected]>
---
drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
index da24e433f82a..4fd16b8f8a7a 100644
--- a/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
+++ b/drivers/gpu/drm/etnaviv/etnaviv_gpu.c
@@ -1824,8 +1824,11 @@ static int etnaviv_gpu_rpm_suspend(struct device *dev)
mask = gpu->idle_mask & ~(VIVS_HI_IDLE_STATE_FE |
VIVS_HI_IDLE_STATE_MC);
idle = gpu_read(gpu, VIVS_HI_IDLE_STATE) & mask;
- if (idle != mask)
+ if (idle != mask) {
+ dev_warn_ratelimited(dev, "GPU not yet idle, mask: 0x%08x\n",
+ idle);
return -EBUSY;
+ }

return etnaviv_gpu_hw_suspend(gpu);
}
--
2.23.0

2020-03-03 11:58:51

by Lucas Stach

[permalink] [raw]
Subject: Re: [PATCH 0/5] drm/etnaviv: Ignore MC bit when checking for runtime suspend

On Mo, 2020-03-02 at 20:13 +0100, Guido Günther wrote:
> At least GC7000 fails to enter runtime suspend for long periods of time since
> the MC becomes busy again even when the FE is idle. The rest of the series
> makes detecting similar issues easier to debug in the future by checking
> all known bits in debugfs and also warning in the EBUSY case.

Thanks, series applied to etnaviv/next.

> Tested on GC7000 with a reduced runtime delay of 50ms. Patches are
> against next-20200226.

I've already wondered if 200ms is too long, 50ms sounds more
reasonable. Do you have any numbers on the power draw on the i.MX8M
with idle GPU, vs. being fully power gated?

Regards,
Lucas

> Thanks to Lucas Stach for pointing me in the right direction.
>
> Guido Günther (5):
> drm/etnaviv: Fix typo in comment
> drm/etnaviv: Update idle bits
> drm/etnaviv: Consider all kwnown idle bits in debugfs
> drm/etnaviv: Ignore MC when checking runtime suspend idleness
> drm/etnaviv: Warn when GPU doesn't idle fast enough
>
> drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 26 ++++++++++++++++++++++----
> drivers/gpu/drm/etnaviv/state_hi.xml.h | 7 +++++++
> 2 files changed, 29 insertions(+), 4 deletions(-)
>

2020-03-03 15:57:43

by Guido Günther

[permalink] [raw]
Subject: Re: [PATCH 0/5] drm/etnaviv: Ignore MC bit when checking for runtime suspend

Hi Lucas,
On Tue, Mar 03, 2020 at 12:55:04PM +0100, Lucas Stach wrote:
> On Mo, 2020-03-02 at 20:13 +0100, Guido G?nther wrote:
> > At least GC7000 fails to enter runtime suspend for long periods of time since
> > the MC becomes busy again even when the FE is idle. The rest of the series
> > makes detecting similar issues easier to debug in the future by checking
> > all known bits in debugfs and also warning in the EBUSY case.
>
> Thanks, series applied to etnaviv/next.

Thanks!

>
> > Tested on GC7000 with a reduced runtime delay of 50ms. Patches are
> > against next-20200226.
>
> I've already wondered if 200ms is too long, 50ms sounds more
> reasonable. Do you have any numbers on the power draw on the i.MX8M
> with idle GPU, vs. being fully power gated?

I don't have good numbers yet but i'll post here once i do.
Cheers,
-- Guido

>
> Regards,
> Lucas
>
> > Thanks to Lucas Stach for pointing me in the right direction.
> >
> > Guido G?nther (5):
> > drm/etnaviv: Fix typo in comment
> > drm/etnaviv: Update idle bits
> > drm/etnaviv: Consider all kwnown idle bits in debugfs
> > drm/etnaviv: Ignore MC when checking runtime suspend idleness
> > drm/etnaviv: Warn when GPU doesn't idle fast enough
> >
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 26 ++++++++++++++++++++++----
> > drivers/gpu/drm/etnaviv/state_hi.xml.h | 7 +++++++
> > 2 files changed, 29 insertions(+), 4 deletions(-)
> >
>

2020-06-25 14:56:52

by Guido Günther

[permalink] [raw]
Subject: Re: [PATCH 0/5] drm/etnaviv: Ignore MC bit when checking for runtime suspend

Hi,
On Tue, Mar 03, 2020 at 12:55:04PM +0100, Lucas Stach wrote:
> On Mo, 2020-03-02 at 20:13 +0100, Guido G?nther wrote:
> > At least GC7000 fails to enter runtime suspend for long periods of time since
> > the MC becomes busy again even when the FE is idle. The rest of the series
> > makes detecting similar issues easier to debug in the future by checking
> > all known bits in debugfs and also warning in the EBUSY case.
>
> Thanks, series applied to etnaviv/next.
>
> > Tested on GC7000 with a reduced runtime delay of 50ms. Patches are
> > against next-20200226.
>
> I've already wondered if 200ms is too long, 50ms sounds more
> reasonable. Do you have any numbers on the power draw on the i.MX8M
> with idle GPU, vs. being fully power gated?

The difference is at least 250mW. It makes a huge difference over here.
We hit
https://lore.kernel.org/dri-devel/[email protected]/
recently and you notice instantly when that happens when looking at the
SoC temperature.

Cheers,
-- Guido
>
> Regards,
> Lucas
>
> > Thanks to Lucas Stach for pointing me in the right direction.
> >
> > Guido G?nther (5):
> > drm/etnaviv: Fix typo in comment
> > drm/etnaviv: Update idle bits
> > drm/etnaviv: Consider all kwnown idle bits in debugfs
> > drm/etnaviv: Ignore MC when checking runtime suspend idleness
> > drm/etnaviv: Warn when GPU doesn't idle fast enough
> >
> > drivers/gpu/drm/etnaviv/etnaviv_gpu.c | 26 ++++++++++++++++++++++----
> > drivers/gpu/drm/etnaviv/state_hi.xml.h | 7 +++++++
> > 2 files changed, 29 insertions(+), 4 deletions(-)
> >
>