* [PATCH RFC v2 0/4] Add support for DisplayPort link training information report
@ 2026-06-19 14:08 Kory Maincent
2026-06-19 14:08 ` [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state Kory Maincent
` (5 more replies)
0 siblings, 6 replies; 11+ messages in thread
From: Kory Maincent @ 2026-06-19 14:08 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, Luca Ceresoli, dri-devel,
linux-kernel, intel-gfx, intel-xe, linux-mediatek,
linux-arm-kernel, Kory Maincent
DisplayPort link training negotiates the physical-layer parameters needed
for a reliable connection: lane count, link rate, and optionally Display
Stream Compression (DSC). Currently, each driver exposes this state in
its own way, often through driver-specific debugfs entries, with no
standard interface for userspace diagnostic and monitoring tools.
This series introduces generic, managed and unmanaged DisplayPort
connector initialization helpers, for exposing DP link capabilities and
state as standard sysfs entries, modeled after the existing HDMI helper
drmm_connector_hdmi_init().
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.
These new drmm_connector_dp_init() and drm_connector_dp_init_with_ddc()
helpers initialize a DP connector and expose link training capabilities
and state to userspace via sysfs attributes under dp_link.
Additional helpers are provided to manage link capabilities and parameters
at runtime.
Two drivers are updated as reference implementations: i915 (direct
connector path) and MediaTek (via the bridge connector framework using a
new DRM_BRIDGE_OP_DP flag).
The changes updating the i915 driver to use DRM managed resources have been
removed due to cleanup path issues. The core problem is that some functions
do not consistently propagate errors through their call paths (whether this
is intentional or not) making it difficult to properly handle cleanup of
DRM objects (planes, encoders, connectors). A potential solution would be
to implement something similar to devres_group for each DRM object type,
but this represents a substantial undertaking that falls outside the scope
of this patch series.
The MST case in i915 driver is not supported yet.
Patch 1: Introduce the core drmm_connector_dp_init() framework
Patch 2: Wire the i915 DP connector to use the new helpers
Patch 3: Introduce DRM_BRIDGE_OP_DP and wire bridge connectors
Patch 4: Wire the MediaTek DP bridge to the new helpers [untested]
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Changes in v2:
- Removed work converting i915 to DRM managed resource
- Remove voltage swing and pre-emphasis properties
- Expose link training state via sysfs dp_link/ group instead of
connector properties
- Add comprehensive sysfs attributes for both source and sink capabilities
- Add new helpers for managing sink capabilities and for current link
parameters
- Link to v1: https://lore.kernel.org/r/20260409-feat_link_cap-v1-0-7069e8199ce2@bootlin.com
---
Kory Maincent (4):
drm: Introduce DisplayPort connector helpers with link training state
drm/i915/display/dp: Adopt dp_connector helpers to expose link training state
drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag
drm/mediatek: Use dp_connector helpers to report link training state
drivers/gpu/drm/display/drm_bridge_connector.c | 24 ++++
drivers/gpu/drm/display/drm_dp_helper.c | 144 +++++++++++++++++++++
drivers/gpu/drm/drm_connector.c | 122 +++++++++++++++++
drivers/gpu/drm/drm_sysfs.c | 100 ++++++++++++++
drivers/gpu/drm/i915/display/intel_dp.c | 26 +++-
.../gpu/drm/i915/display/intel_dp_link_training.c | 17 +++
drivers/gpu/drm/mediatek/mtk_dp.c | 23 ++++
include/drm/display/drm_dp_helper.h | 7 +
include/drm/drm_bridge.h | 13 ++
include/drm/drm_connector.h | 105 +++++++++++++++
10 files changed, 577 insertions(+), 4 deletions(-)
---
base-commit: 4d75f8bd845c10f126e0e66bcdd264e1f9772bde
change-id: 20260226-feat_link_cap-20cbb6f31d40
Best regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
@ 2026-06-19 14:08 ` Kory Maincent
2026-06-19 14:23 ` sashiko-bot
2026-06-19 14:08 ` [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose " Kory Maincent
` (4 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Kory Maincent @ 2026-06-19 14:08 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, Luca Ceresoli, dri-devel,
linux-kernel, intel-gfx, intel-xe, linux-mediatek,
linux-arm-kernel, Kory Maincent
Add managed and unmanaged DisplayPort connector initialization helpers,
drmm_connector_dp_init() and drm_connector_dp_init_with_ddc(), modeled
after the existing HDMI counterpart drmm_connector_hdmi_init().
These helpers initialize DP-specific connector state and expose link
training capabilities and state to userspace via sysfs attributes under
dp_link:
- source_link_rates_caps: Array of source-supported link rates
- source_max_lane_count_caps: Source maximum lane count capability
- source_dsc_caps: Source Display Stream Compression support
- sink_max_link_rate_caps: Sink maximum link rate capability
- sink_max_lane_count_caps: Sink maximum lane count capability
- sink_dsc_caps: Sink DSC support
- cur_link_rate: Current negotiated link rate
- cur_lane_count: Current negotiated lane count
- dsc_en: DSC enabled in current link training
- max_link_rate: Maximum achievable link rate (limited by source/sink)
- max_lane_count: Maximum achievable lane count (limited by source/sink)
Link rates are passed by the driver in deca-kbps, following the DRM
convention, but exposed to userspace in kbps for clarity.
Additional helpers are provided to manage link capabilities and parameters
at runtime:
- drm_dp_sink_set_link_caps(): Set sink capabilities after DPCD read
- drm_dp_sink_reset_link_caps(): Reset sink capabilities on disconnect
- drm_dp_set_cur_link_params(): Update current link training parameters
- drm_dp_set_max_link_params(): Update maximum achievable parameters
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>
---
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
---
drivers/gpu/drm/display/drm_dp_helper.c | 144 ++++++++++++++++++++++++++++++++
drivers/gpu/drm/drm_connector.c | 122 +++++++++++++++++++++++++++
drivers/gpu/drm/drm_sysfs.c | 100 ++++++++++++++++++++++
include/drm/display/drm_dp_helper.h | 7 ++
include/drm/drm_connector.h | 105 +++++++++++++++++++++++
5 files changed, 478 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
index 9c31e14cc413b..bd0e1eb657412 100644
--- a/drivers/gpu/drm/display/drm_dp_helper.c
+++ b/drivers/gpu/drm/display/drm_dp_helper.c
@@ -4900,3 +4900,147 @@ int drm_dp_max_dprx_data_rate(int max_link_rate, int max_lanes)
1000000 * 8);
}
EXPORT_SYMBOL(drm_dp_max_dprx_data_rate);
+
+static int drm_dp_dpcd_read_link_rate_caps(struct drm_dp_aux *aux)
+{
+ u8 data;
+ int ret;
+
+ ret = drm_dp_dpcd_read_byte(aux, DP_DP13_DPCD_REV + DP_MAIN_LINK_CHANNEL_CODING, &data);
+ if (ret < 0)
+ return ret;
+
+ if (data & DP_CAP_ANSI_128B132B) {
+ ret = drm_dp_dpcd_read_byte(aux, DP_128B132B_SUPPORTED_LINK_RATES, &data);
+ if (ret < 0)
+ return ret;
+
+ if (data & DP_UHBR20)
+ return 20000000;
+ if (data & DP_UHBR13_5)
+ return 13500000;
+ if (data & DP_UHBR10)
+ return 10000000;
+ }
+
+ ret = drm_dp_dpcd_read_byte(aux, DP_MAX_LINK_RATE, &data);
+ if (ret < 0)
+ return ret;
+
+ return data * 270000;
+}
+
+/**
+ * drm_dp_sink_set_link_caps - Set DisplayPort sink link capabilities
+ * @connector: DisplayPort connector
+ * @aux: The DP AUX channel to use
+ *
+ * This function sets the DisplayPort sink (monitor) link training capabilities
+ * for the given connector. These capabilities are typically read from the
+ * sink's DPCD registers during HPD processing.
+ */
+void drm_dp_sink_set_link_caps(struct drm_connector *connector,
+ struct drm_dp_aux *aux)
+{
+ u32 lane_count, link_rate;
+ u8 data;
+ int ret;
+
+ if (!connector)
+ return;
+
+ WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
+
+ ret = drm_dp_dpcd_read_byte(aux, DP_MAX_LANE_COUNT, &data);
+ if (ret < 0)
+ return;
+
+ lane_count = data & DP_MAX_LANE_COUNT_MASK;
+
+ ret = drm_dp_dpcd_read_link_rate_caps(aux);
+ if (ret < 0)
+ return;
+
+ link_rate = ret;
+
+ ret = drm_dp_dpcd_read_byte(aux, DP_DSC_SUPPORT, &data);
+ if (ret < 0)
+ return;
+
+ connector->dp.sink_max_lane_count_caps = lane_count;
+ connector->dp.sink_max_link_rate_caps = link_rate;
+ connector->dp.sink_dsc_caps = ret & DP_DSC_DECOMPRESSION_IS_SUPPORTED;
+}
+EXPORT_SYMBOL_GPL(drm_dp_sink_set_link_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,
+ u32 link_rate, u32 lane_count, bool dsc_en)
+{
+ if (!connector)
+ return;
+
+ WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
+
+ /* Convert deca-kbps in kbps */
+ connector->dp.cur_link_rate = link_rate * 10;
+ connector->dp.cur_lane_count = lane_count;
+ connector->dp.dsc_en = dsc_en;
+}
+EXPORT_SYMBOL_GPL(drm_dp_set_cur_link_params);
+
+/**
+ * drm_dp_set_max_link_params - Set maximum DisplayPort link parameters
+ * @connector: DisplayPort connector
+ * @link_rate: Maximum link rate in kbps
+ * @lane_count: Maximum lane count
+ *
+ * This function sets the maximum achievable DisplayPort link parameters,
+ * which represent the intersection of source and sink capabilities. These
+ * values are the upper bounds for link training attempts.
+ */
+void drm_dp_set_max_link_params(struct drm_connector *connector, u32 link_rate,
+ u32 lane_count)
+{
+ if (!connector)
+ return;
+
+ WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
+
+ connector->dp.max_link_rate = link_rate;
+ connector->dp.max_lane_count = lane_count;
+}
+EXPORT_SYMBOL_GPL(drm_dp_set_max_link_params);
+
+/**
+ * drm_dp_sink_reset_link_caps - Reset DisplayPort sink capabilities
+ * @connector: DisplayPort connector
+ *
+ * This function resets all DisplayPort sink link capabilities and parameters
+ * to their default state. This should be called when a sink is disconnected
+ * to clear stale capability information.
+ */
+void drm_dp_sink_reset_link_caps(struct drm_connector *connector)
+{
+ if (!connector)
+ return;
+
+ WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
+
+ drm_dp_set_cur_link_params(connector, 0, 0, false);
+ drm_dp_set_max_link_params(connector, 0, 0);
+ connector->dp.sink_max_link_rate_caps = 0;
+ connector->dp.sink_max_lane_count_caps = 0;
+ connector->dp.sink_dsc_caps = false;
+}
+EXPORT_SYMBOL_GPL(drm_dp_sink_reset_link_caps);
diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
index a5d13b92b665c..259af3240c057 100644
--- a/drivers/gpu/drm/drm_connector.c
+++ b/drivers/gpu/drm/drm_connector.c
@@ -489,6 +489,128 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
}
EXPORT_SYMBOL(drm_connector_init_with_ddc);
+static int drm_dp_source_set_link_caps(struct drm_connector *connector,
+ const struct drm_connector_dp_link_caps *link_caps)
+{
+ u32 *_link_rates;
+
+ _link_rates = devm_kmemdup_array(connector->dev->dev,
+ link_caps->link_rates,
+ link_caps->nlink_rates,
+ sizeof(*link_caps->link_rates),
+ GFP_KERNEL);
+ if (!_link_rates)
+ return -ENOMEM;
+
+ for (int i = 0; i < link_caps->nlink_rates; i++)
+ /* Convert deca-kbps in kbps */
+ _link_rates[i] *= 10;
+
+ connector->dp.source_link_rates_caps = _link_rates;
+ connector->dp.source_num_link_rates_caps = link_caps->nlink_rates;
+ connector->dp.source_max_lane_count_caps = link_caps->nlanes;
+ connector->dp.source_dsc_caps = link_caps->dsc;
+
+ return 0;
+}
+
+/**
+ * drmm_connector_dp_init - Init a preallocated DisplayPort connector
+ * @dev: DRM device
+ * @connector: A pointer to the DisplayPort connector to init
+ * @funcs: callbacks for this connector
+ * @dp_link_caps: DisplayPort link training capabilities. The pointer
+ * is not kept by the DRM core
+ * @connector_type: user visible type of the connector
+ * @ddc: optional pointer to the associated ddc adapter
+ *
+ * Initialises a preallocated DisplayPort connector. Connectors can be
+ * subclassed as part of driver connector objects.
+ *
+ * Cleanup is automatically handled with a call to
+ * drm_connector_cleanup() in a DRM-managed action.
+ *
+ * The connector structure should be allocated with drmm_kzalloc().
+ *
+ * The @drm_connector_funcs.destroy hook must be NULL.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drmm_connector_dp_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_dp_link_caps *dp_link_caps,
+ int connector_type,
+ struct i2c_adapter *ddc)
+{
+ int ret;
+
+ if (!(connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector_type == DRM_MODE_CONNECTOR_eDP))
+ return -EINVAL;
+
+ if (!dp_link_caps)
+ return -EINVAL;
+
+ ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
+ if (ret)
+ return ret;
+
+ return drm_dp_source_set_link_caps(connector, dp_link_caps);
+}
+EXPORT_SYMBOL(drmm_connector_dp_init);
+
+/**
+ * drm_connector_dp_init_with_ddc - Init a preallocated DisplayPort connector
+ * @dev: DRM device
+ * @connector: A pointer to the DisplayPort connector to init
+ * @funcs: callbacks for this connector
+ * @dp_link_caps: DisplayPort link training capabilities. The pointer
+ * is not kept by the DRM core
+ * @connector_type: user visible type of the connector
+ * @ddc: optional pointer to the associated ddc adapter
+ *
+ * Initialises a preallocated connector. Connectors should be
+ * subclassed as part of driver connector objects.
+ *
+ * At driver unload time the driver's &drm_connector_funcs.destroy hook
+ * should call drm_connector_cleanup() and free the connector structure.
+ * The connector structure should not be allocated with devm_kzalloc().
+ *
+ * Ensures that the ddc field of the connector is correctly set.
+ *
+ * Note: consider using drmm_connector_dp_init() instead of
+ * drm_connector_dp_init_with_ddc() to let the DRM managed resource
+ * infrastructure take care of cleanup and deallocation.
+ *
+ * Returns:
+ * Zero on success, error code on failure.
+ */
+int drm_connector_dp_init_with_ddc(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_dp_link_caps *dp_link_caps,
+ int connector_type,
+ struct i2c_adapter *ddc)
+{
+ int ret;
+
+ if (!(connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
+ connector_type == DRM_MODE_CONNECTOR_eDP))
+ return -EINVAL;
+
+ if (!dp_link_caps)
+ return -EINVAL;
+
+ ret = drm_connector_init_with_ddc(dev, connector, funcs, connector_type, ddc);
+ if (ret)
+ return ret;
+
+ return drm_dp_source_set_link_caps(connector, dp_link_caps);
+}
+EXPORT_SYMBOL(drm_connector_dp_init_with_ddc);
+
static void drm_connector_cleanup_action(struct drm_device *dev,
void *ptr)
{
diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
index ef4e923a87284..653fecf23d717 100644
--- a/drivers/gpu/drm/drm_sysfs.c
+++ b/drivers/gpu/drm/drm_sysfs.c
@@ -340,6 +340,95 @@ static const struct attribute_group *connector_dev_groups[] = {
NULL
};
+static ssize_t drm_link_rates_show(u32 num_link_rates, const u32 *link_rates, char *buf)
+{
+ int size = 0;
+
+ if (!num_link_rates)
+ return 0;
+
+ size += sysfs_emit_at(buf, size, "%d", link_rates[0]);
+ for (int i = 1; i < num_link_rates; i++)
+ size += sysfs_emit_at(buf, size, " %d", link_rates[i]);
+
+ size += sysfs_emit_at(buf, size, "\n");
+ return size;
+}
+
+static ssize_t source_link_rates_caps_show(struct device *device,
+ struct device_attribute *attr,
+ char *buf)
+{
+ struct drm_connector *connector = to_drm_connector(device);
+ ssize_t size;
+
+ drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL);
+ size = drm_link_rates_show(connector->dp.source_num_link_rates_caps,
+ connector->dp.source_link_rates_caps, buf);
+ drm_modeset_unlock(&connector->dev->mode_config.connection_mutex);
+ return size;
+}
+
+#define DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(_name) \
+static ssize_t _name##_show(struct device *device, \
+ struct device_attribute *attr, \
+ char *buf) \
+{ \
+ struct drm_connector *connector = to_drm_connector(device); \
+ int ret; \
+ drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL); \
+ if (!connector->dp._name) { \
+ drm_modeset_unlock(&connector->dev->mode_config.connection_mutex); \
+ return 0; \
+ } \
+ ret = sysfs_emit(buf, "%d\n", connector->dp._name); \
+ drm_modeset_unlock(&connector->dev->mode_config.connection_mutex); \
+ return ret; \
+}
+
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(source_max_lane_count_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(source_dsc_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(sink_dsc_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(sink_max_link_rate_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(sink_max_lane_count_caps);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(cur_link_rate);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(cur_lane_count);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(dsc_en);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(max_link_rate);
+DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(max_lane_count);
+
+static DEVICE_ATTR_RO(source_link_rates_caps);
+static DEVICE_ATTR_RO(source_max_lane_count_caps);
+static DEVICE_ATTR_RO(source_dsc_caps);
+static DEVICE_ATTR_RO(sink_max_link_rate_caps);
+static DEVICE_ATTR_RO(sink_max_lane_count_caps);
+static DEVICE_ATTR_RO(sink_dsc_caps);
+static DEVICE_ATTR_RO(cur_link_rate);
+static DEVICE_ATTR_RO(cur_lane_count);
+static DEVICE_ATTR_RO(dsc_en);
+static DEVICE_ATTR_RO(max_link_rate);
+static DEVICE_ATTR_RO(max_lane_count);
+
+static struct attribute *connector_dp_link_attrs[] = {
+ &dev_attr_source_link_rates_caps.attr,
+ &dev_attr_source_max_lane_count_caps.attr,
+ &dev_attr_source_dsc_caps.attr,
+ &dev_attr_sink_max_link_rate_caps.attr,
+ &dev_attr_sink_max_lane_count_caps.attr,
+ &dev_attr_sink_dsc_caps.attr,
+ &dev_attr_cur_link_rate.attr,
+ &dev_attr_cur_lane_count.attr,
+ &dev_attr_dsc_en.attr,
+ &dev_attr_max_link_rate.attr,
+ &dev_attr_max_lane_count.attr,
+ NULL
+};
+
+static const struct attribute_group connector_dp_link_group = {
+ .name = "dp_link",
+ .attrs = connector_dp_link_attrs,
+};
+
int drm_sysfs_connector_add(struct drm_connector *connector)
{
struct drm_device *dev = connector->dev;
@@ -376,6 +465,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 +482,8 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
return 0;
+err_dp_sysfs:
+ 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 8c2d77a032f06..e7620ecb2380a 100644
--- a/include/drm/display/drm_dp_helper.h
+++ b/include/drm/display/drm_dp_helper.h
@@ -1031,4 +1031,11 @@ 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);
+void drm_dp_sink_set_link_caps(struct drm_connector *connector, struct drm_dp_aux *aux);
+void drm_dp_sink_reset_link_caps(struct drm_connector *connector);
+void drm_dp_set_cur_link_params(struct drm_connector *connector, u32 link_rate,
+ u32 lane_count, bool dsc_en);
+void drm_dp_set_max_link_params(struct drm_connector *connector, u32 link_rate,
+ u32 lane_count);
+
#endif /* _DRM_DP_HELPER_H_ */
diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
index 529755c2e8620..a08b8e9766abe 100644
--- a/include/drm/drm_connector.h
+++ b/include/drm/drm_connector.h
@@ -2003,6 +2003,93 @@ 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
+ */
+ u8 nlanes;
+
+ /**
+ * @nlink_rates: Number of link rates supported
+ */
+ u32 nlink_rates;
+
+ /**
+ * @link_rates: Array listing the supported link rates in deca-kbps
+ */
+ const u32 *link_rates;
+
+ /**
+ * @dsc: Display Stream Compression supported
+ */
+ bool dsc;
+};
+
+/**
+ * struct drm_connector_dp - DRM Connector DisplayPort-related structure
+ */
+struct drm_connector_dp {
+ /**
+ * @source_link_rates_caps: Array of supported link rates by the
+ * source in kbps
+ */
+ const u32 *source_link_rates_caps;
+ /**
+ * @source_num_link_rates_caps: Number of link rates in
+ * @source_link_rates_caps array
+ */
+ u32 source_num_link_rates_caps;
+ /**
+ * @source_max_lane_count_caps: Maximum number of lanes supported by
+ * the source
+ */
+ u32 source_max_lane_count_caps;
+ /**
+ * @source_dsc_caps: Display Stream Compression capability of the
+ * source
+ */
+ bool source_dsc_caps;
+ /**
+ * @sink_max_link_rate_caps: Maximum link rate supported by the sink
+ * in kbps
+ */
+ u32 sink_max_link_rate_caps;
+ /**
+ * @sink_max_lane_count_caps: Maximum number of lanes supported by the
+ * sink
+ */
+ u32 sink_max_lane_count_caps;
+ /**
+ * @sink_dsc_caps: Display Stream Compression capability of the sink
+ */
+ bool sink_dsc_caps;
+ /**
+ * @cur_link_rate: Current negotiated link rate in kbps
+ */
+ u32 cur_link_rate;
+ /**
+ * @cur_lane_count: Current negotiated number of lanes
+ */
+ u32 cur_lane_count;
+ /**
+ * @dsc_en: Display Stream Compression enabled status
+ */
+ bool dsc_en;
+ /**
+ * @max_link_rate: Maximum achievable link rate considering both
+ * source and sink capabilities in deca-kbps
+ */
+ u32 max_link_rate;
+ /**
+ * @max_lane_count: Maximum achievable lane count considering both
+ * source and sink capabilities
+ */
+ u32 max_lane_count;
+};
+
/**
* struct drm_connector - central DRM connector control structure
*
@@ -2426,6 +2513,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)
@@ -2458,6 +2550,19 @@ int drmm_connector_hdmi_init(struct drm_device *dev,
struct i2c_adapter *ddc,
unsigned long supported_formats,
unsigned int max_bpc);
+int drm_connector_dp_init_with_ddc(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_dp_link_caps *dp_link_caps,
+ int connector_type,
+ struct i2c_adapter *ddc);
+
+int drmm_connector_dp_init(struct drm_device *dev,
+ struct drm_connector *connector,
+ const struct drm_connector_funcs *funcs,
+ const struct drm_connector_dp_link_caps *dp_link_caps,
+ int connector_type,
+ struct i2c_adapter *ddc);
void drm_connector_attach_edid_property(struct drm_connector *connector);
int drm_connector_register(struct drm_connector *connector);
int drm_connector_dynamic_register(struct drm_connector *connector);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose link training state
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
2026-06-19 14:08 ` [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state Kory Maincent
@ 2026-06-19 14:08 ` Kory Maincent
2026-06-19 14:37 ` sashiko-bot
2026-06-19 14:08 ` [PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag Kory Maincent
` (3 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Kory Maincent @ 2026-06-19 14:08 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, Luca Ceresoli, dri-devel,
linux-kernel, intel-gfx, intel-xe, linux-mediatek,
linux-arm-kernel, Kory Maincent
Switch the i915 DP connector initialization from
drm_connector_init_with_ddc() to drm_connector_dp_init_with_ddc(),
providing the source link capabilities (supported lane counts, link rates
and DSC support).
Add intel_dp_report_link_train() to collect the negotiated link
parameters (rate, lane count and DSC enable) and report them via
drm_dp_set_max_link_params() and drm_dp_set_cur_link_params() once
link training completes successfully.
Reset the link properties via drm_dp_reset_link_params()
when the connector is reported as disconnected or when the display device
is disabled, so the exposed state always reflects the current link status.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
Changes in v2:
- Remove voltage swing and pre emphasis properties.
---
drivers/gpu/drm/i915/display/intel_dp.c | 26 ++++++++++++++++++----
.../gpu/drm/i915/display/intel_dp_link_training.c | 17 ++++++++++++++
2 files changed, 39 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
index f01a6eed38395..46c06c76952e0 100644
--- a/drivers/gpu/drm/i915/display/intel_dp.c
+++ b/drivers/gpu/drm/i915/display/intel_dp.c
@@ -6414,8 +6414,10 @@ intel_dp_detect(struct drm_connector *_connector,
drm_WARN_ON(display->drm,
!drm_modeset_is_locked(&display->drm->mode_config.connection_mutex));
- if (!intel_display_device_enabled(display))
+ if (!intel_display_device_enabled(display)) {
+ drm_dp_sink_reset_link_caps(_connector);
return connector_status_disconnected;
+ }
if (!intel_display_driver_check_access(display))
return connector->base.status;
@@ -6465,6 +6467,8 @@ intel_dp_detect(struct drm_connector *_connector,
intel_dp_tunnel_disconnect(intel_dp);
+ drm_dp_sink_reset_link_caps(_connector);
+
goto out_unset_edid;
}
@@ -7240,10 +7244,12 @@ 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;
+ u32 *rates;
int type;
if (drm_WARN(dev, dig_port->max_lanes < 1,
@@ -7291,8 +7297,21 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
type == DRM_MODE_CONNECTOR_eDP ? "eDP" : "DP",
encoder->base.base.id, encoder->base.name);
- drm_connector_init_with_ddc(dev, &connector->base, &intel_dp_connector_funcs,
- type, &intel_dp->aux.ddc);
+ intel_dp_set_source_rates(intel_dp);
+ link_caps.nlanes = 4;
+ link_caps.nlink_rates = intel_dp->num_source_rates;
+ rates = kmemdup_array(intel_dp->source_rates, intel_dp->num_source_rates,
+ sizeof(*rates), GFP_KERNEL);
+ if (!rates)
+ goto fail;
+
+ link_caps.link_rates = rates;
+ link_caps.dsc = HAS_DSC(display);
+
+ drm_connector_dp_init_with_ddc(dev, &connector->base, &intel_dp_connector_funcs,
+ &link_caps, type, &intel_dp->aux.ddc);
+ kfree(rates);
+
drm_connector_helper_add(&connector->base, &intel_dp_connector_helper_funcs);
if (!HAS_GMCH(display) && DISPLAY_VER(display) < 12)
@@ -7315,7 +7334,6 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
goto fail;
}
- intel_dp_set_source_rates(intel_dp);
intel_dp_set_common_rates(intel_dp);
intel_dp_reset_link_params(intel_dp);
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 a26094223f780..25e0e957fe36d 100644
--- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
+++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
@@ -1231,6 +1231,18 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
return sink_status & DP_INTRA_HOP_AUX_REPLY_INDICATION ? 1 : 0;
}
+static void intel_dp_report_link_train(struct intel_dp *intel_dp)
+{
+ struct intel_connector *connector = intel_dp->attached_connector;
+
+ drm_dp_set_max_link_params(&connector->base, intel_dp->link_rate,
+ intel_dp->lane_count);
+
+ 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
@@ -1259,6 +1271,9 @@ 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);
+ if (!intel_dp->is_mst)
+ 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,
@@ -1772,6 +1787,8 @@ void intel_dp_start_link_train(struct intel_atomic_state *state,
*/
int lttpr_count;
+ drm_dp_sink_set_link_caps(&intel_dp->attached_connector->base, &intel_dp->aux);
+
intel_hpd_block(encoder);
lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
2026-06-19 14:08 ` [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state Kory Maincent
2026-06-19 14:08 ` [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose " Kory Maincent
@ 2026-06-19 14:08 ` Kory Maincent
2026-06-19 14:18 ` sashiko-bot
2026-06-19 14:08 ` [PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state Kory Maincent
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Kory Maincent @ 2026-06-19 14:08 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, Luca Ceresoli, 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.
In drm_bridge_connector_init(), when a bridge sets DRM_BRIDGE_OP_DP,
use drmm_connector_dp_init() instead of the generic drmm_connector_init()
so the connector exposes link training state properties to userspace.
This mirrors the existing pattern used for HDMI bridges.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
drivers/gpu/drm/display/drm_bridge_connector.c | 24 ++++++++++++++++++++++++
include/drm/drm_bridge.h | 13 +++++++++++++
2 files changed, 37 insertions(+)
diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index cafa498c38482..6ea4d45e3146b 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -108,6 +108,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:
*
@@ -773,6 +780,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);
}
/**
@@ -908,6 +916,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);
@@ -996,6 +1013,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
max_bpc);
if (ret)
return ERR_PTR(ret);
+ } else if (bridge_connector->bridge_dp) {
+ ret = drmm_connector_dp_init(drm, connector,
+ &drm_bridge_connector_funcs,
+ bridge_connector->bridge_dp->dp_link_caps,
+ connector_type, ddc);
+ 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 4ba3a5deef9a6..02411e0b71c35 100644
--- a/include/drm/drm_bridge.h
+++ b/include/drm/drm_bridge.h
@@ -1092,6 +1092,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_train_caps link training
+ * capabilities.
+ */
+ DRM_BRIDGE_OP_DP = BIT(11),
};
/**
@@ -1267,6 +1275,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] 11+ messages in thread
* [PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
` (2 preceding siblings ...)
2026-06-19 14:08 ` [PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag Kory Maincent
@ 2026-06-19 14:08 ` Kory Maincent
2026-06-19 14:20 ` sashiko-bot
2026-06-19 14:18 ` ✗ Fi.CI.BUILD: failure for Add support for DisplayPort link training information report (rev2) Patchwork
2026-06-19 17:49 ` [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
5 siblings, 1 reply; 11+ messages in thread
From: Kory Maincent @ 2026-06-19 14:08 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, Luca Ceresoli, dri-devel,
linux-kernel, intel-gfx, intel-xe, linux-mediatek,
linux-arm-kernel, Kory Maincent
Set DRM_BRIDGE_OP_DP and populate dp_link_train_caps with the supported
link rates, lane counts, voltage swing and pre-emphasis levels so the
bridge connector uses drmm_connector_dp_init() and exposes the link
training state properties to userspace.
Store per-lane voltage swing and pre-emphasis values in
mtk_dp_train_info and report the negotiated link parameters via
drm_connector_dp_set_link_train_properties() on training completion.
Clear them via drm_connector_dp_reset_link_train_properties() when a
disconnect is detected in the HPD event thread.
Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---
This patch has still not been tested. I am in the process of finding the
hardware for that.
Changes in v2:
- Remove voltage swing and pre emphasis properties.
---
drivers/gpu/drm/mediatek/mtk_dp.c | 23 +++++++++++++++++++++++
1 file changed, 23 insertions(+)
diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
index eefbc7e0f9c8d..c1e7086104116 100644
--- a/drivers/gpu/drm/mediatek/mtk_dp.c
+++ b/drivers/gpu/drm/mediatek/mtk_dp.c
@@ -1789,6 +1789,8 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
}
}
+ drm_dp_sink_set_link_caps(mtk_dp->conn, &mtk_dp->aux);
+
return 0;
}
@@ -1812,6 +1814,17 @@ 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;
+
+ 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, false);
+ drm_dp_set_max_link_params(mtk_dp->conn, rate, nlanes);
+}
+
static int mtk_dp_training(struct mtk_dp *mtk_dp)
{
int ret;
@@ -1892,6 +1905,7 @@ static int mtk_dp_training(struct mtk_dp *mtk_dp)
mtk_dp_training_set_scramble(mtk_dp, true);
mtk_dp_set_enhanced_frame_mode(mtk_dp);
+ mtk_dp_report_link_train(mtk_dp);
return 0;
}
@@ -2004,6 +2018,7 @@ static irqreturn_t mtk_dp_hpd_event_thread(int hpd, void *dev)
mtk_dp->need_debounce = false;
mod_timer(&mtk_dp->debounce_timer,
jiffies + msecs_to_jiffies(100) - 1);
+ drm_dp_sink_reset_link_caps(mtk_dp->conn);
} else {
mtk_dp_aux_panel_poweron(mtk_dp, true);
@@ -2742,6 +2757,12 @@ static int mtk_dp_edp_link_panel(struct drm_dp_aux *mtk_aux)
static int mtk_dp_probe(struct platform_device *pdev)
{
+ static const u32 dp_rates[] = {162000, 270000, 540000, 810000};
+ static const struct drm_connector_dp_link_caps dp_link_caps = {
+ .nlanes = 4,
+ .nlink_rates = ARRAY_SIZE(dp_rates),
+ .link_rates = dp_rates,
+ };
struct mtk_dp *mtk_dp;
struct device *dev = &pdev->dev;
int ret;
@@ -2809,6 +2830,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 = &dp_link_caps;
+ mtk_dp->bridge.ops = DRM_BRIDGE_OP_DP;
if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
/*
--
2.43.0
^ permalink raw reply related [flat|nested] 11+ messages in thread
* ✗ Fi.CI.BUILD: failure for Add support for DisplayPort link training information report (rev2)
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
` (3 preceding siblings ...)
2026-06-19 14:08 ` [PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state Kory Maincent
@ 2026-06-19 14:18 ` Patchwork
2026-06-19 17:49 ` [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
5 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2026-06-19 14:18 UTC (permalink / raw)
To: Kory Maincent; +Cc: intel-gfx
== Series Details ==
Series: Add support for DisplayPort link training information report (rev2)
URL : https://patchwork.freedesktop.org/series/164797/
State : failure
== Summary ==
Error: patch https://patchwork.freedesktop.org/api/1.0/series/164797/revisions/2/mbox/ not applied
Applying: drm: Introduce DisplayPort connector helpers with link training state
Applying: drm/i915/display/dp: Adopt dp_connector helpers to expose link training state
Using index info to reconstruct a base tree...
M drivers/gpu/drm/i915/display/intel_dp.c
M drivers/gpu/drm/i915/display/intel_dp_link_training.c
Falling back to patching base and 3-way merge...
Auto-merging drivers/gpu/drm/i915/display/intel_dp_link_training.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_dp_link_training.c
Auto-merging drivers/gpu/drm/i915/display/intel_dp.c
CONFLICT (content): Merge conflict in drivers/gpu/drm/i915/display/intel_dp.c
error: Failed to merge in the changes.
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0002 drm/i915/display/dp: Adopt dp_connector helpers to expose link training state
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
Build failed, no error log produced
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag
2026-06-19 14:08 ` [PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag Kory Maincent
@ 2026-06-19 14:18 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-19 14:18 UTC (permalink / raw)
To: Kory Maincent; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional call to drmm_connector_dp_init() breaks bridge chains with downstream format converters.
- [Low] Kernel-doc for DRM_BRIDGE_OP_DP incorrectly references a non-existent structure member.
--
commit 95b54bb01e2f24e6a00ddf825b25aaa6985fa864
Author: Kory Maincent <kory.maincent@bootlin.com>
drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag
Introduce DRM_BRIDGE_OP_DP to indicate bridges that provide DP connector
operations. Update drm_bridge_connector_init() to use
drmm_connector_dp_init() when this flag is present to expose link training
state properties.
> diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
> index 39cc18f78eda1..5238b1ec738ce 100644
> --- a/drivers/gpu/drm/display/drm_bridge_connector.c
> +++ b/drivers/gpu/drm/display/drm_bridge_connector.c
[ ... ]
> @@ -986,6 +1003,13 @@ struct drm_connector *drm_bridge_connector_init(struct drm_device *drm,
> max_bpc);
> if (ret)
> return ERR_PTR(ret);
> + } else if (bridge_connector->bridge_dp) {
> + ret = drmm_connector_dp_init(drm, connector,
> + &drm_bridge_connector_funcs,
> + bridge_connector->bridge_dp->dp_link_caps,
> + connector_type, ddc);
> + if (ret)
> + return ERR_PTR(ret);
[Severity: High]
If a DRM bridge chain consists of a DisplayPort transmitter followed by a
format-converting bridge (e.g., DP-to-VGA or DP-to-LVDS), the final
connector_type of the chain will be the output format (like
DRM_MODE_CONNECTOR_VGA).
Since drmm_connector_dp_init() explicitly enforces that connector_type must
be DRM_MODE_CONNECTOR_DisplayPort or DRM_MODE_CONNECTOR_eDP, will this call
immediately return -EINVAL and break display initialization for legitimate
format-converted hardware combinations?
Does the branch condition need to verify the final connector_type is actually
DP/eDP before forcing the DP initialization path?
> } else {
> ret = drmm_connector_init(drm, connector,
[ ... ]
> diff --git a/include/drm/drm_bridge.h b/include/drm/drm_bridge.h
> index a8d67bd9ee505..63db754d21146 100644
> --- a/include/drm/drm_bridge.h
> +++ b/include/drm/drm_bridge.h
> @@ -1092,6 +1092,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_train_caps link training
[Severity: Low]
This isn't a bug, but there's a typo in the documentation. It references
&drm_bridge->dp_link_train_caps, but the actual structure member added
in this patch is named dp_link_caps.
> + * capabilities.
> + */
> + DRM_BRIDGE_OP_DP = BIT(11),
> };
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-feat_link_cap-v2-0-a3dec4c02ad9@bootlin.com?part=3
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state
2026-06-19 14:08 ` [PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state Kory Maincent
@ 2026-06-19 14:20 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-19 14:20 UTC (permalink / raw)
To: Kory Maincent; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:
- [High] The DRM_BRIDGE_OP_DP flag is unintentionally overwritten and lost for external DP configurations, breaking the patch's main goal.
- [High] Calling DRM DP connector state update helpers from the HPD event thread and atomic_enable without holding the connection mutex triggers kernel WARN_ON traces.
- [High] DP link capabilities and parameters are silently dropped because mtk_dp->conn is NULL during the first hotplug event and during probe for eDP.
- [Medium] The patch fails to implement the voltage swing and pre-emphasis link training properties claimed in the commit message.
--
commit 329f586786239579aef1351b83fdceecb28dd1e1
Author: Kory Maincent <kory.maincent@bootlin.com>
drm/mediatek: Use dp_connector helpers to report link training state
Set DRM_BRIDGE_OP_DP and populate dp_link_train_caps with the supported
link rates, lane counts, voltage swing and pre-emphasis levels so the
bridge connector uses drmm_connector_dp_init() and exposes the link
training state properties to userspace.
Store per-lane voltage swing and pre-emphasis values in
mtk_dp_train_info and report the negotiated link parameters via
drm_connector_dp_set_link_train_properties() on training completion.
Clear them via drm_connector_dp_reset_link_train_properties() when a
disconnect is detected in the HPD event thread.
> Set DRM_BRIDGE_OP_DP and populate dp_link_train_caps with the supported
> link rates, lane counts, voltage swing and pre-emphasis levels so the
> bridge connector uses drmm_connector_dp_init() and exposes the link
> training state properties to userspace.
>
> Store per-lane voltage swing and pre-emphasis values in
> mtk_dp_train_info and report the negotiated link parameters via
> drm_connector_dp_set_link_train_properties() on training completion.
[Severity: Medium]
The commit message mentions populating voltage swing and pre-emphasis levels
and using drm_connector_dp_set_link_train_properties(). Is this description
accurate?
The actual patch only sets up lanes and link rates in dp_link_caps, and there
are no calls to drm_connector_dp_set_link_train_properties() in the diff.
> diff --git a/drivers/gpu/drm/mediatek/mtk_dp.c b/drivers/gpu/drm/mediatek/mtk_dp.c
> index c52cc7c2e2006..439b9496632fe 100644
> --- a/drivers/gpu/drm/mediatek/mtk_dp.c
> +++ b/drivers/gpu/drm/mediatek/mtk_dp.c
> @@ -1789,6 +1789,8 @@ static int mtk_dp_parse_capabilities(struct mtk_dp *mtk_dp)
> }
> }
>
> + drm_dp_sink_set_link_caps(mtk_dp->conn, &mtk_dp->aux);
> +
> return 0;
> }
[Severity: High]
Will mtk_dp->conn be NULL when this is called during eDP probe or the first
hotplug event?
The connector appears to only be initialized later during
mtk_dp_bridge_atomic_enable(). If mtk_dp->conn is NULL,
drm_dp_sink_set_link_caps() will silently return early and discard the
parsed capabilities.
[Severity: High]
Also, drm_dp_sink_set_link_caps() expects the caller to hold the connection
mutex, enforcing it with:
WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
Are we guaranteed to hold the modeset lock when reaching this from the
HPD event thread or eDP probe path?
[ ... ]
> @@ -1812,6 +1814,17 @@ 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;
> +
> + 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, false);
> + drm_dp_set_max_link_params(mtk_dp->conn, rate, nlanes);
> +}
[Severity: High]
Similar to the capability parsing above, can mtk_dp->conn be NULL here if
called from the HPD thread before the first modeset?
Additionally, these helper functions also require the connection mutex. Since
mtk_dp_report_link_train() is called from mtk_dp_training() which can be
reached from the lockless non-blocking atomic_enable path, could this
trigger a WARN_ON?
[ ... ]
> @@ -2004,6 +2018,7 @@ static irqreturn_t mtk_dp_hpd_event_thread(int hpd, void *dev)
> mtk_dp->need_debounce = false;
> mod_timer(&mtk_dp->debounce_timer,
> jiffies + msecs_to_jiffies(100) - 1);
> + drm_dp_sink_reset_link_caps(mtk_dp->conn);
> } else {
> mtk_dp_aux_panel_poweron(mtk_dp, true);
[Severity: High]
Calling drm_dp_sink_reset_link_caps() directly from the asynchronous HPD IRQ
thread will trigger a kernel warning because the connection mutex is not
held. Is there a way to defer this update to a workqueue or safely grab the
required modeset locks?
[ ... ]
> @@ -2809,6 +2830,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 = &dp_link_caps;
> + mtk_dp->bridge.ops = DRM_BRIDGE_OP_DP;
>
> if (mtk_dp->bridge.type == DRM_MODE_CONNECTOR_eDP) {
> /*
[Severity: High]
Does this assignment get lost for external DP configurations?
Further down in mtk_dp_probe(), the non-eDP branch assigns the ops directly
using an equals sign instead of a bitwise OR:
} else {
mtk_dp->bridge.ops = DRM_BRIDGE_OP_DETECT |
DRM_BRIDGE_OP_EDID | DRM_BRIDGE_OP_HPD;
...
}
This appears to completely overwrite the DRM_BRIDGE_OP_DP flag added above,
which would prevent the bridge connector from exposing link training
properties to userspace for external DP.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-feat_link_cap-v2-0-a3dec4c02ad9@bootlin.com?part=4
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state
2026-06-19 14:08 ` [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state Kory Maincent
@ 2026-06-19 14:23 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-19 14:23 UTC (permalink / raw)
To: Kory Maincent; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 9 potential issue(s) to consider:
- [High] Unconditional read of DP 1.3 Extended Receiver Capability space breaks capability detection on legacy DP sinks.
- [High] Incorrect use of `devm_kmemdup_array` binds memory to the physical device, causing memory leaks for dynamic connectors.
- [High] Incorrect assignment of `sink_dsc_caps` uses the return value of the DPCD read instead of the actual data read.
- [High] Missing `drm_connector_cleanup` on error paths leaves partially initialized connectors on global DRM lists.
- [High] Dangling pointer `connector->kdev` in `drm_sysfs_connector_add` leads to UAF if sysfs group creation fails.
- [High] Raw AUX reads of base DPCD registers bypass DP 1.3+ Extended Receiver Capability overrides.
- [Medium] Sysfs scalar attributes return an empty file (EOF) instead of '0' for valid zero values.
- [Medium] Unconditional sysfs group creation litters existing DP drivers with empty files.
- [Medium] Dangerous unit mismatch for max_link_rate causes 10x value scaling errors in sysfs.
--
commit 9797fc077ec92710d6cc77dd41b9fa876502ade2
Author: Kory Maincent <kory.maincent@bootlin.com>
Subject: drm: Introduce DisplayPort connector helpers with link training state
This commit adds managed and unmanaged DisplayPort connector initialization
helpers to expose link training capabilities and state to userspace via
sysfs attributes under dp_link.
> diff --git a/drivers/gpu/drm/display/drm_dp_helper.c b/drivers/gpu/drm/display/drm_dp_helper.c
> index a697cc227e289..4603efa46d912 100644
> --- a/drivers/gpu/drm/display/drm_dp_helper.c
> +++ b/drivers/gpu/drm/display/drm_dp_helper.c
> @@ -4867,3 +4867,147 @@ int drm_dp_max_dprx_data_rate(int max_link_rate, int max_lanes)
[ ... ]
> +static int drm_dp_dpcd_read_link_rate_caps(struct drm_dp_aux *aux)
> +{
> + u8 data;
> + int ret;
> +
> + ret = drm_dp_dpcd_read_byte(aux, DP_DP13_DPCD_REV + DP_MAIN_LINK_CHANNEL_CODING, &data);
> + if (ret < 0)
> + return ret;
[Severity: High]
Does this unconditional read break capability detection on older sinks?
Legacy sinks (DP 1.2 and earlier) do not implement the 0x2200 register space
and typically NACK AUX reads to unimplemented addresses. If this returns an
error, will it abort drm_dp_sink_set_link_caps() and leave capabilities
uninitialized for legacy hardware?
[ ... ]
> +void drm_dp_sink_set_link_caps(struct drm_connector *connector,
> + struct drm_dp_aux *aux)
> +{
> + u32 lane_count, link_rate;
> + u8 data;
> + int ret;
> +
> + if (!connector)
> + return;
> +
> + WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
> +
> + ret = drm_dp_dpcd_read_byte(aux, DP_MAX_LANE_COUNT, &data);
[Severity: High]
Does this raw AUX read miss DP 1.3+ Extended Receiver Capability overrides?
For DP 1.3+ sinks with the DP_EXTENDED_RECEIVER_CAP_FIELD_PRESENT bit set,
these base DPCD registers are overridden by values at 0x2200-0x220F. By
bypassing drm_dp_read_dpcd_caps() which handles patching these overrides,
will this report legacy capabilities for modern DP 1.4+ sinks?
> + if (ret < 0)
> + return;
> +
> + lane_count = data & DP_MAX_LANE_COUNT_MASK;
> +
> + ret = drm_dp_dpcd_read_link_rate_caps(aux);
> + if (ret < 0)
> + return;
> +
> + link_rate = ret;
> +
> + ret = drm_dp_dpcd_read_byte(aux, DP_DSC_SUPPORT, &data);
> + if (ret < 0)
> + return;
> +
> + connector->dp.sink_max_lane_count_caps = lane_count;
> + connector->dp.sink_max_link_rate_caps = link_rate;
> + connector->dp.sink_dsc_caps = ret & DP_DSC_DECOMPRESSION_IS_SUPPORTED;
[Severity: High]
Is the wrong variable used for this assignment?
The assignment uses the return code ret instead of the payload variable
data. Since the return code is 1 on success, and
DP_DSC_DECOMPRESSION_IS_SUPPORTED is also 1, will this always evaluate
to true for any sink that acknowledges the DPCD read, regardless of actual
DSC support?
> +}
> +EXPORT_SYMBOL_GPL(drm_dp_sink_set_link_caps);
[ ... ]
> +void drm_dp_set_max_link_params(struct drm_connector *connector, u32 link_rate,
> + u32 lane_count)
> +{
> + if (!connector)
> + return;
> +
> + WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
> +
> + connector->dp.max_link_rate = link_rate;
> + connector->dp.max_lane_count = lane_count;
[Severity: Medium]
Is there a unit conversion mismatch here?
Unlike drm_dp_set_cur_link_params() which converts deca-kbps to kbps by
multiplying by 10, this assigns link_rate directly to max_link_rate without
conversion. Since both are exposed via sysfs, will max_link_rate be presented
10x smaller than the actual rate?
> +}
> +EXPORT_SYMBOL_GPL(drm_dp_set_max_link_params);
> +
> +/**
> + * drm_dp_sink_reset_link_caps - Reset DisplayPort sink capabilities
> + * @connector: DisplayPort connector
> + *
> + * This function resets all DisplayPort sink link capabilities and parameters
> + * to their default state. This should be called when a sink is disconnected
> + * to clear stale capability information.
> + */
> +void drm_dp_sink_reset_link_caps(struct drm_connector *connector)
> +{
> + if (!connector)
> + return;
> +
> + WARN_ON(!drm_modeset_is_locked(&connector->dev->mode_config.connection_mutex));
> +
> + drm_dp_set_cur_link_params(connector, 0, 0, false);
> + drm_dp_set_max_link_params(connector, 0, 0);
> + connector->dp.sink_max_link_rate_caps = 0;
> + connector->dp.sink_max_lane_count_caps = 0;
> + connector->dp.sink_dsc_caps = false;
> +}
> +EXPORT_SYMBOL_GPL(drm_dp_sink_reset_link_caps);
> diff --git a/drivers/gpu/drm/drm_connector.c b/drivers/gpu/drm/drm_connector.c
> index 47dc53c4a738f..aac99fbb44804 100644
> --- a/drivers/gpu/drm/drm_connector.c
> +++ b/drivers/gpu/drm/drm_connector.c
> @@ -489,6 +489,128 @@ int drm_connector_init_with_ddc(struct drm_device *dev,
> }
> EXPORT_SYMBOL(drm_connector_init_with_ddc);
>
> +static int drm_dp_source_set_link_caps(struct drm_connector *connector,
> + const struct drm_connector_dp_link_caps *link_caps)
> +{
> + u32 *_link_rates;
> +
> + _link_rates = devm_kmemdup_array(connector->dev->dev,
> + link_caps->link_rates,
> + link_caps->nlink_rates,
> + sizeof(*link_caps->link_rates),
> + GFP_KERNEL);
[Severity: High]
Can this allocation leak memory for dynamically plugged connectors?
By using devm_kmemdup_array() tied to the physical device
(connector->dev->dev), the memory is bound to the GPU driver's lifecycle.
Since connectors like MST can be hot-plugged and destroyed independently,
will this leak the array on every plug/unplug cycle until the entire GPU
driver unbinds?
> + if (!_link_rates)
> + return -ENOMEM;
[ ... ]
> +int drm_connector_dp_init_with_ddc(struct drm_device *dev,
> + struct drm_connector *connector,
> + const struct drm_connector_funcs *funcs,
> + const struct drm_connector_dp_link_caps *dp_link_caps,
> + int connector_type,
> + struct i2c_adapter *ddc)
> +{
> + int ret;
> +
> + if (!(connector_type == DRM_MODE_CONNECTOR_DisplayPort ||
> + connector_type == DRM_MODE_CONNECTOR_eDP))
> + return -EINVAL;
> +
> + if (!dp_link_caps)
> + return -EINVAL;
> +
> + ret = drm_connector_init_with_ddc(dev, connector, funcs, connector_type, ddc);
> + if (ret)
> + return ret;
> +
> + return drm_dp_source_set_link_caps(connector, dp_link_caps);
[Severity: High]
Does this error path leave a partially initialized connector on the global DRM
lists?
If drm_connector_init_with_ddc() succeeds, it adds the connector to the
DRM device's lists. If drm_dp_source_set_link_caps() then fails, the error
is returned directly without calling drm_connector_cleanup().
Will the caller perceive this as a failure and free the structure, leaving
a dangling pointer in dev->mode_config.connector_list?
> +}
> +EXPORT_SYMBOL(drm_connector_dp_init_with_ddc);
> +
> static void drm_connector_cleanup_action(struct drm_device *dev,
> void *ptr)
> {
> diff --git a/drivers/gpu/drm/drm_sysfs.c b/drivers/gpu/drm/drm_sysfs.c
> index ef4e923a87284..653fecf23d717 100644
> --- a/drivers/gpu/drm/drm_sysfs.c
> +++ b/drivers/gpu/drm/drm_sysfs.c
> @@ -340,6 +340,95 @@ static const struct attribute_group *connector_dev_groups[] = {
[ ... ]
> +#define DRM_CONNECTOR_DP_ATTR_SHOW_SIMPLE(_name) \
> +static ssize_t _name##_show(struct device *device, \
> + struct device_attribute *attr, \
> + char *buf) \
> +{ \
> + struct drm_connector *connector = to_drm_connector(device); \
> + int ret; \
> + drm_modeset_lock(&connector->dev->mode_config.connection_mutex, NULL); \
> + if (!connector->dp._name) { \
> + drm_modeset_unlock(&connector->dev->mode_config.connection_mutex); \
> + return 0; \
> + } \
[Severity: Medium]
Will this return an empty file (EOF) when the property evaluates to false
or 0?
By returning 0 from the sysfs show callback here, userspace will read 0 bytes
rather than seeing "0\n". Since 0 and false are valid states for properties
like dsc_en and cur_link_rate, will this prevent userspace from correctly
parsing them?
> + ret = sysfs_emit(buf, "%d\n", connector->dp._name); \
> + drm_modeset_unlock(&connector->dev->mode_config.connection_mutex); \
> + return ret; \
> +}
[ ... ]
> @@ -376,6 +465,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);
[Severity: Medium]
Does this unconditionally expose empty sysfs files for existing DP drivers?
For drivers that have not migrated to use the new drm_connector_dp_init*
helpers, the dp capability struct remains zero-initialized. Will this create
a dp_link directory populated with useless, empty files on all legacy systems?
> + 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 +482,8 @@ int drm_sysfs_connector_add(struct drm_connector *connector)
>
> return 0;
>
> +err_dp_sysfs:
> + device_del(kdev);
[Severity: High]
Does this leave a dangling pointer in connector->kdev?
If sysfs_create_group() fails, the code jumps to err_dp_sysfs which
deletes kdev, but connector->kdev is never reset to NULL. When
drm_sysfs_connector_remove() is called later, could it dereference the
freed pointer?
> err_free:
> put_device(kdev);
> return r;
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-feat_link_cap-v2-0-a3dec4c02ad9@bootlin.com?part=1
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose link training state
2026-06-19 14:08 ` [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose " Kory Maincent
@ 2026-06-19 14:37 ` sashiko-bot
0 siblings, 0 replies; 11+ messages in thread
From: sashiko-bot @ 2026-06-19 14:37 UTC (permalink / raw)
To: Kory Maincent; +Cc: dri-devel
Thank you for your contribution! Sashiko AI review found 6 potential issue(s) to consider:
- [Critical] Kernel panic in drm_connector_cleanup() due to an uninitialized connector when kmemdup_array fails.
- [High] Calling DRM DP helpers from atomic commit tail violates locking contracts and introduces a data race.
- [Medium] The current negotiated link parameters are incorrectly reported as the maximum link parameters.
- [Medium] The source maximum lane count capability is incorrectly hardcoded to 4, misrepresenting actual port capabilities.
- [Medium] The return value of drm_connector_dp_init_with_ddc() is ignored, potentially leaving the connector with missing source capabilities on allocation failure.
- [Medium] The failure path for the rates allocation misses the cleanup for intel_dp_aux_init(), leaking resources.
--
commit 77ebe85e37823b9a8f351840bfca5631c84a3dac
Author: Kory Maincent <kory.maincent@bootlin.com>
drm/i915/display/dp: Adopt dp_connector helpers to expose link training state
This commit switches the i915 DP connector initialization to use
drm_connector_dp_init_with_ddc() to provide source link capabilities.
It also adds intel_dp_report_link_train() to collect and expose
negotiated link parameters via the DRM DP helpers.
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 5c3e816b01352..5f934257771a7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
[ ... ]
> @@ -7174,10 +7178,12 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
> struct intel_connector *connector)
> {
[ ... ]
> + intel_dp_set_source_rates(intel_dp);
> + link_caps.nlanes = 4;
[Severity: Medium]
Is it intentional to hardcode the maximum lane count to 4 here?
Some digital encoder ports may be restricted to 1 or 2 lanes depending on
the hardware platform or Type-C port configuration. The actual capability
is stored in dig_port->max_lanes. Hardcoding this to 4 might misrepresent
the actual port capabilities.
> + link_caps.nlink_rates = intel_dp->num_source_rates;
> + rates = kmemdup_array(intel_dp->source_rates, intel_dp->num_source_rates,
> + sizeof(*rates), GFP_KERNEL);
> + if (!rates)
> + goto fail;
[Severity: Critical]
Does this error path trigger a NULL pointer dereference?
At this point in the initialization, drm_connector_dp_init_with_ddc()
hasn't been called yet, so connector->base.dev is still NULL.
The jump to the fail label calls drm_connector_cleanup(&connector->base).
This function unconditionally dereferences the device pointer, which will
cause a kernel panic.
[Severity: Medium]
Does this error path also leak the resources allocated by intel_dp_aux_init()
earlier in this function?
The direct jump to the fail label bypasses the intel_dp_aux_fini(intel_dp)
call, which means the aux channel name allocation and the PM QoS request
will permanently leak.
> +
> + link_caps.link_rates = rates;
> + link_caps.dsc = HAS_DSC(display);
> +
> + drm_connector_dp_init_with_ddc(dev, &connector->base, &intel_dp_connector_funcs,
> + &link_caps, type, &intel_dp->aux.ddc);
[Severity: Medium]
Should we check the return value here?
drm_connector_dp_init_with_ddc() can fail with -ENOMEM when allocating the
link capabilities array internally. If it fails, the connector will proceed
partially initialized without its source capabilities.
> + kfree(rates);
> +
> drm_connector_helper_add(&connector->base, &intel_dp_connector_helper_funcs);
[ ... ]
> 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 54c585c59b900..aaa6eba058af5 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -1116,6 +1116,18 @@ intel_dp_128b132b_intra_hop(struct intel_dp *intel_dp,
> return sink_status & DP_INTRA_HOP_AUX_REPLY_INDICATION ? 1 : 0;
> }
>
> +static void intel_dp_report_link_train(struct intel_dp *intel_dp)
> +{
> + struct intel_connector *connector = intel_dp->attached_connector;
> +
> + drm_dp_set_max_link_params(&connector->base, intel_dp->link_rate,
> + intel_dp->lane_count);
[Severity: Medium]
Should this report the architectural maximums instead of the currently
negotiated parameters?
intel_dp->link_rate and intel_dp->lane_count reflect the current link
configuration, which might have been downgraded due to training failures
or signal degradation. The hardware maximum capabilities are stored in
intel_dp->link.max_rate and intel_dp->link.max_lane_count.
[Severity: High]
Can this helper be safely called from this context?
drm_dp_set_max_link_params() and drm_dp_set_cur_link_params() both require
the connection_mutex to be held. However, intel_dp_stop_link_train()
(which calls this) executes during the hardware enable phase (commit_tail).
For non-blocking commits, this runs asynchronously in a workqueue after
the modeset locks have already been released. This will trigger a lockdep
warning and could race against concurrent userspace sysfs readers.
> +
> + drm_dp_set_cur_link_params(&connector->base, intel_dp->link_rate,
> + intel_dp->lane_count,
> + connector->dp.dsc_decompression_enabled);
> +}
[ ... ]
> @@ -1652,6 +1667,8 @@ void intel_dp_start_link_train(struct intel_atomic_state *state,
> */
> int lttpr_count;
>
> + drm_dp_sink_set_link_caps(&intel_dp->attached_connector->base, &intel_dp->aux);
[Severity: High]
Similar to the helpers above, drm_dp_sink_set_link_caps() explicitly warns
if the connection_mutex is not locked.
Since intel_dp_start_link_train() is executed from the asynchronous
commit_tail for non-blocking commits, the modeset locks are already
released. Can this cause a lockdep warning and data race during asynchronous
commits?
> +
> intel_hpd_block(encoder);
>
> lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);
--
Sashiko AI review · https://sashiko.dev/#/patchset/20260619-feat_link_cap-v2-0-a3dec4c02ad9@bootlin.com?part=2
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH RFC v2 0/4] Add support for DisplayPort link training information report
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
` (4 preceding siblings ...)
2026-06-19 14:18 ` ✗ Fi.CI.BUILD: failure for Add support for DisplayPort link training information report (rev2) Patchwork
@ 2026-06-19 17:49 ` Kory Maincent
5 siblings, 0 replies; 11+ messages in thread
From: Kory Maincent @ 2026-06-19 17:49 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
Hello
On Fri, 19 Jun 2026 16:08:42 +0200
Kory Maincent <kory.maincent@bootlin.com> wrote:
> DisplayPort link training negotiates the physical-layer parameters needed
> for a reliable connection: lane count, link rate, and optionally Display
> Stream Compression (DSC). Currently, each driver exposes this state in
> its own way, often through driver-specific debugfs entries, with no
> standard interface for userspace diagnostic and monitoring tools.
>
> This series introduces generic, managed and unmanaged DisplayPort
> connector initialization helpers, for exposing DP link capabilities and
> state as standard sysfs entries, modeled after the existing HDMI helper
> drmm_connector_hdmi_init().
>
> 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.
>
> These new drmm_connector_dp_init() and drm_connector_dp_init_with_ddc()
> helpers initialize a DP connector and expose link training capabilities
> and state to userspace via sysfs attributes under dp_link.
>
> Additional helpers are provided to manage link capabilities and parameters
> at runtime.
>
> Two drivers are updated as reference implementations: i915 (direct
> connector path) and MediaTek (via the bridge connector framework using a
> new DRM_BRIDGE_OP_DP flag).
>
> The changes updating the i915 driver to use DRM managed resources have been
> removed due to cleanup path issues. The core problem is that some functions
> do not consistently propagate errors through their call paths (whether this
> is intentional or not) making it difficult to properly handle cleanup of
> DRM objects (planes, encoders, connectors). A potential solution would be
> to implement something similar to devres_group for each DRM object type,
> but this represents a substantial undertaking that falls outside the scope
> of this patch series.
>
> The MST case in i915 driver is not supported yet.
I have seen and fixed the reviews from Sashiko.
I prefer to wait for human reviews about the core design before sending a v3, so
don't hesitate to look at the series. Mainly the first patch which tackle core
DRM changes.
Regards,
--
Köry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2026-06-19 17:49 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-06-19 14:08 [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
2026-06-19 14:08 ` [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state Kory Maincent
2026-06-19 14:23 ` sashiko-bot
2026-06-19 14:08 ` [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose " Kory Maincent
2026-06-19 14:37 ` sashiko-bot
2026-06-19 14:08 ` [PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag Kory Maincent
2026-06-19 14:18 ` sashiko-bot
2026-06-19 14:08 ` [PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state Kory Maincent
2026-06-19 14:20 ` sashiko-bot
2026-06-19 14:18 ` ✗ Fi.CI.BUILD: failure for Add support for DisplayPort link training information report (rev2) Patchwork
2026-06-19 17:49 ` [PATCH RFC v2 0/4] Add support for DisplayPort link training information report Kory Maincent
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.