Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20230526091052.2169044-1-kherbst@redhat.com> <CAKb7Uvha74HOvny25aqnwzvvRLpc7+DNQm6QEAk4oxfTa1urJg@mail.gmail.com>
In-Reply-To: <CAKb7Uvha74HOvny25aqnwzvvRLpc7+DNQm6QEAk4oxfTa1urJg@mail.gmail.com>
From:   Karol Herbst <kherbst@redhat.com>
Date:   Tue, 6 Jun 2023 12:38:50 +0200
Message-ID: <CACO55ts7gJfJ5ver+qvL5Tp9X+Kzk9ByvmXUD1hLH31NR+3pBg@mail.gmail.com>
Subject: Re: [Nouveau] [PATCH v2] drm/nouveau: bring back blit subchannel for
 pre nv50 GPUs
To:     Ilia Mirkin <imirkin@alum.mit.edu>
Cc:     linux-kernel@vger.kernel.org, nouveau@lists.freedesktop.org,
        Ben Skeggs <bskeggs@redhat.com>,
        dri-devel@lists.freedesktop.org
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

On Fri, May 26, 2023 at 5:21=E2=80=AFPM Ilia Mirkin <imirkin@alum.mit.edu> =
wrote:
>
> On Fri, May 26, 2023 at 5:11=E2=80=AFAM Karol Herbst <kherbst@redhat.com>=
 wrote:
> >
> > 1ba6113a90a0 removed a lot of the kernel GPU channel, but method 0x128
> > was important as otherwise the GPU spams us with `CACHE_ERROR` messages=
.
> >
> > We use the blit subchannel inside our vblank handling, so we should kee=
p
> > at least this part.
> >
> > v2: Only do it for NV11+ GPUs
> >
> > Closes: https://gitlab.freedesktop.org/drm/nouveau/-/issues/201
> > Fixes: 4a16dd9d18a0 ("drm/nouveau/kms: switch to drm fbdev helpers")
> > Signed-off-by: Karol Herbst <kherbst@redhat.com>
> > ---
> >  drivers/gpu/drm/nouveau/nouveau_chan.c |  1 +
> >  drivers/gpu/drm/nouveau/nouveau_chan.h |  1 +
> >  drivers/gpu/drm/nouveau/nouveau_drm.c  | 20 +++++++++++++++++---
> >  3 files changed, 19 insertions(+), 3 deletions(-)
> >
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.c b/drivers/gpu/drm/n=
ouveau/nouveau_chan.c
> > index e648ecd0c1a0..3dfbc374478e 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_chan.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_chan.c
> > @@ -90,6 +90,7 @@ nouveau_channel_del(struct nouveau_channel **pchan)
> >                 if (cli)
> >                         nouveau_svmm_part(chan->vmm->svmm, chan->inst);
> >
> > +               nvif_object_dtor(&chan->blit);
> >                 nvif_object_dtor(&chan->nvsw);
> >                 nvif_object_dtor(&chan->gart);
> >                 nvif_object_dtor(&chan->vram);
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_chan.h b/drivers/gpu/drm/n=
ouveau/nouveau_chan.h
> > index e06a8ffed31a..bad7466bd0d5 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_chan.h
> > +++ b/drivers/gpu/drm/nouveau/nouveau_chan.h
> > @@ -53,6 +53,7 @@ struct nouveau_channel {
> >         u32 user_put;
> >
> >         struct nvif_object user;
> > +       struct nvif_object blit;
> >
> >         struct nvif_event kill;
> >         atomic_t killed;
> > diff --git a/drivers/gpu/drm/nouveau/nouveau_drm.c b/drivers/gpu/drm/no=
uveau/nouveau_drm.c
> > index cc7c5b4a05fd..9512f1c2f871 100644
> > --- a/drivers/gpu/drm/nouveau/nouveau_drm.c
> > +++ b/drivers/gpu/drm/nouveau/nouveau_drm.c
> > @@ -369,15 +369,29 @@ nouveau_accel_gr_init(struct nouveau_drm *drm)
> >                 ret =3D nvif_object_ctor(&drm->channel->user, "drmNvsw"=
,
> >                                        NVDRM_NVSW, nouveau_abi16_swclas=
s(drm),
> >                                        NULL, 0, &drm->channel->nvsw);
> > +
> > +               if (ret =3D=3D 0 && device->info.chipset >=3D 0x11) {
>
> Can you double-check that this is needed on NV15? IIRC there's some
> non-linearity of chipsets here which is why we had (some long time
> ago, not sure if it's still there), a chip class which would simplify
> such checks.
>

yeah, it's fine. The old code before 4a16dd9d18a0 had a
"device->info.chipset >=3D 0x11 ? 0x009f : 0x005f" check when creating
the blit object.

> Cheers,
>
>   -ilia
>