| Message ID | 20211007214117.231472-1-marex@denx.de (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | drm/nouveau/fifo: Reinstate the correct engine bit programming | expand |
Reviewed-by: Karol Herbst <kherbst@redhat.com> I haven't checked if other places need fixing up yet, and I still want to test this patch, but I won't get to it until Monday. But if everything is in place we can get this pushed next week so we can finally fix this annoying issue :) I was also seeing some minor graphical corruptions which would be cool if this patch fixes it as well. Thanks for the patch and poking us about the bug again. On Thu, Oct 7, 2021 at 11:41 PM Marek Vasut <marex@denx.de> wrote: > > Commit 64f7c698bea9 ("drm/nouveau/fifo: add engine_id hook") replaced > fifo/chang84.c g84_fifo_chan_engine() call with an indirect call of > fifo/g84.c g84_fifo_engine_id(). The G84_FIFO_ENGN_* values returned > from the later g84_fifo_engine_id() are incremented by 1 compared to > the previous g84_fifo_chan_engine() return values. > > This is fine either way for most of the code, except this one line > where an engine bit programmed into the hardware is derived from the > return value. Decrement the return value accordingly, otherwise the > wrong engine bit is programmed into the hardware and that leads to > the following failure: > nouveau 0000:01:00.0: gr: 00000030 [ILLEGAL_MTHD ILLEGAL_CLASS] ch 1 [003fbce000 DRM] subc 3 class 0000 mthd 085c data 00000420 > > On the following hardware: > lspci -s 01:00.0 > 01:00.0 VGA compatible controller: NVIDIA Corporation GT216GLM [Quadro FX 880M] (rev a2) > lspci -ns 01:00.0 > 01:00.0 0300: 10de:0a3c (rev a2) > > Fixes: 64f7c698bea9 ("drm/nouveau/fifo: add engine_id hook") > Signed-off-by: Marek Vasut <marex@denx.de> > Cc: <stable@vger.kernel.org> # 5.12+ > Cc: Ben Skeggs <bskeggs@redhat.com> > Cc: Karol Herbst <kherbst@redhat.com> > Cc: Lyude Paul <lyude@redhat.com> > --- > drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c > index 353b77d9b3dc..3492c561f2cf 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c > @@ -82,7 +82,7 @@ g84_fifo_chan_engine_fini(struct nvkm_fifo_chan *base, > if (offset < 0) > return 0; > > - engn = fifo->base.func->engine_id(&fifo->base, engine); > + engn = fifo->base.func->engine_id(&fifo->base, engine) - 1; > save = nvkm_mask(device, 0x002520, 0x0000003f, 1 << engn); > nvkm_wr32(device, 0x0032fc, chan->base.inst->addr >> 12); > done = nvkm_msec(device, 2000, > -- > 2.33.0 >
On Fri, 8 Oct 2021 at 07:46, Karol Herbst <kherbst@redhat.com> wrote: > > Reviewed-by: Karol Herbst <kherbst@redhat.com> Reviewed-by: Ben Skeggs <bskeggs@redhat.com> > > I haven't checked if other places need fixing up yet, and I still want > to test this patch, but I won't get to it until Monday. But if > everything is in place we can get this pushed next week so we can > finally fix this annoying issue :) I was also seeing some minor > graphical corruptions which would be cool if this patch fixes it as > well. > > Thanks for the patch and poking us about the bug again. > > On Thu, Oct 7, 2021 at 11:41 PM Marek Vasut <marex@denx.de> wrote: > > > > Commit 64f7c698bea9 ("drm/nouveau/fifo: add engine_id hook") replaced > > fifo/chang84.c g84_fifo_chan_engine() call with an indirect call of > > fifo/g84.c g84_fifo_engine_id(). The G84_FIFO_ENGN_* values returned > > from the later g84_fifo_engine_id() are incremented by 1 compared to > > the previous g84_fifo_chan_engine() return values. > > > > This is fine either way for most of the code, except this one line > > where an engine bit programmed into the hardware is derived from the > > return value. Decrement the return value accordingly, otherwise the > > wrong engine bit is programmed into the hardware and that leads to > > the following failure: > > nouveau 0000:01:00.0: gr: 00000030 [ILLEGAL_MTHD ILLEGAL_CLASS] ch 1 [003fbce000 DRM] subc 3 class 0000 mthd 085c data 00000420 > > > > On the following hardware: > > lspci -s 01:00.0 > > 01:00.0 VGA compatible controller: NVIDIA Corporation GT216GLM [Quadro FX 880M] (rev a2) > > lspci -ns 01:00.0 > > 01:00.0 0300: 10de:0a3c (rev a2) > > > > Fixes: 64f7c698bea9 ("drm/nouveau/fifo: add engine_id hook") > > Signed-off-by: Marek Vasut <marex@denx.de> > > Cc: <stable@vger.kernel.org> # 5.12+ > > Cc: Ben Skeggs <bskeggs@redhat.com> > > Cc: Karol Herbst <kherbst@redhat.com> > > Cc: Lyude Paul <lyude@redhat.com> > > --- > > drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c > > index 353b77d9b3dc..3492c561f2cf 100644 > > --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c > > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c > > @@ -82,7 +82,7 @@ g84_fifo_chan_engine_fini(struct nvkm_fifo_chan *base, > > if (offset < 0) > > return 0; > > > > - engn = fifo->base.func->engine_id(&fifo->base, engine); > > + engn = fifo->base.func->engine_id(&fifo->base, engine) - 1; > > save = nvkm_mask(device, 0x002520, 0x0000003f, 1 << engn); > > nvkm_wr32(device, 0x0032fc, chan->base.inst->addr >> 12); > > done = nvkm_msec(device, 2000, > > -- > > 2.33.0 > > >
diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c index 353b77d9b3dc..3492c561f2cf 100644 --- a/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c +++ b/drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c @@ -82,7 +82,7 @@ g84_fifo_chan_engine_fini(struct nvkm_fifo_chan *base, if (offset < 0) return 0; - engn = fifo->base.func->engine_id(&fifo->base, engine); + engn = fifo->base.func->engine_id(&fifo->base, engine) - 1; save = nvkm_mask(device, 0x002520, 0x0000003f, 1 << engn); nvkm_wr32(device, 0x0032fc, chan->base.inst->addr >> 12); done = nvkm_msec(device, 2000,
Commit 64f7c698bea9 ("drm/nouveau/fifo: add engine_id hook") replaced fifo/chang84.c g84_fifo_chan_engine() call with an indirect call of fifo/g84.c g84_fifo_engine_id(). The G84_FIFO_ENGN_* values returned from the later g84_fifo_engine_id() are incremented by 1 compared to the previous g84_fifo_chan_engine() return values. This is fine either way for most of the code, except this one line where an engine bit programmed into the hardware is derived from the return value. Decrement the return value accordingly, otherwise the wrong engine bit is programmed into the hardware and that leads to the following failure: nouveau 0000:01:00.0: gr: 00000030 [ILLEGAL_MTHD ILLEGAL_CLASS] ch 1 [003fbce000 DRM] subc 3 class 0000 mthd 085c data 00000420 On the following hardware: lspci -s 01:00.0 01:00.0 VGA compatible controller: NVIDIA Corporation GT216GLM [Quadro FX 880M] (rev a2) lspci -ns 01:00.0 01:00.0 0300: 10de:0a3c (rev a2) Fixes: 64f7c698bea9 ("drm/nouveau/fifo: add engine_id hook") Signed-off-by: Marek Vasut <marex@denx.de> Cc: <stable@vger.kernel.org> # 5.12+ Cc: Ben Skeggs <bskeggs@redhat.com> Cc: Karol Herbst <kherbst@redhat.com> Cc: Lyude Paul <lyude@redhat.com> --- drivers/gpu/drm/nouveau/nvkm/engine/fifo/chang84.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)