* [PATCH RFC v3 1/8] drm: Move sink link rates reading helpers out of i915
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
@ 2026-10-12 15:10 ` Kory Maincent
2026-10-12 15:26 ` sashiko-bot
2026-10-12 15:10 ` [PATCH RFC v3 2/8] drm: Move DSC capability reading helper " Kory Maincent
` (6 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:10 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
In preparation for adding generic DRM properties to report source and
sink capabilities, move the sink link rates reading functions out of
the i915 driver into the generic DRM helpers. These functions provide
everything needed to properly extract link rate capabilities from the
DPCD.
Keep the Intel-specific quirk handling in the driver.
No functional change intended.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Change in v3:
- New patch
---
drivers/gpu/drm/display/drm_dp_helper.c | 237 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.c | 169 ++++++-----------------
include/drm/display/drm_dp.h | 1 +
include/drm/display/drm_dp_helper.h | 11 ++
4 files changed, 293 insertions(+), 125 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index e623ccb4c1d8a..8ef096a0138e4 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4912,3 +4912,240 @@ int drm_dp_max_dprx_data_rate(int max_link_rate, int max_lanes)
1000000 * 8);
}
EXPORT_SYMBOL(drm_dp_max_dprx_data_rate);
+
+/**
+ * drm_dp_read_dpcd_supported_link_rates() - read DPCD supported link rates
+ * @aux: DisplayPort AUX channel
+ * @link_rates: Buffer to store the resulting link rates supported
+ *
+ * Attempts to read the DPCD supported link rates for @aux.
+ *
+ * Returns: The number or supported link rates, negative error code otherwise.
+ */
+static int drm_dp_read_dpcd_supported_link_rates(struct drm_dp_aux *aux,
+ int link_rates[DP_MAX_SUPPORTED_RATES])
+{
+ __le16 dpcd_link_rates[DP_MAX_SUPPORTED_RATES] = {0};
+ int ret, i;
+
+ ret = drm_dp_dpcd_read_data(aux, DP_SUPPORTED_LINK_RATES,
+ dpcd_link_rates, sizeof(dpcd_link_rates));
+ if (ret < 0)
+ return ret;
+
+ for (i = 0; i < ARRAY_SIZE(dpcd_link_rates) && dpcd_link_rates[i]; i++)
+ link_rates[i] = le16_to_cpu(dpcd_link_rates[i]) * 20;
+
+ return i;
+}
+
+#if IS_ENABLED(CONFIG_DRM_DISPLAY_DP_TUNNEL)
+
+static u8 drm_dprx_128b132b_link_rates(const struct drm_dp_tunnel *tunnel,
+ u8 uhbr_rates)
+{
+ if (!drm_dp_tunnel_bw_alloc_is_enabled(tunnel))
+ return uhbr_rates;
+
+ static_assert(DP_TUNNELING_10GBPS_PER_LANE_SUPPORT == DP_UHBR10 &&
+ DP_TUNNELING_13_5GBPS_PER_LANE_SUPPORT == DP_UHBR13_5 &&
+ DP_TUNNELING_20GBPS_PER_LANE_SUPPORT == DP_UHBR20);
+
+ return drm_dp_tunnel_128b132b_dprx_rates(tunnel);
+}
+
+static bool drm_dprx_supports_128b132b(const struct drm_dp_tunnel *tunnel,
+ const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ if (tunnel && drm_dp_tunnel_bw_alloc_is_enabled(tunnel))
+ return drm_dp_tunnel_128b132b_supported(tunnel);
+ else
+ return drm_dp_128b132b_supported(dpcd);
+}
+
+#else
+
+static u8 drm_dprx_128b132b_link_rates(const struct drm_dp_tunnel *tunnel,
+ u8 uhbr_rates)
+{
+ return uhbr_rates;
+}
+
+static bool drm_dprx_supports_128b132b(const struct drm_dp_tunnel *tunnel,
+ const u8 dpcd[DP_RECEIVER_CAP_SIZE])
+{
+ return drm_dp_128b132b_supported(dpcd);
+}
+
+#endif
+
+/**
+ * drm_dp_get_128b132b_link_rates() - get the supported 128b/132b link rates
+ * @lttpr_common_caps: LTTPR common capabilities
+ * @tunnel: DP tunnel object (NULL if not supported)
+ * @uhbr_rates: UHBR rate mask read from the DPCD
+ * DP_128B132B_SUPPORTED_LINK_RATES register
+ * @link_rates: Buffer of DP_MAX_128B132B_SUPPORTED_RATES entries to store the
+ * resulting link rates
+ *
+ * Limit the UHBR rate mask @uhbr_rates by the capabilities of the DP tunnel
+ * and of the LTTPRs, if any is present, then expand the remaining rates into
+ * @link_rates in ascending order.
+ *
+ * Returns: The number of link rates stored in @link_rates.
+ */
+static int
+drm_dp_get_128b132b_link_rates(const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE],
+ const struct drm_dp_tunnel *tunnel,
+ u8 uhbr_rates,
+ int link_rates[DP_MAX_128B132B_SUPPORTED_RATES])
+{
+ int i = 0;
+
+ if (tunnel)
+ uhbr_rates = drm_dprx_128b132b_link_rates(tunnel, uhbr_rates);
+
+ if (drm_dp_lttpr_count(lttpr_common_caps)) {
+ /* We have a repeater */
+ if (lttpr_common_caps[0] >= 0x20 &&
+ dp_lttpr_common_cap(lttpr_common_caps,
+ DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER) &
+ DP_PHY_REPEATER_128B132B_SUPPORTED) {
+ /* Repeater supports 128b/132b, valid UHBR rates */
+ uhbr_rates &= dp_lttpr_common_cap(lttpr_common_caps,
+ DP_PHY_REPEATER_128B132B_RATES);
+ } else {
+ /* Does not support 128b/132b */
+ uhbr_rates = 0;
+ }
+ }
+
+ if (uhbr_rates & DP_UHBR10)
+ link_rates[i++] = 1000000;
+ if (uhbr_rates & DP_UHBR13_5)
+ link_rates[i++] = 1350000;
+ if (uhbr_rates & DP_UHBR20)
+ link_rates[i++] = 2000000;
+
+ return i;
+}
+
+/**
+ * drm_dp_read_dpcd_sink_rates() - read the sink rates from the DPCD
+ * @aux: DisplayPort AUX channel
+ * @dpcd: DPCD receiver capabilities
+ * @lttpr_common_caps: LTTPR common capabilities
+ * @desc: DisplayPort sink descriptor
+ * @tunnel: Tunnel object (NULL if not supported)
+ * @sink_rates: Array of DP_MAX_SUPPORTED_RATES entries to store the sink rates
+ *
+ * Read the link rates supported by a DisplayPort sink from the DPCD
+ * DP_MAX_LINK_RATE and DP_128B132B_SUPPORTED_LINK_RATES registers, limited by
+ * the capabilities of the LTTPRs if any is present. The rates are stored in
+ * @sink_rates in ascending order.
+ *
+ * The DPCD receiver capabilities, the LTTPR common capabilities and the sink
+ * descriptor are passed by the caller instead of being read from the DPCD.
+ * Drivers which already read them, like i915, can pass them to avoid
+ * duplicate DPCD reads.
+ *
+ * Returns: The number of supported sink rates.
+ */
+int drm_dp_read_dpcd_sink_rates(struct drm_dp_aux *aux,
+ const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+ const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE],
+ const struct drm_dp_desc *desc,
+ const struct drm_dp_tunnel *tunnel,
+ int sink_rates[DP_MAX_SUPPORTED_RATES])
+{
+ static const int dp_rates[] = {
+ 162000, 270000, 540000, 810000
+ };
+ int max_rate, max_lttpr_rate = 0, ret, i;
+ u8 uhbr_rates = 0;
+
+ BUILD_BUG_ON(ARRAY_SIZE(dp_rates) + DP_MAX_128B132B_SUPPORTED_RATES >
+ DP_MAX_SUPPORTED_RATES);
+
+ if (drm_dp_has_quirk(desc, DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) {
+ /* Needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panel */
+ static const int quirk_rates[] = { 162000, 270000, 324000 };
+
+ memcpy(sink_rates, quirk_rates, sizeof(quirk_rates));
+
+ return ARRAY_SIZE(quirk_rates);
+ }
+
+ /*
+ * Sink rates for 8b/10b.
+ */
+ max_rate = drm_dp_bw_code_to_link_rate(dpcd[DP_MAX_LINK_RATE]);
+ if (lttpr_common_caps)
+ max_lttpr_rate = drm_dp_lttpr_max_link_rate(lttpr_common_caps);
+ if (max_lttpr_rate)
+ max_rate = min(max_rate, max_lttpr_rate);
+
+ for (i = 0; i < ARRAY_SIZE(dp_rates); i++) {
+ if (dp_rates[i] > max_rate)
+ break;
+ sink_rates[i] = dp_rates[i];
+ }
+
+ /*
+ * The following register must be read unconditionally for the later
+ * DP tunnel 128b132b detection to work, see DP Standard v2.1 5.14.3 .
+ */
+ ret = drm_dp_dpcd_read_byte(aux, DP_128B132B_SUPPORTED_LINK_RATES,
+ &uhbr_rates);
+ if (ret < 0) {
+ drm_dbg_kms(aux->drm_dev,
+ "%s: Failed to read DPCD 128B/132B supported link rates (%d)\n",
+ aux->name, ret);
+ uhbr_rates = 0;
+ }
+
+ /*
+ * Sink rates for 128b/132b. If set, sink should support all 8b/10b
+ * rates and 10 Gbps.
+ */
+ if (drm_dprx_supports_128b132b(tunnel, dpcd))
+ i += drm_dp_get_128b132b_link_rates(lttpr_common_caps,
+ tunnel,
+ uhbr_rates,
+ &sink_rates[i]);
+
+ return i;
+}
+EXPORT_SYMBOL_GPL(drm_dp_read_dpcd_sink_rates);
+
+/**
+ * drm_dp_read_edp_sink_rates() - read the sink rates from the eDP DPCD
+ * @aux: DisplayPort AUX channel
+ * @edp_dpcd_rev: eDP DPCD revision of the sink
+ * @sink_rates: Array of DP_MAX_SUPPORTED_RATES entries to store the sink rates
+ *
+ * Read the link rates supported by an eDP sink from the DPCD
+ * DP_SUPPORTED_LINK_RATES registers, available on eDP 1.4 and later sinks.
+ *
+ * Returns: The number of supported sink rates, 0 if the sink does not support
+ * the DP_SUPPORTED_LINK_RATES register, negative error code otherwise.
+ */
+int drm_dp_read_edp_sink_rates(struct drm_dp_aux *aux, u8 edp_dpcd_rev,
+ int sink_rates[DP_MAX_SUPPORTED_RATES])
+{
+ int num_sink_rates;
+
+ if (edp_dpcd_rev < DP_EDP_14)
+ return 0;
+
+ num_sink_rates = drm_dp_read_dpcd_supported_link_rates(aux, sink_rates);
+ if (num_sink_rates < 0) {
+ drm_dbg_kms(aux->drm_dev,
+ "%s: Unable to read eDP supported link rates (%d)\n",
+ aux->name, num_sink_rates);
+ return 0;
+ }
+
+ return num_sink_rates;
+}
+EXPORT_SYMBOL_GPL(drm_dp_read_edp_sink_rates);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 0cd5e6b5034cf..da73ad11e9cac 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -191,102 +191,47 @@ static void intel_dp_set_default_sink_rates(struct intel_dp *intel_dp)
intel_dp->num_sink_rates = 1;
}
-static bool dprx_supports_128b132b(struct intel_dp *intel_dp)
-{
- if (intel_dp_tunnel_bw_alloc_is_enabled(intel_dp))
- return drm_dp_tunnel_128b132b_supported(intel_dp->tunnel);
- else
- return drm_dp_128b132b_supported(intel_dp->dpcd);
-}
-
-static u8 dprx_128b132b_link_rates(struct intel_dp *intel_dp, u8 no_bwa_rates)
-{
- u8 ret;
-
- if (!intel_dp_tunnel_bw_alloc_is_enabled(intel_dp))
- return no_bwa_rates;
-
- ret = drm_dp_tunnel_128b132b_dprx_rates(intel_dp->tunnel);
- static_assert(DP_TUNNELING_10GBPS_PER_LANE_SUPPORT == DP_UHBR10 &&
- DP_TUNNELING_13_5GBPS_PER_LANE_SUPPORT == DP_UHBR13_5 &&
- DP_TUNNELING_20GBPS_PER_LANE_SUPPORT == DP_UHBR20);
-
- return ret;
-}
-
/* update sink rates from dpcd */
static void intel_dp_set_dpcd_sink_rates(struct intel_dp *intel_dp)
{
- static const int dp_rates[] = {
- 162000, 270000, 540000, 810000
- };
- int i, max_rate;
- int max_lttpr_rate;
- u8 uhbr_rates = 0;
-
- if (drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) {
- /* Needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panel */
- static const int quirk_rates[] = { 162000, 270000, 324000 };
-
- memcpy(intel_dp->sink_rates, quirk_rates, sizeof(quirk_rates));
- intel_dp->num_sink_rates = ARRAY_SIZE(quirk_rates);
-
- return;
- }
-
- /*
- * Sink rates for 8b/10b.
- */
- max_rate = max_dprx_rate(intel_dp);
- max_lttpr_rate = drm_dp_lttpr_max_link_rate(intel_dp->lttpr_common_caps);
- if (max_lttpr_rate)
- max_rate = min(max_rate, max_lttpr_rate);
+ int num_sink_rates;
+ int max_rate;
+ int i, num;
- for (i = 0; i < ARRAY_SIZE(dp_rates); i++) {
- if (dp_rates[i] > max_rate)
- break;
- intel_dp->sink_rates[i] = dp_rates[i];
- }
+ num_sink_rates = drm_dp_read_dpcd_sink_rates(&intel_dp->aux,
+ intel_dp->dpcd,
+ intel_dp->lttpr_common_caps,
+ &intel_dp->desc,
+ intel_dp->tunnel,
+ intel_dp->sink_rates);
+ if (num_sink_rates < 0)
+ num_sink_rates = 0;
/*
- * The following register must be read unconditionally for the later
- * DP tunnel 128b132b detection to work, see DP Standard v2.1 5.14.3 .
+ * Don't limit the rates of sinks with the 3.24 Gbps DPCD quirk
+ * (needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panels),
+ * they support rates not advertised by DP_MAX_LINK_RATE.
*/
- drm_dp_dpcd_read_byte(&intel_dp->aux, DP_128B132B_SUPPORTED_LINK_RATES, &uhbr_rates);
+ if (!drm_dp_has_quirk(&intel_dp->desc, DP_DPCD_QUIRK_CAN_DO_MAX_LINK_RATE_3_24_GBPS)) {
+ /*
+ * Limit the 8b/10b rates to the maximum rate the source can
+ * drive. The 128b/132b rates are all above the maximum 8b/10b
+ * rate and are not limited by the source's DP_MAX_LINK_RATE.
+ */
+ max_rate = max_dprx_rate(intel_dp);
+ num = 0;
+ for (i = 0; i < num_sink_rates; i++) {
+ if (!drm_dp_is_uhbr_rate(intel_dp->sink_rates[i]) &&
+ intel_dp->sink_rates[i] > max_rate)
+ continue;
- /*
- * Sink rates for 128b/132b. If set, sink should support all 8b/10b
- * rates and 10 Gbps.
- */
- if (dprx_supports_128b132b(intel_dp)) {
- BUILD_BUG_ON(ARRAY_SIZE(intel_dp->sink_rates) < ARRAY_SIZE(dp_rates) + 3);
-
- uhbr_rates = dprx_128b132b_link_rates(intel_dp, uhbr_rates);
-
- if (drm_dp_lttpr_count(intel_dp->lttpr_common_caps)) {
- /* We have a repeater */
- if (intel_dp->lttpr_common_caps[0] >= 0x20 &&
- intel_dp->lttpr_common_caps[DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER -
- DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV] &
- DP_PHY_REPEATER_128B132B_SUPPORTED) {
- /* Repeater supports 128b/132b, valid UHBR rates */
- uhbr_rates &= intel_dp->lttpr_common_caps[DP_PHY_REPEATER_128B132B_RATES -
- DP_LT_TUNABLE_PHY_REPEATER_FIELD_DATA_STRUCTURE_REV];
- } else {
- /* Does not support 128b/132b */
- uhbr_rates = 0;
- }
+ intel_dp->sink_rates[num++] = intel_dp->sink_rates[i];
}
- if (uhbr_rates & DP_UHBR10)
- intel_dp->sink_rates[i++] = 1000000;
- if (uhbr_rates & DP_UHBR13_5)
- intel_dp->sink_rates[i++] = 1350000;
- if (uhbr_rates & DP_UHBR20)
- intel_dp->sink_rates[i++] = 2000000;
+ num_sink_rates = num;
}
- intel_dp->num_sink_rates = i;
+ intel_dp->num_sink_rates = num_sink_rates;
}
static void intel_dp_set_sink_rates(struct intel_dp *intel_dp)
@@ -4722,50 +4667,24 @@ static void
intel_edp_set_sink_rates(struct intel_dp *intel_dp)
{
struct intel_display *display = to_intel_display(intel_dp);
+ int num_sink_rates;
- intel_dp->num_sink_rates = 0;
-
- if (intel_dp->edp_dpcd[0] >= DP_EDP_14) {
- __le16 sink_rates[DP_MAX_SUPPORTED_RATES];
- int ret;
- int i;
-
- ret = drm_dp_dpcd_read_data(&intel_dp->aux,
- DP_SUPPORTED_LINK_RATES,
- sink_rates, sizeof(sink_rates));
- if (ret < 0) {
- drm_dbg_kms(display->drm,
- "Unable to read eDP supported link rates, using default rates\n");
- memset(sink_rates, 0, sizeof(sink_rates));
- }
-
- for (i = 0; i < ARRAY_SIZE(sink_rates); i++) {
- int rate;
+ num_sink_rates = drm_dp_read_edp_sink_rates(&intel_dp->aux,
+ intel_dp->edp_dpcd[0],
+ intel_dp->sink_rates);
+ if (num_sink_rates < 0)
+ num_sink_rates = 0;
- /* Value read multiplied by 200kHz gives the per-lane
- * link rate in kHz. The source rates are, however,
- * stored in terms of LS_Clk kHz. The full conversion
- * 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 platforms cannot reliably drive HBR3 rates due to PHY limitations,
- * even if the sink advertises support. Reject any sink rates above HBR2 on
- * the known machines for stable output.
- */
- if (rate > 540000 &&
- intel_has_quirk(display, QUIRK_EDP_LIMIT_RATE_HBR2))
- break;
+ /*
+ * Some platforms cannot reliably drive HBR3 rates due to PHY limitations,
+ * even if the sink advertises support. Reject any sink rates above HBR2 on
+ * the known machines for stable output.
+ */
+ if (num_sink_rates && intel_has_quirk(display, QUIRK_EDP_LIMIT_RATE_HBR2))
+ num_sink_rates = intel_dp_rate_limit_len(intel_dp->sink_rates,
+ num_sink_rates, 540000);
- intel_dp->sink_rates[i] = rate;
- }
- intel_dp->num_sink_rates = i;
- }
+ intel_dp->num_sink_rates = num_sink_rates;
/*
* Use DP_LINK_RATE_SET if DP_SUPPORTED_LINK_RATES are available,
diff --git a/include/drm/display/drm_dp.h b/include/drm/display/drm_dp.h
index e63c7f075f54c..9db924ff7c53e 100644
--- a/include/drm/display/drm_dp.h
+++ b/include/drm/display/drm_dp.h
@@ -1228,6 +1228,7 @@
# define DP_UHBR10 (1 << 0)
# define DP_UHBR20 (1 << 1)
# define DP_UHBR13_5 (1 << 2)
+# define DP_MAX_128B132B_SUPPORTED_RATES 3
#define DP_128B132B_TRAINING_AUX_RD_INTERVAL 0x2216 /* 2.0 */
# define DP_128B132B_TRAINING_AUX_RD_INTERVAL_1MS_UNIT (1 << 7)
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 5b05a7fbe174a..d7f4c6f2ba4c7 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -32,6 +32,8 @@
struct drm_device;
struct drm_dp_aux;
struct drm_panel;
+struct drm_dp_tunnel;
+struct drm_dp_desc;
bool drm_dp_channel_eq_ok(const u8 link_status[DP_LINK_STATUS_SIZE],
int lane_count);
@@ -700,6 +702,15 @@ static inline int drm_dp_dpcd_write_byte(struct drm_dp_aux *aux,
int drm_dp_read_dpcd_caps(struct drm_dp_aux *aux,
u8 dpcd[DP_RECEIVER_CAP_SIZE]);
+int drm_dp_read_dpcd_sink_rates(struct drm_dp_aux *aux,
+ const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+ const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE],
+ const struct drm_dp_desc *desc,
+ const struct drm_dp_tunnel *tunnel,
+ int sink_rates[DP_MAX_SUPPORTED_RATES]);
+int drm_dp_read_edp_sink_rates(struct drm_dp_aux *aux, u8 edp_dpcd_rev,
+ int sink_rates[DP_MAX_SUPPORTED_RATES]);
+
int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
u8 status[DP_LINK_STATUS_SIZE]);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC v3 1/8] drm: Move sink link rates reading helpers out of i915
2026-10-12 15:10 ` [PATCH RFC v3 1/8] drm: Move sink link rates reading helpers out of i915 Kory Maincent
@ 2026-10-12 15:26 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-12 15:26 UTC (permalink / raw)
To: Kory Maincent; +Cc: intel-xe, intel-gfx, dri-devel
Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [Medium] drm: drm_dp_lttpr_count() negative error code evaluated as true
- [Low] drm/display: Missing <drm/display/drm_dp_tunnel.h> include in drm_dp_helper.c
- [Low] drm/display: API doc mismatch in drm_dp_read_edp_sink_rates()
--
commit fd61f2a21aaa6e7f861570be192ba592894c3a4d
Author: Kory Maincent <kory.maincent@bootlin.com>
drm: Move sink link rates reading helpers out of i915
The commit moves sink link rates reading helpers from i915 into generic DRM
helpers for reuse. It leaves Intel-specific quirks in the driver.
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index e623ccb4c1d8a..8ef096a0138e4 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
[ ... ]
> @@ -4912,3 +4912,240 @@ int drm_dp_max_dprx_data_rate(int max_link_rate, int max_lanes)
[ ... ]
> +#if IS_ENABLED(CONFIG_DRM_DISPLAY_DP_TUNNEL)
> +
> +static u8 drm_dprx_128b132b_link_rates(const struct drm_dp_tunnel *tunnel,
> + u8 uhbr_rates)
> +{
> + if (!drm_dp_tunnel_bw_alloc_is_enabled(tunnel))
[Severity: Low]
Does this code build correctly when CONFIG_DRM_DISPLAY_DP_TUNNEL is enabled?
The patch adds calls to drm_dp_tunnel_bw_alloc_is_enabled(),
drm_dp_tunnel_128b132b_dprx_rates(), and drm_dp_tunnel_128b132b_supported() in
drm_dp_helper.c. These are defined in <drm/display/drm_dp_tunnel.h>.
However, neither drm_dp_helper.c nor its included headers appear to include
this header, which could result in implicit function declaration errors.
> + return uhbr_rates;
> +
> + static_assert(DP_TUNNELING_10GBPS_PER_LANE_SUPPORT == DP_UHBR10 &&
> + DP_TUNNELING_13_5GBPS_PER_LANE_SUPPORT == DP_UHBR13_5 &&
> + DP_TUNNELING_20GBPS_PER_LANE_SUPPORT == DP_UHBR20);
> +
> + return drm_dp_tunnel_128b132b_dprx_rates(tunnel);
> +}
[ ... ]
> +static int
> +drm_dp_get_128b132b_link_rates(const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE],
> + const struct drm_dp_tunnel *tunnel,
> + u8 uhbr_rates,
> + int link_rates[DP_MAX_128B132B_SUPPORTED_RATES])
> +{
> + int i = 0;
> +
> + if (tunnel)
> + uhbr_rates = drm_dprx_128b132b_link_rates(tunnel, uhbr_rates);
> +
> + if (drm_dp_lttpr_count(lttpr_common_caps)) {
[Severity: Medium]
Will this evaluate negative error codes as a positive repeater count?
drm_dp_lttpr_count() can return negative error codes (like -EINVAL or -ERANGE)
for invalid DPCD LTTPR counts. This condition evaluates those negative errors as
true, falsely assuming a repeater is present.
If the DPCD revision lttpr_common_caps[0] is < 0x20, the else branch executes
and clears uhbr_rates = 0, wrongly discarding the sink's supported UHBR rates.
> + /* We have a repeater */
> + if (lttpr_common_caps[0] >= 0x20 &&
> + dp_lttpr_common_cap(lttpr_common_caps,
> + DP_MAIN_LINK_CHANNEL_CODING_PHY_REPEATER) &
> + DP_PHY_REPEATER_128B132B_SUPPORTED) {
[ ... ]
> +int drm_dp_read_dpcd_sink_rates(struct drm_dp_aux *aux,
> + const u8 dpcd[DP_RECEIVER_CAP_SIZE],
> + const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE],
> + const struct drm_dp_desc *desc,
> + const struct drm_dp_tunnel *tunnel,
> + int sink_rates[DP_MAX_SUPPORTED_RATES])
> +{
[ ... ]
> + /*
> + * Sink rates for 128b/132b. If set, sink should support all 8b/10b
> + * rates and 10 Gbps.
> + */
> + if (drm_dprx_supports_128b132b(tunnel, dpcd))
> + i += drm_dp_get_128b132b_link_rates(lttpr_common_caps,
> + tunnel,
> + uhbr_rates,
> + &sink_rates[i]);
[Severity: Medium]
Could this result in a NULL pointer dereference inside
drm_dp_get_128b132b_link_rates()?
Earlier in drm_dp_read_dpcd_sink_rates(), lttpr_common_caps is allowed to be
NULL:
if (lttpr_common_caps)
max_lttpr_rate = drm_dp_lttpr_max_link_rate(lttpr_common_caps);
However, it is passed blindly to drm_dp_get_128b132b_link_rates() here. If a
caller passes a NULL lttpr_common_caps, drm_dp_lttpr_count() will attempt to
access it, triggering a NULL pointer dereference.
> +
> + return i;
> +}
> +EXPORT_SYMBOL_GPL(drm_dp_read_dpcd_sink_rates);
> +
> +/**
> + * drm_dp_read_edp_sink_rates() - read the sink rates from the eDP DPCD
[ ... ]
> +int drm_dp_read_edp_sink_rates(struct drm_dp_aux *aux, u8 edp_dpcd_rev,
> + int sink_rates[DP_MAX_SUPPORTED_RATES])
> +{
> + int num_sink_rates;
> +
> + if (edp_dpcd_rev < DP_EDP_14)
> + return 0;
> +
> + num_sink_rates = drm_dp_read_dpcd_supported_link_rates(aux, sink_rates);
> + if (num_sink_rates < 0) {
> + drm_dbg_kms(aux->drm_dev,
> + "%s: Unable to read eDP supported link rates (%d)\n",
> + aux->name, num_sink_rates);
> + return 0;
[Severity: Low]
Does this implementation match its API documentation?
The kernel-doc for drm_dp_read_edp_sink_rates() explicitly states:
"Returns: The number of supported sink rates, 0 if the sink does not support
the DP_SUPPORTED_LINK_RATES register, negative error code otherwise."
However, this implementation traps the negative error code internally and
explicitly returns 0 on failure. This creates a confusing API contract for
drivers using this newly exported helper.
> + }
> +
> + return num_sink_rates;
> +}
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=1
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC v3 2/8] drm: Move DSC capability reading helper out of i915
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
2026-10-12 15:10 ` [PATCH RFC v3 1/8] drm: Move sink link rates reading helpers out of i915 Kory Maincent
@ 2026-10-12 15:10 ` Kory Maincent
2026-10-12 15:10 ` [PATCH RFC v3 3/8] drm: Introduce DisplayPort link training capabilities and state helpers Kory Maincent
` (5 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:10 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
In preparation for adding generic DRM properties to report source and
sink capabilities, move the DSC capability reading helper out of the
i915 driver into the generic DRM helpers.
No functional change intended.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Change in v3:
- New patch
---
drivers/gpu/drm/display/drm_dp_helper.c | 30 ++++++++++++++++++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.c | 28 ++++------------------------
include/drm/display/drm_dp_helper.h | 2 ++
3 files changed, 36 insertions(+), 24 deletions(-)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 8ef096a0138e4..93cf43d166c6e 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -5149,3 +5149,33 @@ int drm_dp_read_edp_sink_rates(struct drm_dp_aux *aux, u8 edp_dpcd_rev,
return num_sink_rates;
}
EXPORT_SYMBOL_GPL(drm_dp_read_edp_sink_rates);
+
+/**
+ * drm_dp_read_dsc_dpcd() - read the DSC capabilities from the DPCD
+ * @aux: DisplayPort AUX channel
+ * @dsc_dpcd: Buffer to store the DSC capabilities read from the DPCD
+ *
+ * Attempts to read the sink DSC capabilities from the DPCD of @aux.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+int drm_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
+ u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
+{
+ int ret;
+
+ ret = drm_dp_dpcd_read_data(aux, DP_DSC_SUPPORT, dsc_dpcd,
+ DP_DSC_RECEIVER_CAP_SIZE);
+ if (ret) {
+ drm_dbg_kms(aux->drm_dev,
+ "Could not read DSC DPCD register 0x%x Error: %pe\n",
+ DP_DSC_SUPPORT, ERR_PTR(ret));
+ return ret;
+ }
+
+ drm_dbg_kms(aux->drm_dev, "DSC DPCD: %*ph\n",
+ DP_DSC_RECEIVER_CAP_SIZE,
+ dsc_dpcd);
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drm_dp_read_dsc_dpcd);
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index da73ad11e9cac..830fe6eda17d7 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4430,26 +4430,6 @@ static bool intel_dp_get_sst_split_sdp_status(u8 dprx)
return dprx & DP_SST_SPLIT_SDP_CAP;
}
-static int intel_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
- u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE])
-{
- int ret;
-
- ret = drm_dp_dpcd_read_data(aux, DP_DSC_SUPPORT, dsc_dpcd,
- DP_DSC_RECEIVER_CAP_SIZE);
- if (ret) {
- drm_dbg_kms(aux->drm_dev,
- "Could not read DSC DPCD register 0x%x Error: %pe\n",
- DP_DSC_SUPPORT, ERR_PTR(ret));
- return ret;
- }
-
- drm_dbg_kms(aux->drm_dev, "DSC DPCD: %*ph\n",
- DP_DSC_RECEIVER_CAP_SIZE,
- dsc_dpcd);
- return 0;
-}
-
static void init_dsc_overall_throughput_limits(struct intel_connector *connector, bool is_branch)
{
u8 branch_caps[DP_DSC_BRANCH_CAP_SIZE];
@@ -4498,8 +4478,8 @@ void intel_dp_get_dsc_sink_cap(u8 dpcd_rev,
if (dpcd_rev < DP_DPCD_REV_14)
return;
- if (intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
- connector->dp.dsc_dpcd) < 0)
+ if (drm_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
+ connector->dp.dsc_dpcd) < 0)
return;
if (drm_dp_dpcd_readb(connector->dp.dsc_decompression_aux, DP_FEC_CAPABILITY,
@@ -4530,8 +4510,8 @@ static void intel_edp_get_dsc_sink_cap(u8 edp_dpcd_rev, struct intel_connector *
if (edp_dpcd_rev < DP_EDP_14)
return;
- if (intel_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
- connector->dp.dsc_dpcd) < 0)
+ if (drm_dp_read_dsc_dpcd(connector->dp.dsc_decompression_aux,
+ connector->dp.dsc_dpcd) < 0)
return;
if (connector->dp.dsc_dpcd[0] & DP_DSC_DECOMPRESSION_IS_SUPPORTED)
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index d7f4c6f2ba4c7..7bc3b36f38b82 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -215,6 +215,8 @@ drm_dp_is_branch(const u8 dpcd[DP_RECEIVER_CAP_SIZE])
}
/* DP/eDP DSC support */
+int drm_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
+ u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
u8 drm_dp_dsc_sink_bpp_incr(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE]);
u32 drm_dp_dsc_slice_count_to_mask(int slice_count);
u32 drm_dp_dsc_sink_slice_count_mask(const u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE],
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH RFC v3 3/8] drm: Introduce DisplayPort link training capabilities and state helpers
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
2026-10-12 15:10 ` [PATCH RFC v3 1/8] drm: Move sink link rates reading helpers out of i915 Kory Maincent
2026-10-12 15:10 ` [PATCH RFC v3 2/8] drm: Move DSC capability reading helper " Kory Maincent
@ 2026-10-12 15:10 ` Kory Maincent
2026-10-12 15:23 ` sashiko-bot
2026-10-12 15:10 ` [PATCH RFC v3 4/8] drm/i915/display/dp: Read DPCD DSC capabilities in any case Kory Maincent
` (4 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:10 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
Add helpers to expose link training capabilities and state to userspace
via sysfs attributes.
Link rates are passed by the driver in deca-kbps, following the DRM
convention, but exposed to sysfs userspace in kbps for clarity.
The aim of such development is to guide users to select the most suitable
DisplayPort connector for their needs. For example, if you have a USB-C
hub with lesser capabilities than your computer’s native DisplayPort
connector (such as HBR2 versus HBR3 support), the system could recommend
connecting high-resolution displays directly to the computer’s port
instead of through the hub to ensure optimal performance.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
If the needs arise we can add sysfs files for sink and source common
capabilities and hardware achievable capabilities. Let's begin with this
for now.
Changes in v3:
- Use drm_connector_dp struct for source caps, sync caps and current link
information.
- Remove drmm_connector_dp_init and use an helper instead to report
source capabilities.
- Report all sink link rates capabilities instead of only the max link
rates.
- Expose sysfs attributes only if nlanes is set.
- Add a new mutex to protect sysfs read write access instead of using
the global connection_mutex.
- Add documentation
Changes in v2:
- Remove voltage swing and pre-emphasis properties
- Expose link training state via sysfs dp_link/ group instead of
connector properties
- Rename variables from link_train to link as they relate directly to
the link capabilities
- Add comprehensive sysfs attributes for both source and sink capabilities
- Add separate helpers for managing sink capabilities and for current and
maximum link parameters
---
Documentation/ABI/testing/sysfs-class-drm-dp | 85 ++++++++
drivers/gpu/drm/display/drm_dp_helper.c | 304 +++++++++++++++++++++++++++
drivers/gpu/drm/drm_connector.c | 11 +
drivers/gpu/drm/drm_sysfs.c | 155 ++++++++++++++
include/drm/display/drm_dp_helper.h | 12 ++
include/drm/drm_connector.h | 53 +++++
include/drm/drm_sysfs.h | 1 +
7 files changed, 621 insertions(+)
diff --git a/Documentation/ABI/testing/sysfs-class-drm-dp b/Documentation/ABI/testing/sysfs-class-drm-dp
new file mode 100644
index 0000000000000..21c95a1b8dce0
--- /dev/null
+++ b/Documentation/ABI/testing/sysfs-class-drm-dp
@@ -0,0 +1,85 @@
+What: /sys/class/drm/card<X>-<connector>/dp_link/
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Directory of DisplayPort link attributes. It exists for
+ DisplayPort and eDP connectors and is empty until the
+ driver reports the source and the sink capabilities.
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/source_link_caps_nlanes
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Maximum number of DisplayPort lanes the source of this
+ connector can drive.
+
+ Valid values: 1, 2 or 4
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/source_link_caps_link_rates
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Space-separated list of the DisplayPort link rates the
+ source of this connector can drive, in kbps, sorted in
+ ascending order.
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/source_link_caps_dsc
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Indicates whether the source of this connector supports
+ Display Stream Compression (DSC).
+
+ Valid values: 0 (not supported), 1 (supported)
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/sink_link_caps_nlanes
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Maximum number of DisplayPort lanes supported by the
+ connected sink, as reported by the sink capabilities.
+
+ Valid values: 1, 2 or 4
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/sink_link_caps_link_rates
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Space-separated list of the DisplayPort link rates
+ supported by the connected sink, in kbps, sorted in
+ ascending order. The list is empty if the sink link
+ rates could not be read.
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/sink_link_caps_dsc
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Indicates whether the connected sink supports Display
+ Stream Compression (DSC).
+
+ Valid values: 0 (not supported), 1 (supported)
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/cur_link_info_nlanes
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Number of lanes used by the currently trained DisplayPort
+ link of this connector.
+
+ Valid values: 1, 2 or 4
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/cur_link_info_link_rate
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Link rate of the currently trained DisplayPort link of
+ this connector, in kbps.
+
+What: /sys/class/drm/card<X>-<connector>/dp_link/cur_link_info_dsc
+Date: October 2026
+Contact: dri-devel@lists.freedesktop.org
+Description:
+ Indicates whether Display Stream Compression (DSC) is
+ active on the currently trained DisplayPort link.
+
+ Valid values: 0 (disabled), 1 (enabled)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 93cf43d166c6e..c37826ad4d5a8 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -40,6 +40,7 @@
#include <drm/drm_edid.h>
#include <drm/drm_fixed.h>
#include <drm/drm_print.h>
+#include <drm/drm_sysfs.h>
#include <drm/drm_vblank.h>
#include <drm/drm_panel.h>
@@ -5118,6 +5119,30 @@ int drm_dp_read_dpcd_sink_rates(struct drm_dp_aux *aux,
}
EXPORT_SYMBOL_GPL(drm_dp_read_dpcd_sink_rates);
+/**
+ * drm_dp_get_max_lane_count - Get the max lane count from DPCD and LTTPR
+ * capabilities
+ * @dpcd: DPCD receiver capabilities
+ * @lttpr_common_caps: LTTPR common capabilities (NULL if no LTTPR supported)
+ *
+ * Returns: Number of max lane count
+ */
+int drm_dp_get_max_lane_count(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+ const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE])
+{
+ int nlanes, lttpr_nlanes;
+
+ nlanes = dpcd[DP_MAX_LANE_COUNT] & DP_MAX_LANE_COUNT_MASK;
+ if (lttpr_common_caps) {
+ lttpr_nlanes = drm_dp_lttpr_max_lane_count(lttpr_common_caps);
+ if (lttpr_nlanes)
+ nlanes = min(nlanes, lttpr_nlanes);
+ }
+
+ return nlanes;
+}
+EXPORT_SYMBOL_GPL(drm_dp_get_max_lane_count);
+
/**
* drm_dp_read_edp_sink_rates() - read the sink rates from the eDP DPCD
* @aux: DisplayPort AUX channel
@@ -5179,3 +5204,282 @@ int drm_dp_read_dsc_dpcd(struct drm_dp_aux *aux,
return 0;
}
EXPORT_SYMBOL_GPL(drm_dp_read_dsc_dpcd);
+
+static int drm_dp_copy_caps(struct drm_connector_dp_link_caps *dest_caps,
+ const struct drm_connector_dp_link_caps *src_caps)
+{
+ int *link_rates = NULL;
+ int nlink_rates = 0;
+
+ if (src_caps->link_rates && src_caps->nlink_rates > 0) {
+ link_rates = kmemdup_array(src_caps->link_rates,
+ src_caps->nlink_rates,
+ sizeof(*src_caps->link_rates),
+ GFP_KERNEL);
+ if (!link_rates)
+ return -ENOMEM;
+
+ nlink_rates = src_caps->nlink_rates;
+ }
+
+ kfree(dest_caps->link_rates);
+
+ dest_caps->nlanes = src_caps->nlanes;
+ dest_caps->nlink_rates = nlink_rates;
+ dest_caps->link_rates = link_rates;
+ dest_caps->dsc = src_caps->dsc;
+
+ return 0;
+}
+
+/**
+ * drm_dp_source_set_caps - Set DisplayPort source capabilities
+ * @connector: DisplayPort connector
+ * @link_caps: DisplayPort link training capabilities. The link_rates pointer
+ * is not kept by the DRM core
+ *
+ * This function sets the DisplayPort source link training capabilities
+ * for the given connector. This function has to be called after
+ * drm_connector_init() and before drm_dev_register().
+ */
+int drm_dp_source_set_caps(struct drm_connector *connector,
+ const struct drm_connector_dp_link_caps *link_caps)
+{
+ int ret;
+
+ if (!connector)
+ return -ENODEV;
+
+ if (!(connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector->connector_type == DRM_MODE_CONNECTOR_eDP))
+ return -EINVAL;
+
+ WARN_ON(connector->dev->registered);
+
+ /* No need to take the dp mutex as the connector sysfs is not yet
+ * created
+ */
+ ret = drm_dp_copy_caps(&connector->dp.source_link_caps, link_caps);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drm_dp_source_set_caps);
+
+/**
+ * drm_dp_sink_set_caps - Set DisplayPort sink link capabilities
+ * @connector: DisplayPort connector
+ * @link_caps: DisplayPort link training capabilities. The link_rates pointer
+ * is not kept by the DRM core
+ *
+ * This function sets the DisplayPort sink (monitor) link training
+ * capabilities for the given connector. This function lets the controller
+ * driver read the DPCD on its own, to avoid too many read of the same
+ * DPCD data.
+ * This should be called when a sink is connected.
+ *
+ * Returns: 0 if the capabilities were set successfully, negative error code
+ * otherwise.
+ */
+int drm_dp_sink_set_caps(struct drm_connector *connector,
+ const struct drm_connector_dp_link_caps *link_caps)
+{
+ int ret;
+
+ if (!connector)
+ return -ENODEV;
+
+ drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);
+
+ scoped_guard(mutex, &connector->dp.mutex) {
+ ret = drm_dp_copy_caps(&connector->dp.sink_link_caps, link_caps);
+ if (ret)
+ return ret;
+
+ drm_sysfs_connector_dp_link_update(connector);
+ }
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(drm_dp_sink_set_caps);
+
+/**
+ * drm_dp_sink_sync_caps - Fetch and set DisplayPort sink link capabilities
+ * @connector: DisplayPort connector
+ * @aux: DisplayPort AUX channel
+ * @tunnel: Tunnel object (NULL if not supported)
+ *
+ * This function fetches the DisplayPort sink link capabilities from DPCD and
+ * sets them for the given connector.
+ * This should be called when a sink is connected.
+ *
+ * Returns: 0 if the capabilities were fetched and set successfully, negative
+ * error code otherwise.
+ */
+int drm_dp_sink_sync_caps(struct drm_connector *connector, struct drm_dp_aux *aux,
+ const struct drm_dp_tunnel *tunnel)
+{
+ u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE] = {0};
+ struct drm_connector_dp_link_caps link_caps;
+ int sink_rates[DP_MAX_SUPPORTED_RATES] = {0};
+ u8 dsc_dpcd[DP_DSC_RECEIVER_CAP_SIZE] = {0};
+ u8 dpcd_caps[DP_RECEIVER_CAP_SIZE] = {0};
+ struct drm_dp_desc desc = {0};
+ int num_sink_rates, ret;
+ u8 edp_dpcd_rev = 0;
+
+ if (!connector)
+ return -ENODEV;
+
+ drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);
+
+ ret = drm_dp_read_dpcd_caps(aux, dpcd_caps);
+ if (ret < 0) {
+ drm_dbg_kms(aux->drm_dev, "%s: error reading DPCD capabilities (%d)\n",
+ aux->name, ret);
+ return ret;
+ }
+
+ if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+ ret = drm_dp_dpcd_read_byte(aux, DP_EDP_DPCD_REV, &edp_dpcd_rev);
+ if (ret < 0)
+ drm_dbg_kms(aux->drm_dev, "%s: No DPCD eDP revision found\n",
+ aux->name);
+ }
+
+ if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
+ dpcd_caps[DP_DPCD_REV] >= DP_DPCD_REV_13) {
+ ret = drm_dp_read_lttpr_common_caps(aux, dpcd_caps, lttpr_common_caps);
+ if (ret < 0) {
+ drm_dbg_kms(aux->drm_dev,
+ "%s: error reading LTTPR caps, assuming no LTTPR (%d)\n",
+ aux->name, ret);
+ /* LTTPR capabilities are read block by block. Clean the
+ * entire capabilities if the read fails to avoid
+ * partial garbage
+ */
+ memset(lttpr_common_caps, 0, sizeof(lttpr_common_caps));
+ }
+ }
+
+ ret = drm_dp_read_desc(aux, &desc, drm_dp_is_branch(dpcd_caps));
+ if (ret)
+ drm_dbg_kms(aux->drm_dev, "%s: No DPCD descriptor found\n",
+ aux->name);
+
+ if (connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+ num_sink_rates = drm_dp_read_edp_sink_rates(aux, edp_dpcd_rev,
+ sink_rates);
+
+ /*
+ * Use the rates of the DP_MAX_LINK_RATE and
+ * DP_128B132B_SUPPORTED_LINK_RATES registers if the sink does
+ * not support the DP_SUPPORTED_LINK_RATES register.
+ */
+ if (num_sink_rates <= 0)
+ num_sink_rates = drm_dp_read_dpcd_sink_rates(aux, dpcd_caps,
+ lttpr_common_caps,
+ &desc,
+ tunnel,
+ sink_rates);
+ } else {
+ num_sink_rates = drm_dp_read_dpcd_sink_rates(aux, dpcd_caps,
+ lttpr_common_caps,
+ &desc,
+ tunnel,
+ sink_rates);
+ }
+
+ if ((connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort &&
+ dpcd_caps[DP_DPCD_REV] >= DP_DPCD_REV_14) ||
+ (connector->connector_type == DRM_MODE_CONNECTOR_eDP &&
+ edp_dpcd_rev >= DP_EDP_14)) {
+ ret = drm_dp_read_dsc_dpcd(aux, dsc_dpcd);
+ if (ret < 0)
+ drm_dbg_kms(aux->drm_dev,
+ "%s: error reading DPCD DSC capabilities (%d)\n",
+ aux->name, ret);
+ }
+
+ link_caps.nlanes = drm_dp_get_max_lane_count(dpcd_caps, lttpr_common_caps);
+ link_caps.nlink_rates = num_sink_rates;
+ link_caps.link_rates = sink_rates;
+ link_caps.dsc = drm_dp_sink_supports_dsc(dsc_dpcd);
+
+ return drm_dp_sink_set_caps(connector, &link_caps);
+}
+EXPORT_SYMBOL_GPL(drm_dp_sink_sync_caps);
+
+/**
+ * drm_dp_set_cur_link_params - Set current DisplayPort link parameters
+ * @connector: DisplayPort connector
+ * @link_rate: Current link rate in deca-kbps
+ * @lane_count: Current lane count
+ * @dsc_en: Display Stream Compression enabled
+ *
+ * This function sets the current active DisplayPort link parameters after
+ * link training has completed. These parameters represent the actual link
+ * configuration being used for display output.
+ */
+void drm_dp_set_cur_link_params(struct drm_connector *connector,
+ int link_rate, int lane_count, bool dsc_en)
+{
+ bool update = false;
+ int *_link_rates;
+
+ if (!connector)
+ return;
+
+ drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);
+
+ scoped_guard(mutex, &connector->dp.mutex) {
+ if (!connector->dp.cur_link_info.nlanes || !lane_count)
+ update = true;
+
+ if (!connector->dp.cur_link_info.link_rates) {
+ _link_rates = kzalloc_obj(*_link_rates);
+ if (!_link_rates)
+ return;
+
+ connector->dp.cur_link_info.link_rates = _link_rates;
+ connector->dp.cur_link_info.nlink_rates = 1;
+ }
+
+ *connector->dp.cur_link_info.link_rates = link_rate;
+ connector->dp.cur_link_info.nlanes = lane_count;
+ connector->dp.cur_link_info.dsc = dsc_en;
+
+ if (update)
+ drm_sysfs_connector_dp_link_update(connector);
+ }
+}
+EXPORT_SYMBOL_GPL(drm_dp_set_cur_link_params);
+
+/**
+ * drm_dp_sink_reset_caps - Reset DisplayPort sink capabilities
+ * @connector: DisplayPort connector
+ *
+ * This function clears all DisplayPort sink link capabilities and parameters.
+ * This should be called when a sink is disconnected to clear stale
+ * capability information.
+ */
+void drm_dp_sink_reset_caps(struct drm_connector *connector)
+{
+ struct drm_connector_dp_link_caps zero_caps = {0};
+
+ if (!connector)
+ return;
+
+ drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);
+
+ scoped_guard(mutex, &connector->dp.mutex) {
+ kfree(connector->dp.cur_link_info.link_rates);
+ memcpy(&connector->dp.cur_link_info, &zero_caps, sizeof(zero_caps));
+ kfree(connector->dp.sink_link_caps.link_rates);
+ memcpy(&connector->dp.sink_link_caps, &zero_caps, sizeof(zero_caps));
+
+ drm_sysfs_connector_dp_link_update(connector);
+ }
+}
+EXPORT_SYMBOL_GPL(drm_dp_sink_reset_caps);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index 893716a9e4895..f9b780fb340b5 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -285,6 +285,7 @@ static int drm_connector_init_only(struct drm_device *dev,
mutex_init(&connector->edid_override_mutex);
mutex_init(&connector->hdmi.infoframes.lock);
mutex_init(&connector->hdmi_audio.lock);
+ mutex_init(&connector->dp.mutex);
connector->edid_blob_ptr = NULL;
connector->epoch_counter = 0;
connector->tile_blob_ptr = NULL;
@@ -941,6 +942,16 @@ void drm_connector_cleanup(struct drm_connector *connector)
connector->tile_group = NULL;
}
+ scoped_guard(mutex, &connector->dp.mutex) {
+ kfree(connector->dp.source_link_caps.link_rates);
+ connector->dp.source_link_caps.link_rates = NULL;
+ kfree(connector->dp.sink_link_caps.link_rates);
+ connector->dp.sink_link_caps.link_rates = NULL;
+ kfree(connector->dp.cur_link_info.link_rates);
+ connector->dp.cur_link_info.link_rates = NULL;
+ }
+ mutex_destroy(&connector->dp.mutex);
+
list_for_each_entry_safe(mode, t, &connector->probed_modes, head)
drm_mode_remove(connector, mode);
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index 7b16d32b6ff65..1949b46419c6d 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -340,6 +340,149 @@ static const struct attribute_group *connector_dev_groups[] = {
NULL
};
+static ssize_t drm_link_rates_show(int num_link_rates, const int *link_rates, char *buf)
+{
+ ssize_t size = 0;
+
+ if (!num_link_rates)
+ return 0;
+
+ /* We are using deca-kbps in the kernel historically, due to
+ * calculation over the link rates that could overflow 32 bits.
+ * Report link rates in kbps to avoid confusing the user
+ */
+ size += sysfs_emit_at(buf, size, "%d", link_rates[0] * 10);
+ for (int i = 1; i < num_link_rates; i++)
+ size += sysfs_emit_at(buf, size, " %d", link_rates[i] * 10);
+
+ size += sysfs_emit_at(buf, size, "\n");
+ return size;
+}
+
+#define DRM_CONNECTOR_DP_ATTR_SHOW_LINK_RATES_NESTED(_name) \
+static ssize_t _name##_link_rates_show(struct device *device, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ struct drm_connector *connector = to_drm_connector(device); \
+ ssize_t size; \
+ scoped_guard(mutex, &connector->dp.mutex) \
+ size = drm_link_rates_show(connector->dp._name.nlink_rates, \
+ connector->dp._name.link_rates, buf); \
+ return size; \
+}
+
+#define DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(_name, _info) \
+static ssize_t _name##_##_info##_show(struct device *device, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ struct drm_connector *connector = to_drm_connector(device); \
+ ssize_t size; \
+ scoped_guard(mutex, &connector->dp.mutex) \
+ size = sysfs_emit(buf, "%d\n", connector->dp._name._info); \
+ return size; \
+}
+
+static ssize_t cur_link_info_link_rate_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(device);
+ ssize_t size;
+
+ scoped_guard(mutex, &connector->dp.mutex)
+ size = drm_link_rates_show(connector->dp.cur_link_info.nlink_rates,
+ connector->dp.cur_link_info.link_rates, buf);
+ return size;
+}
+
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(source_link_caps, nlanes);
+DRM_CONNECTOR_DP_ATTR_SHOW_LINK_RATES_NESTED(source_link_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(source_link_caps, dsc);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(sink_link_caps, nlanes);
+DRM_CONNECTOR_DP_ATTR_SHOW_LINK_RATES_NESTED(sink_link_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(sink_link_caps, dsc);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(cur_link_info, nlanes);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(cur_link_info, dsc);
+
+static DEVICE_ATTR_RO(source_link_caps_nlanes);
+static DEVICE_ATTR_RO(source_link_caps_link_rates);
+static DEVICE_ATTR_RO(source_link_caps_dsc);
+static DEVICE_ATTR_RO(sink_link_caps_nlanes);
+static DEVICE_ATTR_RO(sink_link_caps_link_rates);
+static DEVICE_ATTR_RO(sink_link_caps_dsc);
+static DEVICE_ATTR_RO(cur_link_info_nlanes);
+static DEVICE_ATTR_RO(cur_link_info_link_rate);
+static DEVICE_ATTR_RO(cur_link_info_dsc);
+
+static struct attribute *connector_dp_link_attrs[] = {
+ &dev_attr_source_link_caps_nlanes.attr,
+ &dev_attr_source_link_caps_link_rates.attr,
+ &dev_attr_source_link_caps_dsc.attr,
+ &dev_attr_sink_link_caps_nlanes.attr,
+ &dev_attr_sink_link_caps_link_rates.attr,
+ &dev_attr_sink_link_caps_dsc.attr,
+ &dev_attr_cur_link_info_nlanes.attr,
+ &dev_attr_cur_link_info_link_rate.attr,
+ &dev_attr_cur_link_info_dsc.attr,
+ NULL
+};
+
+static umode_t dp_link_attr_visible(struct kobject *kobj,
+ struct attribute *a, int n)
+{
+ struct device *dev = kobj_to_dev(kobj);
+ struct drm_connector *connector = to_drm_connector(dev);
+ struct drm_connector_dp_link_caps *caps;
+
+ if (a == &dev_attr_source_link_caps_nlanes.attr ||
+ a == &dev_attr_source_link_caps_link_rates.attr ||
+ a == &dev_attr_source_link_caps_dsc.attr)
+ caps = &connector->dp.source_link_caps;
+ else if (a == &dev_attr_sink_link_caps_nlanes.attr ||
+ a == &dev_attr_sink_link_caps_link_rates.attr ||
+ a == &dev_attr_sink_link_caps_dsc.attr)
+ caps = &connector->dp.sink_link_caps;
+ else
+ caps = &connector->dp.cur_link_info;
+
+ /* Checking nlanes is enough to decide whether the attributes should
+ * be exposed: without lanes there is no working DisplayPort link
+ */
+ return caps->nlanes ? a->mode : 0;
+}
+
+static const struct attribute_group connector_dp_link_group = {
+ .name = "dp_link",
+ .attrs = connector_dp_link_attrs,
+ .is_visible = dp_link_attr_visible,
+};
+
+/**
+ * drm_sysfs_connector_dp_link_update - Update the visibility of the DP link
+ * attributes
+ * @connector: DisplayPort connector
+ */
+void drm_sysfs_connector_dp_link_update(struct drm_connector *connector)
+{
+ int ret;
+
+ if (!connector->kdev)
+ return;
+
+ if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
+ connector->connector_type != DRM_MODE_CONNECTOR_eDP)
+ return;
+
+ ret = sysfs_update_group(&connector->kdev->kobj,
+ &connector_dp_link_group);
+ if (ret)
+ drm_err(connector->dev,
+ "failed to update DP link sysfs attributes: %d\n", ret);
+}
+EXPORT_SYMBOL_GPL(drm_sysfs_connector_dp_link_update);
+
int drm_sysfs_connector_add(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
@@ -376,6 +519,15 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
connector->kdev = kdev;
+ if (connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector->connector_type == DRM_MODE_CONNECTOR_eDP) {
+ r = sysfs_create_group(&connector->kdev->kobj, &connector_dp_link_group);
+ if (r) {
+ drm_err(dev, "failed to create DP connector sysfs: %d\n", r);
+ goto err_dp_sysfs;
+ }
+ }
+
if (dev_fwnode(kdev)) {
r = component_add(kdev, &typec_connector_ops);
if (r)
@@ -384,6 +536,9 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
return 0;
+err_dp_sysfs:
+ connector->kdev = NULL;
+ device_del(kdev);
err_free:
put_device(kdev);
return r;
diff --git a/include/drm/display/drm_dp_helper.h b/include/drm/display/drm_dp_helper.h
index 7bc3b36f38b82..ad899ac18bfd8 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -712,6 +712,8 @@ int drm_dp_read_dpcd_sink_rates(struct drm_dp_aux *aux,
int sink_rates[DP_MAX_SUPPORTED_RATES]);
int drm_dp_read_edp_sink_rates(struct drm_dp_aux *aux, u8 edp_dpcd_rev,
int sink_rates[DP_MAX_SUPPORTED_RATES]);
+int drm_dp_get_max_lane_count(const u8 dpcd[DP_RECEIVER_CAP_SIZE],
+ const u8 lttpr_common_caps[DP_LTTPR_COMMON_CAP_SIZE]);
int drm_dp_dpcd_read_link_status(struct drm_dp_aux *aux,
u8 status[DP_LINK_STATUS_SIZE]);
@@ -1051,4 +1053,14 @@ ssize_t drm_dp_vsc_sdp_pack(const struct drm_dp_vsc_sdp *vsc, struct dp_sdp *sdp
int drm_dp_link_symbol_cycles(int lane_count, int pixels, int dsc_slice_count,
int bpp_x16, int symbol_size, bool is_mst);
+int drm_dp_source_set_caps(struct drm_connector *connector,
+ const struct drm_connector_dp_link_caps *link_caps);
+int drm_dp_sink_sync_caps(struct drm_connector *connector, struct drm_dp_aux *aux,
+ const struct drm_dp_tunnel *tunnel);
+int drm_dp_sink_set_caps(struct drm_connector *connector,
+ const struct drm_connector_dp_link_caps *link_caps);
+void drm_dp_sink_reset_caps(struct drm_connector *connector);
+void drm_dp_set_cur_link_params(struct drm_connector *connector, int link_rate,
+ int lane_count, bool dsc_en);
+
#endif /* _DRM_DP_HELPER_H_ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 2ee7c59329aae..415fa5652d3c4 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2282,6 +2282,54 @@ struct drm_connector_cec {
void *data;
};
+/**
+ * struct drm_connector_dp_link_caps - DRM DisplayPort link capabilities
+ */
+struct drm_connector_dp_link_caps {
+ /**
+ * @nlanes: Maximum number of lanes number supported
+ */
+ int nlanes;
+
+ /**
+ * @nlink_rates: Number of link rates supported
+ */
+ int nlink_rates;
+
+ /**
+ * @link_rates: Array listing the supported link rates in deca-kbps
+ */
+ int *link_rates;
+
+ /**
+ * @dsc: Display Stream Compression supported
+ */
+ bool dsc;
+};
+
+/**
+ * struct drm_connector_dp - DRM Connector DisplayPort-related structure
+ */
+struct drm_connector_dp {
+ /**
+ * @source_link_caps: Source link capabilities
+ */
+ struct drm_connector_dp_link_caps source_link_caps;
+ /**
+ * @sink_link_caps: Sink link capabilities
+ */
+ struct drm_connector_dp_link_caps sink_link_caps;
+ /**
+ * @cur_link_info: Current negotiated link information
+ */
+ struct drm_connector_dp_link_caps cur_link_info;
+
+ /**
+ * @mutex: Lock to protect read write access
+ */
+ struct mutex mutex;
+};
+
/**
* struct drm_connector - central DRM connector control structure
*
@@ -2714,6 +2762,11 @@ struct drm_connector {
* @cec: CEC-related data.
*/
struct drm_connector_cec cec;
+
+ /**
+ * @dp: DisplayPort-related variable and properties.
+ */
+ struct drm_connector_dp dp;
};
#define obj_to_connector(x) container_of(x, struct drm_connector, base)
diff --git a/include/drm/drm_sysfs.h b/include/drm/drm_sysfs.h
index 7695873a94565..366a02e27f366 100644
--- a/include/drm/drm_sysfs.h
+++ b/include/drm/drm_sysfs.h
@@ -10,4 +10,5 @@ void drm_sysfs_hotplug_event(struct drm_device *dev);
void drm_sysfs_connector_hotplug_event(struct drm_connector *connector);
void drm_sysfs_connector_property_event(struct drm_connector *connector,
struct drm_property *property);
+void drm_sysfs_connector_dp_link_update(struct drm_connector *connector);
#endif
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC v3 3/8] drm: Introduce DisplayPort link training capabilities and state helpers
2026-10-12 15:10 ` [PATCH RFC v3 3/8] drm: Introduce DisplayPort link training capabilities and state helpers Kory Maincent
@ 2026-10-12 15:23 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-12 15:23 UTC (permalink / raw)
To: Kory Maincent; +Cc: intel-gfx, intel-xe, dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm: AB-BA deadlock between sysfs active ref and dp.mutex
- [High] drm: UAF/NULL deref of connector->kdev in sysfs_connector_dp_link_update
--
commit 1620ef6dfb353b92f81d9907563ffd328e9eb367
Author: Kory Maincent <kory.maincent@bootlin.com>
Subject: drm: Introduce DisplayPort link training capabilities and state helpers
This commit introduces helpers to expose DisplayPort link training capabilities
and state to userspace via sysfs attributes. It adds functions to manage
sink and source capabilities and exposes them through a new dp_link sysfs
group.
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
[ ... ]
> +void drm_dp_sink_reset_caps(struct drm_connector *connector)
> +{
> + struct drm_connector_dp_link_caps zero_caps = {0};
> +
> + if (!connector)
> + return;
> +
> + drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);
> +
> + scoped_guard(mutex, &connector->dp.mutex) {
> + kfree(connector->dp.cur_link_info.link_rates);
> + memcpy(&connector->dp.cur_link_info, &zero_caps, sizeof(zero_caps));
> + kfree(connector->dp.sink_link_caps.link_rates);
> + memcpy(&connector->dp.sink_link_caps, &zero_caps, sizeof(zero_caps));
> +
> + drm_sysfs_connector_dp_link_update(connector);
> + }
> +}
[ ... ]
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
[ ... ]
> +#define DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE_NESTED(_name, _info) \
> +static ssize_t _name##_##_info##_show(struct device *device, \
> + struct device_attribute *attr, \
> + char *buf) \
> +{ \
> + struct drm_connector *connector = to_drm_connector(device); \
> + ssize_t size; \
> + scoped_guard(mutex, &connector->dp.mutex) \
> + size = sysfs_emit(buf, "%d\n", connector->dp._name._info); \
> + return size; \
> +}
[Severity: High]
Does this sequence introduce an AB-BA deadlock between the sysfs active
reference and connector->dp.mutex?
If a userspace thread reads a sysfs attribute like cur_link_info_nlanes,
the VFS/kernfs core acquires an active reference (s_active) on the
kernfs_node. The show callback then attempts to lock connector->dp.mutex via
scoped_guard().
Concurrently, a hotplug or link retraining event can call
drm_dp_sink_reset_caps() as seen in the drm_dp_helper.c snippet above. That
thread acquires connector->dp.mutex first, and then calls
drm_sysfs_connector_dp_link_update().
When drm_sysfs_connector_dp_link_update() invokes sysfs_update_group(),
and the visibility drops to 0 (because nlanes is cleared to 0), it calls
kernfs_remove_by_name(). This in turn calls kernfs_drain(), which blocks
indefinitely waiting for all active references to drop.
This leaves the kernel thread holding dp.mutex waiting for s_active to
drop, and the userspace thread holding s_active waiting for dp.mutex.
[ ... ]
> +void drm_sysfs_connector_dp_link_update(struct drm_connector *connector)
> +{
> + int ret;
> +
> + if (!connector->kdev)
> + return;
> +
> + if (connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort &&
> + connector->connector_type != DRM_MODE_CONNECTOR_eDP)
> + return;
> +
> + ret = sysfs_update_group(&connector->kdev->kobj,
> + &connector_dp_link_group);
[Severity: High]
Is it possible for connector->kdev to be unregistered and freed between
the NULL check and the sysfs_update_group() call?
MST hot-unplug processes (like drm_dp_delayed_destroy_work()) can invoke
drm_connector_unregister() and device_unregister(connector->kdev) without
holding connection_mutex or connector->dp.mutex.
Since drm_sysfs_connector_dp_link_update() does not hold a lock that
serializes against connector unregistration, this looks like a TOCTOU race
condition where connector->kdev->kobj could be a use-after-free or a
NULL pointer dereference.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=3
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC v3 4/8] drm/i915/display/dp: Read DPCD DSC capabilities in any case
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
` (2 preceding siblings ...)
2026-10-12 15:10 ` [PATCH RFC v3 3/8] drm: Introduce DisplayPort link training capabilities and state helpers Kory Maincent
@ 2026-10-12 15:10 ` Kory Maincent
2026-10-12 15:11 ` [PATCH RFC v3 5/8] drm/i915/display/dp: Use dp_connector helpers to expose link training state Kory Maincent
` (3 subsequent siblings)
7 siblings, 0 replies; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:10 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
In preparation for the support of reporting the sink capabilities we
need to read the DSC sink capabilities even if the source does not
support it.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Change in v3:
- New patch
---
drivers/gpu/drm/i915/display/intel_dp.c | 5 -----
1 file changed, 5 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index 830fe6eda17d7..b769723a1c0fd 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -4521,12 +4521,7 @@ static void intel_edp_get_dsc_sink_cap(u8 edp_dpcd_rev, struct intel_connector *
static void
intel_dp_detect_dsc_caps(struct intel_dp *intel_dp, struct intel_connector *connector)
{
- struct intel_display *display = to_intel_display(intel_dp);
-
/* Read DP Sink DSC Cap DPCD regs for DP v1.4 */
- if (!HAS_DSC(display))
- return;
-
if (intel_dp_is_edp(intel_dp))
intel_edp_get_dsc_sink_cap(intel_dp->edp_dpcd[0],
connector);
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* [PATCH RFC v3 5/8] drm/i915/display/dp: Use dp_connector helpers to expose link training state
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
` (3 preceding siblings ...)
2026-10-12 15:10 ` [PATCH RFC v3 4/8] drm/i915/display/dp: Read DPCD DSC capabilities in any case Kory Maincent
@ 2026-10-12 15:11 ` Kory Maincent
2026-10-12 15:24 ` sashiko-bot
2026-10-12 15:11 ` [PATCH RFC v3 6/8] drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag Kory Maincent
` (2 subsequent siblings)
7 siblings, 1 reply; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:11 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
Adopt the new DRM core DisplayPort helpers, which expose source and sink
capabilities along with the maximum and current link parameters to
userspace through the dp_link/ sysfs attributes.
Keep the sink's supported link rates unclamped in the new
dpcd_sink_rates/num_dpcd_sink_rates fields, rather than limiting them to
the rates the source supports.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Changes in v3:
- Move the call to new helpers around.
- Use real sink rates instead of the modified ones.
Changes in v2:
- Remove voltage swing and pre emphasis properties.
---
drivers/gpu/drm/i915/display/intel_display_types.h | 2 +
drivers/gpu/drm/i915/display/intel_dp.c | 55 +++++++++++++++++++++-
.../gpu/drm/i915/display/intel_dp_link_training.c | 14 ++++++
3 files changed, 69 insertions(+), 2 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_display_types.h b/drivers/gpu/drm/i915/display/intel_display_types.h
index 5f0fe18c0614e..025523ae58e6c 100644
--- a/drivers/gpu/drm/i915/display/intel_display_types.h
+++ b/drivers/gpu/drm/i915/display/intel_display_types.h
@@ -1845,6 +1845,8 @@ struct intel_dp {
/* sink rates as reported by DP_MAX_LINK_RATE/DP_SUPPORTED_LINK_RATES */
int num_sink_rates;
int sink_rates[DP_MAX_SUPPORTED_RATES];
+ int num_dpcd_sink_rates;
+ int dpcd_sink_rates[DP_MAX_SUPPORTED_RATES];
bool use_rate_select;
/* Max sink lane count as reported by DP_MAX_LANE_COUNT */
int max_sink_lane_count;
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index b769723a1c0fd..6af59e75bd666 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -189,6 +189,7 @@ static void intel_dp_set_default_sink_rates(struct intel_dp *intel_dp)
{
intel_dp->sink_rates[0] = 162000;
intel_dp->num_sink_rates = 1;
+ intel_dp->num_dpcd_sink_rates = 0;
}
/* update sink rates from dpcd */
@@ -203,10 +204,14 @@ static void intel_dp_set_dpcd_sink_rates(struct intel_dp *intel_dp)
intel_dp->lttpr_common_caps,
&intel_dp->desc,
intel_dp->tunnel,
- intel_dp->sink_rates);
+ intel_dp->dpcd_sink_rates);
if (num_sink_rates < 0)
num_sink_rates = 0;
+ intel_dp->num_dpcd_sink_rates = num_sink_rates;
+ memcpy(intel_dp->sink_rates, intel_dp->dpcd_sink_rates,
+ sizeof(int) * num_sink_rates);
+
/*
* Don't limit the rates of sinks with the 3.24 Gbps DPCD quirk
* (needed, e.g., for Apple MBP 2017, 15 inch eDP Retina panels),
@@ -4646,10 +4651,14 @@ intel_edp_set_sink_rates(struct intel_dp *intel_dp)
num_sink_rates = drm_dp_read_edp_sink_rates(&intel_dp->aux,
intel_dp->edp_dpcd[0],
- intel_dp->sink_rates);
+ intel_dp->dpcd_sink_rates);
if (num_sink_rates < 0)
num_sink_rates = 0;
+ intel_dp->num_dpcd_sink_rates = num_sink_rates;
+ memcpy(intel_dp->sink_rates, intel_dp->dpcd_sink_rates,
+ sizeof(int) * num_sink_rates);
+
/*
* Some platforms cannot reliably drive HBR3 rates due to PHY limitations,
* even if the sink advertises support. Reject any sink rates above HBR2 on
@@ -6188,6 +6197,20 @@ void intel_dp_dpcd_set_probe(struct intel_dp *intel_dp, bool force_on_external)
intel_dp_needs_dpcd_probe(intel_dp, force_on_external));
}
+static void intel_dp_set_sink_caps(struct intel_connector *connector,
+ struct intel_dp *intel_dp)
+{
+ struct drm_connector_dp_link_caps link_caps;
+
+ link_caps.nlanes = drm_dp_get_max_lane_count(intel_dp->dpcd,
+ intel_dp->lttpr_common_caps);
+ link_caps.link_rates = intel_dp->dpcd_sink_rates;
+ link_caps.nlink_rates = intel_dp->num_dpcd_sink_rates;
+ link_caps.dsc = drm_dp_sink_supports_dsc(connector->dp.dsc_dpcd);
+
+ drm_dp_sink_set_caps(&connector->base, &link_caps);
+}
+
static int
intel_dp_detect(struct drm_connector *_connector,
struct drm_modeset_acquire_ctx *ctx,
@@ -6259,6 +6282,8 @@ intel_dp_detect(struct drm_connector *_connector,
intel_dp_tunnel_uhbr_lanes_wa_reset(intel_dp);
+ drm_dp_sink_reset_caps(_connector);
+
goto out_unset_edid;
}
@@ -6290,6 +6315,8 @@ intel_dp_detect(struct drm_connector *_connector,
intel_dp_print_rates(intel_dp);
+ intel_dp_set_sink_caps(connector, intel_dp);
+
if (intel_dp->is_mst) {
/*
* If we are in MST mode then this connector
@@ -7037,11 +7064,14 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
struct intel_connector *connector)
{
struct intel_display *display = to_intel_display(dig_port);
+ struct drm_connector_dp_link_caps link_caps;
struct intel_dp *intel_dp = &dig_port->dp;
struct intel_encoder *encoder = &dig_port->base;
struct drm_device *dev = encoder->base.dev;
enum port port = encoder->port;
+ int *rates;
int type;
+ int ret;
if (drm_WARN(dev, dig_port->max_lanes < 1,
"Not enough lanes (%d) for DP on [ENCODER:%d:%s]\n",
@@ -7116,6 +7146,27 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
intel_dp_set_common_link_params(intel_dp);
intel_dp_reset_link_params(intel_dp);
+ rates = kmemdup_array(intel_dp->source_rates,
+ intel_dp->num_source_rates,
+ sizeof(*rates),
+ GFP_KERNEL);
+ if (!rates) {
+ intel_dp_aux_fini(intel_dp);
+ goto fail;
+ }
+
+ link_caps.nlanes = dig_port->max_lanes;
+ link_caps.nlink_rates = intel_dp->num_source_rates;
+ link_caps.link_rates = rates;
+ link_caps.dsc = HAS_DSC(display);
+
+ ret = drm_dp_source_set_caps(&connector->base, &link_caps);
+ if (ret)
+ drm_dbg_kms(display->drm,
+ "failed to set source link rates, skipping.\n");
+
+ kfree(rates);
+
/* init MST on ports that can support it */
intel_dp_mst_encoder_init(dig_port, connector->base.base.id);
diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
index cb92cff906146..c77cb124f1419 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1758,6 +1758,18 @@ static void link_recovery_reset(struct intel_dp_link_training *link_training)
link_training->recovery_state = INTEL_DP_LINK_RECOVERY_IDLE;
}
+static void intel_dp_report_link_train(struct intel_dp *intel_dp)
+{
+ struct intel_connector *connector = intel_dp->attached_connector;
+
+ if (!connector)
+ return;
+
+ drm_dp_set_cur_link_params(&connector->base, intel_dp->link_rate,
+ intel_dp->lane_count,
+ connector->dp.dsc_decompression_enabled);
+}
+
/**
* intel_dp_stop_link_train - stop link training
* @intel_dp: DP struct
@@ -1787,6 +1799,8 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
intel_dp_program_link_training_pattern(intel_dp, crtc_state, DP_PHY_DPRX,
DP_TRAINING_PATTERN_DISABLE);
+ intel_dp_report_link_train(intel_dp);
+
if (intel_dp_is_uhbr(crtc_state)) {
ret = poll_timeout_us(ret = intel_dp_128b132b_intra_hop(intel_dp, crtc_state),
ret == 0,
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC v3 5/8] drm/i915/display/dp: Use dp_connector helpers to expose link training state
2026-10-12 15:11 ` [PATCH RFC v3 5/8] drm/i915/display/dp: Use dp_connector helpers to expose link training state Kory Maincent
@ 2026-10-12 15:24 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-12 15:24 UTC (permalink / raw)
To: Kory Maincent; +Cc: intel-xe, dri-devel, intel-gfx
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm/i915: lockdep splat and sleeping in atomic context via intel_dp_report_link_train
--
commit a5f66f1f6cf0713414f9ea60cce11fc6b162d6e4
Author: Kory Maincent <kory.maincent@bootlin.com>
drm/i915/display/dp: Use dp_connector helpers to expose link training state
Adopt the new DRM core DisplayPort helpers, which expose source and sink
capabilities along with the maximum and current link parameters to
userspace through the dp_link/ sysfs attributes.
[ ... ]
> diff --git a/drivers/gpu/drm/i915/display/intel_dp_link_training.c b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> index cb92cff906146..c77cb124f1419 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
[ ... ]
> @@ -1787,6 +1799,8 @@ void intel_dp_stop_link_train(struct intel_dp *intel_dp,
> intel_dp_program_link_training_pattern(intel_dp, crtc_state, DP_PHY_DPRX,
> DP_TRAINING_PATTERN_DISABLE);
>
> + intel_dp_report_link_train(intel_dp);
> +
[Severity: High]
Does this new call introduce a lockdep splat and sleep in an atomic context?
intel_dp_stop_link_train() executes during encoder atomic enable callbacks as
part of the intel_atomic_commit_tail() sequence, which runs in an atomic
context.
The new intel_dp_report_link_train() delegates to the DRM core helper
drm_dp_set_cur_link_params():
void drm_dp_set_cur_link_params(...)
{
...
drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex);
scoped_guard(mutex, &connector->dp.mutex) {
...
_link_rates = kzalloc_obj(*_link_rates);
...
}
During non-blocking commits, the commit tail runs asynchronously in a worker
thread after the initial IOCTL has dropped all modeset locks. This causes the
drm_modeset_lock_assert_held() to trigger a lockdep splat.
Furthermore, acquiring the sleeping mutex connector->dp.mutex and calling
kzalloc_obj() (which can sleep) in the atomic commit path violates DRM
subsystem invariants. Could this lead to kernel warnings and potential
deadlocks?
> if (intel_dp_is_uhbr(crtc_state)) {
> ret = poll_timeout_us(ret = intel_dp_128b132b_intra_hop(intel_dp, crtc_state),
> ret == 0,
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=5
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC v3 6/8] drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
` (4 preceding siblings ...)
2026-10-12 15:11 ` [PATCH RFC v3 5/8] drm/i915/display/dp: Use dp_connector helpers to expose link training state Kory Maincent
@ 2026-10-12 15:11 ` Kory Maincent
2026-10-12 15:20 ` sashiko-bot
2026-10-12 15:11 ` [PATCH RFC v3 7/8] drm/analogix/anx7625: Use dp_connector helpers to expose link training state Kory Maincent
2026-10-12 15:11 ` [PATCH RFC v3 8/8] drm/mediatek: " Kory Maincent
7 siblings, 1 reply; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:11 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
Introduce DRM_BRIDGE_OP_DP, a new bridge operation flag for bridges
that provide DisplayPort connector operations with link training support.
Bridges advertising this flag must fill the dp_link_caps field in
struct drm_bridge with their link capabilities.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Changes in v3:
- Mone on from drmm_connector_dp_init() to drm_dp_source_set_caps().
Change in v2:
- New patch
---
drivers/gpu/drm/display/Kconfig | 1 +
drivers/gpu/drm/display/drm_bridge_connector.c | 29 ++++++++++++++++++++++++++
include/drm/drm_bridge.h | 13 ++++++++++++
3 files changed, 43 insertions(+)
diff --git a/drivers/gpu/drm/display/Kconfig b/drivers/gpu/drm/display/Kconfig
index df09cf9a8ca19..ce2f6d62c76d0 100644
--- a/drivers/gpu/drm/display/Kconfig
+++ b/drivers/gpu/drm/display/Kconfig
@@ -19,6 +19,7 @@ config DRM_BRIDGE_CONNECTOR
select DRM_DISPLAY_HDMI_AUDIO_HELPER
select DRM_DISPLAY_HDMI_CEC_HELPER
select DRM_DISPLAY_HDMI_STATE_HELPER
+ select DRM_DISPLAY_DP_HELPER
help
DRM connector implementation terminating DRM bridge chains.
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 81f3e26f6fdf5..5b58e86da6259 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -26,6 +26,7 @@
#include <drm/display/drm_hdmi_cec_helper.h>
#include <drm/display/drm_hdmi_helper.h>
#include <drm/display/drm_hdmi_state_helper.h>
+#include <drm/display/drm_dp_helper.h>
/**
* DOC: overview
@@ -109,6 +110,13 @@ struct drm_bridge_connector {
* HDMI Audio infrastructure, if any (see &DRM_BRIDGE_OP_HDMI_AUDIO).
*/
struct drm_bridge *bridge_hdmi_audio;
+ /**
+ * @bridge_dp:
+ *
+ * The bridge in the chain that implements necessary support for the
+ * DisplayPort connector infrastructure, if any (see &DRM_BRIDGE_OP_DP).
+ */
+ struct drm_bridge *bridge_dp;
/**
* @bridge_dp_audio:
*
@@ -835,6 +843,7 @@ static void drm_bridge_connector_put_bridges(struct drm_device *dev, void *data)
drm_bridge_put(bridge_connector->bridge_hdmi_audio);
drm_bridge_put(bridge_connector->bridge_dp_audio);
drm_bridge_put(bridge_connector->bridge_hdmi_cec);
+ drm_bridge_put(bridge_connector->bridge_dp);
}
/**
@@ -968,6 +977,15 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
bridge_connector->bridge_hdmi_audio = drm_bridge_get(bridge);
}
+ if (bridge->ops & DRM_BRIDGE_OP_DP) {
+ if (bridge_connector->bridge_dp)
+ return ERR_PTR(-EBUSY);
+ if (!bridge->dp_link_caps)
+ return ERR_PTR(-EINVAL);
+
+ bridge_connector->bridge_dp = drm_bridge_get(bridge);
+ }
+
if (bridge->ops & DRM_BRIDGE_OP_DP_AUDIO) {
if (bridge_connector->bridge_dp_audio)
return ERR_PTR(-EBUSY);
@@ -1079,6 +1097,17 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
connector_type, ddc);
if (ret)
return ERR_PTR(ret);
+ } else if (bridge_connector->bridge_dp) {
+ ret = drmm_connector_init(drm, connector,
+ &drm_bridge_connector_funcs,
+ connector_type, ddc);
+ if (ret)
+ return ERR_PTR(ret);
+
+ ret = drm_dp_source_set_caps(connector,
+ bridge_connector->bridge_dp->dp_link_caps);
+ if (ret)
+ return ERR_PTR(ret);
} else {
ret = drmm_connector_init(drm, connector,
&drm_bridge_connector_funcs,
diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
index 1981d24a700d2..d1430b2b72518 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1021,6 +1021,14 @@ enum drm_bridge_ops {
* &drm_bridge_funcs->hdmi_clear_spd_infoframe callbacks.
*/
DRM_BRIDGE_OP_HDMI_SPD_INFOFRAME = BIT(10),
+ /**
+ * @DRM_BRIDGE_OP_DP: The bridge provides DisplayPort connector
+ * operations, including link training support. Bridges that set
+ * this flag must provide DisplayPort-related information and
+ * fill the &drm_bridge->dp_link_caps link training
+ * capabilities.
+ */
+ DRM_BRIDGE_OP_DP = BIT(11),
};
/**
@@ -1214,6 +1222,11 @@ struct drm_bridge {
*/
void *hpd_data;
+ /**
+ * @dp_link_caps: DisplayPort link capabilities
+ */
+ const struct drm_connector_dp_link_caps *dp_link_caps;
+
/**
* @next_bridge: Pointer to the following bridge, automatically put
* when this bridge is freed (i.e. at destroy time). This is for
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC v3 6/8] drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag
2026-10-12 15:11 ` [PATCH RFC v3 6/8] drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag Kory Maincent
@ 2026-10-12 15:20 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-12 15:20 UTC (permalink / raw)
To: Kory Maincent; +Cc: intel-gfx, dri-devel, intel-xe
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] drm: Probe failure for DP pipelines ending in non-DP connectors
--
commit 1f392a73217bd39ddc3ecb68c1400cefdd77a913
Author: Kory Maincent <kory.maincent@bootlin.com>
Subject: drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag
This commit introduces DRM_BRIDGE_OP_DP, a new bridge operation flag for
bridges that provide DisplayPort connector operations with link training
support. Bridges advertising this flag must fill the dp_link_caps field
with their link capabilities.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -1079,6 +1097,17 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> connector_type, ddc);
> if (ret)
> return ERR_PTR(ret);
> + } else if (bridge_connector->bridge_dp) {
> + ret = drmm_connector_init(drm, connector,
> + &drm_bridge_connector_funcs,
> + connector_type, ddc);
> + if (ret)
> + return ERR_PTR(ret);
> +
> + ret = drm_dp_source_set_caps(connector,
> + bridge_connector->bridge_dp->dp_link_caps);
> + if (ret)
> + return ERR_PTR(ret);
[Severity: High]
Does this cause a probe failure for DisplayPort pipelines that end in a non-DP
connector?
Earlier in drm_bridge_connector_init(), the final connector_type is determined
by the last bridge in the chain. If a display pipeline ends with an active
converter (like a DP-to-VGA or DP-to-LVDS bridge), the resulting connector
type will be non-DP (e.g., DRM_MODE_CONNECTOR_VGA).
However, this block unconditionally calls drm_dp_source_set_caps() as long as
any upstream bridge advertises DRM_BRIDGE_OP_DP and there is no HDMI bridge.
Looking at drm_dp_source_set_caps() in drm_dp_helper.c, it has a strict check
for the connector type:
if (!(connector->connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
connector->connector_type == DRM_MODE_CONNECTOR_eDP))
return -EINVAL;
If the pipeline ends in a VGA or LVDS connector, this will return -EINVAL,
which is then propagated up by drm_bridge_connector_init(), causing the entire
display pipeline initialization to fail for otherwise valid bridge chains.
Could we add a check here to ensure the connector type is actually DP or eDP
before calling drm_dp_source_set_caps()?
> } else {
> ret = drmm_connector_init(drm, connector,
> &drm_bridge_connector_funcs,
[ ... ]
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=6
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC v3 7/8] drm/analogix/anx7625: Use dp_connector helpers to expose link training state
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
` (5 preceding siblings ...)
2026-10-12 15:11 ` [PATCH RFC v3 6/8] drm/bridge: Expose link training state via new DRM_BRIDGE_OP_DP flag Kory Maincent
@ 2026-10-12 15:11 ` Kory Maincent
2026-10-12 15:26 ` sashiko-bot
2026-10-12 15:11 ` [PATCH RFC v3 8/8] drm/mediatek: " Kory Maincent
7 siblings, 1 reply; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:11 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
Adopt the DRM core DisplayPort helpers by setting DRM_BRIDGE_OP_DP and
populating the bridge dp_link_caps at probe time. Report the sink
capabilities on hotplug, reset them on disconnect, and report the link
parameters once link training succeeds.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Change in v3:
- New patch
---
drivers/gpu/drm/bridge/analogix/anx7625.c | 68 +++++++++++++++++++++++++++++++
drivers/gpu/drm/bridge/analogix/anx7625.h | 1 +
2 files changed, 69 insertions(+)
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
index c3af88e14f8b2..2b1235efc9fa4 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.c
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
@@ -2558,6 +2558,31 @@ static int anx7625_bridge_atomic_check(struct drm_bridge *bridge,
return 0;
}
+static void anx7625_report_link_train(struct anx7625_data *ctx,
+ struct drm_connector *connector)
+{
+ int bw_code, rate, nlanes;
+
+ /*
+ * The DP link training is handled by the internal OCM firmware, read
+ * back the negotiated link rate and lane count.
+ */
+ bw_code = anx7625_reg_read(ctx, ctx->i2c.tx_p0_client,
+ SP_TX_LINK_BW_SET_REG);
+ if (bw_code < 0)
+ return;
+
+ rate = drm_dp_bw_code_to_link_rate(bw_code & SP_TX_LINK_BW_SET_MASK);
+
+ nlanes = anx7625_reg_read(ctx, ctx->i2c.tx_p0_client,
+ SP_TX_LANE_COUNT_SET_REG);
+ if (nlanes < 0)
+ return;
+ nlanes &= DP_MAX_LANE_COUNT_MASK;
+
+ drm_dp_set_cur_link_params(connector, rate, nlanes, false);
+}
+
static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
struct drm_atomic_commit *state)
{
@@ -2565,6 +2590,7 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
struct device *dev = ctx->dev;
struct drm_connector *connector;
struct drm_connector_state *conn_state;
+ int ret;
dev_dbg(dev, "drm atomic enable\n");
@@ -2580,6 +2606,17 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
anx7625_dp_start(ctx);
+ /* On DisplayPort case, the caps are set in the detect callback. */
+ if (bridge->type == DRM_MODE_CONNECTOR_eDP) {
+ ret = drm_dp_sink_sync_caps(connector, &ctx->aux, NULL);
+ if (ret)
+ DRM_DEV_DEBUG_DRIVER(dev,
+ "error sink caps synchronisation (%d)\n",
+ ret);
+ }
+
+ anx7625_report_link_train(ctx, connector);
+
conn_state = drm_atomic_get_new_connector_state(state, connector);
if (WARN_ON(!conn_state))
@@ -2616,6 +2653,12 @@ static void anx7625_bridge_atomic_disable(struct drm_bridge *bridge,
dev_dbg(dev, "update CP to DESIRE\n");
}
+ /* On DisplayPort unplug, the caps are already reset in the detect
+ * callback.
+ */
+ if (bridge->type == DRM_MODE_CONNECTOR_eDP)
+ drm_dp_sink_reset_caps(ctx->connector);
+
ctx->connector = NULL;
anx7625_dp_stop(ctx);
@@ -2634,11 +2677,25 @@ anx7625_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector
struct anx7625_data *ctx = bridge_to_anx7625(bridge);
struct device *dev = ctx->dev;
enum drm_connector_status status;
+ int ret;
DRM_DEV_DEBUG_DRIVER(dev, "drm bridge detect\n");
status = anx7625_sink_detect(ctx);
anx7625_audio_update_connector_status(ctx, status);
+
+ if (status == connector_status_connected) {
+ ret = drm_dp_sink_sync_caps(connector, &ctx->aux, NULL);
+ if (ret)
+ DRM_DEV_DEBUG_DRIVER(dev,
+ "error sink caps synchronisation (%d)\n",
+ ret);
+
+ anx7625_report_link_train(ctx, connector);
+ } else {
+ drm_dp_sink_reset_caps(connector);
+ }
+
return status;
}
@@ -2765,6 +2822,14 @@ static const struct dev_pm_ops anx7625_pm_ops = {
anx7625_runtime_pm_resume, NULL)
};
+static int anx7625_dp_rates[] = {162000, 270000, 540000, 675000};
+
+static struct drm_connector_dp_link_caps anx7625_dp_link_caps = {
+ .nlanes = 2,
+ .nlink_rates = ARRAY_SIZE(anx7625_dp_rates),
+ .link_rates = anx7625_dp_rates,
+};
+
static int anx7625_link_bridge(struct drm_dp_aux *aux)
{
struct anx7625_data *platform = container_of(aux, struct anx7625_data, aux);
@@ -2787,6 +2852,9 @@ static int anx7625_link_bridge(struct drm_dp_aux *aux)
DRM_MODE_CONNECTOR_DisplayPort;
platform->bridge.support_hdcp = true;
+ platform->bridge.dp_link_caps = &anx7625_dp_link_caps;
+ platform->bridge.ops |= DRM_BRIDGE_OP_DP;
+
drm_bridge_add(&platform->bridge);
if (!platform->pdata.is_dpi) {
diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.h b/drivers/gpu/drm/bridge/analogix/anx7625.h
index 957d234ec07c8..b9a0f62f6398c 100644
--- a/drivers/gpu/drm/bridge/analogix/anx7625.h
+++ b/drivers/gpu/drm/bridge/analogix/anx7625.h
@@ -90,6 +90,7 @@
#define HDCP2TX_FW_EN BIT(4)
#define SP_TX_LINK_BW_SET_REG 0xA0
+#define SP_TX_LINK_BW_SET_MASK 0x1F
#define SP_TX_LANE_COUNT_SET_REG 0xA1
#define M_VID_0 0xC0
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC v3 7/8] drm/analogix/anx7625: Use dp_connector helpers to expose link training state
2026-10-12 15:11 ` [PATCH RFC v3 7/8] drm/analogix/anx7625: Use dp_connector helpers to expose link training state Kory Maincent
@ 2026-10-12 15:26 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-12 15:26 UTC (permalink / raw)
To: Kory Maincent; +Cc: dri-devel, intel-xe, intel-gfx
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] drm: lockdep assertion failure in DP helpers during atomic commit
- [High] drm/analogix: lockdep splat and sleep-in-atomic via dp_connector helpers in atomic callbacks
- [Medium] drm: eDP sink capabilities incorrectly erased on atomic_disable
- [Medium] drm: reading stale DP link parameters in detect callback
--
commit 1548e3d30cfb50d6bc0f342acba2b2ddf8b97bab
Author: Kory Maincent <kory.maincent@bootlin.com>
drm/analogix/anx7625: Use dp_connector helpers to expose link training state
Adopt the DRM core DisplayPort helpers by setting DRM_BRIDGE_OP_DP and
populating the bridge dp_link_caps at probe time. Report the sink
capabilities on hotplug, reset them on disconnect, and report the link
parameters once link training succeeds.
> diff --git a/drivers/gpu/drm/bridge/analogix/anx7625.c b/drivers/gpu/drm/bridge/analogix/anx7625.c
> index c3af88e14f8b2..2b1235efc9fa4 100644
> --- a/drivers/gpu/drm/bridge/analogix/anx7625.c
> +++ b/drivers/gpu/drm/bridge/analogix/anx7625.c
[ ... ]
> @@ -2580,6 +2606,17 @@ static void anx7625_bridge_atomic_enable(struct drm_bridge *bridge,
>
> anx7625_dp_start(ctx);
>
> + /* On DisplayPort case, the caps are set in the detect callback. */
> + if (bridge->type == DRM_MODE_CONNECTOR_eDP) {
> + ret = drm_dp_sink_sync_caps(connector, &ctx->aux, NULL);
> + if (ret)
> + DRM_DEV_DEBUG_DRIVER(dev,
> + "error sink caps synchronisation (%d)\n",
> + ret);
> + }
> +
> + anx7625_report_link_train(ctx, connector);
> +
[Severity: High]
Does calling drm_dp_sink_sync_caps() and anx7625_report_link_train() here
violate atomic context invariants and locking expectations?
During non-blocking atomic commits, the commit_tail worker executes this
callback asynchronously after the originating thread has dropped all modeset
locks. However, drm_dp_sink_sync_caps() and drm_dp_set_cur_link_params()
(called by anx7625_report_link_train) explicitly assert that
connector->dev->mode_config.connection_mutex is held.
Additionally, atomic_enable callbacks execute in atomic context where sleeping
is forbidden. The DRM DP helpers perform I2C AUX transfers, allocate memory,
and acquire standard mutexes (like connector->dp.mutex), which are sleeping
operations.
> conn_state = drm_atomic_get_new_connector_state(state, connector);
>
> if (WARN_ON(!conn_state))
[ ... ]
> @@ -2616,6 +2653,12 @@ static void anx7625_bridge_atomic_disable(struct drm_bridge *bridge,
> dev_dbg(dev, "update CP to DESIRE\n");
> }
>
> + /* On DisplayPort unplug, the caps are already reset in the detect
> + * callback.
> + */
> + if (bridge->type == DRM_MODE_CONNECTOR_eDP)
> + drm_dp_sink_reset_caps(ctx->connector);
> +
[Severity: High]
Similar to atomic_enable, does calling drm_dp_sink_reset_caps() from an
atomic_disable callback trigger lockdep and sleep-in-atomic warnings?
The helper asserts connection_mutex is held, but the lock is dropped before
asynchronous commits reach this callback. It also acquires standard mutexes,
violating the atomic context requirement.
[Severity: Medium]
Is it intentional to clear eDP sink capabilities on display disable?
Unlike hotpluggable DisplayPort monitors where capabilities must be cleared
upon physical unplug, an eDP panel is permanently attached to the system and
its capabilities are static. Erasing the capabilities on atomic_disable
causes sysfs DP capability attributes to read empty or zero whenever the
screen is in a DPMS off state or entering suspend, even though the panel
is still present.
> ctx->connector = NULL;
> anx7625_dp_stop(ctx);
>
[ ... ]
> @@ -2634,11 +2677,25 @@ anx7625_bridge_detect(struct drm_bridge *bridge, struct drm_connector *connector
> struct anx7625_data *ctx = bridge_to_anx7625(bridge);
> struct device *dev = ctx->dev;
> enum drm_connector_status status;
> + int ret;
>
> DRM_DEV_DEBUG_DRIVER(dev, "drm bridge detect\n");
>
> status = anx7625_sink_detect(ctx);
> anx7625_audio_update_connector_status(ctx, status);
> +
> + if (status == connector_status_connected) {
> + ret = drm_dp_sink_sync_caps(connector, &ctx->aux, NULL);
> + if (ret)
> + DRM_DEV_DEBUG_DRIVER(dev,
> + "error sink caps synchronisation (%d)\n",
> + ret);
> +
> + anx7625_report_link_train(ctx, connector);
> + } else {
> + drm_dp_sink_reset_caps(connector);
> + }
> +
[Severity: Medium]
Will anx7625_report_link_train() read stale hardware registers here?
During the detect phase, the DRM core is merely probing for a monitor
connection. The display pipeline has not been enabled, the sink hasn't been
powered up, and DP link training hasn't taken place yet. Reading the
negotiated link bandwidth and lane count directly from the hardware registers
at this point seems like it would yield uninitialized or stale values from a
previous display session.
These invalid values are then erroneously reported to the DRM core via
drm_dp_set_cur_link_params(), circumventing the logic of reporting them only
after successful link training.
> return status;
> }
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=7
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH RFC v3 8/8] drm/mediatek: Use dp_connector helpers to expose link training state
2026-10-12 15:10 [PATCH RFC v3 0/8] Add support for DisplayPort link training information report Kory Maincent
` (6 preceding siblings ...)
2026-10-12 15:11 ` [PATCH RFC v3 7/8] drm/analogix/anx7625: Use dp_connector helpers to expose link training state Kory Maincent
@ 2026-10-12 15:11 ` Kory Maincent
2026-10-12 15:26 ` sashiko-bot
7 siblings, 1 reply; 15+ messages in thread
From: Kory Maincent @ 2026-10-12 15:11 UTC (permalink / raw)
To: Maarten Lankhorst, Maxime Ripard, Thomas Zimmermann, David Airlie,
Simona Vetter, Jani Nikula, Rodrigo Vivi, Joonas Lahtinen,
Tvrtko Ursulin, Andrzej Hajda, Neil Armstrong, Robert Foss,
Laurent Pinchart, Jonas Karlman, Jernej Skrabec, Luca Ceresoli,
Chun-Kuang Hu, Philipp Zabel, Matthias Brugger,
AngeloGioacchino Del Regno, Dmitry Baryshkov, Daniel Stone
Cc: Thomas Petazzoni, Mark Yacoub, Sean Paul, Manasi Navare,
Drew Davenport, Louis Chauvet, dri-devel, linux-kernel, intel-gfx,
intel-xe, linux-mediatek, linux-arm-kernel, Kory Maincent
Adopt the DRM core DisplayPort helpers by setting DRM_BRIDGE_OP_DP and
populating the bridge dp_link_caps at probe time. Report the sink
capabilities on hotplug, reset them on disconnect, and report the link
parameters once link training succeeds.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Not tested. I believed I had a board with this device, but it actually uses
an anx7625 bridge. I'm unable to test it; it would be appreciated if
someone else could, so the change isn't dropped.
Changes in v3:
- Sync and report sink capabilities from the HPD thread for DisplayPort,
or in atomic_enable for eDP. Reset them on unplug, or atomic disable
and only report link parameters after successful training.
- Add connection_mutex lock for DisplayPort in the IRQ thread.
Changes in v2:
- Remove voltage swing and pre emphasis properties.
---
drivers/gpu/drm/mediatek/mtk_dp.c | 79 +++++++++++++++++++++++++++++++++++++--
1 file changed, 75 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index f656b85b84212..2e2596eaa342e 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1812,6 +1812,35 @@ static void mtk_dp_train_change_mode(struct mtk_dp *mtk_dp)
mtk_dp_reset_swing_pre_emphasis(mtk_dp);
}
+static void _mtk_dp_report_link_train(struct mtk_dp *mtk_dp)
+{
+ u32 rate, nlanes;
+ bool dsc = false;
+
+ rate = drm_dp_bw_code_to_link_rate(mtk_dp->train_info.link_rate);
+ nlanes = mtk_dp->train_info.lane_count;
+
+ drm_dp_set_cur_link_params(mtk_dp->conn, rate, nlanes, dsc);
+}
+
+static void mtk_dp_report_link_train(struct mtk_dp *mtk_dp)
+{
+ struct drm_modeset_lock *lock;
+ int ret;
+
+ if (!mtk_dp->conn)
+ return;
+
+ lock = &mtk_dp->conn->dev->mode_config.connection_mutex;
+ ret = drm_modeset_lock_single_interruptible(lock);
+ if (ret)
+ return;
+
+ _mtk_dp_report_link_train(mtk_dp);
+
+ drm_modeset_unlock(lock);
+}
+
static int mtk_dp_training(struct mtk_dp *mtk_dp)
{
int ret;
@@ -2015,6 +2044,8 @@ static irqreturn_t mtk_dp_hpd_event_thread(int hpd, void *dev)
ret = mtk_dp_training(mtk_dp);
if (ret)
drm_err(mtk_dp->drm_dev, "Training failed, %d\n", ret);
+ else
+ mtk_dp_report_link_train(mtk_dp);
mtk_dp->enabled = true;
}
@@ -2145,9 +2176,12 @@ mtk_dp_bdg_detect(struct drm_bridge *bridge, struct drm_connector *connector)
struct mtk_dp *mtk_dp = mtk_dp_from_bridge(bridge);
enum drm_connector_status ret = connector_status_disconnected;
bool enabled = mtk_dp->enabled;
+ int err;
- if (!mtk_dp->train_info.cable_plugged_in)
+ if (!mtk_dp->train_info.cable_plugged_in) {
+ drm_dp_sink_reset_caps(connector);
return ret;
+ }
if (!enabled)
mtk_dp_aux_panel_poweron(mtk_dp, true);
@@ -2160,9 +2194,18 @@ mtk_dp_bdg_detect(struct drm_bridge *bridge, struct drm_connector *connector)
* whether we connect to a sink device.
*/
- if (drm_dp_read_sink_count(&mtk_dp->aux) > 0)
+ if (drm_dp_read_sink_count(&mtk_dp->aux) > 0) {
ret = connector_status_connected;
+ err = drm_dp_sink_sync_caps(connector, &mtk_dp->aux, NULL);
+ if (err)
+ drm_dbg_kms(mtk_dp->aux.drm_dev,
+ "%s: error sink caps synchronisation (%d)\n",
+ mtk_dp->aux.name, err);
+ } else {
+ drm_dp_sink_reset_caps(connector);
+ }
+
if (!enabled)
mtk_dp_aux_panel_poweron(mtk_dp, false);
@@ -2390,12 +2433,20 @@ static void mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
if (mtk_dp->data->bridge_type == DRM_MODE_CONNECTOR_eDP) {
mtk_dp_aux_panel_poweron(mtk_dp, true);
+ ret = drm_dp_sink_sync_caps(mtk_dp->conn, &mtk_dp->aux, NULL);
+ if (ret)
+ drm_dbg_kms(mtk_dp->aux.drm_dev,
+ "%s: error sink caps synchronisation (%d)\n",
+ mtk_dp->aux.name, ret);
+
/* Training */
ret = mtk_dp_training(mtk_dp);
if (ret) {
drm_err(mtk_dp->drm_dev, "Training failed, %d\n", ret);
goto power_off_aux;
}
+
+ _mtk_dp_report_link_train(mtk_dp);
}
ret = mtk_dp_video_config(mtk_dp);
@@ -2435,6 +2486,7 @@ static void mtk_dp_bridge_atomic_disable(struct drm_bridge *bridge,
if (mtk_dp->data->bridge_type == DRM_MODE_CONNECTOR_eDP) {
mtk_dp->enabled = false;
mtk_dp_aux_panel_poweron(mtk_dp, false);
+ drm_dp_sink_reset_caps(mtk_dp->conn);
}
mtk_dp_update_plugged_status(mtk_dp);
@@ -2740,11 +2792,14 @@ static int mtk_dp_edp_link_panel(struct drm_dp_aux *mtk_aux)
return 0;
}
+static int mtk_dp_rates[] = {162000, 270000, 540000, 810000};
+
static int mtk_dp_probe(struct platform_device *pdev)
{
+ struct drm_connector_dp_link_caps *source_caps;
struct mtk_dp *mtk_dp;
struct device *dev = &pdev->dev;
- int ret;
+ int ret, i, max_rate;
mtk_dp = devm_drm_bridge_alloc(dev, struct mtk_dp, bridge,
&mtk_dp_bridge_funcs);
@@ -2758,6 +2813,20 @@ static int mtk_dp_probe(struct platform_device *pdev)
if (ret)
return dev_err_probe(dev, ret, "Failed to parse dt\n");
+ source_caps = devm_kzalloc(dev, sizeof(*source_caps), GFP_KERNEL);
+ if (!source_caps)
+ return -ENOMEM;
+
+ source_caps->nlanes = mtk_dp->max_lanes;
+ source_caps->link_rates = mtk_dp_rates;
+
+ max_rate = drm_dp_bw_code_to_link_rate(mtk_dp->max_linkrate);
+ for (i = 0; i < ARRAY_SIZE(mtk_dp_rates); i++)
+ if (mtk_dp_rates[i] > max_rate)
+ break;
+
+ source_caps->nlink_rates = i;
+
/*
* Request the interrupt and install service routine only if we are
* on full DisplayPort.
@@ -2809,6 +2878,8 @@ static int mtk_dp_probe(struct platform_device *pdev)
mtk_dp->bridge.of_node = dev->of_node;
mtk_dp->bridge.type = mtk_dp->data->bridge_type;
+ mtk_dp->bridge.dp_link_caps = source_caps;
+ mtk_dp->bridge.ops = DRM_BRIDGE_OP_DP;
if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
/*
@@ -2849,7 +2920,7 @@ static int mtk_dp_probe(struct platform_device *pdev)
}
}
} else {
- mtk_dp->bridge.ops = DRM_BRIDGE_OP_DETECT |
+ mtk_dp->bridge.ops |= DRM_BRIDGE_OP_DETECT |
DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
ret = devm_drm_bridge_add(dev, &mtk_dp->bridge);
if (ret)
--
2.43.0
^ permalink raw reply related [flat|nested] 15+ messages in thread* Re: [PATCH RFC v3 8/8] drm/mediatek: Use dp_connector helpers to expose link training state
2026-10-12 15:11 ` [PATCH RFC v3 8/8] drm/mediatek: " Kory Maincent
@ 2026-10-12 15:26 ` sashiko-bot
0 siblings, 0 replies; 15+ messages in thread
From: sashiko-bot @ 2026-10-12 15:26 UTC (permalink / raw)
To: Kory Maincent; +Cc: dri-devel, intel-xe, intel-gfx
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] drm: connection_mutex not held during atomic_enable/disable DP sink helpers
- [Medium] drm: missed DP link parameter reporting on initial hotplug
--
commit db3d1199eb9fddde4ac0ef014ef0885a214a3723
Author: Kory Maincent <kory.maincent@bootlin.com>
drm/mediatek: Use dp_connector helpers to expose link training state
Adopt the DRM core DisplayPort helpers by setting DRM_BRIDGE_OP_DP and
populating the bridge dp_link_caps at probe time. Report the sink
capabilities on hotplug, reset them on disconnect, and report the link
parameters once link training succeeds.
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index f656b85b84212..2e2596eaa342e 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
[ ... ]
> @@ -1812,6 +1812,35 @@ static void mtk_dp_train_change_mode(struct mtk_dp *mtk_dp)
> mtk_dp_reset_swing_pre_emphasis(mtk_dp);
> }
>
> +static void _mtk_dp_report_link_train(struct mtk_dp *mtk_dp)
> +{
> + u32 rate, nlanes;
> + bool dsc = false;
> +
> + rate = drm_dp_bw_code_to_link_rate(mtk_dp->train_info.link_rate);
> + nlanes = mtk_dp->train_info.lane_count;
> +
> + drm_dp_set_cur_link_params(mtk_dp->conn, rate, nlanes, dsc);
> +}
> +
> +static void mtk_dp_report_link_train(struct mtk_dp *mtk_dp)
> +{
> + struct drm_modeset_lock *lock;
> + int ret;
> +
> + if (!mtk_dp->conn)
> + return;
[Severity: Medium]
Will this silently discard the link training parameters on the initial
hotplug?
Because the initial HPD interrupt fires before any atomic modeset has
occurred, mtk_dp->conn is only initialized later in
mtk_dp_bridge_atomic_enable(). At this point, it will be NULL, causing
mtk_dp_report_link_train() to return immediately.
Furthermore, while mtk_dp_bridge_atomic_enable() does report link training,
it restricts it to eDP. Will standard DP connections ever report their
current link parameters to the DRM core for the initial connection?
[ ... ]
> @@ -2390,12 +2433,20 @@ static void mtk_dp_bridge_atomic_enable(struct drm_bridge *bridge,
> if (mtk_dp->data->bridge_type == DRM_MODE_CONNECTOR_eDP) {
> mtk_dp_aux_panel_poweron(mtk_dp, true);
>
> + ret = drm_dp_sink_sync_caps(mtk_dp->conn, &mtk_dp->aux, NULL);
[Severity: High]
Does this code call drm_dp_sink_sync_caps() while the modeset locks are
dropped?
During a non-blocking atomic commit, the atomic_enable and atomic_disable
callbacks are executed in a worker thread asynchronously, and all modeset
locks (including connection_mutex) are intentionally dropped by the
submitter thread beforehand.
Since drm_dp_sink_sync_caps() explicitly enforces that the connector's
connection_mutex is held by asserting
drm_modeset_lock_assert_held(&connector->dev->mode_config.connection_mutex),
will this trigger a lockdep splat?
> + if (ret)
> + drm_dbg_kms(mtk_dp->aux.drm_dev,
> + "%s: error sink caps synchronisation (%d)\n",
> + mtk_dp->aux.name, ret);
> +
> /* Training */
> ret = mtk_dp_training(mtk_dp);
> if (ret) {
> drm_err(mtk_dp->drm_dev, "Training failed, %d\n", ret);
> goto power_off_aux;
> }
> +
> + _mtk_dp_report_link_train(mtk_dp);
[Severity: High]
Does calling _mtk_dp_report_link_train() here bypass the locking that is
normally handled by the mtk_dp_report_link_train() wrapper?
_mtk_dp_report_link_train() eventually calls drm_dp_set_cur_link_params(),
which also asserts that connection_mutex is held. Since the lock is dropped
in atomic_enable, is it safe to expose the connector's properties to
concurrent data races without the lock?
> }
>
> ret = mtk_dp_video_config(mtk_dp);
[ ... ]
> @@ -2435,6 +2486,7 @@ static void mtk_dp_bridge_atomic_disable(struct drm_bridge *bridge,
> if (mtk_dp->data->bridge_type == DRM_MODE_CONNECTOR_eDP) {
> mtk_dp->enabled = false;
> mtk_dp_aux_panel_poweron(mtk_dp, false);
> + drm_dp_sink_reset_caps(mtk_dp->conn);
[Severity: High]
Similar to the issue above, does this call drm_dp_sink_reset_caps() in an
unlocked context?
This function also asserts that connection_mutex is held, which would lead
to a lockdep splat during an asynchronous atomic commit.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261012-feat_link_cap-v3-0-7517f4aba0f3@bootlin.com?part=8
^ permalink raw reply [flat|nested] 15+ messages in thread