Re: [PATCH] drm/i915/dp: Allow HBR3 without TPS4 support for eDP panels

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

On 5/20/2025 5:55 PM, Jani Nikula wrote:
> On Tue, 20 May 2025, Ankit Nautiyal <ankit.k.nautiyal@intel.com> wrote:
>> 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.
>>
>> 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.
>>
>> To restore functionality for such panels do not reject HBR3 when sink
>> doesn't support TPS4. Instead reject HBR3 for specific panel that are
>> not able to handle HBR3 [1].
>>
>> [1] https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/5969
>>
>> Fixes: 584cf613c24a ("drm/i915/dp: Reject HBR3 when sink doesn't support TPS4")
>> Cc: stable@vger.kernel.org
>> Cc: Jani Nikula <jani.nikula@linux.intel.com>
>> Cc: Ville Syrjälä <ville.syrjala@linux.intel.com>
>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>> ---
>>   drivers/gpu/drm/display/drm_dp_helper.c |  2 ++
>>   drivers/gpu/drm/i915/display/intel_dp.c | 21 ++++++++++-----------
>>   include/drm/display/drm_dp_helper.h     |  7 +++++++
>>   3 files changed, 19 insertions(+), 11 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
>> index f2a6559a2710..bf66489c9202 100644
>> --- a/drivers/gpu/drm/display/drm_dp_helper.c
>> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
>> @@ -2526,6 +2526,8 @@ static const struct dpcd_quirk dpcd_quirk_list[] = {
>>   	{ OUI(0x00, 0x0C, 0xE7), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC) },
>>   	/* Apple MacBookPro 2017 15 inch eDP Retina panel reports too low DP_MAX_LINK_RATE */
>>   	{ OUI(0x00, 0x10, 0xfa), DEVICE_ID(101, 68, 21, 101, 98, 97), false, BIT(DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS) },
>> +	/* Novatek panel */
>> +	{ OUI(0x38, 0xEC, 0x11), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_HBR3) },
>>   };
>>   
>>   #undef OUI
>> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
>> index 21297bc4cc00..0bfc84cbd50d 100644
>> --- a/drivers/gpu/drm/i915/display/intel_dp.c
>> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
>> @@ -184,13 +184,13 @@ static int max_dprx_rate(struct intel_dp *intel_dp)
>>   		max_rate = drm_dp_bw_code_to_link_rate(intel_dp->dpcd[DP_MAX_LINK_RATE]);
>>   
>>   	/*
>> -	 * 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.
>> +	 * Some broken eDP sinks declare support for HBR3 but are unable to
>> +	 * produce a stable output. For these panel reject HBR3.
>>   	 */
>> -	if (max_rate >= 810000 && !drm_dp_tps4_supported(intel_dp->dpcd)) {
>> +	if (max_rate >= 810000 &&
>> +	    drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_HBR3)) {
> This does work, but I was thinking the quirk would be that the max is
> HBR2. Same thing, but more generic? DP_DPCD_QUIRK_MAX_HBR2 maybe?

Makes sense..

Something like:

                        if (rate >= 540000 &&
drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_HBR2)) {
                                 drm_dbg_kms(display->drm,
                                             "[ENCODER:%d:%s] Rejecting 
rates above HBR2 due to DP_DPCD_QUIRK_HBR2\n",
encoder->base.base.id, encoder->base.name);
                                 break;
                         }


>
> With that, you could drop the max_rate >= 810000 from here. (Though the
> next check below does need the rate check as it stops the loop.)
>
>>   		drm_dbg_kms(display->drm,
>> -			    "[ENCODER:%d:%s] Rejecting HBR3 due to missing TPS4 support\n",
>> +			    "[ENCODER:%d:%s] Rejecting HBR3 due to DP_DPCD_QUIRK_HBR3\n",
>>   			    encoder->base.base.id, encoder->base.name);
>>   		max_rate = 540000;
>>   	}
>> @@ -4296,15 +4296,14 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
>>   
>>   			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.
>> +			 * Some broken eDP sinks declare support for HBR3 but are unable to
>> +			 * produce a stable output. For these panel reject HBR3.
>>   			 */
>> -			if (rate >= 810000 && !drm_dp_tps4_supported(intel_dp->dpcd)) {
>> +			if (rate >= 810000 &&
>> +			    drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_HBR3)) {
>>   				drm_dbg_kms(display->drm,
>> -					    "[ENCODER:%d:%s] Rejecting HBR3 due to missing TPS4 support\n",
>> +					    "[ENCODER:%d:%s] Rejecting HBR3 due to DP_DPCD_QUIRK_HBR3\n",
>>   					    encoder->base.base.id, encoder->base.name);
>>   				break;
>>   			}
>> diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
>> index e4ca35143ff9..5e60a37b61ce 100644
>> --- a/include/drm/display/drm_dp_helper.h
>> +++ b/include/drm/display/drm_dp_helper.h
>> @@ -814,6 +814,13 @@ enum drm_dp_quirk {
>>   	 * requires enabling DSC.
>>   	 */
>>   	DP_DPCD_QUIRK_HBLANK_EXPANSION_REQUIRES_DSC,
>> +
>> +	/**
>> +	 * @DP_DPCD_QUIRK_HBR3:
>> +	 *
>> +	 * The device supports HBR3 but is unable to produce stable output.
> I think DP_DPCD_QUIRK_MAX_HBR2 is easier to explain too.
>
> What do you think?


I agree. Thanks for the suggestion.

Besides this, I have one doubt regarding the quirk.

Currently I have added OUI details from the logs from gitlab issue, but 
DEVICE_ID field was blank so I have used DEVICE_ID_ANY:

+	/* Novatek panel */
+	{ OUI(0x38, 0xEC, 0x11), DEVICE_ID_ANY, false, BIT(DP_DPCD_QUIRK_HBR3) },


This can probably clamp the rate for many panels. Is there any way to identify the panel other than the description from DPCD

Regards,

Ankit

> BR,
> Jani.
>
>
>
>> +	 */
>> +	DP_DPCD_QUIRK_HBR3,
>>   };
>>   
>>   /**