2023-05-26 09:38:24

by Karol Herbst

Subject: [PATCH v2] drm/nouveau: bring back blit subchannel for pre nv50 GPUs

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 keep
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 <[email protected]>
---
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/nouveau/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/nouveau/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/nouveau/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 = nvif_object_ctor(&drm->channel->user, "drmNvsw",
NVDRM_NVSW, nouveau_abi16_swclass(drm),
NULL, 0, &drm->channel->nvsw);
+
+ if (ret == 0 && device->info.chipset >= 0x11) {
+ ret = nvif_object_ctor(&drm->channel->user, "drmBlit",
+ 0x005f, 0x009f,
+ NULL, 0, &drm->channel->blit);
+ }
+
if (ret == 0) {
struct nvif_push *push = drm->channel->chan.push;
- ret = PUSH_WAIT(push, 2);
- if (ret == 0)
+ ret = PUSH_WAIT(push, 8);
+ if (ret == 0) {
+ if (device->info.chipset >= 0x11) {
+ PUSH_NVSQ(push, NV05F, 0x0000, drm->channel->blit.handle);
+ PUSH_NVSQ(push, NV09F, 0x0120, 0,
+ 0x0124, 1,
+ 0x0128, 2);
+ }
PUSH_NVSQ(push, NV_SW, 0x0000, drm->channel->nvsw.handle);
+ }
}

if (ret) {
- NV_ERROR(drm, "failed to allocate sw class, %d\n", ret);
+ NV_ERROR(drm, "failed to allocate sw or blit class, %d\n", ret);
nouveau_accel_gr_fini(drm);
return;
}
--
2.40.1

2023-05-26 15:24:42

by Ilia Mirkin

Subject: Re: [Nouveau] [PATCH v2] drm/nouveau: bring back blit subchannel for pre nv50 GPUs

On Fri, May 26, 2023 at 5:11 AM Karol Herbst <[email protected]> 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 keep
> 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 <[email protected]>
> ---
> 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/nouveau/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/nouveau/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/nouveau/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 = nvif_object_ctor(&drm->channel->user, "drmNvsw",
> NVDRM_NVSW, nouveau_abi16_swclass(drm),
> NULL, 0, &drm->channel->nvsw);
> +
> + if (ret == 0 && device->info.chipset >= 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.

Cheers,

-ilia

2023-06-06 10:59:30

by Karol Herbst

Subject: Re: [Nouveau] [PATCH v2] drm/nouveau: bring back blit subchannel for pre nv50 GPUs

On Fri, May 26, 2023 at 5:21 PM Ilia Mirkin <[email protected]> wrote:
>
> On Fri, May 26, 2023 at 5:11 AM Karol Herbst <[email protected]> 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 keep
> > 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 <[email protected]>
> > ---
> > 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/nouveau/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/nouveau/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/nouveau/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 = nvif_object_ctor(&drm->channel->user, "drmNvsw",
> > NVDRM_NVSW, nouveau_abi16_swclass(drm),
> > NULL, 0, &drm->channel->nvsw);
> > +
> > + if (ret == 0 && device->info.chipset >= 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 >= 0x11 ? 0x009f : 0x005f" check when creating
the blit object.

> Cheers,
>
> -ilia
>

2023-07-12 21:12:12

by Ben Skeggs

Subject: Re: [Nouveau] [PATCH v2] drm/nouveau: bring back blit subchannel for pre nv50 GPUs

On Fri, 26 May 2023 at 19:11, Karol Herbst <[email protected]> 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 keep
> 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 <[email protected]>
Reviewed-by: Ben Skeggs <[email protected]>

> ---
> 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/nouveau/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/nouveau/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/nouveau/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 = nvif_object_ctor(&drm->channel->user, "drmNvsw",
> NVDRM_NVSW, nouveau_abi16_swclass(drm),
> NULL, 0, &drm->channel->nvsw);
> +
> + if (ret == 0 && device->info.chipset >= 0x11) {
> + ret = nvif_object_ctor(&drm->channel->user, "drmBlit",
> + 0x005f, 0x009f,
> + NULL, 0, &drm->channel->blit);
> + }
> +
> if (ret == 0) {
> struct nvif_push *push = drm->channel->chan.push;
> - ret = PUSH_WAIT(push, 2);
> - if (ret == 0)
> + ret = PUSH_WAIT(push, 8);
> + if (ret == 0) {
> + if (device->info.chipset >= 0x11) {
> + PUSH_NVSQ(push, NV05F, 0x0000, drm->channel->blit.handle);
> + PUSH_NVSQ(push, NV09F, 0x0120, 0,
> + 0x0124, 1,
> + 0x0128, 2);
> + }
> PUSH_NVSQ(push, NV_SW, 0x0000, drm->channel->nvsw.handle);
> + }
> }
>
> if (ret) {
> - NV_ERROR(drm, "failed to allocate sw class, %d\n", ret);
> + NV_ERROR(drm, "failed to allocate sw or blit class, %d\n", ret);
> nouveau_accel_gr_fini(drm);
> return;
> }
> --
> 2.40.1
>