Re: [PATCH RFC 00/12] Add support for DisplayPort link training information report

Re: [PATCH RFC 00/12] Add support for DisplayPort link training information report

[Ville Syrjälä]

On Thu, Apr 09, 2026 at 07:08:16PM +0200, Kory Maincent wrote:
> DisplayPort link training negotiates the physical-layer parameters needed
> for a reliable connection: lane count, link rate, voltage swing,
> pre-emphasis, 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 a generic, DRM-managed framework for exposing DP
> link training state as standard connector properties, modeled after the
> existing HDMI helper drmm_connector_hdmi_init().
> 
> The new drmm_connector_dp_init() helper initializes a DP connector and
> registers the following connector properties to expose the negotiated link
> state to userspace:
> 
> - num_lanes:      negotiated lane count (1, 2 or 4)
> - link_rate:      negotiated link rate
> - dsc_en:         whether Display Stream Compression is active
> - voltage_swingN: per-lane voltage swing level (lanes 0-3)
> - pre_emphasisN:  per-lane pre-emphasis level (lanes 0-3)

I don't see why any real userspace would be interested in those (apart
from maybe DSC). If this is just for diagnostics and whatnot then I
think sysfs/debugfs could be a better fit.

> 
> Two runtime helpers update and clear these properties when link training
> completes or the link goes down:
> - drm_connector_dp_set_link_train_properties()
> - drm_connector_dp_reset_link_train_properties()
> 
> 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 i915 patches are preceded by a series of
> conversions to DRM managed resources, which are required before adopting
> drmm_connector_dp_init().
> 
> The MST case in i915 driver is not supported yet.
> 
> Patches 1-3:  Fix two error-path cleanup bugs in i915 sdvo and lvds
> [Will probably be sent standalone]
> Patches 4-8: Convert i915 display resources to DRM managed lifetime
> Patch 9: Introduce the core drmm_connector_dp_init() framework
> Patch 10: Wire the i915 DP connector to use the new helpers
> Patch 11: Introduce DRM_BRIDGE_OP_DP and wire bridge connectors
> Patch 12: Wire the MediaTek DP bridge to the new helpers [untested]
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
> Kory Maincent (12):
>       drm/i915/display/intel_sdvo: Fix double connector destroy in error paths
>       drm/i915/display/intel_lvds: Drop redundant manual cleanup on init failure
>       drm/i915/display/intel_dp: Drop redundant intel_dp_aux_fini() on init failure
>       drm/i915/display: Switch to drmm_mode_config_init() and drop manual cleanup
>       drm/i915/display: Switch to managed for crtc
>       drm/i915/display: Switch to managed for plane
>       drm/i915/display: Switch to managed for encoder
>       drm/i915/display: Switch to managed for connector
>       drm: Introduce drmm_connector_dp_init() with link training state properties
>       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/Makefile                           |   1 +
>  drivers/gpu/drm/display/drm_bridge_connector.c     |  26 +-
>  drivers/gpu/drm/drm_dp_connector.c                 | 344 +++++++++++++++++++++
>  drivers/gpu/drm/i915/display/g4x_dp.c              |  39 +--
>  drivers/gpu/drm/i915/display/g4x_hdmi.c            |  27 +-
>  drivers/gpu/drm/i915/display/i9xx_plane.c          |  97 +++---
>  drivers/gpu/drm/i915/display/icl_dsi.c             |  50 ++-
>  drivers/gpu/drm/i915/display/intel_connector.c     |  26 +-
>  drivers/gpu/drm/i915/display/intel_connector.h     |   5 +-
>  drivers/gpu/drm/i915/display/intel_crt.c           |  28 +-
>  drivers/gpu/drm/i915/display/intel_crtc.c          | 102 +++---
>  drivers/gpu/drm/i915/display/intel_cursor.c        |  41 ++-
>  drivers/gpu/drm/i915/display/intel_ddi.c           |  64 ++--
>  drivers/gpu/drm/i915/display/intel_display.c       |   8 -
>  drivers/gpu/drm/i915/display/intel_display.h       |   1 -
>  .../gpu/drm/i915/display/intel_display_driver.c    |  37 ++-
>  drivers/gpu/drm/i915/display/intel_dp.c            |  43 ++-
>  .../gpu/drm/i915/display/intel_dp_link_training.c  |  25 ++
>  drivers/gpu/drm/i915/display/intel_dp_mst.c        |  33 +-
>  drivers/gpu/drm/i915/display/intel_dvo.c           |  43 +--
>  drivers/gpu/drm/i915/display/intel_encoder.c       |   6 +-
>  drivers/gpu/drm/i915/display/intel_encoder.h       |   3 +-
>  drivers/gpu/drm/i915/display/intel_hdmi.c          |  15 +-
>  drivers/gpu/drm/i915/display/intel_lvds.c          |  45 ++-
>  drivers/gpu/drm/i915/display/intel_plane.c         |  45 +--
>  drivers/gpu/drm/i915/display/intel_plane.h         |   5 +-
>  drivers/gpu/drm/i915/display/intel_sdvo.c          | 134 +++-----
>  drivers/gpu/drm/i915/display/intel_sprite.c        | 119 ++++---
>  drivers/gpu/drm/i915/display/intel_tv.c            |  26 +-
>  drivers/gpu/drm/i915/display/skl_universal_plane.c | 102 +++---
>  drivers/gpu/drm/i915/display/vlv_dsi.c             |  42 +--
>  drivers/gpu/drm/mediatek/mtk_dp.c                  |  34 +-
>  include/drm/drm_bridge.h                           |  13 +
>  include/drm/drm_connector.h                        |  38 +++
>  include/drm/drm_dp_connector.h                     | 109 +++++++
>  35 files changed, 1125 insertions(+), 651 deletions(-)
> ---
> base-commit: db5a75cfd29766536be62aece9f19c6e7a858fa6
> change-id: 20260226-feat_link_cap-20cbb6f31d40
> 
> Best regards,
> -- 
> Köry Maincent, Bootlin
> Embedded Linux and kernel engineering
> https://bootlin.com

-- 
Ville Syrjälä
Intel

Re: [PATCH RFC 00/12] Add support for DisplayPort link training information report

[Dmitry Baryshkov]

On Thu, Apr 09, 2026 at 11:36:21PM +0300, Ville Syrjälä wrote:
> On Thu, Apr 09, 2026 at 07:08:16PM +0200, Kory Maincent wrote:
> > DisplayPort link training negotiates the physical-layer parameters needed
> > for a reliable connection: lane count, link rate, voltage swing,
> > pre-emphasis, 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 a generic, DRM-managed framework for exposing DP
> > link training state as standard connector properties, modeled after the
> > existing HDMI helper drmm_connector_hdmi_init().
> > 
> > The new drmm_connector_dp_init() helper initializes a DP connector and
> > registers the following connector properties to expose the negotiated link
> > state to userspace:
> > 
> > - num_lanes:      negotiated lane count (1, 2 or 4)
> > - link_rate:      negotiated link rate
> > - dsc_en:         whether Display Stream Compression is active
> > - voltage_swingN: per-lane voltage swing level (lanes 0-3)
> > - pre_emphasisN:  per-lane pre-emphasis level (lanes 0-3)
> 
> I don't see why any real userspace would be interested in those (apart
> from maybe DSC). If this is just for diagnostics and whatnot then I
> think sysfs/debugfs could be a better fit.

I'd agree here. Please consider implementing it as a debugfs interface,
possibly reusing the Intel's format.

-- 
With best wishes
Dmitry

Re: [PATCH RFC 09/12] drm: Introduce drmm_connector_dp_init() with link training state properties

[Dmitry Baryshkov]

On Thu, Apr 09, 2026 at 07:08:25PM +0200, Kory Maincent wrote:
> Add a managed DisplayPort connector initialization helper,
> drmm_connector_dp_init(), modeled after the existing HDMI counterpart
> drmm_connector_hdmi_init(). Cleanup is handled automatically via a
> DRM-managed action.
> 
> The helper creates the following immutable connector properties to expose
> DP link training capabilities and state to userspace:
> 
>   - num_lanes: bitmask of supported lane counts (1, 2, 4)
>   - link_rate: Array of supported link rates.
>   - dsc_en: Display Stream Compression supported
>   - voltage_swingN: per-lane voltage swing level bitmask
>   - pre-emphasisN: per-lane pre-emphasis level bitmask
> 
> Link rates are passed by the driver in deca-kbps, following the DRM
> convention, but exposed to userspace in kbps for clarity.
> 
> Two additional helpers are provided to update and reset those properties
> at runtime:
>   - drm_connector_dp_set_link_train_properties()
>   - drm_connector_dp_reset_link_train_properties()
> 
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  drivers/gpu/drm/Makefile           |   1 +
>  drivers/gpu/drm/drm_dp_connector.c | 344 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h        |  38 ++++
>  include/drm/drm_dp_connector.h     | 109 ++++++++++++
>  4 files changed, 492 insertions(+)
> 
> diff --git a/include/drm/drm_dp_connector.h b/include/drm/drm_dp_connector.h
> new file mode 100644
> index 0000000000000..77d2f4bb6df68
> --- /dev/null
> +++ b/include/drm/drm_dp_connector.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef DRM_DP_CONNECTOR_H_
> +#define DRM_DP_CONNECTOR_H_
> +
> +#include <drm/drm_connector.h>
> +
> +#define DRM_DP_1LANE	BIT(0)
> +#define DRM_DP_2LANE	BIT(1)
> +#define DRM_DP_4LANE	BIT(2)
> +#define DRM_NLANES_MASK (DRM_DP_1LANE | DRM_DP_2LANE | DRM_DP_4LANE)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_0 BIT(0)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_1 BIT(1)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_2 BIT(2)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_3 BIT(3)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_MASK (DRM_DP_VOLTAGE_SWING_LEVEL_0 | \
> +					 DRM_DP_VOLTAGE_SWING_LEVEL_1 | \
> +					 DRM_DP_VOLTAGE_SWING_LEVEL_2 | \
> +					 DRM_DP_VOLTAGE_SWING_LEVEL_3)
> +#define DRM_DP_PRE_EMPH_LEVEL_0 BIT(0)
> +#define DRM_DP_PRE_EMPH_LEVEL_1 BIT(1)
> +#define DRM_DP_PRE_EMPH_LEVEL_2 BIT(2)
> +#define DRM_DP_PRE_EMPH_LEVEL_3 BIT(3)
> +#define DRM_DP_PRE_EMPH_LEVEL_MASK (DRM_DP_PRE_EMPH_LEVEL_0 | \
> +				    DRM_DP_PRE_EMPH_LEVEL_1 | \
> +				    DRM_DP_PRE_EMPH_LEVEL_2 | \
> +				    DRM_DP_PRE_EMPH_LEVEL_3)
> +
> +/**
> + * struct drm_connector_dp_link_train_caps - DRM DisplayPort link training
> + * capabilities

Those are not just link training caps. It is more like DP link caps.
They make sense to be a part of the DP-related drm_connector part.

> + */
> +struct drm_connector_dp_link_train_caps {
> +	/**
> +	 * @nlanes: Bitmask of lanes number supported
> +	 */
> +	u8 nlanes;
> +
> +	/**
> +	 * @nrates: Number of link rates supported
> +	 */
> +	u32 nrates;
> +
> +	/**
> +	 * @rates: Array listing the supported link rates in deca-kbps
> +	 */
> +	const u32 *rates;
> +
> +	/**
> +	 * @dsc: Display Stream Compression supported
> +	 */
> +	bool dsc;
> +
> +	/**
> +	 * @v_swings: Bitmask of voltage swing level supported
> +	 */
> +	u8 v_swings;
> +
> +	/**
> +	 * @pre_emphs: Bitmask of pre-emphasis level supported
> +	 */
> +	u8 pre_emphs;
> +};
> +
> +/**
> + * struct drm_connector_dp_link_train - DRM DisplayPort link training
> + * information report
> + */
> +struct drm_connector_dp_link_train {

THese define the current DP state. As such, they definitely make sense
to be a part of the drm_connector.
> +	/**
> +	 * @nlanes: The number of lanes used
> +	 */
> +	u8 nlanes;
> +
> +	/**
> +	 * @rates: Link rate value selected in deca-kbps
> +	 */
> +	u32 rate;
> +
> +	/**
> +	 * @dsc: Display Stream Compression enabled
> +	 */
> +	bool dsc_en;
> +
> +	/**
> +	 * @v_swings: Array listing the bitmask voltage swing level per lanes
> +	 */
> +	u8 v_swing[4];
> +
> +	/**
> +	 * @pre_emph: Array listing the bitmask pre-emphasis level per lanes
> +	 */
> +	u8 pre_emph[4];

Please consider following struct phy_configure_opts_dp (or using it as
is). Overall, please refer the talk and (more important) the lightning
resumee at this XDC. I have some bits and pieces ready in spite of that
proposal, but I didn't have time to finish it.

> +};
> +

-- 
With best wishes
Dmitry

Re: [PATCH RFC 09/12] drm: Introduce drmm_connector_dp_init() with link training state properties

[Jani Nikula]

On Thu, 09 Apr 2026, Kory Maincent <kory.maincent@bootlin.com> wrote:
> Add a managed DisplayPort connector initialization helper,
> drmm_connector_dp_init(), modeled after the existing HDMI counterpart
> drmm_connector_hdmi_init(). Cleanup is handled automatically via a
> DRM-managed action.
>
> The helper creates the following immutable connector properties to expose
> DP link training capabilities and state to userspace:
>
>   - num_lanes: bitmask of supported lane counts (1, 2, 4)
>   - link_rate: Array of supported link rates.
>   - dsc_en: Display Stream Compression supported
>   - voltage_swingN: per-lane voltage swing level bitmask
>   - pre-emphasisN: per-lane pre-emphasis level bitmask

The main question is, why do we need to provide these details to the
userspace via ABI?

I mean yeah, we can unify on a debugfs interface, but connector
properties and ABI seems a bit over the top. The userspace should not
have to act on any of this information, except perhaps in a test
scenario, for which debugfs should be just fine.

I'm also concerned about the duplication of data here. I think debugfs
could be constructed in a way to query the actual information from the
driver right then and there, instead of having to copy data over to
properties, which can go stale.

Oh, there's also too much going on in one patch here.


BR,
Jani.



>
> Link rates are passed by the driver in deca-kbps, following the DRM
> convention, but exposed to userspace in kbps for clarity.
>
> Two additional helpers are provided to update and reset those properties
> at runtime:
>   - drm_connector_dp_set_link_train_properties()
>   - drm_connector_dp_reset_link_train_properties()
>
> Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
> ---
>  drivers/gpu/drm/Makefile           |   1 +
>  drivers/gpu/drm/drm_dp_connector.c | 344 +++++++++++++++++++++++++++++++++++++
>  include/drm/drm_connector.h        |  38 ++++
>  include/drm/drm_dp_connector.h     | 109 ++++++++++++
>  4 files changed, 492 insertions(+)
>
> diff --git a/drivers/gpu/drm/Makefile b/drivers/gpu/drm/Makefile
> index e97faabcd7830..8ff08c2fb863e 100644
> --- a/drivers/gpu/drm/Makefile
> +++ b/drivers/gpu/drm/Makefile
> @@ -42,6 +42,7 @@ drm-y := \
>  	drm_color_mgmt.o \
>  	drm_colorop.o \
>  	drm_connector.o \
> +	drm_dp_connector.o \
>  	drm_crtc.o \
>  	drm_displayid.o \
>  	drm_drv.o \
> diff --git a/drivers/gpu/drm/drm_dp_connector.c b/drivers/gpu/drm/drm_dp_connector.c
> new file mode 100644
> index 0000000000000..b25637a4378d5
> --- /dev/null
> +++ b/drivers/gpu/drm/drm_dp_connector.c
> @@ -0,0 +1,344 @@
> +// SPDX-License-Identifier: GPL-2.0-only
> +/*
> + * Copyright (C) 2026 Google
> + * Author: Kory Maincent <kory.maincent@bootlin.com>
> + */
> +#include <drm/drm_dp_connector.h>
> +#include <drm/drm_print.h>
> +#include <linux/list.h>
> +
> +/**
> + * drm_connector_dp_link_reset_properties() - Reset DisplayPort link configuration
> + * @connector: DRM connector
> + * @dp_link: Link training informations
> + *
> + * Returns: Zero on success, or an errno code otherwise.
> + */
> +int
> +drm_connector_dp_set_link_train_properties(struct drm_connector *connector,
> +					   const struct drm_connector_dp_link_train *dp_link_train)
> +{
> +	u32 lrate = 0;
> +	int ret;
> +
> +	if (!connector)
> +		return -ENODEV;
> +
> +	if (dp_link_train->nlanes && !is_power_of_2(dp_link_train->nlanes & DRM_NLANES_MASK)) {
> +		drm_err(connector->dev, "Wrong lane number");
> +		return -EINVAL;
> +	}
> +
> +	if (dp_link_train->rate) {
> +		struct drm_property_enum *prop_enum;
> +		bool found = false;
> +
> +		list_for_each_entry(prop_enum, &connector->dp.link_rate_property->enum_list, head) {
> +			u32 parsed_rate;
> +
> +			/* Convert dp_link_train->rate from deca-kbps to kbps */
> +			if (!kstrtou32(prop_enum->name, 10, &parsed_rate) &&
> +			    dp_link_train->rate * 10 == parsed_rate) {
> +				lrate = 1 << prop_enum->value;
> +				found = true;
> +				break;
> +			}
> +		}
> +
> +		if (!found) {
> +			drm_err(connector->dev, "Wrong rate value");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	ret = drm_object_property_set_value(&connector->base, connector->dp.nlanes_property,
> +					    dp_link_train->nlanes);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_object_property_set_value(&connector->base, connector->dp.link_rate_property,
> +					    lrate);
> +	if (ret)
> +		return ret;
> +
> +	if (connector->dp.dsc_en_property) {
> +		ret = drm_object_property_set_value(&connector->base, connector->dp.dsc_en_property,
> +						    dp_link_train->dsc_en);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	for (int i = 0; i < 4; i++) {
> +		if (connector->dp.v_swing_property[i]) {
> +			ret = drm_object_property_set_value(&connector->base,
> +							    connector->dp.v_swing_property[i],
> +							    dp_link_train->v_swing[i]);
> +			if (ret)
> +				return ret;
> +		}
> +
> +		if (connector->dp.pre_emph_property[i]) {
> +			ret = drm_object_property_set_value(&connector->base,
> +							    connector->dp.pre_emph_property[i],
> +							    dp_link_train->pre_emph[i]);
> +			if (ret)
> +				return ret;
> +		}
> +	}
> +
> +	return ret;
> +}
> +EXPORT_SYMBOL(drm_connector_dp_set_link_train_properties);
> +
> +/**
> + * drm_connector_dp_link_reset_properties() - Reset DisplayPort link configuration
> + * @connector: DRM connector
> + */
> +void drm_connector_dp_reset_link_train_properties(struct drm_connector *connector)
> +{
> +	struct drm_connector_dp_link_train dp_link_train = {0};
> +
> +	drm_connector_dp_set_link_train_properties(connector, &dp_link_train);
> +}
> +EXPORT_SYMBOL(drm_connector_dp_reset_link_train_properties);
> +
> +static int drm_connector_create_nlanes_prop(struct drm_connector *connector,
> +					    u8 sup_nlanes)
> +{
> +	static const struct drm_prop_enum_list props[] = {
> +		{__builtin_ffs(DRM_DP_1LANE) - 1, "1" },
> +		{__builtin_ffs(DRM_DP_2LANE) - 1, "2" },
> +		{__builtin_ffs(DRM_DP_4LANE) - 1, "4" },
> +	};
> +	struct drm_property *prop;
> +
> +	if (drm_WARN_ON(connector->dev, sup_nlanes != (sup_nlanes & DRM_NLANES_MASK)))
> +		return -EINVAL;
> +
> +	prop = drm_property_create_bitmask(connector->dev, DRM_MODE_PROP_IMMUTABLE,
> +					   "num_lanes", props, ARRAY_SIZE(props),
> +					   sup_nlanes);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +
> +	connector->dp.nlanes_property = prop;
> +
> +	return 0;
> +}
> +
> +static int drm_connector_create_lrate_prop(struct drm_connector *connector,
> +					   u32 sup_nlrates,
> +					   const u32 *sup_lrates)
> +{
> +	struct drm_prop_enum_list *props;
> +	u32 supp_nlrates_bitmask = 0;
> +	struct drm_property *prop;
> +	int ret = 0;
> +
> +	if (!sup_nlrates || !sup_lrates)
> +		return 0;
> +
> +	props = kcalloc(sup_nlrates, sizeof(*props), GFP_KERNEL);
> +	if (!props)
> +		return -ENOMEM;
> +
> +	for (int i = 0; i < sup_nlrates; i++) {
> +		props[i].type = i;
> +		/* Convert deca-kbps to kbps */
> +		props[i].name = kasprintf(GFP_KERNEL, "%d", sup_lrates[i] * 10);
> +		if (!props[i].name) {
> +			while (i--)
> +				kfree(props[i].name);
> +			kfree(props);
> +			return -ENOMEM;
> +		}
> +		supp_nlrates_bitmask |= 1 << i;
> +	}
> +
> +	prop = drm_property_create_bitmask(connector->dev, DRM_MODE_PROP_IMMUTABLE,
> +					   "link_rate", props, sup_nlrates,
> +					   supp_nlrates_bitmask);
> +	if (!prop) {
> +		ret = -ENOMEM;
> +		goto out;
> +	}
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +
> +	connector->dp.link_rate_property = prop;
> +
> +out:
> +	for (int i = 0; i < sup_nlrates; i++)
> +		kfree(props[i].name);
> +
> +	kfree(props);
> +	return ret;
> +}
> +
> +static int drm_connector_create_dsc_prop(struct drm_connector *connector)
> +{
> +	struct drm_property *prop;
> +
> +	prop = drm_property_create_bool(connector->dev, DRM_MODE_PROP_IMMUTABLE, "dsc_en");
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +
> +	connector->dp.dsc_en_property = prop;
> +
> +	return 0;
> +}
> +
> +static int drm_connector_create_vswing_prop(struct drm_connector *connector,
> +					    u8 sup_v_swings, int id)
> +{
> +	static const struct drm_prop_enum_list props[] = {
> +		{__builtin_ffs(DRM_DP_VOLTAGE_SWING_LEVEL_0) - 1, "level_0" },
> +		{__builtin_ffs(DRM_DP_VOLTAGE_SWING_LEVEL_1) - 1, "level_1" },
> +		{__builtin_ffs(DRM_DP_VOLTAGE_SWING_LEVEL_2) - 1, "level_2" },
> +		{__builtin_ffs(DRM_DP_VOLTAGE_SWING_LEVEL_3) - 1, "level_3" },
> +	};
> +	struct drm_property *prop;
> +	char str[16];
> +
> +	if (!sup_v_swings)
> +		return 0;
> +
> +	if (drm_WARN_ON(connector->dev, sup_v_swings != (sup_v_swings &
> +						   DRM_DP_VOLTAGE_SWING_LEVEL_MASK)))
> +		return -EINVAL;
> +
> +	snprintf(str, sizeof(str), "voltage_swing%d", id);
> +	prop = drm_property_create_bitmask(connector->dev, DRM_MODE_PROP_IMMUTABLE,
> +					   str, props, ARRAY_SIZE(props),
> +					   sup_v_swings);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +
> +	connector->dp.v_swing_property[id] = prop;
> +
> +	return 0;
> +}
> +
> +static int drm_connector_create_pre_emph_prop(struct drm_connector *connector,
> +					      u8 sup_pre_emph, int id)
> +{
> +	static const struct drm_prop_enum_list props[] = {
> +		{__builtin_ffs(DRM_DP_PRE_EMPH_LEVEL_0) - 1, "level_0" },
> +		{__builtin_ffs(DRM_DP_PRE_EMPH_LEVEL_1) - 1, "level_1" },
> +		{__builtin_ffs(DRM_DP_PRE_EMPH_LEVEL_2) - 1, "level_2" },
> +		{__builtin_ffs(DRM_DP_PRE_EMPH_LEVEL_3) - 1, "level_3" },
> +	};
> +	struct drm_property *prop;
> +	char str[16];
> +
> +	if (!sup_pre_emph)
> +		return 0;
> +
> +	if (drm_WARN_ON(connector->dev, sup_pre_emph != (sup_pre_emph &
> +						   DRM_DP_PRE_EMPH_LEVEL_MASK)))
> +		return -EINVAL;
> +
> +	snprintf(str, sizeof(str), "pre_emphasis%d", id);
> +	prop = drm_property_create_bitmask(connector->dev, DRM_MODE_PROP_IMMUTABLE,
> +					   str, props, ARRAY_SIZE(props),
> +					   sup_pre_emph);
> +	if (!prop)
> +		return -ENOMEM;
> +
> +	drm_object_attach_property(&connector->base, prop, 0);
> +
> +	connector->dp.pre_emph_property[id] = prop;
> +
> +	return 0;
> +}
> +
> +static int
> +drm_connector_dp_create_props(struct drm_connector *connector,
> +			      const struct drm_connector_dp_link_train_caps *dp_link_train_caps)
> +{
> +	u8 nlanes;
> +	int ret;
> +
> +	ret = drm_connector_create_nlanes_prop(connector, dp_link_train_caps->nlanes);
> +	if (ret)
> +		return ret;
> +
> +	ret = drm_connector_create_lrate_prop(connector, dp_link_train_caps->nrates,
> +					      dp_link_train_caps->rates);
> +	if (ret)
> +		return ret;
> +
> +	if (dp_link_train_caps->dsc) {
> +		ret = drm_connector_create_dsc_prop(connector);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	nlanes = 1 << (fls(dp_link_train_caps->nlanes) - 1);
> +	for (int i = 0; i < nlanes; i++) {
> +		ret = drm_connector_create_vswing_prop(connector,
> +						       dp_link_train_caps->v_swings, i);
> +		if (ret)
> +			return ret;
> +
> +		ret = drm_connector_create_pre_emph_prop(connector,
> +							 dp_link_train_caps->pre_emphs, i);
> +		if (ret)
> +			return ret;
> +	}
> +
> +	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_train_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_train_caps *dp_link_train_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_train_caps)
> +		return -EINVAL;
> +
> +	ret = drmm_connector_init(dev, connector, funcs, connector_type, ddc);
> +	if (ret)
> +		return ret;
> +
> +	return drm_connector_dp_create_props(connector, dp_link_train_caps);
> +}
> +EXPORT_SYMBOL(drmm_connector_dp_init);
> diff --git a/include/drm/drm_connector.h b/include/drm/drm_connector.h
> index f83f28cae2075..df3a71fed35b1 100644
> --- a/include/drm/drm_connector.h
> +++ b/include/drm/drm_connector.h
> @@ -1987,6 +1987,39 @@ struct drm_connector_cec {
>  	void *data;
>  };
>  
> +/**
> + * struct drm_connector_dp - DRM Connector DisplayPort-related structure
> + */
> +struct drm_connector_dp {
> +	/**
> +	 * @nlanes_property: Connector property to report the number of lanes
> +	 */
> +	struct drm_property *nlanes_property;
> +
> +	/**
> +	 * @link_rate_property: Connector property to report the link rate
> +	 */
> +	struct drm_property *link_rate_property;
> +
> +	/**
> +	 * @dsc_en_property: Connector property to report the Display Stream
> +	 * Compression supporrt
> +	 */
> +	struct drm_property *dsc_en_property;
> +
> +	/**
> +	 * @v_swing_property: Connector property to report the voltage
> +	 * swing per lane
> +	 */
> +	struct drm_property *v_swing_property[4];
> +
> +	/**
> +	 * @pre_emph_property: Connector property to report the
> +	 * pre-emphasis per lane
> +	 */
> +	struct drm_property *pre_emph_property[4];
> +};
> +
>  /**
>   * struct drm_connector - central DRM connector control structure
>   *
> @@ -2410,6 +2443,11 @@ struct drm_connector {
>  	 * @cec: CEC-related data.
>  	 */
>  	struct drm_connector_cec cec;
> +
> +	/**
> +	 * @dp: DisplayPort-related variable and properties.
> +	 */
> +	struct drm_connector_dp dp;
>  };
>  
>  #define obj_to_connector(x) container_of(x, struct drm_connector, base)
> diff --git a/include/drm/drm_dp_connector.h b/include/drm/drm_dp_connector.h
> new file mode 100644
> index 0000000000000..77d2f4bb6df68
> --- /dev/null
> +++ b/include/drm/drm_dp_connector.h
> @@ -0,0 +1,109 @@
> +/* SPDX-License-Identifier: GPL-2.0-only */
> +
> +#ifndef DRM_DP_CONNECTOR_H_
> +#define DRM_DP_CONNECTOR_H_
> +
> +#include <drm/drm_connector.h>
> +
> +#define DRM_DP_1LANE	BIT(0)
> +#define DRM_DP_2LANE	BIT(1)
> +#define DRM_DP_4LANE	BIT(2)
> +#define DRM_NLANES_MASK (DRM_DP_1LANE | DRM_DP_2LANE | DRM_DP_4LANE)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_0 BIT(0)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_1 BIT(1)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_2 BIT(2)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_3 BIT(3)
> +#define DRM_DP_VOLTAGE_SWING_LEVEL_MASK (DRM_DP_VOLTAGE_SWING_LEVEL_0 | \
> +					 DRM_DP_VOLTAGE_SWING_LEVEL_1 | \
> +					 DRM_DP_VOLTAGE_SWING_LEVEL_2 | \
> +					 DRM_DP_VOLTAGE_SWING_LEVEL_3)
> +#define DRM_DP_PRE_EMPH_LEVEL_0 BIT(0)
> +#define DRM_DP_PRE_EMPH_LEVEL_1 BIT(1)
> +#define DRM_DP_PRE_EMPH_LEVEL_2 BIT(2)
> +#define DRM_DP_PRE_EMPH_LEVEL_3 BIT(3)
> +#define DRM_DP_PRE_EMPH_LEVEL_MASK (DRM_DP_PRE_EMPH_LEVEL_0 | \
> +				    DRM_DP_PRE_EMPH_LEVEL_1 | \
> +				    DRM_DP_PRE_EMPH_LEVEL_2 | \
> +				    DRM_DP_PRE_EMPH_LEVEL_3)
> +
> +/**
> + * struct drm_connector_dp_link_train_caps - DRM DisplayPort link training
> + * capabilities
> + */
> +struct drm_connector_dp_link_train_caps {
> +	/**
> +	 * @nlanes: Bitmask of lanes number supported
> +	 */
> +	u8 nlanes;
> +
> +	/**
> +	 * @nrates: Number of link rates supported
> +	 */
> +	u32 nrates;
> +
> +	/**
> +	 * @rates: Array listing the supported link rates in deca-kbps
> +	 */
> +	const u32 *rates;
> +
> +	/**
> +	 * @dsc: Display Stream Compression supported
> +	 */
> +	bool dsc;
> +
> +	/**
> +	 * @v_swings: Bitmask of voltage swing level supported
> +	 */
> +	u8 v_swings;
> +
> +	/**
> +	 * @pre_emphs: Bitmask of pre-emphasis level supported
> +	 */
> +	u8 pre_emphs;
> +};
> +
> +/**
> + * struct drm_connector_dp_link_train - DRM DisplayPort link training
> + * information report
> + */
> +struct drm_connector_dp_link_train {
> +	/**
> +	 * @nlanes: The number of lanes used
> +	 */
> +	u8 nlanes;
> +
> +	/**
> +	 * @rates: Link rate value selected in deca-kbps
> +	 */
> +	u32 rate;
> +
> +	/**
> +	 * @dsc: Display Stream Compression enabled
> +	 */
> +	bool dsc_en;
> +
> +	/**
> +	 * @v_swings: Array listing the bitmask voltage swing level per lanes
> +	 */
> +	u8 v_swing[4];
> +
> +	/**
> +	 * @pre_emph: Array listing the bitmask pre-emphasis level per lanes
> +	 */
> +	u8 pre_emph[4];
> +};
> +
> +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_train_caps *dp_link_train_caps,
> +			   int connector_type,
> +			   struct i2c_adapter *ddc);
> +
> +int
> +drm_connector_dp_set_link_train_properties(struct drm_connector *con,
> +					   const struct drm_connector_dp_link_train *dp_link_train);
> +
> +void drm_connector_dp_reset_link_train_properties(struct drm_connector *connector);
> +
> +#endif // DRM_DP_CONNECTOR_H_

-- 
Jani Nikula, Intel

Re: [PATCH RFC 10/12] drm/i915/display/dp: Adopt dp_connector helpers to expose link training state

[Jani Nikula]

On Thu, 09 Apr 2026, Kory Maincent <kory.maincent@bootlin.com> wrote:
> Switch the i915 DP connector initialization from drmm_connector_init()
> to drmm_connector_dp_init(), providing the source link capabilities
> (supported lane counts, link rates, DSC support, voltage swing and
> pre-emphasis levels).
>
> Add intel_dp_report_link_train() to collect the negotiated link
> parameters (rate, lane count, DSC enable, per-lane voltage swing and
> pre-emphasis) and report them via drm_connector_dp_set_link_train_properties()
> once link training completes successfully.
>
> Reset the link training properties via
> drm_connector_dp_reset_link_train_properties() 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>
> ---
>  drivers/gpu/drm/i915/display/intel_dp.c            | 31 +++++++++++++++++++---
>  .../gpu/drm/i915/display/intel_dp_link_training.c  | 25 +++++++++++++++++
>  2 files changed, 52 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 2af64de9c81de..641406bdc0cc9 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -45,6 +45,7 @@
>  #include <drm/display/drm_hdmi_helper.h>
>  #include <drm/drm_atomic_helper.h>
>  #include <drm/drm_crtc.h>
> +#include <drm/drm_dp_connector.h>
>  #include <drm/drm_edid.h>
>  #include <drm/drm_fixed.h>
>  #include <drm/drm_managed.h>
> @@ -6337,8 +6338,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_connector_dp_reset_link_train_properties(_connector);
>  		return connector_status_disconnected;
> +	}
>  
>  	if (!intel_display_driver_check_access(display))
>  		return connector->base.status;
> @@ -6388,6 +6391,8 @@ intel_dp_detect(struct drm_connector *_connector,
>  
>  		intel_dp_tunnel_disconnect(intel_dp);
>  
> +		drm_connector_dp_reset_link_train_properties(_connector);
> +
>  		goto out_unset_edid;
>  	}
>  
> @@ -7162,10 +7167,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_train_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,
> @@ -7213,8 +7220,25 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
>  		    type == DRM_MODE_CONNECTOR_eDP ? "eDP" : "DP",
>  		    encoder->base.base.id, encoder->base.name);
>  
> -	drmm_connector_init(dev, &connector->base, &intel_dp_connector_funcs,
> -			    type, &intel_dp->aux.ddc);
> +	intel_dp_set_source_rates(intel_dp);
> +	link_caps.nlanes = DRM_DP_1LANE | DRM_DP_2LANE | DRM_DP_4LANE;
> +	link_caps.nrates = intel_dp->num_source_rates;
> +	rates = kzalloc_objs(*rates, intel_dp->num_source_rates);
> +	if (!rates)
> +		goto fail;
> +
> +	for (int i = 0; i < intel_dp->num_source_rates; i++)
> +		rates[i] = intel_dp->source_rates[i];
> +
> +	link_caps.rates = rates;
> +	link_caps.dsc = true;

You have a source, you have a sink, and you have a link between the two.

Source rates do not reflect the link rates common between source and
sink.

DSC depends on source and sink, and it's not statically "true" for
either, and depends on a bunch of things.

BR,
Jani.

> +	link_caps.v_swings = DRM_DP_VOLTAGE_SWING_LEVEL_MASK;
> +	link_caps.pre_emphs = DRM_DP_PRE_EMPH_LEVEL_MASK;
> +
> +	drmm_connector_dp_init(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 (drmm_add_action_or_reset(dev, intel_connector_destroy, connector)) {
> @@ -7240,7 +7264,6 @@ intel_dp_init_connector(struct intel_digital_port *dig_port,
>  	if (!intel_edp_init_connector(intel_dp, connector))
>  		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 54c585c59b900..c2fd46a323650 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp_link_training.c
> @@ -25,6 +25,7 @@
>  #include <linux/iopoll.h>
>  
>  #include <drm/display/drm_dp_helper.h>
> +#include <drm/drm_dp_connector.h>
>  #include <drm/drm_print.h>
>  
>  #include "intel_display_core.h"
> @@ -1116,6 +1117,27 @@ 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;
> +	struct drm_connector_dp_link_train dp_link_train;
> +
> +	dp_link_train.rate = intel_dp->link_rate;
> +	dp_link_train.nlanes = intel_dp->lane_count;
> +	dp_link_train.dsc_en = connector->dp.dsc_decompression_enabled;
> +
> +	for (int i = 0; i < intel_dp->lane_count; i++) {
> +		int v_swing_level = (intel_dp->train_set[i] &
> +				     DP_TRAIN_VOLTAGE_SWING_MASK) >> DP_TRAIN_VOLTAGE_SWING_SHIFT;
> +		int pre_emph_level = (intel_dp->train_set[i] &
> +				      DP_TRAIN_PRE_EMPHASIS_MASK) >> DP_TRAIN_PRE_EMPHASIS_SHIFT;
> +		dp_link_train.v_swing[i] = 1 << v_swing_level;
> +		dp_link_train.pre_emph[i] = 1 << pre_emph_level;
> +	}
> +
> +	drm_connector_dp_set_link_train_properties(&connector->base, &dp_link_train);
> +}
> +
>  /**
>   * intel_dp_stop_link_train - stop link training
>   * @intel_dp: DP struct
> @@ -1144,6 +1166,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,

-- 
Jani Nikula, Intel