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.Zuo@amd.com Signed-off-by: David Francis David.Francis@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.Kazlauskas@amd.com Sent: August 19, 2019 3:21 PM To: Francis, David; dri-devel@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.Zuo@amd.com Signed-off-by: David Francis David.Francis@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)