On 8/19/19 11:50 AM, 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) >> >> Use the DC helpers (dc_bandwidth_in_kbps_from_timing, >> dc_link_bandwidth_kbps) instead >> >> Cc: Jerry Zuo <jerry....@amd.com> >> Signed-off-by: David Francis <david.fran...@amd.com> > >Wouldn't this be a good candidate for shared DRM helpers or to modify >the existing ones to support this usecase? > >Seems like this would be shared across drivers. > >Nicholas Kazlauskas >
These functions use information which is not stored in drm structs but tracked in a driver-specific way - Whether or not FEC is enabled - Whether or not DSC is enabled, and with what config - The current link setting This could be shifted into drm helpers, but it would require functions with signatures like drm_dp_calc_pbn_mode(clock, bpp, dsc_bpp) and drm_dp_find_vcpi_slots(mst_mgr, pbn, fec_enabled, lane_count, link_rate) and each driver would need to convert their own structs into those fields before calling these helpers. Is that worth it? >> --- >> .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 41 ++++--------------- >> 1 file changed, 8 insertions(+), 33 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..b32c0790399a 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; >> >> aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; >> >> @@ -205,40 +205,15 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( >> mst_port = aconnector->port; >> >> if (enable) { >> - clock = stream->timing.pix_clk_100hz / 10; >> - >> - switch (stream->timing.display_color_depth) { >> - >> - case COLOR_DEPTH_666: >> - bpp = 6; >> - break; >> - case COLOR_DEPTH_888: >> - bpp = 8; >> - break; >> - case COLOR_DEPTH_101010: >> - bpp = 10; >> - break; >> - case COLOR_DEPTH_121212: >> - bpp = 12; >> - break; >> - case COLOR_DEPTH_141414: >> - bpp = 14; >> - break; >> - case COLOR_DEPTH_161616: >> - bpp = 16; >> - break; >> - default: >> - ASSERT(bpp != 0); >> - break; >> - } >> - >> - bpp = bpp * 3; >> - >> - /* TODO need to know link rate */ >> + clock = dc_bandwidth_in_kbps_from_timing(&stream->timing); >> >> - pbn = drm_dp_calc_pbn_mode(clock, bpp); >> + /* dc_bandwidth_in_kbps_from_timing already takes bpp into >> account */ >> + pbn = drm_dp_calc_pbn_mode(clock, 1); >> >> - 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) ________________________________________ From: Kazlauskas, Nicholas <nicholas.kazlaus...@amd.com> Sent: August 19, 2019 3:21 PM To: Francis, David; dri-de...@lists.freedesktop.org; amd-gfx@lists.freedesktop.org Cc: Zuo, Jerry Subject: Re: [PATCH 06/14] drm/amd/display: Use dc helpers to compute timeslot distribution On 8/19/19 11:50 AM, 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) > > Use the DC helpers (dc_bandwidth_in_kbps_from_timing, > dc_link_bandwidth_kbps) instead > > Cc: Jerry Zuo <jerry....@amd.com> > Signed-off-by: David Francis <david.fran...@amd.com> Wouldn't this be a good candidate for shared DRM helpers or to modify the existing ones to support this usecase? Seems like this would be shared across drivers. Nicholas Kazlauskas > --- > .../amd/display/amdgpu_dm/amdgpu_dm_helpers.c | 41 ++++--------------- > 1 file changed, 8 insertions(+), 33 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..b32c0790399a 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; > > aconnector = (struct amdgpu_dm_connector *)stream->dm_stream_context; > > @@ -205,40 +205,15 @@ bool dm_helpers_dp_mst_write_payload_allocation_table( > mst_port = aconnector->port; > > if (enable) { > - clock = stream->timing.pix_clk_100hz / 10; > - > - switch (stream->timing.display_color_depth) { > - > - case COLOR_DEPTH_666: > - bpp = 6; > - break; > - case COLOR_DEPTH_888: > - bpp = 8; > - break; > - case COLOR_DEPTH_101010: > - bpp = 10; > - break; > - case COLOR_DEPTH_121212: > - bpp = 12; > - break; > - case COLOR_DEPTH_141414: > - bpp = 14; > - break; > - case COLOR_DEPTH_161616: > - bpp = 16; > - break; > - default: > - ASSERT(bpp != 0); > - break; > - } > - > - bpp = bpp * 3; > - > - /* TODO need to know link rate */ > + clock = dc_bandwidth_in_kbps_from_timing(&stream->timing); > > - pbn = drm_dp_calc_pbn_mode(clock, bpp); > + /* dc_bandwidth_in_kbps_from_timing already takes bpp into > account */ > + pbn = drm_dp_calc_pbn_mode(clock, 1); > > - 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) > _______________________________________________ amd-gfx mailing list amd-gfx@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/amd-gfx