[PATCH 1/2] Revert "drm/i915/dp: Reject HBR3 when sink doesn't support TPS4"

[Nautiyal, Ankit K]

On 6/23/2025 8:12 PM, Ville Syrjälä wrote:
> On Fri, Jun 20, 2025 at 06:14:16PM +0530, Ankit Nautiyal wrote:
>> This reverts commit 584cf613c24a4250d9be4819efc841aa2624d5b6.
>> Commit 584cf613c24a ("drm/i915/dp: Reject HBR3 when sink doesn't support
>> TPS4") introduced a blanket rejection of HBR3 link rate when the sink does
>> not support TPS4. While this was intended to address instability observed
>> on certain eDP panels [1], the TPS4 requirement is only mandated for DPRX
>> and not for eDPRX.
> I see no exception given for eDP regarding this rule. The only exception
> allowed is that eDP can say DPCD_REV=1.4 + TPS4_SUPPORTED=0. So I still
> claim that these eDP sinks are violating the spec.

Hmm.. Yes the spec allows eDP sinks to report DPCD_REV=1.4 and 
TPS4_SUPPORTED, so perhaps eDPs claiming HBR3 with DPCD rev other than 
rev 1.4 and not supporting TPS4 are indeed violating the spec.

Would it make sense to add a condition that checks for DPCD_REV=1.4.

Specifically:

if DPCD_REV=1.4 and TPS4_SUPPORTED = 0, then do not prune the HBR3 rate?

Or otherway if DPCD_REV!=1.4 and TPS4_SUPPORTER = 0, prune the HBR3 rate

This way the patch need not be reverted, but modified to address 
instability issues for eDP panels that are not aligned with the spec.

That said, the gitlab issue#5969 [1] will still need another solution 
since it seems to have DPCD rev 14 as per logs:

DPCD: 14 1e 44 41 00 00 01 80 02 00 02 00 00 0b 80


Regards,

Ankit

>
>> This change inadvertently causes blank screens on some eDP panels that do
>> not advertise TPS4 support, and require HBR3 to operate at their fixed
>> native resolution.
>>
>> Revert the commit to restore functionality for such panels.
>>
>> [1] https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5969
>>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal at intel.com>
>> Acked-by: Jani Nikula <jani.nikula at intel.com>
>> ---
>>   drivers/gpu/drm/i915/display/intel_dp.c | 49 ++++---------------------
>>   1 file changed, 7 insertions(+), 42 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 277b40b13948..74f331ae97ff 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -173,28 +173,10 @@ int intel_dp_link_symbol_clock(int rate)
>>   
>>   static int max_dprx_rate(struct intel_dp *intel_dp)
>>   {
>> -	struct intel_display *display = to_intel_display(intel_dp);
>> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> -	int max_rate;
>> -
>>   	if (intel_dp_tunnel_bw_alloc_is_enabled(intel_dp))
>> -		max_rate = drm_dp_tunnel_max_dprx_rate(intel_dp->tunnel);
>> -	else
>> -		max_rate = drm_dp_bw_code_to_link_rate(intel_dp->dpcd[DP_MAX_LINK_RATE]);
>> +		return drm_dp_tunnel_max_dprx_rate(intel_dp->tunnel);
>>   
>> -	/*
>> -	 * Some broken eDP sinks illegally declare support for
>> -	 * HBR3 without TPS4, and are unable to produce a stable
>> -	 * output. Reject HBR3 when TPS4 is not available.
>> -	 */
>> -	if (max_rate >= 810000 && !drm_dp_tps4_supported(intel_dp->dpcd)) {
>> -		drm_dbg_kms(display->drm,
>> -			    "[ENCODER:%d:%s] Rejecting HBR3 due to missing TPS4 support\n",
>> -			    encoder->base.base.id, encoder->base.name);
>> -		max_rate = 540000;
>> -	}
>> -
>> -	return max_rate;
>> +	return drm_dp_bw_code_to_link_rate(intel_dp->dpcd[DP_MAX_LINK_RATE]);
>>   }
>>   
>>   static int max_dprx_lane_count(struct intel_dp *intel_dp)
>> @@ -4270,9 +4252,6 @@ static void intel_edp_mso_init(struct intel_dp *intel_dp)
>>   static void
>>   intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>>   {
>> -	struct intel_display *display = to_intel_display(intel_dp);
>> -	struct intel_encoder *encoder = &dp_to_dig_port(intel_dp)->base;
>> -
>>   	intel_dp->num_sink_rates = 0;
>>   
>>   	if (intel_dp->edp_dpcd[0] >= DP_EDP_14) {
>> @@ -4283,7 +4262,10 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>>   				 sink_rates, sizeof(sink_rates));
>>   
>>   		for (i = 0; i < ARRAY_SIZE(sink_rates); i++) {
>> -			int rate;
>> +			int val = le16_to_cpu(sink_rates[i]);
>> +
>> +			if (val == 0)
>> +				break;
>>   
>>   			/* Value read multiplied by 200kHz gives the per-lane
>>   			 * link rate in kHz. The source rates are, however,
>> @@ -4291,24 +4273,7 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>>   			 * back to symbols is
>>   			 * (val * 200kHz)*(8/10 ch. encoding)*(1/8 bit to Byte)
>>   			 */
>> -			rate = le16_to_cpu(sink_rates[i]) * 200 / 10;
>> -
>> -			if (rate == 0)
>> -				break;
>> -
>> -			/*
>> -			 * Some broken eDP sinks illegally declare support for
>> -			 * HBR3 without TPS4, and are unable to produce a stable
>> -			 * output. Reject HBR3 when TPS4 is not available.
>> -			 */
>> -			if (rate >= 810000 && !drm_dp_tps4_supported(intel_dp->dpcd)) {
>> -				drm_dbg_kms(display->drm,
>> -					    "[ENCODER:%d:%s] Rejecting HBR3 due to missing TPS4 support\n",
>> -					    encoder->base.base.id, encoder->base.name);
>> -				break;
>> -			}
>> -
>> -			intel_dp->sink_rates[i] = rate;
>> +			intel_dp->sink_rates[i] = (val * 200) / 10;
>>   		}
>>   		intel_dp->num_sink_rates = i;
>>   	}
>> -- 
>> 2.45.2