2023-12-14 00:44:18

by Lyude Paul

[permalink] [raw]
Subject: [PATCH] drm/nouveau/kms/nv50-: Don't allow inheritance of headless iors

Turns out we made a silly mistake when coming up with OR inheritance on
nouveau. On pre-DCB 4.1, iors are statically routed to output paths via the
DCB. On later generations iors are only routed to an output path if they're
actually being used. Unfortunately, it appears with NVIF_OUTP_INHERIT_V0 we
make the mistake of assuming the later is true on all generations, which is
currently leading us to return bogus ior -> head assignments through nvif,
which causes WARN_ON().

So - fix this by verifying that we actually know that there's a head
assigned to an ior before allowing it to be inherited through nvif. This
-should- hopefully fix the WARN_ON on GT218 reported by Borislav.

Signed-off-by: Lyude Paul <[email protected]>
Cc: Borislav Petkov <[email protected]>
---
drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
index e4279f1772a1b..377d0e0cef848 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
@@ -385,7 +385,7 @@ nvkm_uoutp_mthd_inherit(struct nvkm_outp *outp, void *argv, u32 argc)

/* Ensure an ior is hooked up to this outp already */
ior = outp->func->inherit(outp);
- if (!ior)
+ if (!ior || !ior->arm.head)
return -ENODEV;

/* With iors, there will be a separate output path for each type of connector - and all of
--
2.43.0


2023-12-15 15:32:15

by Borislav Petkov

[permalink] [raw]
Subject: Re: [PATCH] drm/nouveau/kms/nv50-: Don't allow inheritance of headless iors

On Wed, Dec 13, 2023 at 07:43:57PM -0500, Lyude Paul wrote:
> Turns out we made a silly mistake when coming up with OR inheritance on
> nouveau. On pre-DCB 4.1, iors are statically routed to output paths via the
> DCB. On later generations iors are only routed to an output path if they're
> actually being used. Unfortunately, it appears with NVIF_OUTP_INHERIT_V0 we
> make the mistake of assuming the later is true on all generations, which is
> currently leading us to return bogus ior -> head assignments through nvif,
> which causes WARN_ON().
>
> So - fix this by verifying that we actually know that there's a head
> assigned to an ior before allowing it to be inherited through nvif. This
> -should- hopefully fix the WARN_ON on GT218 reported by Borislav.
>
> Signed-off-by: Lyude Paul <[email protected]>
> Cc: Borislav Petkov <[email protected]>
> ---
> drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
> index e4279f1772a1b..377d0e0cef848 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/uoutp.c
> @@ -385,7 +385,7 @@ nvkm_uoutp_mthd_inherit(struct nvkm_outp *outp, void *argv, u32 argc)
>
> /* Ensure an ior is hooked up to this outp already */
> ior = outp->func->inherit(outp);
> - if (!ior)
> + if (!ior || !ior->arm.head)
> return -ENODEV;
>
> /* With iors, there will be a separate output path for each type of connector - and all of
> --

Thanks, that fixes it!

Reported-by: Borislav Petkov (AMD) <[email protected]>
Tested-by: Borislav Petkov (AMD) <[email protected]>

--
Regards/Gruss,
Boris.

https://people.kernel.org/tglx/notes-about-netiquette