* [PATCH v2] drm/nouveau/pmu: fix timeout on GP108
@ 2021-02-16 16:19 Diego Viola
2021-02-17 3:30 ` Alexandre Courbot
0 siblings, 1 reply; 6+ messages in thread
From: Diego Viola @ 2021-02-16 16:19 UTC (permalink / raw)
To: bskeggs; +Cc: linux-kernel, dri-devel, nouveau, gnurou, Diego Viola
This code times out on GP108, probably because the BIOS puts it into a
bad state.
Since we reset the PMU on driver load anyway, we are at no risk from
missing a response from it since we are not waiting for one to begin
with.
Signed-off-by: Diego Viola <diego.viola@gmail.com>
---
drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c | 6 +-----
1 file changed, 1 insertion(+), 5 deletions(-)
diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
index a0fe607c9c07..5c802f2d00cb 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
@@ -102,12 +102,8 @@ nvkm_pmu_reset(struct nvkm_pmu *pmu)
if (!pmu->func->enabled(pmu))
return 0;
- /* Inhibit interrupts, and wait for idle. */
+ /* Inhibit interrupts. */
nvkm_wr32(device, 0x10a014, 0x0000ffff);
- nvkm_msec(device, 2000,
- if (!nvkm_rd32(device, 0x10a04c))
- break;
- );
/* Reset. */
if (pmu->func->reset)
--
2.30.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/nouveau/pmu: fix timeout on GP108
2021-02-16 16:19 [PATCH v2] drm/nouveau/pmu: fix timeout on GP108 Diego Viola
@ 2021-02-17 3:30 ` Alexandre Courbot
2021-02-24 9:50 ` Ben Skeggs
0 siblings, 1 reply; 6+ messages in thread
From: Alexandre Courbot @ 2021-02-17 3:30 UTC (permalink / raw)
To: Diego Viola; +Cc: Ben Skeggs, Linux Kernel Mailing List, dri-devel, nouveau
On Wed, Feb 17, 2021 at 1:20 AM Diego Viola <diego.viola@gmail.com> wrote:
>
> This code times out on GP108, probably because the BIOS puts it into a
> bad state.
>
> Since we reset the PMU on driver load anyway, we are at no risk from
> missing a response from it since we are not waiting for one to begin
> with.
This looks safe to me, provided indeed that the PMU's reset is not
called outside of initialization (which for GP108 is shouldn't be
IIRC?).
>
> Signed-off-by: Diego Viola <diego.viola@gmail.com>
> ---
> drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c | 6 +-----
> 1 file changed, 1 insertion(+), 5 deletions(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> index a0fe607c9c07..5c802f2d00cb 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> @@ -102,12 +102,8 @@ nvkm_pmu_reset(struct nvkm_pmu *pmu)
> if (!pmu->func->enabled(pmu))
> return 0;
>
> - /* Inhibit interrupts, and wait for idle. */
> + /* Inhibit interrupts. */
> nvkm_wr32(device, 0x10a014, 0x0000ffff);
> - nvkm_msec(device, 2000,
> - if (!nvkm_rd32(device, 0x10a04c))
> - break;
> - );
>
> /* Reset. */
> if (pmu->func->reset)
> --
> 2.30.1
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/nouveau/pmu: fix timeout on GP108
2021-02-17 3:30 ` Alexandre Courbot
@ 2021-02-24 9:50 ` Ben Skeggs
2021-02-25 4:04 ` Diego Viola
2021-02-25 5:22 ` Diego Viola
0 siblings, 2 replies; 6+ messages in thread
From: Ben Skeggs @ 2021-02-24 9:50 UTC (permalink / raw)
To: Alexandre Courbot
Cc: Diego Viola, Roy Spliet, Ben Skeggs, ML dri-devel,
Linux Kernel Mailing List
On Wed, 17 Feb 2021 at 13:30, Alexandre Courbot <gnurou@gmail.com> wrote:
>
> On Wed, Feb 17, 2021 at 1:20 AM Diego Viola <diego.viola@gmail.com> wrote:
> >
> > This code times out on GP108, probably because the BIOS puts it into a
> > bad state.
> >
> > Since we reset the PMU on driver load anyway, we are at no risk from
> > missing a response from it since we are not waiting for one to begin
> > with.
>
> This looks safe to me, provided indeed that the PMU's reset is not
> called outside of initialization (which for GP108 is shouldn't be
> IIRC?).
ISTR that the PMU FW we use prior to GM200 might depend on that being there.
I've posted a proposed alternate fix here[1], as we probably shouldn't
have been touching PMU there anyway on those GPUs.
Ben.
[1] https://github.com/skeggsb/linux/commit/90224a17437b1f39dbecbb385567c1fce958f992
>
> >
> > Signed-off-by: Diego Viola <diego.viola@gmail.com>
> > ---
> > drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c | 6 +-----
> > 1 file changed, 1 insertion(+), 5 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > index a0fe607c9c07..5c802f2d00cb 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > @@ -102,12 +102,8 @@ nvkm_pmu_reset(struct nvkm_pmu *pmu)
> > if (!pmu->func->enabled(pmu))
> > return 0;
> >
> > - /* Inhibit interrupts, and wait for idle. */
> > + /* Inhibit interrupts. */
> > nvkm_wr32(device, 0x10a014, 0x0000ffff);
> > - nvkm_msec(device, 2000,
> > - if (!nvkm_rd32(device, 0x10a04c))
> > - break;
> > - );
> >
> > /* Reset. */
> > if (pmu->func->reset)
> > --
> > 2.30.1
> >
> _______________________________________________
> dri-devel mailing list
> dri-devel@lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/nouveau/pmu: fix timeout on GP108
2021-02-24 9:50 ` Ben Skeggs
@ 2021-02-25 4:04 ` Diego Viola
2021-02-25 5:22 ` Diego Viola
1 sibling, 0 replies; 6+ messages in thread
From: Diego Viola @ 2021-02-25 4:04 UTC (permalink / raw)
To: Ben Skeggs
Cc: Alexandre Courbot, Roy Spliet, Ben Skeggs, ML dri-devel,
Linux Kernel Mailing List
Hi Ben,
On Wed, Feb 24, 2021 at 6:50 AM Ben Skeggs <skeggsb@gmail.com> wrote:
>
> On Wed, 17 Feb 2021 at 13:30, Alexandre Courbot <gnurou@gmail.com> wrote:
> >
> > On Wed, Feb 17, 2021 at 1:20 AM Diego Viola <diego.viola@gmail.com> wrote:
> > >
> > > This code times out on GP108, probably because the BIOS puts it into a
> > > bad state.
> > >
> > > Since we reset the PMU on driver load anyway, we are at no risk from
> > > missing a response from it since we are not waiting for one to begin
> > > with.
> >
> > This looks safe to me, provided indeed that the PMU's reset is not
> > called outside of initialization (which for GP108 is shouldn't be
> > IIRC?).
> ISTR that the PMU FW we use prior to GM200 might depend on that being there.
>
> I've posted a proposed alternate fix here[1], as we probably shouldn't
> have been touching PMU there anyway on those GPUs.
>
> Ben.
>
> [1] https://github.com/skeggsb/linux/commit/90224a17437b1f39dbecbb385567c1fce958f992
>
> >
> > >
> > > Signed-off-by: Diego Viola <diego.viola@gmail.com>
> > > ---
> > > drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c | 6 +-----
> > > 1 file changed, 1 insertion(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > index a0fe607c9c07..5c802f2d00cb 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > @@ -102,12 +102,8 @@ nvkm_pmu_reset(struct nvkm_pmu *pmu)
> > > if (!pmu->func->enabled(pmu))
> > > return 0;
> > >
> > > - /* Inhibit interrupts, and wait for idle. */
> > > + /* Inhibit interrupts. */
> > > nvkm_wr32(device, 0x10a014, 0x0000ffff);
> > > - nvkm_msec(device, 2000,
> > > - if (!nvkm_rd32(device, 0x10a04c))
> > > - break;
> > > - );
> > >
> > > /* Reset. */
> > > if (pmu->func->reset)
> > > --
> > > 2.30.1
> > >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
I tested your patch and can confirm that the timeout is gone after
booting my system, but unfortunately it's back after doing a
suspend/resume.
Any ideas about that?
Thanks,
Diego
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/nouveau/pmu: fix timeout on GP108
2021-02-24 9:50 ` Ben Skeggs
2021-02-25 4:04 ` Diego Viola
@ 2021-02-25 5:22 ` Diego Viola
2021-02-25 5:37 ` Diego Viola
1 sibling, 1 reply; 6+ messages in thread
From: Diego Viola @ 2021-02-25 5:22 UTC (permalink / raw)
To: Ben Skeggs
Cc: Alexandre Courbot, Roy Spliet, Ben Skeggs, ML dri-devel,
Linux Kernel Mailing List
Hi Ben,
I can confirm that your last two patches[0][1] fix the timeout issues
(those from a normal boot and from suspend/resume).
[0] https://github.com/skeggsb/linux/commit/90224a17437b1f39dbecbb385567c1fce958f992
[1] https://github.com/skeggsb/linux/commit/0ee6dc49601359042fd254bbd8ba6b4685b4d0d7
Tested-by: Diego Viola <diego.viola@gmail.com>
on both patches.
Thanks, I appreciate it a lot.
Diego
On Wed, Feb 24, 2021 at 6:50 AM Ben Skeggs <skeggsb@gmail.com> wrote:
>
> On Wed, 17 Feb 2021 at 13:30, Alexandre Courbot <gnurou@gmail.com> wrote:
> >
> > On Wed, Feb 17, 2021 at 1:20 AM Diego Viola <diego.viola@gmail.com> wrote:
> > >
> > > This code times out on GP108, probably because the BIOS puts it into a
> > > bad state.
> > >
> > > Since we reset the PMU on driver load anyway, we are at no risk from
> > > missing a response from it since we are not waiting for one to begin
> > > with.
> >
> > This looks safe to me, provided indeed that the PMU's reset is not
> > called outside of initialization (which for GP108 is shouldn't be
> > IIRC?).
> ISTR that the PMU FW we use prior to GM200 might depend on that being there.
>
> I've posted a proposed alternate fix here[1], as we probably shouldn't
> have been touching PMU there anyway on those GPUs.
>
> Ben.
>
> [1] https://github.com/skeggsb/linux/commit/90224a17437b1f39dbecbb385567c1fce958f992
>
> >
> > >
> > > Signed-off-by: Diego Viola <diego.viola@gmail.com>
> > > ---
> > > drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c | 6 +-----
> > > 1 file changed, 1 insertion(+), 5 deletions(-)
> > >
> > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > index a0fe607c9c07..5c802f2d00cb 100644
> > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > @@ -102,12 +102,8 @@ nvkm_pmu_reset(struct nvkm_pmu *pmu)
> > > if (!pmu->func->enabled(pmu))
> > > return 0;
> > >
> > > - /* Inhibit interrupts, and wait for idle. */
> > > + /* Inhibit interrupts. */
> > > nvkm_wr32(device, 0x10a014, 0x0000ffff);
> > > - nvkm_msec(device, 2000,
> > > - if (!nvkm_rd32(device, 0x10a04c))
> > > - break;
> > > - );
> > >
> > > /* Reset. */
> > > if (pmu->func->reset)
> > > --
> > > 2.30.1
> > >
> > _______________________________________________
> > dri-devel mailing list
> > dri-devel@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/dri-devel
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] drm/nouveau/pmu: fix timeout on GP108
2021-02-25 5:22 ` Diego Viola
@ 2021-02-25 5:37 ` Diego Viola
0 siblings, 0 replies; 6+ messages in thread
From: Diego Viola @ 2021-02-25 5:37 UTC (permalink / raw)
To: Ben Skeggs
Cc: Alexandre Courbot, Roy Spliet, Ben Skeggs, ML dri-devel,
Linux Kernel Mailing List
On Thu, Feb 25, 2021 at 2:22 AM Diego Viola <diego.viola@gmail.com> wrote:
>
> Hi Ben,
>
> I can confirm that your last two patches[0][1] fix the timeout issues
> (those from a normal boot and from suspend/resume).
>
> [0] https://github.com/skeggsb/linux/commit/90224a17437b1f39dbecbb385567c1fce958f992
> [1] https://github.com/skeggsb/linux/commit/0ee6dc49601359042fd254bbd8ba6b4685b4d0d7
>
> Tested-by: Diego Viola <diego.viola@gmail.com>
>
> on both patches.
>
> Thanks, I appreciate it a lot.
>
> Diego
>
> On Wed, Feb 24, 2021 at 6:50 AM Ben Skeggs <skeggsb@gmail.com> wrote:
> >
> > On Wed, 17 Feb 2021 at 13:30, Alexandre Courbot <gnurou@gmail.com> wrote:
> > >
> > > On Wed, Feb 17, 2021 at 1:20 AM Diego Viola <diego.viola@gmail.com> wrote:
> > > >
> > > > This code times out on GP108, probably because the BIOS puts it into a
> > > > bad state.
> > > >
> > > > Since we reset the PMU on driver load anyway, we are at no risk from
> > > > missing a response from it since we are not waiting for one to begin
> > > > with.
> > >
> > > This looks safe to me, provided indeed that the PMU's reset is not
> > > called outside of initialization (which for GP108 is shouldn't be
> > > IIRC?).
> > ISTR that the PMU FW we use prior to GM200 might depend on that being there.
> >
> > I've posted a proposed alternate fix here[1], as we probably shouldn't
> > have been touching PMU there anyway on those GPUs.
> >
> > Ben.
> >
> > [1] https://github.com/skeggsb/linux/commit/90224a17437b1f39dbecbb385567c1fce958f992
> >
> > >
> > > >
> > > > Signed-off-by: Diego Viola <diego.viola@gmail.com>
> > > > ---
> > > > drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c | 6 +-----
> > > > 1 file changed, 1 insertion(+), 5 deletions(-)
> > > >
> > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > > index a0fe607c9c07..5c802f2d00cb 100644
> > > > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/pmu/base.c
> > > > @@ -102,12 +102,8 @@ nvkm_pmu_reset(struct nvkm_pmu *pmu)
> > > > if (!pmu->func->enabled(pmu))
> > > > return 0;
> > > >
> > > > - /* Inhibit interrupts, and wait for idle. */
> > > > + /* Inhibit interrupts. */
> > > > nvkm_wr32(device, 0x10a014, 0x0000ffff);
> > > > - nvkm_msec(device, 2000,
> > > > - if (!nvkm_rd32(device, 0x10a04c))
> > > > - break;
> > > > - );
> > > >
> > > > /* Reset. */
> > > > if (pmu->func->reset)
> > > > --
> > > > 2.30.1
> > > >
> > > _______________________________________________
> > > dri-devel mailing list
> > > dri-devel@lists.freedesktop.org
> > > https://lists.freedesktop.org/mailman/listinfo/dri-devel
Ugh, sorry for breaking the regular email flow/order. Damn you gmail!
Regards,
Diego
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-02-25 5:39 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-16 16:19 [PATCH v2] drm/nouveau/pmu: fix timeout on GP108 Diego Viola
2021-02-17 3:30 ` Alexandre Courbot
2021-02-24 9:50 ` Ben Skeggs
2021-02-25 4:04 ` Diego Viola
2021-02-25 5:22 ` Diego Viola
2021-02-25 5:37 ` Diego Viola
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).