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

[Ben Skeggs]

On Fri, 8 Oct 2021 at 07:46, Karol Herbst <kherbst at redhat.com> wrote:
>
> Reviewed-by: Karol Herbst <kherbst at redhat.com>
Reviewed-by: Ben Skeggs <bskeggs at 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 at 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 at denx.de>
> > Cc: <stable at vger.kernel.org> # 5.12+
> > Cc: Ben Skeggs <bskeggs at redhat.com>
> > Cc: Karol Herbst <kherbst at redhat.com>
> > Cc: Lyude Paul <lyude at 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
> >
>