Re: [PATCH 2/9] drm/i915/dp: Limit m/n ratio to 10 for DP SST

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 7/21/2025 5:11 PM, Imre Deak wrote:
> On Mon, Jul 21, 2025 at 02:45:22PM +0530, Ankit Nautiyal wrote:
>> The hardware cannot support DisplayPort configurations where the
>> ceiling of the Link M/Link N ratio exceeds 10. This limitation has
>> always existed, but it typically wasn't encountered without the use of
>> joiners and DSC.
>>
>> With higher resolutions and combinations involving joiners and DSC,
>> this constraint can now be hit in certain scenarios.
>>
>> Introduce a check during link rate configuration to ensure
>> the computed M/N ratio does not exceed the hardware limit. If no valid
>> link rate satisfies this constraint, the mode will be rejected.
>>
>> Note: This change applies the check only for SST. Support for MST will
>> be added in a subsequent commit.
>>
>> v2:
>> -Move the M/N ratio check to the link rate configuration phase instead
>> of during M/N computation. (Ville)
>> -Prune modes that cannot be supported even with highest link rate due to
>> M/N ratio restriction.
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_display.c |  1 -
>>   drivers/gpu/drm/i915/display/intel_display.h |  3 ++
>>   drivers/gpu/drm/i915/display/intel_dp.c      | 33 +++++++++++++++++++-
>>   3 files changed, 35 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>> index 671491a2a3b6..f32a4956c926 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>> @@ -2506,7 +2506,6 @@ static void compute_m_n(u32 *ret_m, u32 *ret_n,
>>   	intel_reduce_m_n_ratio(ret_m, ret_n);
>>   }
>>   
>> -static
>>   void intel_display_get_link_m_n(u32 *link_m, u32 *link_n,
>>   				u32 pixel_clock,
>>   				u32 link_clock)
>> diff --git a/drivers/gpu/drm/i915/display/intel_display.h b/drivers/gpu/drm/i915/display/intel_display.h
>> index 37e2ab301a80..bfa3db219b9c 100644
>> --- a/drivers/gpu/drm/i915/display/intel_display.h
>> +++ b/drivers/gpu/drm/i915/display/intel_display.h
>> @@ -559,5 +559,8 @@ bool assert_port_valid(struct intel_display *display, enum port port);
>>   
>>   bool intel_scanout_needs_vtd_wa(struct intel_display *display);
>>   int intel_crtc_num_joined_pipes(const struct intel_crtc_state *crtc_state);
>> +void intel_display_get_link_m_n(u32 *link_m, u32 *link_n,
>> +				u32 pixel_clock,
>> +				u32 link_clock);
>>   
>>   #endif
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 54d88f24b689..4245dd65b2af 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -1407,6 +1407,28 @@ bool intel_dp_has_dsc(const struct intel_connector *connector)
>>   	return true;
>>   }
>>   
>> +static int
>> +intel_dp_get_max_m_n_ratio(void)
>> +{
>> +	return 10;
>> +}
>> +
>> +static bool
>> +intel_dp_can_support_m_n(int pixel_clock,
>> +			 int link_rate)
>> +{
>> +	int max_m_n_ratio = intel_dp_get_max_m_n_ratio();
>> +	u32 link_m, link_n;
>> +	int m_n_ratio;
>> +
>> +	intel_display_get_link_m_n(&link_m, &link_n,
>> +				   pixel_clock, link_rate);
>> +
>> +	m_n_ratio = DIV_ROUND_UP(link_m, link_n);
>> +
>> +	return m_n_ratio <= max_m_n_ratio;
>> +}
>> +
>>   static enum drm_mode_status
>>   intel_dp_mode_valid(struct drm_connector *_connector,
>>   		    const struct drm_display_mode *mode)
>> @@ -1518,6 +1540,9 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>>   	if (status != MODE_OK)
>>   		return status;
>>   
>> +	if (!intel_dp_can_support_m_n(target_clock, max_rate))
>> +		return MODE_CLOCK_HIGH;
>> +
>>   	return intel_mode_valid_max_plane_size(display, mode, num_joined_pipes);
>>   }
>>   
>> @@ -1789,6 +1814,9 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>>   			    link_rate > limits->max_rate)
>>   				continue;
>>   
>> +			if (!intel_dp_can_support_m_n(clock, link_rate))
>> +				continue;
> Could this limit be set in limits->min_rate, computed already in
> intel_dp_compute_config_limits()? (And fail already there if this is
> bigger than limits->max_rate.)

Yes right this can be done in intel_dp_compute_config_limit.

We can iterate over the array of common_rates and find the rate for 
which the ratio is within limits and bail out early if we cant find such 
a link rate.

Or otherwise instead of iterating over rates, I guess theoretically we 
can calculate the minimum link rate for given pixelclock and ratio and 
set it to limits->min_rate and bail out early if this is more than the 
limits->max_rate.

However this might be bit tricky as symbol size is different for uhbr.

I will check this out, and move this in intel_dp_compute_config_limits.

Thanks & Regards,

Ankit


>
>> +
>>   			for (lane_count = limits->min_lane_count;
>>   			     lane_count <= limits->max_lane_count;
>>   			     lane_count <<= 1) {
>> @@ -1796,7 +1824,6 @@ intel_dp_compute_link_config_wide(struct intel_dp *intel_dp,
>>   									 link_rate,
>>   									 lane_count);
>>   
>> -
>>   				if (mode_rate <= link_avail) {
>>   					pipe_config->lane_count = lane_count;
>>   					pipe_config->pipe_bpp = bpp;
>> @@ -1983,6 +2010,10 @@ static int dsc_compute_link_config(struct intel_dp *intel_dp,
>>   		if (link_rate < limits->min_rate || link_rate > limits->max_rate)
>>   			continue;
>>   
>> +		if (!intel_dp_can_support_m_n(adjusted_mode->clock,
>> +					      link_rate))
>> +			continue;
>> +
>>   		for (lane_count = limits->min_lane_count;
>>   		     lane_count <= limits->max_lane_count;
>>   		     lane_count <<= 1) {
>> -- 
>> 2.45.2
>>