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,
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
dri-devel@lists.freedesktop.org
-
Ben Skeggs -
Karol Herbst -
Marek Vasut