Re: [PATCH v9 01/18] drm/dp_mst: Add PBN calculation for DSC modes
Actually, one comment on this that should be very simple to add
On Fri, 2019-12-13 at 15:08 -0500, mikita.lip...@amd.com wrote:
> From: David Francis
>
> 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
> Reviewed-by: Lyude Paul
> Reviewed-by: Harry Wentland
> Signed-off-by: David Francis
> Signed-off-by: Mikita Lipski
> ---
> 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 *= 8convert 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
[PATCH v9 01/18] drm/dp_mst: Add PBN calculation for DSC modes
From: David Francis
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
Reviewed-by: Lyude Paul
Reviewed-by: Harry Wentland
Signed-off-by: David Francis
Signed-off-by: Mikita Lipski
---
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 *= 8convert 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_ms
