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@xxxxxxx>
>> Signed-off-by: David Francis <David.Francis@xxxxxxx>
>
>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@xxxxxxx>
Sent: August 19, 2019 3:21 PM
To: Francis, David; dri-devel@xxxxxxxxxxxxxxxxxxxxx; amd-gfx@xxxxxxxxxxxxxxxxxxxxx
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@xxxxxxx>
> Signed-off-by: David Francis <David.Francis@xxxxxxx>
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)
>
_______________________________________________
dri-devel mailing list
dri-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/dri-devel
