diff mbox series

[v9,01/18] drm/dp_mst: Add PBN calculation for DSC modes

Message ID 20191213200854.31545-2-mikita.lipski@amd.com (mailing list archive)
State New, archived
Headers show
Series DSC MST support for DRM and AMDGPU | expand

Commit Message

Lipski, Mikita Dec. 13, 2019, 8:08 p.m. UTC
From: David Francis <David.Francis@amd.com>

With DSC, bpp can be fractional in multiples of 1/16.

Change drm_dp_calc_pbn_mode to reflect this, adding a new
parameter bool dsc. When this parameter is true, treat the
bpp parameter as having units not of bits per pixel, but
1/16 of a bit per pixel

v2: Don't add separate function for this
v3: In the equation divide bpp by 16 as it is expected
not to leave any remainder

Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
Reviewed-by: Lyude Paul <lyude@redhat.com>
Reviewed-by: Harry Wentland <harry.wentland@amd.com>
Signed-off-by: David Francis <David.Francis@amd.com>
Signed-off-by: Mikita Lipski <mikita.lipski@amd.com>
---
 drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
 drivers/gpu/drm/drm_dp_mst_topology.c              | 12 +++++++++++-
 drivers/gpu/drm/i915/display/intel_dp_mst.c        |  3 ++-
 drivers/gpu/drm/nouveau/dispnv50/disp.c            |  2 +-
 drivers/gpu/drm/radeon/radeon_dp_mst.c             |  2 +-
 drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c | 10 ++++++----
 include/drm/drm_dp_mst_helper.h                    |  3 +--
 7 files changed, 23 insertions(+), 11 deletions(-)

Comments

Lyude Paul Dec. 20, 2019, 9:35 p.m. UTC | #1
Actually, one comment on this that should be very simple to add

On Fri, 2019-12-13 at 15:08 -0500, mikita.lipski@amd.com wrote:
> From: David Francis <David.Francis@amd.com>
> 
> With DSC, bpp can be fractional in multiples of 1/16.
> 
> Change drm_dp_calc_pbn_mode to reflect this, adding a new
> parameter bool dsc. When this parameter is true, treat the
> bpp parameter as having units not of bits per pixel, but
> 1/16 of a bit per pixel
> 
> v2: Don't add separate function for this
> v3: In the equation divide bpp by 16 as it is expected
> not to leave any remainder
> 
> Reviewed-by: Manasi Navare <manasi.d.navare@intel.com>
> Reviewed-by: Lyude Paul <lyude@redhat.com>
> Reviewed-by: Harry Wentland <harry.wentland@amd.com>
> Signed-off-by: David Francis <David.Francis@amd.com>
> Signed-off-by: Mikita Lipski <mikita.lipski@amd.com>
> ---
>  drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c  |  2 +-
>  drivers/gpu/drm/drm_dp_mst_topology.c              | 12 +++++++++++-
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  3 ++-
>  drivers/gpu/drm/nouveau/dispnv50/disp.c            |  2 +-
>  drivers/gpu/drm/radeon/radeon_dp_mst.c             |  2 +-
>  drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c | 10 ++++++----
>  include/drm/drm_dp_mst_helper.h                    |  3 +--
>  7 files changed, 23 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> index 455c51c38720..9fc03fc1017d 100644
> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
> @@ -4967,7 +4967,7 @@ static int dm_encoder_helper_atomic_check(struct
> drm_encoder *encoder,
>  								    is_y420);
>  		bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
>  		clock = adjusted_mode->clock;
> -		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock,
> bpp);
> +		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp,
> false);
>  	}
>  	dm_new_connector_state->vcpi_slots =
> drm_dp_atomic_find_vcpi_slots(state,
>  									   mst
> _mgr,
> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c
> b/drivers/gpu/drm/drm_dp_mst_topology.c
> index ae5809a1f19a..363e7e58e7e7 100644
> --- a/drivers/gpu/drm/drm_dp_mst_topology.c
> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c
> @@ -4342,10 +4342,11 @@ EXPORT_SYMBOL(drm_dp_check_act_status);
>   * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
>   * @clock: dot clock for the mode
>   * @bpp: bpp for the mode.
> + * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
>   *
>   * This uses the formula in the spec to calculate the PBN value for a mode.
>   */
> -int drm_dp_calc_pbn_mode(int clock, int bpp)
> +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
>  {
>  	/*
>  	 * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
> @@ -4356,7 +4357,16 @@ int drm_dp_calc_pbn_mode(int clock, int bpp)
>  	 * peak_kbps *= (1006/1000)
>  	 * peak_kbps *= (64/54)
>  	 * peak_kbps *= 8    convert to bytes
> +	 *
> +	 * If the bpp is in units of 1/16, further divide by 16. Put this
> +	 * factor in the numerator rather than the denominator to avoid
> +	 * integer overflow
>  	 */
> +
> +	if (dsc)
> +		return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 *
> 1006),
> +					8 * 54 * 1000 * 1000);
> +
>  	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006),
>  				8 * 54 * 1000 * 1000);
>  }
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 03d1cba0b696..92be17711287 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -61,7 +61,8 @@ static int intel_dp_mst_compute_link_config(struct
> intel_encoder *encoder,
>  		crtc_state->pipe_bpp = bpp;
>  
>  		crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode-
> >crtc_clock,
> -						       crtc_state->pipe_bpp);
> +						       crtc_state->pipe_bpp,
> +						       false);
>  
>  		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp-
> >mst_mgr,
>  						      port, crtc_state->pbn);
> diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> index 549486f1d937..1c9e23d5a6fd 100644
> --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
> +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
> @@ -782,7 +782,7 @@ nv50_msto_atomic_check(struct drm_encoder *encoder,
>  			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->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp,
> false);
>  		}
>  
>  		slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
> diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> index ee28f5b3785e..28eef9282874 100644
> --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
> +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
> @@ -518,7 +518,7 @@ static bool radeon_mst_mode_fixup(struct drm_encoder
> *encoder,
>  
>  	mst_enc = radeon_encoder->enc_priv;
>  
> -	mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp);
> +	mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp, false);
>  
>  	mst_enc->primary->active_device = mst_enc->primary->devices & mst_enc-
> >connector->devices;
>  	DRM_DEBUG_KMS("setting active device to %08x from %08x %08x for
> encoder %d\n",
> diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> index af2b2de65316..73fc1c485283 100644
> --- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> +++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
> @@ -18,15 +18,17 @@ int igt_dp_mst_calc_pbn_mode(void *ignored)
>  		int rate;
>  		int bpp;
>  		int expected;
> +		bool dsc;
>  	} test_params[] = {
> -		{ 154000, 30, 689 },
> -		{ 234000, 30, 1047 },
> -		{ 297000, 24, 1063 },
> +		{ 154000, 30, 689, false },
> +		{ 234000, 30, 1047, false },
> +		{ 297000, 24, 1063, false },

Mind adding one or two test_params that actually use dsc here?

>  	};
>  
>  	for (i = 0; i < ARRAY_SIZE(test_params); i++) {
>  		pbn = drm_dp_calc_pbn_mode(test_params[i].rate,
> -					   test_params[i].bpp);
> +					   test_params[i].bpp,
> +					   test_params[i].dsc);
>  		FAIL(pbn != test_params[i].expected,
>  		     "Expected PBN %d for clock %d bpp %d, got %d\n",
>  		     test_params[i].expected, test_params[i].rate,
> diff --git a/include/drm/drm_dp_mst_helper.h
> b/include/drm/drm_dp_mst_helper.h
> index d5fc90b30487..68656913cfe5 100644
> --- a/include/drm/drm_dp_mst_helper.h
> +++ b/include/drm/drm_dp_mst_helper.h
> @@ -719,8 +719,7 @@ bool drm_dp_mst_port_has_audio(struct
> drm_dp_mst_topology_mgr *mgr,
>  struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct
> drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
>  
>  
> -int drm_dp_calc_pbn_mode(int clock, int bpp);
> -
> +int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
>  
>  bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
>  			      struct drm_dp_mst_port *port, int pbn, int
> slots);
diff mbox series

Patch

diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
index 455c51c38720..9fc03fc1017d 100644
--- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
+++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c
@@ -4967,7 +4967,7 @@  static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder,
 								    is_y420);
 		bpp = convert_dc_color_depth_into_bpc(color_depth) * 3;
 		clock = adjusted_mode->clock;
-		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp);
+		dm_new_connector_state->pbn = drm_dp_calc_pbn_mode(clock, bpp, false);
 	}
 	dm_new_connector_state->vcpi_slots = drm_dp_atomic_find_vcpi_slots(state,
 									   mst_mgr,
diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c
index ae5809a1f19a..363e7e58e7e7 100644
--- a/drivers/gpu/drm/drm_dp_mst_topology.c
+++ b/drivers/gpu/drm/drm_dp_mst_topology.c
@@ -4342,10 +4342,11 @@  EXPORT_SYMBOL(drm_dp_check_act_status);
  * drm_dp_calc_pbn_mode() - Calculate the PBN for a mode.
  * @clock: dot clock for the mode
  * @bpp: bpp for the mode.
+ * @dsc: DSC mode. If true, bpp has units of 1/16 of a bit per pixel
  *
  * This uses the formula in the spec to calculate the PBN value for a mode.
  */
-int drm_dp_calc_pbn_mode(int clock, int bpp)
+int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc)
 {
 	/*
 	 * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006
@@ -4356,7 +4357,16 @@  int drm_dp_calc_pbn_mode(int clock, int bpp)
 	 * peak_kbps *= (1006/1000)
 	 * peak_kbps *= (64/54)
 	 * peak_kbps *= 8    convert to bytes
+	 *
+	 * If the bpp is in units of 1/16, further divide by 16. Put this
+	 * factor in the numerator rather than the denominator to avoid
+	 * integer overflow
 	 */
+
+	if (dsc)
+		return DIV_ROUND_UP_ULL(mul_u32_u32(clock * (bpp / 16), 64 * 1006),
+					8 * 54 * 1000 * 1000);
+
 	return DIV_ROUND_UP_ULL(mul_u32_u32(clock * bpp, 64 * 1006),
 				8 * 54 * 1000 * 1000);
 }
diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 03d1cba0b696..92be17711287 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -61,7 +61,8 @@  static int intel_dp_mst_compute_link_config(struct intel_encoder *encoder,
 		crtc_state->pipe_bpp = bpp;
 
 		crtc_state->pbn = drm_dp_calc_pbn_mode(adjusted_mode->crtc_clock,
-						       crtc_state->pipe_bpp);
+						       crtc_state->pipe_bpp,
+						       false);
 
 		slots = drm_dp_atomic_find_vcpi_slots(state, &intel_dp->mst_mgr,
 						      port, crtc_state->pbn);
diff --git a/drivers/gpu/drm/nouveau/dispnv50/disp.c b/drivers/gpu/drm/nouveau/dispnv50/disp.c
index 549486f1d937..1c9e23d5a6fd 100644
--- a/drivers/gpu/drm/nouveau/dispnv50/disp.c
+++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c
@@ -782,7 +782,7 @@  nv50_msto_atomic_check(struct drm_encoder *encoder,
 			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->dp.pbn = drm_dp_calc_pbn_mode(clock, bpp, false);
 		}
 
 		slots = drm_dp_atomic_find_vcpi_slots(state, &mstm->mgr,
diff --git a/drivers/gpu/drm/radeon/radeon_dp_mst.c b/drivers/gpu/drm/radeon/radeon_dp_mst.c
index ee28f5b3785e..28eef9282874 100644
--- a/drivers/gpu/drm/radeon/radeon_dp_mst.c
+++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c
@@ -518,7 +518,7 @@  static bool radeon_mst_mode_fixup(struct drm_encoder *encoder,
 
 	mst_enc = radeon_encoder->enc_priv;
 
-	mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp);
+	mst_enc->pbn = drm_dp_calc_pbn_mode(adjusted_mode->clock, bpp, false);
 
 	mst_enc->primary->active_device = mst_enc->primary->devices & mst_enc->connector->devices;
 	DRM_DEBUG_KMS("setting active device to %08x from %08x %08x for encoder %d\n",
diff --git a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
index af2b2de65316..73fc1c485283 100644
--- a/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
+++ b/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c
@@ -18,15 +18,17 @@  int igt_dp_mst_calc_pbn_mode(void *ignored)
 		int rate;
 		int bpp;
 		int expected;
+		bool dsc;
 	} test_params[] = {
-		{ 154000, 30, 689 },
-		{ 234000, 30, 1047 },
-		{ 297000, 24, 1063 },
+		{ 154000, 30, 689, false },
+		{ 234000, 30, 1047, false },
+		{ 297000, 24, 1063, false },
 	};
 
 	for (i = 0; i < ARRAY_SIZE(test_params); i++) {
 		pbn = drm_dp_calc_pbn_mode(test_params[i].rate,
-					   test_params[i].bpp);
+					   test_params[i].bpp,
+					   test_params[i].dsc);
 		FAIL(pbn != test_params[i].expected,
 		     "Expected PBN %d for clock %d bpp %d, got %d\n",
 		     test_params[i].expected, test_params[i].rate,
diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h
index d5fc90b30487..68656913cfe5 100644
--- a/include/drm/drm_dp_mst_helper.h
+++ b/include/drm/drm_dp_mst_helper.h
@@ -719,8 +719,7 @@  bool drm_dp_mst_port_has_audio(struct drm_dp_mst_topology_mgr *mgr,
 struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port);
 
 
-int drm_dp_calc_pbn_mode(int clock, int bpp);
-
+int drm_dp_calc_pbn_mode(int clock, int bpp, bool dsc);
 
 bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr,
 			      struct drm_dp_mst_port *port, int pbn, int slots);