| Message ID | 20191030192431.5798-3-mikita.lipski@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | DSC MST support for AMDGPU | expand |
hey-sorry this took a little bit to get back to. I reviewed this before, but this patch doesn't seem to be rebased against drm-tip otherwise it would reflect the changes that happened with drm_dp_calc_pbn_mode() upstream: https://patchwork.freedesktop.org/patch/332935/ can you rebase these patches against drm-tip and also update the igt_dp_mst_calc_pbn_mode() test here? https://cgit.freedesktop.org/drm-tip/tree/drivers/gpu/drm/selftests/test-drm_dp_mst_helper.c#n14 On Wed, 2019-10-30 at 15:24 -0400, 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 > > 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> > --- > .../gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c | 2 +- > drivers/gpu/drm/drm_dp_mst_topology.c | 16 ++++++++++++---- > drivers/gpu/drm/i915/display/intel_dp_mst.c | 3 ++- > drivers/gpu/drm/nouveau/dispnv50/disp.c | 3 ++- > drivers/gpu/drm/radeon/radeon_dp_mst.c | 2 +- > include/drm/drm_dp_mst_helper.h | 3 +-- > 6 files changed, 19 insertions(+), 10 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 d75726013436..1309e9cfdea3 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c > @@ -4656,7 +4656,7 @@ static int dm_encoder_helper_atomic_check(struct > drm_encoder *encoder, > color_depth = convert_color_depth_from_display_info(connector, > conn_state); > 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 82add736e17d..3e7b7553cf4d 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3534,10 +3534,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) > { > u64 kbps; > s64 peak_kbps; > @@ -3555,11 +3556,18 @@ 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 > */ > > numerator = 64 * 1006; > denominator = 54 * 8 * 1000 * 1000; > > + if (dsc) > + numerator /= 16; > + > kbps *= numerator; > peak_kbps = drm_fixp_from_fraction(kbps, denominator); > > @@ -3570,19 +3578,19 @@ EXPORT_SYMBOL(drm_dp_calc_pbn_mode); > static int test_calc_pbn_mode(void) > { > int ret; > - ret = drm_dp_calc_pbn_mode(154000, 30); > + ret = drm_dp_calc_pbn_mode(154000, 30, false); > if (ret != 689) { > DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, > expected PBN %d, actual PBN %d.\n", > 154000, 30, 689, ret); > return -EINVAL; > } > - ret = drm_dp_calc_pbn_mode(234000, 30); > + ret = drm_dp_calc_pbn_mode(234000, 30, false); > if (ret != 1047) { > DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, > expected PBN %d, actual PBN %d.\n", > 234000, 30, 1047, ret); > return -EINVAL; > } > - ret = drm_dp_calc_pbn_mode(297000, 24); > + ret = drm_dp_calc_pbn_mode(297000, 24, false); > if (ret != 1063) { > DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, > expected PBN %d, actual PBN %d.\n", > 297000, 24, 1063, ret); > diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c > b/drivers/gpu/drm/i915/display/intel_dp_mst.c > index 2c5ac3dd647f..dfac450841df 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 f1dbc7852414..c45832230ccc 100644 > --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c > +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c > @@ -778,7 +778,8 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, > if (!state->duplicated) > asyh->dp.pbn = > drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, > - connector->display_info.bpc * 3); > + connector->display_info.bpc * 3, > + false); > > if (crtc_state->mode_changed) { > 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 2994f07fbad9..c997f88218f2 100644 > --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c > +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c > @@ -514,7 +514,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/include/drm/drm_dp_mst_helper.h > b/include/drm/drm_dp_mst_helper.h > index 2ba6253ea6d3..9116b2c95239 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -610,8 +610,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 d75726013436..1309e9cfdea3 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm.c @@ -4656,7 +4656,7 @@ static int dm_encoder_helper_atomic_check(struct drm_encoder *encoder, color_depth = convert_color_depth_from_display_info(connector, conn_state); 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 82add736e17d..3e7b7553cf4d 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3534,10 +3534,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) { u64 kbps; s64 peak_kbps; @@ -3555,11 +3556,18 @@ 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 */ numerator = 64 * 1006; denominator = 54 * 8 * 1000 * 1000; + if (dsc) + numerator /= 16; + kbps *= numerator; peak_kbps = drm_fixp_from_fraction(kbps, denominator); @@ -3570,19 +3578,19 @@ EXPORT_SYMBOL(drm_dp_calc_pbn_mode); static int test_calc_pbn_mode(void) { int ret; - ret = drm_dp_calc_pbn_mode(154000, 30); + ret = drm_dp_calc_pbn_mode(154000, 30, false); if (ret != 689) { DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n", 154000, 30, 689, ret); return -EINVAL; } - ret = drm_dp_calc_pbn_mode(234000, 30); + ret = drm_dp_calc_pbn_mode(234000, 30, false); if (ret != 1047) { DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n", 234000, 30, 1047, ret); return -EINVAL; } - ret = drm_dp_calc_pbn_mode(297000, 24); + ret = drm_dp_calc_pbn_mode(297000, 24, false); if (ret != 1063) { DRM_ERROR("PBN calculation test failed - clock %d, bpp %d, expected PBN %d, actual PBN %d.\n", 297000, 24, 1063, ret); diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c index 2c5ac3dd647f..dfac450841df 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 f1dbc7852414..c45832230ccc 100644 --- a/drivers/gpu/drm/nouveau/dispnv50/disp.c +++ b/drivers/gpu/drm/nouveau/dispnv50/disp.c @@ -778,7 +778,8 @@ nv50_msto_atomic_check(struct drm_encoder *encoder, if (!state->duplicated) asyh->dp.pbn = drm_dp_calc_pbn_mode(crtc_state->adjusted_mode.clock, - connector->display_info.bpc * 3); + connector->display_info.bpc * 3, + false); if (crtc_state->mode_changed) { 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 2994f07fbad9..c997f88218f2 100644 --- a/drivers/gpu/drm/radeon/radeon_dp_mst.c +++ b/drivers/gpu/drm/radeon/radeon_dp_mst.c @@ -514,7 +514,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/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 2ba6253ea6d3..9116b2c95239 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -610,8 +610,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);