Nouveau - Feb 2018 - [PATCH 0/1] drm/nouveau/disp: prefer identity-mapped route of SOR <-> macro link

Hi Ben,

still _assuming_ it's an issue of the card I thought about why it
works with the NVIDIA binary driver.

And I can image they're just trying to do an identity-mapping first
and if that doesn't work (e.g. the particular SOR is already in use
by another macro link) they just pick the next suitable one.

So the case would be that the NVIDIA binary driver always assignes
the only working mapping (SOR-1 <-> macro link D) on a Gainward
GTX 1070.

Therefore, if this is the case, maybe it is better to change the
routing strategy to prefer a SOR which maps the identity of the
particular macro link over a random one instead of having my
orignally proposed quirk.

So here's the patch doing exactly that.

Regards,
Danilo

Danilo Krummrich (1):
  drm/nouveau/disp: prefer identity-mapped route of SOR <-> macro link

 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c | 38 +++++++++++++++++--------
 1 file changed, 26 insertions(+), 12 deletions(-)

-- 
2.14.1

With DCB 4.1 implemented by VBIOS since GM20x GPUs, SOR crossbar
routing should be possible, such that any SOR sublink can drive
any macro link. Therefore, if crossbar routing is available,
the first suitable SOR was picked.

Unfortunately, there's at least one card where some SORs being
being connected to a particular macro link not matching the identity
of each other are causing failures.

To fix this, when searching for a suitable SOR to be mapped to a
macro link prefer a SOR which maps the identity of the macro link.

Signed-off-by: Danilo Krummrich <danilokrummrich at dk-develop.de>
---
 drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c | 38 +++++++++++++++++--------
 1 file changed, 26 insertions(+), 12 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
index be9e7f8c3b23..b7af9349504e 100644
--- a/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
+++ b/drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c
@@ -112,7 +112,7 @@ nvkm_outp_acquire_ior(struct nvkm_outp *outp, u8 user,
struct nvkm_ior *ior)
 int
 nvkm_outp_acquire(struct nvkm_outp *outp, u8 user)
 {
-	struct nvkm_ior *ior = outp->ior;
+	struct nvkm_ior *_ior, *ior = outp->ior;
 	enum nvkm_ior_proto proto;
 	enum nvkm_ior_type type;
 
@@ -130,26 +130,40 @@ nvkm_outp_acquire(struct nvkm_outp *outp, u8 user)
 	/* First preference is to reuse the OR that is currently armed
 	 * on HW, if any, in order to prevent unnecessary switching.
 	 */
-	list_for_each_entry(ior, &outp->disp->ior, head) {
-		if (!ior->asy.outp && ior->arm.outp == outp)
-			return nvkm_outp_acquire_ior(outp, user, ior);
+	list_for_each_entry(_ior, &outp->disp->ior, head) {
+		if (!_ior->asy.outp && _ior->arm.outp == outp)
+			return nvkm_outp_acquire_ior(outp, user, _ior);
 	}
 
 	/* Failing that, a completely unused OR is the next best thing. */
-	list_for_each_entry(ior, &outp->disp->ior, head) {
-		if (!ior->asy.outp && ior->type == type &&
!ior->arm.outp &&
-		    (ior->func->route.set || ior->id == __ffs(outp->info.or)))
-			return nvkm_outp_acquire_ior(outp, user, ior);
+	ior = NULL;
+	list_for_each_entry(_ior, &outp->disp->ior, head) {
+		if (!_ior->asy.outp && _ior->type == type &&
!_ior->arm.outp) {
+			/* Prefer identity-mapped route. */
+			if (_ior->id == __ffs(outp->info.or))
+				return nvkm_outp_acquire_ior(outp, user, _ior);
+			else if (!ior)
+				ior = _ior;
+		}
 	}
+	if (ior && ior->func->route.set)
+		return nvkm_outp_acquire_ior(outp, user, ior);
 
 	/* Last resort is to assign an OR that's already active on HW,
 	 * but will be released during the next modeset.
 	 */
-	list_for_each_entry(ior, &outp->disp->ior, head) {
-		if (!ior->asy.outp && ior->type == type &&
-		    (ior->func->route.set || ior->id == __ffs(outp->info.or)))
-			return nvkm_outp_acquire_ior(outp, user, ior);
+	ior = NULL;
+	list_for_each_entry(_ior, &outp->disp->ior, head) {
+		if (!_ior->asy.outp && _ior->type == type) {
+			/* Again prefer identity-mapped route. */
+			if (_ior->id == __ffs(outp->info.or))
+				return nvkm_outp_acquire_ior(outp, user, _ior);
+			else if (!ior)
+				ior = _ior;
+		}
 	}
+	if (ior && ior->func->route.set)
+		return nvkm_outp_acquire_ior(outp, user, _ior);
 
 	return -ENOSPC;
 }
-- 
2.14.1

On Tue, Feb 6, 2018 at 9:35 AM, Danilo Krummrich
<danilokrummrich at dk-develop.de> wrote:
> Hi Ben,
>
> still _assuming_ it's an issue of the card I thought about why it
> works with the NVIDIA binary driver.
>
> And I can image they're just trying to do an identity-mapping first
> and if that doesn't work (e.g. the particular SOR is already in use
> by another macro link) they just pick the next suitable one.
>
> So the case would be that the NVIDIA binary driver always assignes
> the only working mapping (SOR-1 <-> macro link D) on a Gainward
> GTX 1070.
>
> Therefore, if this is the case, maybe it is better to change the
> routing strategy to prefer a SOR which maps the identity of the
> particular macro link over a random one instead of having my
> orignally proposed quirk.
Thanks for your work looking into this.  I'm still also investigating
to try and determine for sure if this isn't just working around some
other driver bug that we should fix instead, but your patches are
good, so if I don't discover anything, I'll take them.

I shall let you know when I know more :)

Thanks again,
Ben.
>
> So here's the patch doing exactly that.
>
> Regards,
> Danilo
>
> Danilo Krummrich (1):
>   drm/nouveau/disp: prefer identity-mapped route of SOR <-> macro
link
>
>  drivers/gpu/drm/nouveau/nvkm/engine/disp/outp.c | 38
+++++++++++++++++--------
>  1 file changed, 26 insertions(+), 12 deletions(-)
>
> --
> 2.14.1
>