| Message ID | 20190820191203.25807-7-David.Francis@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Display Stream Compression (DSC) for AMD Navi | expand |
Some nitpicks below On Tue, 2019-08-20 at 15:11 -0400, David Francis wrote: > We were using drm helpers to convert a timing into its > bandwidth, its bandwidth into pbn, and its pbn into timeslots > > These helpers > -Did not take DSC timings into account > -Used the link rate and lane count of the link's aux device, > which are not the same as the link's current cap > -Did not take FEC into account (FEC reduces the PBN per timeslot) > > For converting timing into PBN, add a new function > drm_dp_calc_pbn_mode_dsc that handles the DSC case > > For converting PBN into time slots, amdgpu doesn't use the > 'correct' atomic method (drm_dp_atomic_find_vcpi_slots), so > don't add a new helper to cover our approach. Use the same > means of calculating pbn per time slot as the DSC code. > > v2: Add drm helper for clock to pbn conversion > > Cc: Jerry Zuo <Jerry.Zuo@amd.com> > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > Signed-off-by: David Francis <David.Francis@amd.com> > --- > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 18 +++++--- > drivers/gpu/drm/drm_dp_mst_topology.c | 41 +++++++++++++++++++ > include/drm/drm_dp_mst_helper.h | 2 +- > 3 files changed, 54 insertions(+), 7 deletions(-) > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > index 5f2c315b18ba..dfa99e0d6e64 100644 > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > @@ -189,8 +189,8 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( > int slots = 0; > bool ret; > int clock; > - int bpp = 0; > int pbn = 0; > + int pbn_per_timeslot, bpp = 0; > > aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; > > @@ -208,7 +208,6 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( > clock = stream->timing.pix_clk_100hz / 10; > > switch (stream->timing.display_color_depth) { > - > case COLOR_DEPTH_666: > bpp = 6; > break; > @@ -234,11 +233,18 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( > > bpp = bpp * 3; > > - /* TODO need to know link rate */ > - > - pbn = drm_dp_calc_pbn_mode(clock, bpp); > +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT > + if (stream->timing.flags.DSC) > + pbn = drm_dp_calc_pbn_mode_dsc(clock, > + stream- > >timing.dsc_cfg.bits_per_pixel); > + else > +#endif > + pbn = drm_dp_calc_pbn_mode(clock, bpp); > > - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); > + /* Convert kilobits per second / 64 (for 64 timeslots) to pbn > (54/64 megabytes per second) */ > + pbn_per_timeslot = dc_link_bandwidth_kbps( > + stream->link, dc_link_get_link_cap(stream- > >link)) / (8 * 1000 * 54); > + slots = DIV_ROUND_UP(pbn, pbn_per_timeslot); > ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots); > > if (!ret) > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > b/drivers/gpu/drm/drm_dp_mst_topology.c > index 398e7314ea8b..d789b7af7dbf 100644 > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > @@ -3588,6 +3588,47 @@ static int test_calc_pbn_mode(void) > return 0; > } > > +/** > + * drm_dp_calc_pbn_mode_dsc() - Calculate the PBN for a mode with DSC > enabled. > + * @clock: dot clock for the mode > + * @dsc_bpp: dsc bits per pixel x16 (e.g. dsc_bpp = 136 is 8.5 bpp) > + * > + * This uses the formula in the spec to calculate the PBN value for a mode, > + * given that the mode is using DSC You should use the proper kdoc formatting for documenting return values (not all of the DP MST code does this currently, but that's a bug I haven't taken the time to fix :): /* * foo_bar() - foo the bar * * foos the bar, guaranteed * Returns: * Some magic number */ > + */ > +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp) > +{ > + u64 kbps; > + s64 peak_kbps; > + u32 numerator; > + u32 denominator; > + > + kbps = clock * dsc_bpp; > + > + /* > + * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006 > + * The unit of 54/64Mbytes/sec is an arbitrary unit chosen based on > + * common multiplier to render an integer PBN for all link rate/lane > + * counts combinations > + * calculate > + * peak_kbps *= (1/16) bppx16 to bpp > + * peak_kbps *= (1006/1000) > + * peak_kbps *= (64/54) > + * peak_kbps *= 8 convert to bytes > + * > + * Divide numerator and denominator by 16 to avoid overflow > + */ > + > + numerator = 64 * 1006 / 16; > + denominator = 54 * 8 * 1000 * 1000; > + > + kbps *= numerator; > + peak_kbps = drm_fixp_from_fraction(kbps, denominator); > + > + return drm_fixp2int_ceil(peak_kbps); > +} > +EXPORT_SYMBOL(drm_dp_calc_pbn_mode_dsc); > + > /* we want to kick the TX after we've ack the up/down IRQs. */ > static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr) > { > diff --git a/include/drm/drm_dp_mst_helper.h > b/include/drm/drm_dp_mst_helper.h > index 2ba6253ea6d3..ddb518f2157a 100644 > --- a/include/drm/drm_dp_mst_helper.h > +++ b/include/drm/drm_dp_mst_helper.h > @@ -611,7 +611,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector > *connector, struct drm_dp_ > > > int drm_dp_calc_pbn_mode(int clock, int bpp); > - > +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp); > > bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, > struct drm_dp_mst_port *port, int pbn, int > slots);
On 8/20/19 4:43 PM, Lyude Paul wrote: > Some nitpicks below > > On Tue, 2019-08-20 at 15:11 -0400, David Francis wrote: >> We were using drm helpers to convert a timing into its >> bandwidth, its bandwidth into pbn, and its pbn into timeslots >> >> These helpers >> -Did not take DSC timings into account >> -Used the link rate and lane count of the link's aux device, >> which are not the same as the link's current cap >> -Did not take FEC into account (FEC reduces the PBN per timeslot) >> >> For converting timing into PBN, add a new function >> drm_dp_calc_pbn_mode_dsc that handles the DSC case >> >> For converting PBN into time slots, amdgpu doesn't use the >> 'correct' atomic method (drm_dp_atomic_find_vcpi_slots), so >> don't add a new helper to cover our approach. Use the same >> means of calculating pbn per time slot as the DSC code. >> >> v2: Add drm helper for clock to pbn conversion >> >> Cc: Jerry Zuo <Jerry.Zuo@amd.com> >> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> >> Signed-off-by: David Francis <David.Francis@amd.com> I would like ot see Lyude's nitpicks addressed but also having this patch split into one that just adds the helper and another that uses it in amdgpu. Nicholas Kazlauskas >> --- >> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 18 +++++--- >> drivers/gpu/drm/drm_dp_mst_topology.c | 41 +++++++++++++++++++ >> include/drm/drm_dp_mst_helper.h | 2 +- >> 3 files changed, 54 insertions(+), 7 deletions(-) >> >> diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> index 5f2c315b18ba..dfa99e0d6e64 100644 >> --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c >> @@ -189,8 +189,8 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( >> int slots = 0; >> bool ret; >> int clock; >> - int bpp = 0; >> int pbn = 0; >> + int pbn_per_timeslot, bpp = 0; >> >> aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; >> >> @@ -208,7 +208,6 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( >> clock = stream->timing.pix_clk_100hz / 10; >> >> switch (stream->timing.display_color_depth) { >> - >> case COLOR_DEPTH_666: >> bpp = 6; >> break; >> @@ -234,11 +233,18 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( >> >> bpp = bpp * 3; >> >> - /* TODO need to know link rate */ >> - >> - pbn = drm_dp_calc_pbn_mode(clock, bpp); >> +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT >> + if (stream->timing.flags.DSC) >> + pbn = drm_dp_calc_pbn_mode_dsc(clock, >> + stream- >>> timing.dsc_cfg.bits_per_pixel); >> + else >> +#endif >> + pbn = drm_dp_calc_pbn_mode(clock, bpp); >> >> - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); >> + /* Convert kilobits per second / 64 (for 64 timeslots) to pbn >> (54/64 megabytes per second) */ >> + pbn_per_timeslot = dc_link_bandwidth_kbps( >> + stream->link, dc_link_get_link_cap(stream- >>> link)) / (8 * 1000 * 54); >> + slots = DIV_ROUND_UP(pbn, pbn_per_timeslot); >> ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots); >> >> if (!ret) >> diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c >> b/drivers/gpu/drm/drm_dp_mst_topology.c >> index 398e7314ea8b..d789b7af7dbf 100644 >> --- a/drivers/gpu/drm/drm_dp_mst_topology.c >> +++ b/drivers/gpu/drm/drm_dp_mst_topology.c >> @@ -3588,6 +3588,47 @@ static int test_calc_pbn_mode(void) >> return 0; >> } >> >> +/** >> + * drm_dp_calc_pbn_mode_dsc() - Calculate the PBN for a mode with DSC >> enabled. >> + * @clock: dot clock for the mode >> + * @dsc_bpp: dsc bits per pixel x16 (e.g. dsc_bpp = 136 is 8.5 bpp) >> + * >> + * This uses the formula in the spec to calculate the PBN value for a mode, >> + * given that the mode is using DSC > > You should use the proper kdoc formatting for documenting return values (not > all of the DP MST code does this currently, but that's a bug I haven't taken > the time to fix :): > > /* > * foo_bar() - foo the bar > * > * foos the bar, guaranteed > * Returns: > * Some magic number > */ > >> + */ >> +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp) >> +{ >> + u64 kbps; >> + s64 peak_kbps; >> + u32 numerator; >> + u32 denominator; >> + >> + kbps = clock * dsc_bpp; >> + >> + /* >> + * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006 >> + * The unit of 54/64Mbytes/sec is an arbitrary unit chosen based on >> + * common multiplier to render an integer PBN for all link rate/lane >> + * counts combinations >> + * calculate >> + * peak_kbps *= (1/16) bppx16 to bpp >> + * peak_kbps *= (1006/1000) >> + * peak_kbps *= (64/54) >> + * peak_kbps *= 8 convert to bytes >> + * >> + * Divide numerator and denominator by 16 to avoid overflow >> + */ >> + >> + numerator = 64 * 1006 / 16; >> + denominator = 54 * 8 * 1000 * 1000; >> + >> + kbps *= numerator; >> + peak_kbps = drm_fixp_from_fraction(kbps, denominator); >> + >> + return drm_fixp2int_ceil(peak_kbps); >> +} >> +EXPORT_SYMBOL(drm_dp_calc_pbn_mode_dsc); >> + >> /* we want to kick the TX after we've ack the up/down IRQs. */ >> static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr) >> { >> diff --git a/include/drm/drm_dp_mst_helper.h >> b/include/drm/drm_dp_mst_helper.h >> index 2ba6253ea6d3..ddb518f2157a 100644 >> --- a/include/drm/drm_dp_mst_helper.h >> +++ b/include/drm/drm_dp_mst_helper.h >> @@ -611,7 +611,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector >> *connector, struct drm_dp_ >> >> >> int drm_dp_calc_pbn_mode(int clock, int bpp); >> - >> +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp); >> >> bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, >> struct drm_dp_mst_port *port, int pbn, int >> slots);
On Wed, 2019-08-21 at 12:27 +0000, Kazlauskas, Nicholas wrote: > On 8/20/19 4:43 PM, Lyude Paul wrote: > > Some nitpicks below > > > > On Tue, 2019-08-20 at 15:11 -0400, David Francis wrote: > > > We were using drm helpers to convert a timing into its > > > bandwidth, its bandwidth into pbn, and its pbn into timeslots > > > > > > These helpers > > > -Did not take DSC timings into account > > > -Used the link rate and lane count of the link's aux device, > > > which are not the same as the link's current cap > > > -Did not take FEC into account (FEC reduces the PBN per timeslot) > > > > > > For converting timing into PBN, add a new function > > > drm_dp_calc_pbn_mode_dsc that handles the DSC case > > > > > > For converting PBN into time slots, amdgpu doesn't use the > > > 'correct' atomic method (drm_dp_atomic_find_vcpi_slots), so > > > don't add a new helper to cover our approach. Use the same > > > means of calculating pbn per time slot as the DSC code. > > > > > > v2: Add drm helper for clock to pbn conversion > > > > > > Cc: Jerry Zuo <Jerry.Zuo@amd.com> > > > Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> > > > Signed-off-by: David Francis <David.Francis@amd.com> > > I would like ot see Lyude's nitpicks addressed but also having this > patch split into one that just adds the helper and another that uses it > in amdgpu. Sgtm! > > Nicholas Kazlauskas > > > > --- > > > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 18 +++++--- > > > drivers/gpu/drm/drm_dp_mst_topology.c | 41 +++++++++++++++++++ > > > include/drm/drm_dp_mst_helper.h | 2 +- > > > 3 files changed, 54 insertions(+), 7 deletions(-) > > > > > > diff --git a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > index 5f2c315b18ba..dfa99e0d6e64 100644 > > > --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c > > > @@ -189,8 +189,8 @@ bool > > > dm_helpers_dp_mst_write_payload_allocation_table( > > > int slots = 0; > > > bool ret; > > > int clock; > > > - int bpp = 0; > > > int pbn = 0; > > > + int pbn_per_timeslot, bpp = 0; > > > > > > aconnector = (struct amdgpu_dm_connector *)stream- > > > >dm_stream_context; > > > > > > @@ -208,7 +208,6 @@ bool > > > dm_helpers_dp_mst_write_payload_allocation_table( > > > clock = stream->timing.pix_clk_100hz / 10; > > > > > > switch (stream->timing.display_color_depth) { > > > - > > > case COLOR_DEPTH_666: > > > bpp = 6; > > > break; > > > @@ -234,11 +233,18 @@ bool > > > dm_helpers_dp_mst_write_payload_allocation_table( > > > > > > bpp = bpp * 3; > > > > > > - /* TODO need to know link rate */ > > > - > > > - pbn = drm_dp_calc_pbn_mode(clock, bpp); > > > +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT > > > + if (stream->timing.flags.DSC) > > > + pbn = drm_dp_calc_pbn_mode_dsc(clock, > > > + stream- > > > > timing.dsc_cfg.bits_per_pixel); > > > + else > > > +#endif > > > + pbn = drm_dp_calc_pbn_mode(clock, bpp); > > > > > > - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); > > > + /* Convert kilobits per second / 64 (for 64 timeslots) to pbn > > > (54/64 megabytes per second) */ > > > + pbn_per_timeslot = dc_link_bandwidth_kbps( > > > + stream->link, dc_link_get_link_cap(stream- > > > > link)) / (8 * 1000 * 54); > > > + slots = DIV_ROUND_UP(pbn, pbn_per_timeslot); > > > ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, > > > slots); > > > > > > if (!ret) > > > diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c > > > b/drivers/gpu/drm/drm_dp_mst_topology.c > > > index 398e7314ea8b..d789b7af7dbf 100644 > > > --- a/drivers/gpu/drm/drm_dp_mst_topology.c > > > +++ b/drivers/gpu/drm/drm_dp_mst_topology.c > > > @@ -3588,6 +3588,47 @@ static int test_calc_pbn_mode(void) > > > return 0; > > > } > > > > > > +/** > > > + * drm_dp_calc_pbn_mode_dsc() - Calculate the PBN for a mode with DSC > > > enabled. > > > + * @clock: dot clock for the mode > > > + * @dsc_bpp: dsc bits per pixel x16 (e.g. dsc_bpp = 136 is 8.5 bpp) > > > + * > > > + * This uses the formula in the spec to calculate the PBN value for a > > > mode, > > > + * given that the mode is using DSC > > > > You should use the proper kdoc formatting for documenting return values > > (not > > all of the DP MST code does this currently, but that's a bug I haven't > > taken > > the time to fix :): > > > > /* > > * foo_bar() - foo the bar > > * > > * foos the bar, guaranteed > > * Returns: > > * Some magic number > > */ > > > > > + */ > > > +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp) > > > +{ > > > + u64 kbps; > > > + s64 peak_kbps; > > > + u32 numerator; > > > + u32 denominator; > > > + > > > + kbps = clock * dsc_bpp; > > > + > > > + /* > > > + * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006 > > > + * The unit of 54/64Mbytes/sec is an arbitrary unit chosen based on > > > + * common multiplier to render an integer PBN for all link rate/lane > > > + * counts combinations > > > + * calculate > > > + * peak_kbps *= (1/16) bppx16 to bpp > > > + * peak_kbps *= (1006/1000) > > > + * peak_kbps *= (64/54) > > > + * peak_kbps *= 8 convert to bytes > > > + * > > > + * Divide numerator and denominator by 16 to avoid overflow > > > + */ > > > + > > > + numerator = 64 * 1006 / 16; > > > + denominator = 54 * 8 * 1000 * 1000; > > > + > > > + kbps *= numerator; > > > + peak_kbps = drm_fixp_from_fraction(kbps, denominator); > > > + > > > + return drm_fixp2int_ceil(peak_kbps); > > > +} > > > +EXPORT_SYMBOL(drm_dp_calc_pbn_mode_dsc); > > > + > > > /* we want to kick the TX after we've ack the up/down IRQs. */ > > > static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr) > > > { > > > diff --git a/include/drm/drm_dp_mst_helper.h > > > b/include/drm/drm_dp_mst_helper.h > > > index 2ba6253ea6d3..ddb518f2157a 100644 > > > --- a/include/drm/drm_dp_mst_helper.h > > > +++ b/include/drm/drm_dp_mst_helper.h > > > @@ -611,7 +611,7 @@ struct edid *drm_dp_mst_get_edid(struct > > > drm_connector > > > *connector, struct drm_dp_ > > > > > > > > > int drm_dp_calc_pbn_mode(int clock, int bpp); > > > - > > > +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp); > > > > > > 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_helpers.c b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c index 5f2c315b18ba..dfa99e0d6e64 100644 --- a/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c +++ b/drivers/gpu/drm/amd/display/amdgpu_dm/amdgpu_dm_helpers.c @@ -189,8 +189,8 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( int slots = 0; bool ret; int clock; - int bpp = 0; int pbn = 0; + int pbn_per_timeslot, bpp = 0; aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; @@ -208,7 +208,6 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( clock = stream->timing.pix_clk_100hz / 10; switch (stream->timing.display_color_depth) { - case COLOR_DEPTH_666: bpp = 6; break; @@ -234,11 +233,18 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( bpp = bpp * 3; - /* TODO need to know link rate */ - - pbn = drm_dp_calc_pbn_mode(clock, bpp); +#ifdef CONFIG_DRM_AMD_DC_DSC_SUPPORT + if (stream->timing.flags.DSC) + pbn = drm_dp_calc_pbn_mode_dsc(clock, + stream->timing.dsc_cfg.bits_per_pixel); + else +#endif + pbn = drm_dp_calc_pbn_mode(clock, bpp); - slots = drm_dp_find_vcpi_slots(mst_mgr, pbn); + /* Convert kilobits per second / 64 (for 64 timeslots) to pbn (54/64 megabytes per second) */ + pbn_per_timeslot = dc_link_bandwidth_kbps( + stream->link, dc_link_get_link_cap(stream->link)) / (8 * 1000 * 54); + slots = DIV_ROUND_UP(pbn, pbn_per_timeslot); ret = drm_dp_mst_allocate_vcpi(mst_mgr, mst_port, pbn, slots); if (!ret) diff --git a/drivers/gpu/drm/drm_dp_mst_topology.c b/drivers/gpu/drm/drm_dp_mst_topology.c index 398e7314ea8b..d789b7af7dbf 100644 --- a/drivers/gpu/drm/drm_dp_mst_topology.c +++ b/drivers/gpu/drm/drm_dp_mst_topology.c @@ -3588,6 +3588,47 @@ static int test_calc_pbn_mode(void) return 0; } +/** + * drm_dp_calc_pbn_mode_dsc() - Calculate the PBN for a mode with DSC enabled. + * @clock: dot clock for the mode + * @dsc_bpp: dsc bits per pixel x16 (e.g. dsc_bpp = 136 is 8.5 bpp) + * + * This uses the formula in the spec to calculate the PBN value for a mode, + * given that the mode is using DSC + */ +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp) +{ + u64 kbps; + s64 peak_kbps; + u32 numerator; + u32 denominator; + + kbps = clock * dsc_bpp; + + /* + * margin 5300ppm + 300ppm ~ 0.6% as per spec, factor is 1.006 + * The unit of 54/64Mbytes/sec is an arbitrary unit chosen based on + * common multiplier to render an integer PBN for all link rate/lane + * counts combinations + * calculate + * peak_kbps *= (1/16) bppx16 to bpp + * peak_kbps *= (1006/1000) + * peak_kbps *= (64/54) + * peak_kbps *= 8 convert to bytes + * + * Divide numerator and denominator by 16 to avoid overflow + */ + + numerator = 64 * 1006 / 16; + denominator = 54 * 8 * 1000 * 1000; + + kbps *= numerator; + peak_kbps = drm_fixp_from_fraction(kbps, denominator); + + return drm_fixp2int_ceil(peak_kbps); +} +EXPORT_SYMBOL(drm_dp_calc_pbn_mode_dsc); + /* we want to kick the TX after we've ack the up/down IRQs. */ static void drm_dp_mst_kick_tx(struct drm_dp_mst_topology_mgr *mgr) { diff --git a/include/drm/drm_dp_mst_helper.h b/include/drm/drm_dp_mst_helper.h index 2ba6253ea6d3..ddb518f2157a 100644 --- a/include/drm/drm_dp_mst_helper.h +++ b/include/drm/drm_dp_mst_helper.h @@ -611,7 +611,7 @@ struct edid *drm_dp_mst_get_edid(struct drm_connector *connector, struct drm_dp_ int drm_dp_calc_pbn_mode(int clock, int bpp); - +int drm_dp_calc_pbn_mode_dsc(int clock, int dsc_bpp); bool drm_dp_mst_allocate_vcpi(struct drm_dp_mst_topology_mgr *mgr, struct drm_dp_mst_port *port, int pbn, int slots);
We were using drm helpers to convert a timing into its bandwidth, its bandwidth into pbn, and its pbn into timeslots These helpers -Did not take DSC timings into account -Used the link rate and lane count of the link's aux device, which are not the same as the link's current cap -Did not take FEC into account (FEC reduces the PBN per timeslot) For converting timing into PBN, add a new function drm_dp_calc_pbn_mode_dsc that handles the DSC case For converting PBN into time slots, amdgpu doesn't use the 'correct' atomic method (drm_dp_atomic_find_vcpi_slots), so don't add a new helper to cover our approach. Use the same means of calculating pbn per time slot as the DSC code. v2: Add drm helper for clock to pbn conversion Cc: Jerry Zuo <Jerry.Zuo@amd.com> Cc: Nicholas Kazlauskas <nicholas.kazlauskas@amd.com> Signed-off-by: David Francis <David.Francis@amd.com> --- .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 18 +++++--- drivers/gpu/drm/drm_dp_mst_topology.c | 41 +++++++++++++++++++ include/drm/drm_dp_mst_helper.h | 2 +- 3 files changed, 54 insertions(+), 7 deletions(-)