Re: [PATCH] drm/nouveau/fifo: Reinstate the correct engine bit programming

2021-10-07 Thread Ben Skeggs
On Fri, 8 Oct 2021 at 07:46, Karol Herbst  wrote:
>
> Reviewed-by: Karol Herbst 
Reviewed-by: Ben Skeggs 

>
> 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  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 :01:00.0: gr: 0030 [ILLEGAL_MTHD ILLEGAL_CLASS] ch 1 
> > [003fbce000 DRM] subc 3 class  mthd 085c data 0420
> >
> > 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 
> > Cc:  # 5.12+
> > Cc: Ben Skeggs 
> > Cc: Karol Herbst 
> > Cc: Lyude Paul 
> > ---
> >  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(>base, engine);
> > +   engn = fifo->base.func->engine_id(>base, engine) - 1;
> > save = nvkm_mask(device, 0x002520, 0x003f, 1 << engn);
> > nvkm_wr32(device, 0x0032fc, chan->base.inst->addr >> 12);
> > done = nvkm_msec(device, 2000,
> > --
> > 2.33.0
> >
>


Re: [PATCH] drm/nouveau/fifo: Reinstate the correct engine bit programming

2021-10-07 Thread Karol Herbst
Reviewed-by: Karol Herbst 

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  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 :01:00.0: gr: 0030 [ILLEGAL_MTHD ILLEGAL_CLASS] ch 1 
> [003fbce000 DRM] subc 3 class  mthd 085c data 0420
>
> 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 
> Cc:  # 5.12+
> Cc: Ben Skeggs 
> Cc: Karol Herbst 
> Cc: Lyude Paul 
> ---
>  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(>base, engine);
> +   engn = fifo->base.func->engine_id(>base, engine) - 1;
> save = nvkm_mask(device, 0x002520, 0x003f, 1 << engn);
> nvkm_wr32(device, 0x0032fc, chan->base.inst->addr >> 12);
> done = nvkm_msec(device, 2000,
> --
> 2.33.0
>



[PATCH] drm/nouveau/fifo: Reinstate the correct engine bit programming

2021-10-07 Thread Marek Vasut
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 :01:00.0: gr: 0030 [ILLEGAL_MTHD ILLEGAL_CLASS] ch 1 
[003fbce000 DRM] subc 3 class  mthd 085c data 0420

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 
Cc:  # 5.12+
Cc: Ben Skeggs 
Cc: Karol Herbst 
Cc: Lyude Paul 
---
 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(>base, engine);
+   engn = fifo->base.func->engine_id(>base, engine) - 1;
save = nvkm_mask(device, 0x002520, 0x003f, 1 << engn);
nvkm_wr32(device, 0x0032fc, chan->base.inst->addr >> 12);
done = nvkm_msec(device, 2000,
-- 
2.33.0