thr3ads.net - Nouveau - [Nouveau] [PATCH 0/3] MST BPC fixes for nouveau [Nov 2019]

If this information is useful, please help other people find it:
Share via:

Lyude Paul

2019-Nov-15 21:07 UTC

[Nouveau] [PATCH 0/3] MST BPC fixes for nouveau

Realized when I moved nouveau over to using the atomic DP MST VCPI
helpers that I forgot to ensure that we clamp the BPC to 8 to make us
less likely to run out of bandwidth on a topology when enabling multiple
displays that support >8 BPC - something we want to do until we have
support for dynamically selecting the bpc based on the topology's
available bandwidth, since userspace isn't really using HDR yet anyway.
This matches the behavior that i915 has, along with the behavior of
amdgpu and should fix some people's displays not turning on.

Lyude Paul (3):
  drm/nouveau/kms/nv50-: Call outp_atomic_check_view() before handling
    PBN
  drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom
  drm/nouveau/kms/nv50-: Limit MST BPC to 8

 drivers/gpu/drm/nouveau/dispnv50/atom.h |   1 +
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 102 ++++++++++++++----------
 drivers/gpu/drm/nouveau/dispnv50/head.c |   5 +-
 3 files changed, 64 insertions(+), 44 deletions(-)

-- 
2.21.0

Lyude Paul

2019-Nov-15 21:07 UTC

head link

[Nouveau] [PATCH 1/3] drm/nouveau/kms/nv50-: Call outp_atomic_check_view() before handling PBN

Since nv50_outp_atomic_check_view() can set crtc_state->mode_changed, we
probably should be calling it before handling any PBN changes. Just a
precaution.

Signed-off-by: Lyude Paul <lyude at redhat.com>
Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST")
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: David Airlie <airlied at redhat.com>
Cc: Jerry Zuo <Jerry.Zuo at amd.com>
Cc: Harry Wentland <harry.wentland at amd.com>
Cc: Juston Li <juston.li at intel.com>
Cc: Sean Paul <seanpaul at chromium.org>
Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Cc: <stable at vger.kernel.org> # v5.1+
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 44 ++++++++++++++-----------
 1 file changed, 24 insertions(+), 20 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 549486f1d937..6327aaf37c08 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -770,32 +770,36 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 	struct nv50_mstm *mstm = mstc->mstm;
 	struct nv50_head_atom *asyh = nv50_head_atom(crtc_state);
 	int slots;
+	int ret;
 
-	if (crtc_state->mode_changed || crtc_state->connectors_changed) {
-		/*
-		 * When restoring duplicated states, we need to make sure that
-		 * the bw remains the same and avoid recalculating it, as the
-		 * connector's bpc may have changed after the state was
-		 * duplicated
-		 */
-		if (!state->duplicated) {
-			const int bpp = connector->display_info.bpc * 3;
-			const int clock = crtc_state->adjusted_mode.clock;
+	ret = nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
+					  mstc->native);
+	if (ret)
+		return ret;
 
-			asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
-		}
+	if (!crtc_state->mode_changed &&
!crtc_state->connectors_changed)
+		return 0;
 
-		slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
-						      mstc->port,
-						      asyh->dp.pbn);
-		if (slots < 0)
-			return slots;
+	/*
+	 * When restoring duplicated states, we need to make sure that the bw
+	 * remains the same and avoid recalculating it, as the connector's bpc
+	 * may have changed after the state was duplicated
+	 */
+	if (!state->duplicated) {
+		const int bpp = connector->display_info.bpc * 3;
+		const int clock = crtc_state->adjusted_mode.clock;
 
-		asyh->dp.tu = slots;
+		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
 	}
 
-	return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
-					   mstc->native);
+	slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, mstc->port,
+					      asyh->dp.pbn);
+	if (slots < 0)
+		return slots;
+
+	asyh->dp.tu = slots;
+
+	return 0;
 }
 
 static void
-- 
2.21.0

Lyude Paul

2019-Nov-15 21:07 UTC

head link

[Nouveau] [PATCH 2/3] drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom

In order to be able to use bpc values that are different from what the
connector reports, we want to be able to store the bpc value we decide
on using for an atomic state in nv50_head_atom and refer to that instead
of simply using the value that the connector reports throughout the
whole atomic check phase and commit phase. This will let us (eventually)
implement the max bpc connector property, and will also be needed for
limiting the bpc we use on MST displays to 8 in the next commit.

Signed-off-by: Lyude Paul <lyude at redhat.com>
Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST")
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: David Airlie <airlied at redhat.com>
Cc: Jerry Zuo <Jerry.Zuo at amd.com>
Cc: Harry Wentland <harry.wentland at amd.com>
Cc: Juston Li <juston.li at intel.com>
Cc: Sean Paul <seanpaul at chromium.org>
Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
Cc: <stable at vger.kernel.org> # v5.1+
---
 drivers/gpu/drm/nouveau/dispnv50/atom.h |  1 +
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 57 ++++++++++++++-----------
 drivers/gpu/drm/nouveau/dispnv50/head.c |  5 +--
 3 files changed, 36 insertions(+), 27 deletions(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h
b/drivers/gpu/drm/nouveau/dispnv50/atom.h
index 43df86c38f58..24f7700768da 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/atom.h
+++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h
@@ -114,6 +114,7 @@ struct nv50_head_atom {
 		u8 nhsync:1;
 		u8 nvsync:1;
 		u8 depth:4;
+		u8 bpc;
 	} or;
 
 	/* Currently only used for MST */
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 6327aaf37c08..93665aecce57 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -353,10 +353,20 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
 		       struct drm_crtc_state *crtc_state,
 		       struct drm_connector_state *conn_state)
 {
-	struct nouveau_connector *nv_connector -	
nouveau_connector(conn_state->connector);
-	return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
-					   nv_connector->native_mode);
+	struct drm_connector *connector = conn_state->connector;
+	struct nouveau_connector *nv_connector = nouveau_connector(connector);
+	struct nv50_head_atom *asyh = nv50_head_atom(crtc_state);
+	int ret;
+
+	ret = nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
+					  nv_connector->native_mode);
+	if (ret)
+		return ret;
+
+	if (crtc_state->mode_changed || crtc_state->connectors_changed)
+		asyh->or.bpc = connector->display_info.bpc;
+
+	return 0;
 }
 
 /******************************************************************************
@@ -786,10 +796,10 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 	 * may have changed after the state was duplicated
 	 */
 	if (!state->duplicated) {
-		const int bpp = connector->display_info.bpc * 3;
 		const int clock = crtc_state->adjusted_mode.clock;
 
-		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
+		asyh->or.bpc = connector->display_info.bpc;
+		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3);
 	}
 
 	slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr, mstc->port,
@@ -802,6 +812,17 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 	return 0;
 }
 
+static u8
+nv50_dp_bpc_to_depth(unsigned int bpc)
+{
+	switch (bpc) {
+	case  6: return 0x2;
+	case  8: return 0x5;
+	case 10: /* fall-through */
+	default: return 0x6;
+	}
+}
+
 static void
 nv50_msto_enable(struct drm_encoder *encoder)
 {
@@ -812,7 +833,7 @@ nv50_msto_enable(struct drm_encoder *encoder)
 	struct nv50_mstm *mstm = NULL;
 	struct drm_connector *connector;
 	struct drm_connector_list_iter conn_iter;
-	u8 proto, depth;
+	u8 proto;
 	bool r;
 
 	drm_connector_list_iter_begin(encoder->dev, &conn_iter);
@@ -841,14 +862,8 @@ nv50_msto_enable(struct drm_encoder *encoder)
 	else
 		proto = 0x9;
 
-	switch (mstc->connector.display_info.bpc) {
-	case  6: depth = 0x2; break;
-	case  8: depth = 0x5; break;
-	case 10:
-	default: depth = 0x6; break;
-	}
-
-	mstm->outp->update(mstm->outp, head->base.index, armh, proto,
depth);
+	mstm->outp->update(mstm->outp, head->base.index, armh, proto,
+			   nv50_dp_bpc_to_depth(armh->or.bpc));
 
 	msto->head = head;
 	msto->mstc = mstc;
@@ -1502,20 +1517,14 @@ nv50_sor_enable(struct drm_encoder *encoder)
 					lvds.lvds.script |= 0x0200;
 			}
 
-			if (nv_connector->base.display_info.bpc == 8)
+			if (asyh->or.bpc == 8)
 				lvds.lvds.script |= 0x0200;
 		}
 
 		nvif_mthd(&disp->disp->object, 0, &lvds, sizeof(lvds));
 		break;
 	case DCB_OUTPUT_DP:
-		if (nv_connector->base.display_info.bpc == 6)
-			depth = 0x2;
-		else
-		if (nv_connector->base.display_info.bpc == 8)
-			depth = 0x5;
-		else
-			depth = 0x6;
+		depth = nv50_dp_bpc_to_depth(asyh->or.bpc);
 
 		if (nv_encoder->link & 1)
 			proto = 0x8;
@@ -1666,7 +1675,7 @@ nv50_pior_enable(struct drm_encoder *encoder)
 	nv50_outp_acquire(nv_encoder);
 
 	nv_connector = nouveau_encoder_connector_get(nv_encoder);
-	switch (nv_connector->base.display_info.bpc) {
+	switch (asyh->or.bpc) {
 	case 10: asyh->or.depth = 0x6; break;
 	case  8: asyh->or.depth = 0x5; break;
 	case  6: asyh->or.depth = 0x2; break;
diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
index 71c23bf1fe25..c9692df2b76c 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/head.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
@@ -81,18 +81,17 @@ nv50_head_atomic_check_dither(struct nv50_head_atom *armh,
 			      struct nv50_head_atom *asyh,
 			      struct nouveau_conn_atom *asyc)
 {
-	struct drm_connector *connector = asyc->state.connector;
 	u32 mode = 0x00;
 
 	if (asyc->dither.mode == DITHERING_MODE_AUTO) {
-		if (asyh->base.depth > connector->display_info.bpc * 3)
+		if (asyh->base.depth > asyh->or.bpc * 3)
 			mode = DITHERING_MODE_DYNAMIC2X2;
 	} else {
 		mode = asyc->dither.mode;
 	}
 
 	if (asyc->dither.depth == DITHERING_DEPTH_AUTO) {
-		if (connector->display_info.bpc >= 8)
+		if (asyh->or.bpc >= 8)
 			mode |= DITHERING_DEPTH_8BPC;
 	} else {
 		mode |= asyc->dither.depth;
-- 
2.21.0

Lyude Paul

2019-Nov-15 21:07 UTC

head link

[Nouveau] [PATCH 3/3] drm/nouveau/kms/nv50-: Limit MST BPC to 8

Noticed this while working on some unrelated CRC stuff. Currently,
userspace has very little support for BPCs higher than 8. While this
doesn't matter for most things, on MST topologies we need to be careful
about ensuring that we do our best to make any given display
configuration fit within the bandwidth restraints of the topology, since
otherwise less people's monitor configurations will work.

Allowing for BPC settings higher than 8 dramatically increases the
required bandwidth for displays in most configurations, and consequently
makes it a lot less likely that said display configurations will pass
the atomic check.

In the future we want to fix this correctly by making it so that we
adjust the bpp for each display in a topology to be as high as possible,
while making sure to lower the bpp of each display in the event that we
run out of bandwidth and need to rerun our atomic check. But for now,
follow the behavior that both i915 and amdgpu are sticking to.

Signed-off-by: Lyude Paul <lyude at redhat.com>
Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for MST")
Cc: Ben Skeggs <bskeggs at redhat.com>
Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
Cc: David Airlie <airlied at redhat.com>
Cc: Jerry Zuo <Jerry.Zuo at amd.com>
Cc: Harry Wentland <harry.wentland at amd.com>
Cc: Juston Li <juston.li at intel.com>
Cc: Sam Ravnborg <sam at ravnborg.org>
Cc: Sean Paul <seanpaul at chromium.org>
Cc: <stable at vger.kernel.org> # v5.1+
---
 drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 93665aecce57..9ac47fe519f8 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -798,7 +798,14 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
 	if (!state->duplicated) {
 		const int clock = crtc_state->adjusted_mode.clock;
 
-		asyh->or.bpc = connector->display_info.bpc;
+		/*
+		 * XXX: Since we don't use HDR in userspace quite yet, limit
+		 * the bpc to 8 to save bandwidth on the topology. In the
+		 * future, we'll want to properly fix this by dynamically
+		 * selecting the highest possible bpc that would fit in the
+		 * topology
+		 */
+		asyh->or.bpc = min(connector->display_info.bpc, 8U);
 		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3);
 	}
 
-- 
2.21.0

Thierry Reding

2019-Nov-18 13:35 UTC

head link

[Nouveau] [PATCH 2/3] drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom

On Fri, Nov 15, 2019 at 04:07:19PM -0500, Lyude Paul
wrote:> In order to be able to use bpc values that are different from what the
> connector reports, we want to be able to store the bpc value we decide
> on using for an atomic state in nv50_head_atom and refer to that instead
> of simply using the value that the connector reports throughout the
> whole atomic check phase and commit phase. This will let us (eventually)
> implement the max bpc connector property, and will also be needed for
> limiting the bpc we use on MST displays to 8 in the next commit.
> 
> Signed-off-by: Lyude Paul <lyude at redhat.com>
> Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for
MST")
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: David Airlie <airlied at redhat.com>
> Cc: Jerry Zuo <Jerry.Zuo at amd.com>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Juston Li <juston.li at intel.com>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: <stable at vger.kernel.org> # v5.1+
> ---
>  drivers/gpu/drm/nouveau/dispnv50/atom.h |  1 +
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 57 ++++++++++++++-----------
>  drivers/gpu/drm/nouveau/dispnv50/head.c |  5 +--
>  3 files changed, 36 insertions(+), 27 deletions(-)
> 
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/atom.h
b/drivers/gpu/drm/nouveau/dispnv50/atom.h
> index 43df86c38f58..24f7700768da 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/atom.h
> +++ b/drivers/gpu/drm/nouveau/dispnv50/atom.h
> @@ -114,6 +114,7 @@ struct nv50_head_atom {
>  		u8 nhsync:1;
>  		u8 nvsync:1;
>  		u8 depth:4;
> +		u8 bpc;
>  	} or;
>  
>  	/* Currently only used for MST */
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 6327aaf37c08..93665aecce57 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -353,10 +353,20 @@ nv50_outp_atomic_check(struct drm_encoder *encoder,
>  		       struct drm_crtc_state *crtc_state,
>  		       struct drm_connector_state *conn_state)
>  {
> -	struct nouveau_connector *nv_connector > -	
nouveau_connector(conn_state->connector);
> -	return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
> -					   nv_connector->native_mode);
> +	struct drm_connector *connector = conn_state->connector;
> +	struct nouveau_connector *nv_connector = nouveau_connector(connector);
> +	struct nv50_head_atom *asyh = nv50_head_atom(crtc_state);
> +	int ret;
> +
> +	ret = nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
> +					  nv_connector->native_mode);
> +	if (ret)
> +		return ret;
> +
> +	if (crtc_state->mode_changed || crtc_state->connectors_changed)
> +		asyh->or.bpc = connector->display_info.bpc;
> +
> +	return 0;
>  }
>  
> 
/******************************************************************************
> @@ -786,10 +796,10 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>  	 * may have changed after the state was duplicated
>  	 */
>  	if (!state->duplicated) {
> -		const int bpp = connector->display_info.bpc * 3;
>  		const int clock = crtc_state->adjusted_mode.clock;
>  
> -		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
> +		asyh->or.bpc = connector->display_info.bpc;
> +		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, asyh->or.bpc * 3);
>  	}
>  
>  	slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
mstc->port,
> @@ -802,6 +812,17 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>  	return 0;
>  }
>  
> +static u8
> +nv50_dp_bpc_to_depth(unsigned int bpc)
> +{
> +	switch (bpc) {
> +	case  6: return 0x2;
> +	case  8: return 0x5;
> +	case 10: /* fall-through */
> +	default: return 0x6;
> +	}
This is obviously just refactored from the code below, so this is
probably fine for now. But what about BPC > 10? The OR here seems to be
very similar to what's used on Tegra where the same values are used in
the SOR_STATE1 register, see:

	drivers/gpu/drm/tegra/sor.h

There are additional values for 12 and 16 BPC (see the definitions for
SOR_STATE_ASY_PIXELDEPTH_BPP_*). With the above anything higher than 10
BPC will be treated the same and likely lead to wrong results. So I
think either a WARN for the "default" case or additional cases for the
other values would be good to have.

Like I said, if this even is problematic (and given that userspace does
not really support > 8 BPC yet, it probably isn't) it's a preexisting
problem, so can be done in a different patch.

Other than that this looks good, so:

Reviewed-by: Thierry Reding <treding at nvidia.com>
> +}
> +
>  static void
>  nv50_msto_enable(struct drm_encoder *encoder)
>  {
> @@ -812,7 +833,7 @@ nv50_msto_enable(struct drm_encoder *encoder)
>  	struct nv50_mstm *mstm = NULL;
>  	struct drm_connector *connector;
>  	struct drm_connector_list_iter conn_iter;
> -	u8 proto, depth;
> +	u8 proto;
>  	bool r;
>  
>  	drm_connector_list_iter_begin(encoder->dev, &conn_iter);
> @@ -841,14 +862,8 @@ nv50_msto_enable(struct drm_encoder *encoder)
>  	else
>  		proto = 0x9;
>  
> -	switch (mstc->connector.display_info.bpc) {
> -	case  6: depth = 0x2; break;
> -	case  8: depth = 0x5; break;
> -	case 10:
> -	default: depth = 0x6; break;
> -	}
> -
> -	mstm->outp->update(mstm->outp, head->base.index, armh, proto,
depth);
> +	mstm->outp->update(mstm->outp, head->base.index, armh, proto,
> +			   nv50_dp_bpc_to_depth(armh->or.bpc));
>  
>  	msto->head = head;
>  	msto->mstc = mstc;
> @@ -1502,20 +1517,14 @@ nv50_sor_enable(struct drm_encoder *encoder)
>  					lvds.lvds.script |= 0x0200;
>  			}
>  
> -			if (nv_connector->base.display_info.bpc == 8)
> +			if (asyh->or.bpc == 8)
>  				lvds.lvds.script |= 0x0200;
>  		}
>  
>  		nvif_mthd(&disp->disp->object, 0, &lvds, sizeof(lvds));
>  		break;
>  	case DCB_OUTPUT_DP:
> -		if (nv_connector->base.display_info.bpc == 6)
> -			depth = 0x2;
> -		else
> -		if (nv_connector->base.display_info.bpc == 8)
> -			depth = 0x5;
> -		else
> -			depth = 0x6;
> +		depth = nv50_dp_bpc_to_depth(asyh->or.bpc);
>  
>  		if (nv_encoder->link & 1)
>  			proto = 0x8;
> @@ -1666,7 +1675,7 @@ nv50_pior_enable(struct drm_encoder *encoder)
>  	nv50_outp_acquire(nv_encoder);
>  
>  	nv_connector = nouveau_encoder_connector_get(nv_encoder);
> -	switch (nv_connector->base.display_info.bpc) {
> +	switch (asyh->or.bpc) {
>  	case 10: asyh->or.depth = 0x6; break;
>  	case  8: asyh->or.depth = 0x5; break;
>  	case  6: asyh->or.depth = 0x2; break;
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/head.c
b/drivers/gpu/drm/nouveau/dispnv50/head.c
> index 71c23bf1fe25..c9692df2b76c 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/head.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/head.c
> @@ -81,18 +81,17 @@ nv50_head_atomic_check_dither(struct nv50_head_atom
*armh,
>  			      struct nv50_head_atom *asyh,
>  			      struct nouveau_conn_atom *asyc)
>  {
> -	struct drm_connector *connector = asyc->state.connector;
>  	u32 mode = 0x00;
>  
>  	if (asyc->dither.mode == DITHERING_MODE_AUTO) {
> -		if (asyh->base.depth > connector->display_info.bpc * 3)
> +		if (asyh->base.depth > asyh->or.bpc * 3)
>  			mode = DITHERING_MODE_DYNAMIC2X2;
>  	} else {
>  		mode = asyc->dither.mode;
>  	}
>  
>  	if (asyc->dither.depth == DITHERING_DEPTH_AUTO) {
> -		if (connector->display_info.bpc >= 8)
> +		if (asyh->or.bpc >= 8)
>  			mode |= DITHERING_DEPTH_8BPC;
>  	} else {
>  		mode |= asyc->dither.depth;
> -- 
> 2.21.0
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20191118/56c577df/attachment.sig>

Thierry Reding

2019-Nov-18 13:36 UTC

head link

[Nouveau] [PATCH 3/3] drm/nouveau/kms/nv50-: Limit MST BPC to 8

On Fri, Nov 15, 2019 at 04:07:20PM -0500, Lyude Paul
wrote:> Noticed this while working on some unrelated CRC stuff. Currently,
> userspace has very little support for BPCs higher than 8. While this
> doesn't matter for most things, on MST topologies we need to be careful
> about ensuring that we do our best to make any given display
> configuration fit within the bandwidth restraints of the topology, since
> otherwise less people's monitor configurations will work.
> 
> Allowing for BPC settings higher than 8 dramatically increases the
> required bandwidth for displays in most configurations, and consequently
> makes it a lot less likely that said display configurations will pass
> the atomic check.
> 
> In the future we want to fix this correctly by making it so that we
> adjust the bpp for each display in a topology to be as high as possible,
> while making sure to lower the bpp of each display in the event that we
> run out of bandwidth and need to rerun our atomic check. But for now,
> follow the behavior that both i915 and amdgpu are sticking to.
> 
> Signed-off-by: Lyude Paul <lyude at redhat.com>
> Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for
MST")
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: David Airlie <airlied at redhat.com>
> Cc: Jerry Zuo <Jerry.Zuo at amd.com>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Juston Li <juston.li at intel.com>
> Cc: Sam Ravnborg <sam at ravnborg.org>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: <stable at vger.kernel.org> # v5.1+
> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
Reviewed-by: Thierry Reding <treding at nvidia.com>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20191118/55b4b669/attachment.sig>

Thierry Reding

2019-Nov-18 13:41 UTC

head link

[Nouveau] [PATCH 1/3] drm/nouveau/kms/nv50-: Call outp_atomic_check_view() before handling PBN

On Fri, Nov 15, 2019 at 04:07:18PM -0500, Lyude Paul
wrote:> Since nv50_outp_atomic_check_view() can set crtc_state->mode_changed, we
> probably should be calling it before handling any PBN changes. Just a
> precaution.
> 
> Signed-off-by: Lyude Paul <lyude at redhat.com>
> Fixes: 232c9eec417a ("drm/nouveau: Use atomic VCPI helpers for
MST")
> Cc: Ben Skeggs <bskeggs at redhat.com>
> Cc: Daniel Vetter <daniel.vetter at ffwll.ch>
> Cc: David Airlie <airlied at redhat.com>
> Cc: Jerry Zuo <Jerry.Zuo at amd.com>
> Cc: Harry Wentland <harry.wentland at amd.com>
> Cc: Juston Li <juston.li at intel.com>
> Cc: Sean Paul <seanpaul at chromium.org>
> Cc: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
> Cc: <stable at vger.kernel.org> # v5.1+
> ---
>  drivers/gpu/drm/nouveau/dispnv50/disp.c | 44 ++++++++++++++-----------
>  1 file changed, 24 insertions(+), 20 deletions(-)
Looks reasonable:

Reviewed-by: Thierry Reding <treding at nvidia.com>
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 549486f1d937..6327aaf37c08 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -770,32 +770,36 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>  	struct nv50_mstm *mstm = mstc->mstm;
>  	struct nv50_head_atom *asyh = nv50_head_atom(crtc_state);
>  	int slots;
> +	int ret;
>  
> -	if (crtc_state->mode_changed || crtc_state->connectors_changed) {
> -		/*
> -		 * When restoring duplicated states, we need to make sure that
> -		 * the bw remains the same and avoid recalculating it, as the
> -		 * connector's bpc may have changed after the state was
> -		 * duplicated
> -		 */
> -		if (!state->duplicated) {
> -			const int bpp = connector->display_info.bpc * 3;
> -			const int clock = crtc_state->adjusted_mode.clock;
> +	ret = nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
> +					  mstc->native);
> +	if (ret)
> +		return ret;
>  
> -			asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
> -		}
> +	if (!crtc_state->mode_changed &&
!crtc_state->connectors_changed)
> +		return 0;
>  
> -		slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
> -						      mstc->port,
> -						      asyh->dp.pbn);
> -		if (slots < 0)
> -			return slots;
> +	/*
> +	 * When restoring duplicated states, we need to make sure that the bw
> +	 * remains the same and avoid recalculating it, as the connector's
bpc
> +	 * may have changed after the state was duplicated
> +	 */
> +	if (!state->duplicated) {
> +		const int bpp = connector->display_info.bpc * 3;
> +		const int clock = crtc_state->adjusted_mode.clock;
>  
> -		asyh->dp.tu = slots;
> +		asyh->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp);
>  	}
>  
> -	return nv50_outp_atomic_check_view(encoder, crtc_state, conn_state,
> -					   mstc->native);
> +	slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
mstc->port,
> +					      asyh->dp.pbn);
> +	if (slots < 0)
> +		return slots;
> +
> +	asyh->dp.tu = slots;
> +
> +	return 0;
>  }
>  
>  static void
> -- 
> 2.21.0
> 
> _______________________________________________
> Nouveau mailing list
> Nouveau at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/nouveau-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL:
<https://lists.freedesktop.org/archives/nouveau/attachments/20191118/e804a8f9/attachment.sig>

Possibly Parallel Threads

Search for more reasonably related threads

Nouveau - Nov 2019 - [PATCH 0/3] MST BPC fixes for nouveau

[Nouveau] [PATCH 0/3] MST BPC fixes for nouveau

[Nouveau] [PATCH 1/3] drm/nouveau/kms/nv50-: Call outp_atomic_check_view() before handling PBN

[Nouveau] [PATCH 2/3] drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom

[Nouveau] [PATCH 3/3] drm/nouveau/kms/nv50-: Limit MST BPC to 8

[Nouveau] [PATCH 2/3] drm/nouveau/kms/nv50-: Store the bpc we're using in nv50_head_atom

[Nouveau] [PATCH 3/3] drm/nouveau/kms/nv50-: Limit MST BPC to 8

[Nouveau] [PATCH 1/3] drm/nouveau/kms/nv50-: Call outp_atomic_check_view() before handling PBN

Possibly Parallel Threads