On 5/20/2025 5:55 PM, Jani Nikula wrote:
On Tue, 20 May 2025, Ankit Nautiyal <ankit.k.nauti...@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: sta...@vger.kernel.org
Cc: Jani Nikula <jani.nik...@linux.intel.com>
Cc: Ville Syrjälä <ville.syrj...@linux.intel.com>
Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@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,
};
/**
