[PATCH RFC v2 0/4] Add support for DisplayPort link training information report

[PATCH RFC v2 0/4] Add support for DisplayPort link training information report

[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

[PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state

[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

[PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose link training state

[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

[PATCH RFC v2 3/4] drm/bridge: Wire drmm_connector_dp_init() via new DRM_BRIDGE_OP_DP flag

[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

[PATCH RFC v2 4/4] drm/mediatek: Use dp_connector helpers to report link training state

[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

Re: [PATCH RFC v2 0/4] Add support for DisplayPort link training information report

[Kory Maincent]

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

Re: [PATCH RFC v2 1/4] drm: Introduce DisplayPort connector helpers with link training state

[Jani Nikula]

On Fri, 19 Jun 2026, Kory Maincent <kory.maincent@bootlin.com> wrote:
> 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;
> +}

There are three ways of reporting the sink link rates. DP_MAX_LINK_RATE
is the only one that reports the maximum. The other two can report any
rates. The above doesn't handle DP_SUPPORTED_LINK_RATES.

The drivers need to figure out *all* supported rates. What is the point
of adding a "helper" that reads these DPCD registers but only reports
the max rate? Using this will only mean duplicated reads of the
registers. This may confuse the sinks, as well as the CTS.

This is not a good design.

> +
> +/**
> + * 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;

Why do you have different rates in different places? Ditch the
conversions?

> +
> +	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;

Why do you define a struct, and then not use it as a sub-struct in
connector->dp?

> +
> +	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;

Just make all of the above ints? There's nothing specifically u32 or u8
about any of them. They're just integers.

> +
> +	/**
> +	 * @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;

Why doesn't the above use a struct?

> +	/**
> +	 * @sink_max_link_rate_caps: Maximum link rate supported by the sink
> +	 * in kbps
> +	 */
> +	u32 sink_max_link_rate_caps;

Why all rates for source, but just one rate for sink? Why is it called
_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;

Could use the same struct here.

> +	/**
> +	 * @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
> +	 */

These should be ints.

> +	bool dsc_en;
> +	/**
> +	 * @max_link_rate: Maximum achievable link rate considering both
> +	 * source and sink capabilities in deca-kbps
> +	 */
> +	u32 max_link_rate;

IMO this should be an intersection of source and sink rates.

> +	/**
> +	 * @max_lane_count: Maximum achievable lane count considering both
> +	 * source and sink capabilities
> +	 */
> +	u32 max_lane_count;

These should be ints.

> +};
> +
>  /**
>   * 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);

-- 
Jani Nikula, Intel

Re: [PATCH RFC v2 2/4] drm/i915/display/dp: Adopt dp_connector helpers to expose link training state

[Jani Nikula]

On Fri, 19 Jun 2026, Kory Maincent <kory.maincent@bootlin.com> wrote:
> 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);

Gut feeling is that the sprinkling of these around is error prone.

>  		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);
> +

All of the above feels a bit clumsy in the middle of an already too long
function.

>  	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);

This is not okay.

We've already figured out all the information needed, and this call goes
ahead and reads the same information out again.

Moreover, some sinks are fragile when it comes to reading the info and
starting link training. The CTS might complain about redundant reads as
well.

drm_dp_sink_set_link_caps() as a name implies something about link,
i.e. the thing between the source and the sink. But this only sets sink
capabilities. Which also means it should not happen at link training
time at all. We figure the information out at detect, which means it's
available *before* modeset.

The more I think about the function, the more I question it. Like, if
the source doesn't support UHBR at all, or not on this connector, what's
the point of reading the sink UHBR rates?

> +
>  	intel_hpd_block(encoder);
>  
>  	lttpr_count = intel_dp_init_lttpr_and_dprx_caps(intel_dp);

-- 
Jani Nikula, Intel