| 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 |
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 --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);