Re: [Nouveau] [PATCH] ce/gf100: fix incorrect CE0 address calculation on some GPUs

[Karol Herbst <kherbst@redhat.com>]

On Wed, Nov 3, 2021 at 8:51 AM Karol Herbst <kherbst@redhat.com> wrote:
>
> On Wed, Nov 3, 2021 at 2:11 AM Ben Skeggs <skeggsb@gmail.com> wrote:
> >
> > From: Ben Skeggs <bskeggs@redhat.com>
> >
> > The code which constructs the modules for each engine present on the GPU
> > passes -1 for 'instance' on non-instanced engines, which affects how the
> > name for a sub-device is generated.  This is then stored as 'instance 0'
> > in nvkm_subdev.inst, so code can potentially be shared with earlier GPUs
> > that only had a single instance of an engine.
> >
> > However, GF100's CE constructor uses this value to calculate the address
> > of its falcon before it's translated, resulting in CE0 getting the wrong
> > address.
> >
> > This slightly modifies the approach, always passing a valid instance for
> > engines that *can* have multiple copies, and having the code for earlier
> > GPUs explicitly ask for non-instanced name generation.
> >
> > Bug: https://gitlab.freedesktop.org/drm/nouveau/-/issues/91
> >
> > Fixes: 50551b15c760 ("drm/nouveau/ce: switch to instanced constructor")
> > Cc: <stable@vger.kernel.org> # v5.12+
> > Signed-off-by: Ben Skeggs <bskeggs@redhat.com>
> > ---
> >  drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c    | 2 +-
> >  drivers/gpu/drm/nouveau/nvkm/engine/device/base.c | 3 +--
> >  2 files changed, 2 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c
> > index 704df0f2d1f1..09a112af2f89 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/ce/gt215.c
> > @@ -78,6 +78,6 @@ int
> >  gt215_ce_new(struct nvkm_device *device, enum nvkm_subdev_type type, int inst,
> >              struct nvkm_engine **pengine)
> >  {
> > -       return nvkm_falcon_new_(&gt215_ce, device, type, inst,
> > +       return nvkm_falcon_new_(&gt215_ce, device, type, -1,
> >                                 (device->chipset != 0xaf), 0x104000, pengine);
> >  }
> > diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> > index ca75c5f6ecaf..b51d690f375f 100644
> > --- a/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> > +++ b/drivers/gpu/drm/nouveau/nvkm/engine/device/base.c
> > @@ -3147,8 +3147,7 @@ nvkm_device_ctor(const struct nvkm_device_func *func,
> >         WARN_ON(device->chip->ptr.inst & ~((1 << ARRAY_SIZE(device->ptr)) - 1));             \
> >         for (j = 0; device->chip->ptr.inst && j < ARRAY_SIZE(device->ptr); j++) {            \
> >                 if ((device->chip->ptr.inst & BIT(j)) && (subdev_mask & BIT_ULL(type))) {    \
> > -                       int inst = (device->chip->ptr.inst == 1) ? -1 : (j);                 \
> > -                       ret = device->chip->ptr.ctor(device, (type), inst, &device->ptr[j]); \
> > +                       ret = device->chip->ptr.ctor(device, (type), (j), &device->ptr[j]);  \
> >                         subdev = nvkm_device_subdev(device, (type), (j));                    \
> >                         if (ret) {                                                           \
> >                                 nvkm_subdev_del(&subdev);                                    \
> > --
> > 2.31.1
> >
>
> Reviewed-by: Karol Herbst <kherbst@redhat.com>

Tested that on a GF108, so

Tested-by: Karol Herbst <kherbst@redhat.com>