diff mbox series

[v2,04/11] drm/i915/dp_mst: Account for channel coding efficiency in the DSC DPT bpp limit

Message ID 20240416221010.376865-5-imre.deak@intel.com (mailing list archive)
State New, archived
Headers show
Series drm/i915/dp: Few MTL/DSC and a UHBR monitor fix | expand

Commit Message

Imre Deak April 16, 2024, 10:10 p.m. UTC 
The DSC DPT interface BW limit check should take into account the link
clock's (aka DDI clock in bspec) channel coding efficiency overhead.
Bspec suggests that the FEC overhead needs to be applied, however HW
people claim this isn't the case, nor is any overhead applicable.

However based on testing various 5k/6k modes both on the DELL U3224KBA
monitor and the Unigraf UCD-500 CTS test device, both the channel coding
efficiency (which includes the FEC overhead) and an additional 3%
overhead must be accounted for to get these modes working.

Bspec: 49259

v2:
- Apply an additional 3% overhead, add a commit log and code comment
  about these overheads and the relation to the Bspec BW limit formula.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> (v1)
Signed-off-by: Imre Deak <imre.deak@intel.com>
---
 drivers/gpu/drm/i915/display/intel_dp_mst.c | 23 +++++++++++++++++++--
 1 file changed, 21 insertions(+), 2 deletions(-)

Comments

Nautiyal, Ankit K April 17, 2024, 12:42 p.m. UTC | #1 
On 4/17/2024 3:40 AM, Imre Deak wrote:
> The DSC DPT interface BW limit check should take into account the link
> clock's (aka DDI clock in bspec) channel coding efficiency overhead.
> Bspec suggests that the FEC overhead needs to be applied, however HW
> people claim this isn't the case, nor is any overhead applicable.
>
> However based on testing various 5k/6k modes both on the DELL U3224KBA
> monitor and the Unigraf UCD-500 CTS test device, both the channel coding
> efficiency (which includes the FEC overhead) and an additional 3%
> overhead must be accounted for to get these modes working.
>
> Bspec: 49259
>
> v2:
> - Apply an additional 3% overhead, add a commit log and code comment
>    about these overheads and the relation to the Bspec BW limit formula.
>
> Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com> (v1)
> Signed-off-by: Imre Deak <imre.deak@intel.com>
> ---
>   drivers/gpu/drm/i915/display/intel_dp_mst.c | 23 +++++++++++++++++++--
>   1 file changed, 21 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> index 58eb6bf33c92e..0448cc343a33f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
> @@ -59,11 +59,30 @@ static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp
>   	if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 14 && dsc) {
>   		int output_bpp = bpp;
>   		int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock);
> +		/*
> +		 * Bspec/49259 suggests that the FEC overhead needs to be
> +		 * applied here, though HW people claim that neither this FEC
> +		 * or any other overhead is applicable here (that is the actual
> +		 * available_bw is just symbol_clock * 72). However based on
> +		 * testing on MTL-P the
> +		 * - DELL U3224KBA display
> +		 * - Unigraf UCD-500 CTS test sink
> +		 * devices the
> +		 * - 5120x2880/995.59Mhz
> +		 * - 6016x3384/1357.23Mhz
> +		 * - 6144x3456/1413.39Mhz
> +		 * modes (all which had a DPT limit on the above devices),
nitpick : all 'of' which
> +		 * both the channel coding efficiency and an additional 3%
> +		 * overhead needs to be accounted for.
> +		 */
> +		int available_bw = mul_u32_u32(symbol_clock * 72,
> +					       drm_dp_bw_channel_coding_efficiency(true)) /
> +				   1030000;

IMHO, generally overhead of 3% would be better represented by 
multiplying available bandwidth with 97%, but since this is measured to 
be around 3%, this way seems simpler.

Patch looks good to me.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>



>   
>   		if (output_bpp * adjusted_mode->crtc_clock >
> -		    symbol_clock * 72) {
> +		    available_bw) {
>   			drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n",
> -				    output_bpp * adjusted_mode->crtc_clock, symbol_clock * 72);
> +				    output_bpp * adjusted_mode->crtc_clock, available_bw);
>   			return -EINVAL;
>   		}
>   	}
diff mbox series

Patch

diff --git a/drivers/gpu/drm/i915/display/intel_dp_mst.c b/drivers/gpu/drm/i915/display/intel_dp_mst.c
index 58eb6bf33c92e..0448cc343a33f 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_mst.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_mst.c
@@ -59,11 +59,30 @@  static int intel_dp_mst_check_constraints(struct drm_i915_private *i915, int bpp
 	if (intel_dp_is_uhbr(crtc_state) && DISPLAY_VER(i915) < 14 && dsc) {
 		int output_bpp = bpp;
 		int symbol_clock = intel_dp_link_symbol_clock(crtc_state->port_clock);
+		/*
+		 * Bspec/49259 suggests that the FEC overhead needs to be
+		 * applied here, though HW people claim that neither this FEC
+		 * or any other overhead is applicable here (that is the actual
+		 * available_bw is just symbol_clock * 72). However based on
+		 * testing on MTL-P the
+		 * - DELL U3224KBA display
+		 * - Unigraf UCD-500 CTS test sink
+		 * devices the
+		 * - 5120x2880/995.59Mhz
+		 * - 6016x3384/1357.23Mhz
+		 * - 6144x3456/1413.39Mhz
+		 * modes (all which had a DPT limit on the above devices),
+		 * both the channel coding efficiency and an additional 3%
+		 * overhead needs to be accounted for.
+		 */
+		int available_bw = mul_u32_u32(symbol_clock * 72,
+					       drm_dp_bw_channel_coding_efficiency(true)) /
+				   1030000;
 
 		if (output_bpp * adjusted_mode->crtc_clock >
-		    symbol_clock * 72) {
+		    available_bw) {
 			drm_dbg_kms(&i915->drm, "UHBR check failed(required bw %d available %d)\n",
-				    output_bpp * adjusted_mode->crtc_clock, symbol_clock * 72);
+				    output_bpp * adjusted_mode->crtc_clock, available_bw);
 			return -EINVAL;
 		}
 	}