[PATCH v7 0/5] Add SCDC information to connector debugfs

[PATCH v7 0/5] Add SCDC information to connector debugfs

[Nicolas Frattaroli]

HDMI uses the DDC I2C bus for communicating various bits of link status
out of band with the actual HDMI video signal. This information can be
useful for debugging issues like questionable cables sabotaged by feline
teeth, Enthusiast Grade cables made of cow fencing wire, and other such
problems that ruin one's media viewing plans.

Consequently, this series exposes various bits of pertinent information
from the SCDC protocol in an HDMI connector's debugfs. To continually
poll the link status, userspace can poll the debugfs file.

---
Changes in v7:
- Move HDMI debugfs registration to new state helper, fix up all users
  (sun4i, vc4, bridge)
- Register scdc debugfs in new hdmi debugfs helper
- Link to v6: https://patch.msgid.link/20260611-scdc-link-health-v6-0-6307875a6b5e@collabora.com

Changes in v6:
- Fix off-by-one error in drm_scdc_read_state
- Link to v5: https://patch.msgid.link/20260604-scdc-link-health-v5-0-11173b0ac3de@collabora.com

Changes in v5:
- Read all SCDC data regardless of update flags
- Dump SCDC data as hex before the human-readable output. It's separated
  with "\n----------------\n\n".
- No longer write 0 to read-only registers
- Add Reed-Solomon Corrections counter parsing
- Parsing has been kept. A desire was expressed to get this data without
  any external userspace tooling, and the kernel will need to parse it
  eventually anyway to set the link status.
- Functions have been made static as of right now, since external users
  may do another pass over the function signatures anyway.
- Link to v4: https://patch.msgid.link/20260527-scdc-link-health-v4-0-622ea40a1f59@collabora.com

Changes in v4:
- Don't use C struct bitfields for parsing status flags. Switch to
  bitwise AND for boolean flags, and FIELD_GET for multi-bit values.
- Drop the superfluous !! and parens
- Drop the __pure attributes on static functions
- Initialise stack local arrays with {}, not { 0 }.
- I've kept the print macros and %-30s format. Reason being that I don't
  want to repeat the format specifier and str_yes_no(foo) a bunch, and I
  like the %-30s format because it means all values are aligned with the
  value of the longest field, which is 30 chars long.
- Link to v3: https://patch.msgid.link/20260526-scdc-link-health-v3-0-59e4a4aaead1@collabora.com

Changes in v3:
- Add patch to change return type of drm_scdc_read/write.
- Rework error counter reading to duplicate less code.
- Also check lane 3 counter valid flag when reading its error counter.
- Use memset to clear buf for error counters, rather than doing it in
  the loop.
- Make read_error_counters not accept 0 as num_lanes; fix it up in the
  caller instead.
- Link to v2: https://patch.msgid.link/20260520-scdc-link-health-v2-0-511af18cd64b@collabora.com

Changes in v2:
- Add HDMI 2.1 SCDC status reporting
- Link to v1: https://patch.msgid.link/20260415-scdc-link-health-v1-0-8e731e88eaf0@collabora.com

To: Jani Nikula <jani.nikula@linux.intel.com>
To: Maarten Lankhorst <maarten.lankhorst@linux.intel.com>
To: Maxime Ripard <mripard@kernel.org>
To: Thomas Zimmermann <tzimmermann@suse.de>
To: David Airlie <airlied@gmail.com>
To: Simona Vetter <simona@ffwll.ch>
To: Andrzej Hajda <andrzej.hajda@intel.com>
To: Neil Armstrong <neil.armstrong@linaro.org>
To: Robert Foss <rfoss@kernel.org>
To: Laurent Pinchart <Laurent.pinchart@ideasonboard.com>
To: Jonas Karlman <jonas@kwiboo.se>
To: Jernej Skrabec <jernej.skrabec@gmail.com>
To: Luca Ceresoli <luca.ceresoli@bootlin.com>
To: Daniel Stone <daniel@fooishbar.org>
To: Hans Verkuil <hverkuil+cisco@kernel.org>
To: Chen-Yu Tsai <wens@kernel.org>
To: Samuel Holland <samuel@sholland.org>
To: Dave Stevenson <dave.stevenson@raspberrypi.com>
To: Maíra Canal <mcanal@igalia.com>
To: Raspberry Pi Kernel Maintenance <kernel-list@raspberrypi.com>
Cc: dri-devel@lists.freedesktop.org
Cc: linux-kernel@vger.kernel.org
Cc: kernel@collabora.com
Cc: linux-arm-kernel@lists.infradead.org
Cc: linux-sunxi@lists.linux.dev
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

---
Nicolas Frattaroli (5):
      drm/scdc-helper: Don't use ssize_t return type for scdc_read/write
      drm/scdc-helper: Add scdc_status debugfs entry
      drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields
      drm/debugfs: Move HDMI debugfs registration to state helper
      drm/display: hdmi-state-helper: Init SCDC debugfs for HDMI

 drivers/gpu/drm/display/drm_bridge_connector.c  |   3 +
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 156 +++++++++++++
 drivers/gpu/drm/display/drm_scdc_helper.c       | 285 +++++++++++++++++++++++-
 drivers/gpu/drm/drm_debugfs.c                   | 157 -------------
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c          |   1 +
 drivers/gpu/drm/vc4/vc4_hdmi.c                  |   1 +
 include/drm/display/drm_hdmi_state_helper.h     |   3 +
 include/drm/display/drm_scdc.h                  |  21 +-
 include/drm/display/drm_scdc_helper.h           | 103 ++++++++-
 9 files changed, 564 insertions(+), 166 deletions(-)
---
base-commit: 97aed8a757dcf4d64b13c6387637821a2888586e
change-id: 20260413-scdc-link-health-89326013d96c

Best regards,
--  
Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

[PATCH v7 1/5] drm/scdc-helper: Don't use ssize_t return type for scdc_read/write

[Nicolas Frattaroli]

drm_scdc_read and drm_scdc_write, both of which are only used within
drm_scdc_helper (although exported), use a ssize_t as their return type.

This would make sense if they returned the number of bytes read/written
on success, and negative errno otherwise. However, they return 0 on
success.

Demote them to "int" as their return type, in order to avoid needlessly
using 64 bits when less suffices.

No functional change.

Reviewed-by: Luca Ceresoli <luca.ceresoli@bootlin.com>
Reviewed-by: Hans Verkuil <hverkuil+cisco@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_scdc_helper.c | 8 ++++----
 include/drm/display/drm_scdc_helper.h     | 8 ++++----
 2 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
index df878aad4a36..8403f2390ab6 100644
--- a/drivers/gpu/drm/display/drm_scdc_helper.c
+++ b/drivers/gpu/drm/display/drm_scdc_helper.c
@@ -67,8 +67,8 @@
  * Returns:
  * 0 on success, negative error code on failure.
  */
-ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
-		      size_t size)
+int drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
+		  size_t size)
 {
 	int ret;
 	struct i2c_msg msgs[2] = {
@@ -107,8 +107,8 @@ EXPORT_SYMBOL(drm_scdc_read);
  * Returns:
  * 0 on success, negative error code on failure.
  */
-ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
-		       const void *buffer, size_t size)
+int drm_scdc_write(struct i2c_adapter *adapter, u8 offset, const void *buffer,
+		   size_t size)
 {
 	struct i2c_msg msg = {
 		.addr = SCDC_I2C_SLAVE_ADDRESS,
diff --git a/include/drm/display/drm_scdc_helper.h b/include/drm/display/drm_scdc_helper.h
index 34600476a1b9..e9ccaeba56dd 100644
--- a/include/drm/display/drm_scdc_helper.h
+++ b/include/drm/display/drm_scdc_helper.h
@@ -31,10 +31,10 @@
 struct drm_connector;
 struct i2c_adapter;
 
-ssize_t drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
-		      size_t size);
-ssize_t drm_scdc_write(struct i2c_adapter *adapter, u8 offset,
-		       const void *buffer, size_t size);
+int drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
+		  size_t size);
+int drm_scdc_write(struct i2c_adapter *adapter, u8 offset, const void *buffer,
+		   size_t size);
 
 /**
  * drm_scdc_readb - read a single byte from SCDC

-- 
2.55.0

[PATCH v7 2/5] drm/scdc-helper: Add scdc_status debugfs entry

[Nicolas Frattaroli]

SCDC provides status information on the current display link. At the
very least, it may be useful to expose this info through debugfs.

Add a debugfs entry for it under the connector, which displays a few
more details parsed out of the SCDC registers. A new
drm_scdc_debugfs_init function can be called by the connector
implementation to initialise the debugfs file.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_scdc_helper.c | 184 ++++++++++++++++++++++++++++++
 include/drm/display/drm_scdc_helper.h     |  32 ++++++
 2 files changed, 216 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
index 8403f2390ab6..5871fc101815 100644
--- a/drivers/gpu/drm/display/drm_scdc_helper.c
+++ b/drivers/gpu/drm/display/drm_scdc_helper.c
@@ -24,11 +24,14 @@
 #include <linux/export.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
+#include <linux/debugfs.h>
 #include <linux/delay.h>
+#include <linux/overflow.h>
 
 #include <drm/display/drm_scdc_helper.h>
 #include <drm/drm_connector.h>
 #include <drm/drm_device.h>
+#include <drm/drm_managed.h>
 #include <drm/drm_print.h>
 
 /**
@@ -55,6 +58,11 @@
 
 #define SCDC_I2C_SLAVE_ADDRESS 0x54
 
+struct scdc_debugfs_priv {
+	struct drm_connector *connector;
+	struct drm_scdc_state state;
+};
+
 /**
  * drm_scdc_read - read a block of data from SCDC
  * @adapter: I2C controller
@@ -276,3 +284,179 @@ bool drm_scdc_set_high_tmds_clock_ratio(struct drm_connector *connector,
 	return true;
 }
 EXPORT_SYMBOL(drm_scdc_set_high_tmds_clock_ratio);
+
+static void
+drm_scdc_parse_status0_flags(u8 val, struct drm_scdc_status_flags *flags)
+{
+	flags->clock_detected = val & SCDC_CLOCK_DETECT;
+	flags->ch0_locked = val & SCDC_CH0_LOCK;
+	flags->ch1_locked = val & SCDC_CH1_LOCK;
+	flags->ch2_locked = val & SCDC_CH2_LOCK;
+}
+
+static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[3])
+{
+	u8 sum = 0;
+	int i;
+
+	for (i = SCDC_ERR_DET_0_L; i <= SCDC_ERR_DET_CHECKSUM ; i++)
+		sum = wrapping_add(u8, sum, scdc[i]);
+
+	if (sum)
+		return -EPROTO;
+
+	for (i = 0; i < 3; i++) {
+		if (scdc[SCDC_ERR_DET_0_H + i * 2] & SCDC_CHANNEL_VALID)
+			counter[i] =  (scdc[SCDC_ERR_DET_0_H + i * 2] &
+				       ~SCDC_CHANNEL_VALID) << 8 |
+				       scdc[SCDC_ERR_DET_0_L + i * 2];
+		else
+			counter[i] = 0;
+	}
+
+	return 0;
+}
+
+/**
+ * drm_scdc_read_state - Update state from SCDC
+ * @connector: pointer to a &struct drm_connector on which to operate on
+ * @state: pointer to a &struct drm_scdc_state to fill
+ *
+ * Reads the entire 256 byte SCDC state and parses it.
+ *
+ * Returns: %0 on success, negative errno on failure.
+ */
+int drm_scdc_read_state(struct drm_connector *connector, struct drm_scdc_state *state)
+{
+	struct i2c_adapter *ddc;
+	struct drm_scdc *scdc;
+	u8 *buf = state->scdc;
+	int ret;
+
+	if (!state || !connector)
+		return -ENODEV;
+
+	scdc = &connector->display_info.hdmi.scdc;
+	ddc = connector->ddc;
+
+	if (!scdc->supported)
+		return -EOPNOTSUPP;
+
+	/* Read in 128-byte chunks, to work around DP<->HDMI converters with issues. */
+	ret = drm_scdc_read(ddc, 0, buf, 128);
+	if (ret)
+		return ret;
+
+	ret = drm_scdc_read(ddc, 128, &buf[128], 128);
+	if (ret)
+		return ret;
+
+	state->scrambling_enabled = buf[SCDC_TMDS_CONFIG] & SCDC_SCRAMBLING_ENABLE;
+	state->tmds_bclk_x40 = buf[SCDC_TMDS_CONFIG] & SCDC_TMDS_BIT_CLOCK_RATIO_BY_40;
+
+	state->scrambling_detected = buf[SCDC_SCRAMBLER_STATUS] & SCDC_SCRAMBLING_STATUS;
+
+	drm_scdc_parse_status0_flags(buf[SCDC_STATUS_FLAGS_0], &state->stf);
+	ret = drm_scdc_parse_error_counters(buf, state->error_count);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+EXPORT_SYMBOL(drm_scdc_read_state);
+
+#define scdc_print_str(_f, key, s) \
+	(seq_printf((_f), "%-30s: %s\n", (key), (s)))
+#define scdc_print_flag(_f, key, val) \
+	(scdc_print_str((_f), (key), str_yes_no((val))))
+#define scdc_print_dec(_f, key, val) \
+	(seq_printf((_f), "%-30s: %d\n", (key), (val)))
+
+static int scdc_status_show(struct seq_file *m, void *data)
+{
+	struct scdc_debugfs_priv *priv = m->private;
+	struct drm_scdc_state *st = &priv->state;
+	struct drm_connector *connector = priv->connector;
+	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
+	int i, ret;
+
+	drm_connector_get(connector);
+
+	if (connector->status != connector_status_connected) {
+		ret = -ENODEV;
+		goto err_conn_put;
+	}
+
+	if (scdc->supported) {
+		ret = drm_scdc_read_state(connector, st);
+		if (ret)
+			goto err_conn_put;
+
+		for (i = 0; i < ARRAY_SIZE(st->scdc); i += 16)
+			seq_printf(m, "%*ph\n", 16, &st->scdc[i]);
+
+		seq_puts(m, "\n----------------\n\n");
+	}
+
+	scdc_print_flag(m, "SCDC Supported", scdc->supported);
+	if (!scdc->supported) {
+		ret = 0;
+		goto err_conn_put;
+	}
+
+	scdc_print_flag(m, "Sink Read Request Capable", scdc->read_request);
+	scdc_print_flag(m, "Scrambling Supported", scdc->scrambling.supported);
+	scdc_print_flag(m, "Low Rate Scrambling Supported", scdc->scrambling.low_rates);
+
+	drm_connector_put(connector);
+
+	scdc_print_flag(m, "Scrambling Enabled", st->scrambling_enabled);
+	scdc_print_flag(m, "Scrambling Detected", st->scrambling_detected);
+
+	if (st->tmds_bclk_x40)
+		scdc_print_str(m, "TMDS Bit Clock Ratio", "1/40");
+	else
+		scdc_print_str(m, "TMDS Bit Clock Ratio", "1/10");
+
+	scdc_print_flag(m, "Clock Detected", st->stf.clock_detected);
+	scdc_print_flag(m, "Channel 0 Locked", st->stf.ch0_locked);
+	scdc_print_flag(m, "Channel 1 Locked", st->stf.ch1_locked);
+	scdc_print_flag(m, "Channel 2 Locked", st->stf.ch2_locked);
+
+	scdc_print_dec(m, "Channel 0 Errors", st->error_count[0]);
+	scdc_print_dec(m, "Channel 1 Errors", st->error_count[1]);
+	scdc_print_dec(m, "Channel 2 Errors", st->error_count[2]);
+
+	return 0;
+
+err_conn_put:
+	drm_connector_put(connector);
+
+	return ret;
+}
+DEFINE_SHOW_ATTRIBUTE(scdc_status);
+
+/**
+ * drm_scdc_debugfs_init - Initialize scdc files in connector debugfs
+ * @connector: pointer to &struct drm_connector to operate on
+ * @root: debugfs &struct dentry for the debugfs root of @connector
+ *
+ * Creates SCDC-related debugfs files for @connector. Must be called after
+ * @root is already created.
+ */
+void drm_scdc_debugfs_init(struct drm_connector *connector, struct dentry *root)
+{
+	struct scdc_debugfs_priv *priv;
+
+	if (!root || !connector)
+		return;
+
+	priv = drmm_kzalloc(connector->dev, sizeof(*priv), GFP_KERNEL);
+	if (!priv)
+		return;
+
+	priv->connector = connector;
+
+	debugfs_create_file("scdc_status", 0444, root, priv, &scdc_status_fops);
+}
+EXPORT_SYMBOL(drm_scdc_debugfs_init);
diff --git a/include/drm/display/drm_scdc_helper.h b/include/drm/display/drm_scdc_helper.h
index e9ccaeba56dd..e0b79d79e1ff 100644
--- a/include/drm/display/drm_scdc_helper.h
+++ b/include/drm/display/drm_scdc_helper.h
@@ -30,6 +30,34 @@
 
 struct drm_connector;
 struct i2c_adapter;
+struct dentry;
+
+struct drm_scdc_status_flags {
+	/* Status Register 0 */
+	bool clock_detected;
+	bool ch0_locked;
+	bool ch1_locked;
+	bool ch2_locked;
+};
+
+struct drm_scdc_state {
+	/** @stf: contents of the status flag registers */
+	struct drm_scdc_status_flags stf;
+	/** @scramling_enabled: true if TMDS scrambling is on */
+	bool scrambling_enabled;
+	/** @scrambling_detected: true if the sink actually detected scrambling */
+	bool scrambling_detected;
+	/**
+	 * @tmds_bclk_x40: true if TMDS bit period is 1/40th of the TMDS
+	 * clock period, false if it's 1/10th of the clock period.
+	 */
+	bool tmds_bclk_x40;
+	/** @error_count: character error counts for each channel */
+	u16 error_count[3];
+
+	/** @scdc: raw SCDC data buffer */
+	u8 scdc[256];
+};
 
 int drm_scdc_read(struct i2c_adapter *adapter, u8 offset, void *buffer,
 		  size_t size);
@@ -77,4 +105,8 @@ bool drm_scdc_get_scrambling_status(struct drm_connector *connector);
 bool drm_scdc_set_scrambling(struct drm_connector *connector, bool enable);
 bool drm_scdc_set_high_tmds_clock_ratio(struct drm_connector *connector, bool set);
 
+int drm_scdc_read_state(struct drm_connector *connector,
+			struct drm_scdc_state *state);
+void drm_scdc_debugfs_init(struct drm_connector *connector, struct dentry *root);
+
 #endif

-- 
2.55.0

Re: [PATCH v7 2/5] drm/scdc-helper: Add scdc_status debugfs entry

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Shared `scdc_debugfs_priv` state causes a data race on concurrent debugfs reads and leaks memory for dynamic connectors.
- [Medium] Connector `status` and `display_info` fields are accessed without holding the required DRM mode config mutex.
--

--- Patch [2]: [PATCH v7 2/5] drm/scdc-helper: Add scdc_status debugfs entry ---
commit 16b712ae50ae43d528857efe5b800f5abdef139f
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

drm/scdc-helper: Add scdc_status debugfs entry

This commit adds a debugfs entry under the DRM connector to expose SCDC link
status information.

> diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> index 8403f2390ab6a..5871fc101815d 100644
> --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
[ ... ]
> @@ -276,3 +284,179 @@ bool drm_scdc_set_high_tmds_clock_ratio(struct drm_connector *connector,
[ ... ]
> +static int scdc_status_show(struct seq_file *m, void *data)
> +{
> +	struct scdc_debugfs_priv *priv = m->private;
> +	struct drm_scdc_state *st = &priv->state;
> +	struct drm_connector *connector = priv->connector;
> +	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
> +	int i, ret;
> +
> +	drm_connector_get(connector);
> +
> +	if (connector->status != connector_status_connected) {

[Severity: Medium]
Are we missing a lock here? Reading connector->status and 
connector->display_info (such as scdc->supported below) without holding the 
required drm_mode_config.mutex might race with concurrent modeset or hotplug 
events, potentially leading to torn reads or an inconsistent state.

> +		ret = -ENODEV;
> +		goto err_conn_put;
> +	}
> +
> +	if (scdc->supported) {
> +		ret = drm_scdc_read_state(connector, st);

[Severity: High]
Can this cause a data race? Multiple processes reading this debugfs file
concurrently will simultaneously mutate the shared priv->state buffer inside 
drm_scdc_read_state(), leading to corrupted state data being displayed.

> +		if (ret)
> +			goto err_conn_put;
> +
> +		for (i = 0; i < ARRAY_SIZE(st->scdc); i += 16)
> +			seq_printf(m, "%*ph\n", 16, &st->scdc[i]);
> +
> +		seq_puts(m, "\n----------------\n\n");
> +	}
[ ... ]
> +void drm_scdc_debugfs_init(struct drm_connector *connector, struct dentry *root)
> +{
> +	struct scdc_debugfs_priv *priv;
> +
> +	if (!root || !connector)
> +		return;
> +
> +	priv = drmm_kzalloc(connector->dev, sizeof(*priv), GFP_KERNEL);

[Severity: High]
Does this leak memory on hot-unplug? Allocating the priv structure using 
drmm_kzalloc() ties its lifetime to the DRM device rather than the connector. 
For dynamic hotpluggable connectors (like DP MST), this memory might not be 
freed when the connector is destroyed.

> +	if (!priv)
> +		return;
> +
> +	priv->connector = connector;
> +
> +	debugfs_create_file("scdc_status", 0444, root, priv, &scdc_status_fops);
> +}
> +EXPORT_SYMBOL(drm_scdc_debugfs_init);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-scdc-link-health-v7-0-92df1b6ad5fc@collabora.com?part=2

Re: [PATCH v7 2/5] drm/scdc-helper: Add scdc_status debugfs entry

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 1614 bytes --]

On Tue, Jul 21, 2026 at 10:58:58AM +0200, Nicolas Frattaroli wrote:
> SCDC provides status information on the current display link. At the
> very least, it may be useful to expose this info through debugfs.
>
> Add a debugfs entry for it under the connector, which displays a few
> more details parsed out of the SCDC registers. A new
> drm_scdc_debugfs_init function can be called by the connector
> implementation to initialise the debugfs file.
>
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> ---
>  drivers/gpu/drm/display/drm_scdc_helper.c | 184 ++++++++++++++++++++++++++++++
>  include/drm/display/drm_scdc_helper.h     |  32 ++++++
>  2 files changed, 216 insertions(+)
>
> diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> +/**
> + * drm_scdc_debugfs_init - Initialize scdc files in connector debugfs
> + * @connector: pointer to &struct drm_connector to operate on
> + * @root: debugfs &struct dentry for the debugfs root of @connector
> + *
> + * Creates SCDC-related debugfs files for @connector. Must be called after
> + * @root is already created.
> + */
> +void drm_scdc_debugfs_init(struct drm_connector *connector, struct dentry *root)
> +{
> +	struct scdc_debugfs_priv *priv;
> +
> +	if (!root || !connector)
> +		return;
> +
> +	priv = drmm_kzalloc(connector->dev, sizeof(*priv), GFP_KERNEL);
> +	if (!priv)
> +		return;
> +
> +	priv->connector = connector;

You need to take a reference to the connector pointer here with
drm_connector_get()

With this fixed,
Reviewed-by: Maxime Ripard <mripard@kernel.org>

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

Re: [PATCH v7 2/5] drm/scdc-helper: Add scdc_status debugfs entry

[Nicolas Frattaroli]

On Tuesday, 21 July 2026 14:41:42 Central European Summer Time Maxime Ripard wrote:
> On Tue, Jul 21, 2026 at 10:58:58AM +0200, Nicolas Frattaroli wrote:
> > SCDC provides status information on the current display link. At the
> > very least, it may be useful to expose this info through debugfs.
> >
> > Add a debugfs entry for it under the connector, which displays a few
> > more details parsed out of the SCDC registers. A new
> > drm_scdc_debugfs_init function can be called by the connector
> > implementation to initialise the debugfs file.
> >
> > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > ---
> >  drivers/gpu/drm/display/drm_scdc_helper.c | 184 ++++++++++++++++++++++++++++++
> >  include/drm/display/drm_scdc_helper.h     |  32 ++++++
> >  2 files changed, 216 insertions(+)
> >
> > diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> > +/**
> > + * drm_scdc_debugfs_init - Initialize scdc files in connector debugfs
> > + * @connector: pointer to &struct drm_connector to operate on
> > + * @root: debugfs &struct dentry for the debugfs root of @connector
> > + *
> > + * Creates SCDC-related debugfs files for @connector. Must be called after
> > + * @root is already created.
> > + */
> > +void drm_scdc_debugfs_init(struct drm_connector *connector, struct dentry *root)
> > +{
> > +	struct scdc_debugfs_priv *priv;
> > +
> > +	if (!root || !connector)
> > +		return;
> > +
> > +	priv = drmm_kzalloc(connector->dev, sizeof(*priv), GFP_KERNEL);
> > +	if (!priv)
> > +		return;
> > +
> > +	priv->connector = connector;
> 
> You need to take a reference to the connector pointer here with
> drm_connector_get()
> 
> With this fixed,
> Reviewed-by: Maxime Ripard <mripard@kernel.org>
> 
> Maxime
> 

I think sashiko is onto something here with the memleak and the
entirety of the priv thing should go, which is fairly easy to do.
We don't reuse the state anymore anyways.

I'll then just pass the connector as the priv, which will be valid
for the lifetime of the debugfs entry. This means there's no cleanup.

Kind regards,
Nicolas Frattaroli

Re: [PATCH v7 2/5] drm/scdc-helper: Add scdc_status debugfs entry

[Maxime Ripard]

[-- Attachment #1: Type: text/plain, Size: 2448 bytes --]

On Tue, Jul 21, 2026 at 03:01:12PM +0200, Nicolas Frattaroli wrote:
> On Tuesday, 21 July 2026 14:41:42 Central European Summer Time Maxime Ripard wrote:
> > On Tue, Jul 21, 2026 at 10:58:58AM +0200, Nicolas Frattaroli wrote:
> > > SCDC provides status information on the current display link. At the
> > > very least, it may be useful to expose this info through debugfs.
> > >
> > > Add a debugfs entry for it under the connector, which displays a few
> > > more details parsed out of the SCDC registers. A new
> > > drm_scdc_debugfs_init function can be called by the connector
> > > implementation to initialise the debugfs file.
> > >
> > > Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> > > ---
> > >  drivers/gpu/drm/display/drm_scdc_helper.c | 184 ++++++++++++++++++++++++++++++
> > >  include/drm/display/drm_scdc_helper.h     |  32 ++++++
> > >  2 files changed, 216 insertions(+)
> > >
> > > diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> > > +/**
> > > + * drm_scdc_debugfs_init - Initialize scdc files in connector debugfs
> > > + * @connector: pointer to &struct drm_connector to operate on
> > > + * @root: debugfs &struct dentry for the debugfs root of @connector
> > > + *
> > > + * Creates SCDC-related debugfs files for @connector. Must be called after
> > > + * @root is already created.
> > > + */
> > > +void drm_scdc_debugfs_init(struct drm_connector *connector, struct dentry *root)
> > > +{
> > > +	struct scdc_debugfs_priv *priv;
> > > +
> > > +	if (!root || !connector)
> > > +		return;
> > > +
> > > +	priv = drmm_kzalloc(connector->dev, sizeof(*priv), GFP_KERNEL);
> > > +	if (!priv)
> > > +		return;
> > > +
> > > +	priv->connector = connector;
> > 
> > You need to take a reference to the connector pointer here with
> > drm_connector_get()
> > 
> > With this fixed,
> > Reviewed-by: Maxime Ripard <mripard@kernel.org>
> > 
> > Maxime
> > 
> 
> I think sashiko is onto something here with the memleak and the
> entirety of the priv thing should go, which is fairly easy to do.
> We don't reuse the state anymore anyways.
> 
> I'll then just pass the connector as the priv, which will be valid
> for the lifetime of the debugfs entry. This means there's no cleanup.

Connectors can be hotplugged, so they have a shorter (or equal) lifetime
than what drmm_kzalloc provides.

Maxime

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 273 bytes --]

[PATCH v7 3/5] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

[Nicolas Frattaroli]

HDMI 2.1 redefines previously reserved fields in SCDC for various new
uses. No version check needs to be performed, as an HDMI 2.0 sink's
reserved SCDC fields are well-defined to be 0, and any zero-ness of
these fields for an HDMI 2.0 sink is not a surprise for SCDC parsers for
HDMI 2.1.

Implement reading and outputting these fields over debugfs.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_scdc_helper.c | 99 ++++++++++++++++++++++++++++++-
 include/drm/display/drm_scdc.h            | 21 ++++++-
 include/drm/display/drm_scdc_helper.h     | 69 ++++++++++++++++++++-
 3 files changed, 182 insertions(+), 7 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
index 5871fc101815..7e17272d52f2 100644
--- a/drivers/gpu/drm/display/drm_scdc_helper.c
+++ b/drivers/gpu/drm/display/drm_scdc_helper.c
@@ -21,6 +21,7 @@
  * DEALINGS IN THE SOFTWARE.
  */
 
+#include <linux/bitfield.h>
 #include <linux/export.h>
 #include <linux/i2c.h>
 #include <linux/slab.h>
@@ -63,6 +64,38 @@ struct scdc_debugfs_priv {
 	struct drm_scdc_state state;
 };
 
+static const char *drm_scdc_frl_rate_str(enum drm_scdc_frl_rate rate)
+{
+	switch (rate) {
+	case SCDC_FRL_RATE_OFF:
+		return "Off";
+	case SCDC_FRL_RATE_3X3:
+		return "3 Gbit/s x 3 lanes";
+	case SCDC_FRL_RATE_6X3:
+		return "6 Gbit/s x 3 lanes";
+	case SCDC_FRL_RATE_6X4:
+		return "6 Gbit/s x 4 lanes";
+	case SCDC_FRL_RATE_8X4:
+		return "8 Gbit/s x 4 lanes";
+	case SCDC_FRL_RATE_10X4:
+		return "10 Gbit/s x 4 lanes";
+	case SCDC_FRL_RATE_12X4:
+		return "12 Gbit/s x 4 lanes";
+	case SCDC_FRL_RATE_RESV_7:
+	case SCDC_FRL_RATE_RESV_8:
+	case SCDC_FRL_RATE_RESV_9:
+	case SCDC_FRL_RATE_RESV_10:
+	case SCDC_FRL_RATE_RESV_11:
+	case SCDC_FRL_RATE_RESV_12:
+	case SCDC_FRL_RATE_RESV_13:
+	case SCDC_FRL_RATE_RESV_14:
+	case SCDC_FRL_RATE_RESV_15:
+		return "(Reserved)";
+	default:
+		return NULL;
+	}
+}
+
 /**
  * drm_scdc_read - read a block of data from SCDC
  * @adapter: I2C controller
@@ -292,14 +325,41 @@ drm_scdc_parse_status0_flags(u8 val, struct drm_scdc_status_flags *flags)
 	flags->ch0_locked = val & SCDC_CH0_LOCK;
 	flags->ch1_locked = val & SCDC_CH1_LOCK;
 	flags->ch2_locked = val & SCDC_CH2_LOCK;
+	flags->ln3_locked = val & SCDC_LN3_LOCK;
+	flags->flt_ready = val & SCDC_FLT_READY;
+	flags->dsc_fail = val & SCDC_DSC_FAIL;
+}
+
+static void
+drm_scdc_parse_status1_2_flags(u8 val_flag1, u8 val_flag2,
+			       struct drm_scdc_status_flags *flags)
+{
+	flags->ln0_training_pattern = FIELD_GET(SCDC_LN_EVEN_TRAIN_PTRN, val_flag1);
+	flags->ln1_training_pattern = FIELD_GET(SCDC_LN_ODD_TRAIN_PTRN, val_flag1);
+
+	flags->ln2_training_pattern = FIELD_GET(SCDC_LN_EVEN_TRAIN_PTRN, val_flag2);
+	flags->ln3_training_pattern = FIELD_GET(SCDC_LN_ODD_TRAIN_PTRN, val_flag2);
 }
 
-static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[3])
+static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[4],
+					 unsigned int num_lanes)
 {
+	u8 end_reg;
 	u8 sum = 0;
 	int i;
 
-	for (i = SCDC_ERR_DET_0_L; i <= SCDC_ERR_DET_CHECKSUM ; i++)
+	switch (num_lanes) {
+	case 3:
+		end_reg = SCDC_ERR_DET_CHECKSUM;
+		break;
+	case 4:
+		end_reg = SCDC_ERR_DET_3_H;
+		break;
+	default:
+		return -EINVAL;
+	}
+
+	for (i = SCDC_ERR_DET_0_L; i <= end_reg; i++)
 		sum = wrapping_add(u8, sum, scdc[i]);
 
 	if (sum)
@@ -314,6 +374,12 @@ static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[3])
 			counter[i] = 0;
 	}
 
+	if (num_lanes == 4 && scdc[SCDC_ERR_DET_3_H] & SCDC_CHANNEL_VALID)
+		counter[3] =  (scdc[SCDC_ERR_DET_3_H] & ~SCDC_CHANNEL_VALID) << 8 |
+			      scdc[SCDC_ERR_DET_3_L];
+	else
+		counter[3] = 0;
+
 	return 0;
 }
 
@@ -331,6 +397,7 @@ int drm_scdc_read_state(struct drm_connector *connector, struct drm_scdc_state *
 	struct i2c_adapter *ddc;
 	struct drm_scdc *scdc;
 	u8 *buf = state->scdc;
+	int num_lanes;
 	int ret;
 
 	if (!state || !connector)
@@ -356,11 +423,26 @@ int drm_scdc_read_state(struct drm_connector *connector, struct drm_scdc_state *
 
 	state->scrambling_detected = buf[SCDC_SCRAMBLER_STATUS] & SCDC_SCRAMBLING_STATUS;
 
+	state->rate = FIELD_GET(SCDC_FRL_RATE, buf[SCDC_CONFIG_1]);
+	num_lanes = drm_scdc_num_frl_lanes(state->rate);
+	if (num_lanes < 0)
+		return num_lanes;
+	if (!num_lanes)
+		num_lanes = 3;
+
+	state->ffe_levels = FIELD_GET(SCDC_FFE_LEVELS, buf[SCDC_CONFIG_1]);
+
 	drm_scdc_parse_status0_flags(buf[SCDC_STATUS_FLAGS_0], &state->stf);
-	ret = drm_scdc_parse_error_counters(buf, state->error_count);
+	drm_scdc_parse_status1_2_flags(buf[SCDC_STATUS_FLAGS_1],
+				       buf[SCDC_STATUS_FLAGS_2], &state->stf);
+	ret = drm_scdc_parse_error_counters(buf, state->error_count, num_lanes);
 	if (ret)
 		return ret;
 
+	if (num_lanes == 4 && (buf[SCDC_ERR_DET_RS_H] & SCDC_CHANNEL_VALID))
+		state->rs_corrections = (buf[SCDC_ERR_DET_RS_H] & ~SCDC_CHANNEL_VALID) << 8 |
+					buf[SCDC_ERR_DET_RS_L];
+
 	return 0;
 }
 EXPORT_SYMBOL(drm_scdc_read_state);
@@ -412,6 +494,8 @@ static int scdc_status_show(struct seq_file *m, void *data)
 
 	scdc_print_flag(m, "Scrambling Enabled", st->scrambling_enabled);
 	scdc_print_flag(m, "Scrambling Detected", st->scrambling_detected);
+	scdc_print_str(m, "FRL Rate", drm_scdc_frl_rate_str(st->rate));
+	scdc_print_dec(m, "FFE Levels", st->ffe_levels);
 
 	if (st->tmds_bclk_x40)
 		scdc_print_str(m, "TMDS Bit Clock Ratio", "1/40");
@@ -422,10 +506,19 @@ static int scdc_status_show(struct seq_file *m, void *data)
 	scdc_print_flag(m, "Channel 0 Locked", st->stf.ch0_locked);
 	scdc_print_flag(m, "Channel 1 Locked", st->stf.ch1_locked);
 	scdc_print_flag(m, "Channel 2 Locked", st->stf.ch2_locked);
+	if (drm_scdc_num_frl_lanes(st->rate) == 4)
+		scdc_print_flag(m, "Lane 3 Locked", st->stf.ln3_locked);
+
+	scdc_print_flag(m, "Sink Ready For Link Training", st->stf.flt_ready);
+	scdc_print_flag(m, "Sink Failed To Decode DSC", st->stf.dsc_fail);
 
 	scdc_print_dec(m, "Channel 0 Errors", st->error_count[0]);
 	scdc_print_dec(m, "Channel 1 Errors", st->error_count[1]);
 	scdc_print_dec(m, "Channel 2 Errors", st->error_count[2]);
+	if (drm_scdc_num_frl_lanes(st->rate) == 4) {
+		scdc_print_dec(m, "Lane 3 Errors", st->error_count[3]);
+		scdc_print_dec(m, "Reed-Solomon Corrections", st->rs_corrections);
+	}
 
 	return 0;
 
diff --git a/include/drm/display/drm_scdc.h b/include/drm/display/drm_scdc.h
index 3d58f37e8ed8..7f0b05b2f280 100644
--- a/include/drm/display/drm_scdc.h
+++ b/include/drm/display/drm_scdc.h
@@ -29,6 +29,8 @@
 #define SCDC_SOURCE_VERSION 0x02
 
 #define SCDC_UPDATE_0 0x10
+#define  SCDC_RSED_UPDATE (1 << 6)
+#define  SCDC_FLT_UPDATE (1 << 5)
 #define  SCDC_READ_REQUEST_TEST (1 << 2)
 #define  SCDC_CED_UPDATE (1 << 1)
 #define  SCDC_STATUS_UPDATE (1 << 0)
@@ -46,14 +48,25 @@
 #define SCDC_CONFIG_0 0x30
 #define  SCDC_READ_REQUEST_ENABLE (1 << 0)
 
+#define SCDC_CONFIG_1 0x31
+#define  SCDC_FRL_RATE   0x0f
+#define  SCDC_FFE_LEVELS 0xf0
+
 #define SCDC_STATUS_FLAGS_0 0x40
+#define  SCDC_DSC_FAIL (1 << 7)
+#define  SCDC_FLT_READY (1 << 6)
+#define  SCDC_LN3_LOCK (1 << 4)
 #define  SCDC_CH2_LOCK (1 << 3)
 #define  SCDC_CH1_LOCK (1 << 2)
 #define  SCDC_CH0_LOCK (1 << 1)
-#define  SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK)
+#define  SCDC_CH_LOCK_MASK (SCDC_LN3_LOCK | SCDC_CH2_LOCK | SCDC_CH1_LOCK | \
+			    SCDC_CH0_LOCK)
 #define  SCDC_CLOCK_DETECT (1 << 0)
 
 #define SCDC_STATUS_FLAGS_1 0x41
+#define  SCDC_LN_EVEN_TRAIN_PTRN 0x0f
+#define  SCDC_LN_ODD_TRAIN_PTRN 0xf0
+#define SCDC_STATUS_FLAGS_2 0x42
 
 #define SCDC_ERR_DET_0_L 0x50
 #define SCDC_ERR_DET_0_H 0x51
@@ -65,6 +78,12 @@
 
 #define SCDC_ERR_DET_CHECKSUM 0x56
 
+#define SCDC_ERR_DET_3_L 0x57
+#define SCDC_ERR_DET_3_H 0x58
+
+#define SCDC_ERR_DET_RS_L 0x59
+#define SCDC_ERR_DET_RS_H 0x5a
+
 #define SCDC_TEST_CONFIG_0 0xc0
 #define  SCDC_TEST_READ_REQUEST (1 << 7)
 #define  SCDC_TEST_READ_REQUEST_DELAY(x) ((x) & 0x7f)
diff --git a/include/drm/display/drm_scdc_helper.h b/include/drm/display/drm_scdc_helper.h
index e0b79d79e1ff..a3b20adaac7e 100644
--- a/include/drm/display/drm_scdc_helper.h
+++ b/include/drm/display/drm_scdc_helper.h
@@ -24,6 +24,7 @@
 #ifndef DRM_SCDC_HELPER_H
 #define DRM_SCDC_HELPER_H
 
+#include <linux/errno.h>
 #include <linux/types.h>
 
 #include <drm/display/drm_scdc.h>
@@ -38,8 +39,65 @@ struct drm_scdc_status_flags {
 	bool ch0_locked;
 	bool ch1_locked;
 	bool ch2_locked;
+	bool ln3_locked;
+	bool flt_ready;
+	bool dsc_fail;
+
+	/* Status Register 1 */
+	u8 ln0_training_pattern : 4;
+	u8 ln1_training_pattern : 4;
+
+	/* Status Register 2 */
+	u8 ln2_training_pattern : 4;
+	u8 ln3_training_pattern : 4;
+};
+
+enum drm_scdc_frl_rate {
+	SCDC_FRL_RATE_OFF = 0,
+	SCDC_FRL_RATE_3X3 = 1,
+	SCDC_FRL_RATE_6X3 = 2,
+	SCDC_FRL_RATE_6X4 = 3,
+	SCDC_FRL_RATE_8X4 = 4,
+	SCDC_FRL_RATE_10X4 = 5,
+	SCDC_FRL_RATE_12X4 = 6,
+	SCDC_FRL_RATE_RESV_7 = 7,
+	SCDC_FRL_RATE_RESV_8 = 8,
+	SCDC_FRL_RATE_RESV_9 = 9,
+	SCDC_FRL_RATE_RESV_10 = 10,
+	SCDC_FRL_RATE_RESV_11 = 11,
+	SCDC_FRL_RATE_RESV_12 = 12,
+	SCDC_FRL_RATE_RESV_13 = 13,
+	SCDC_FRL_RATE_RESV_14 = 14,
+	SCDC_FRL_RATE_RESV_15 = 15
 };
 
+/**
+ * drm_scdc_num_frl_lanes - get number of lanes for a given FRL rate
+ * @rate: one of &enum drm_scdc_frl_rate
+ *
+ * For a given @rate, return the number of lanes it uses.
+ *
+ * Returns: %-EINVAL if @rate is not a valid FRL rate, or the number of lanes
+ * for a given &enum drm_scdc_frl_rate on success (including %0 for "off")
+ */
+static inline __pure int drm_scdc_num_frl_lanes(enum drm_scdc_frl_rate rate)
+{
+	switch (rate) {
+	case SCDC_FRL_RATE_OFF:
+		return 0;
+	case SCDC_FRL_RATE_3X3:
+	case SCDC_FRL_RATE_6X3:
+		return 3;
+	case SCDC_FRL_RATE_6X4:
+	case SCDC_FRL_RATE_8X4:
+	case SCDC_FRL_RATE_10X4:
+	case SCDC_FRL_RATE_12X4:
+		return 4;
+	default:
+		return -EINVAL;
+	}
+}
+
 struct drm_scdc_state {
 	/** @stf: contents of the status flag registers */
 	struct drm_scdc_status_flags stf;
@@ -52,9 +110,14 @@ struct drm_scdc_state {
 	 * clock period, false if it's 1/10th of the clock period.
 	 */
 	bool tmds_bclk_x40;
-	/** @error_count: character error counts for each channel */
-	u16 error_count[3];
-
+	/** @rate: FRL rate set by the source */
+	enum drm_scdc_frl_rate rate : 4;
+	/** @ffe_levels: The FFE levels for @rate set by the source */
+	u8 ffe_levels : 4;
+	/** @error_count: character error counts for each channel/link */
+	u16 error_count[4];
+	/** @rs_corrections: number of Reed-Solomon Corrections */
+	u16 rs_corrections;
 	/** @scdc: raw SCDC data buffer */
 	u8 scdc[256];
 };

-- 
2.55.0

Re: [PATCH v7 3/5] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [High] Incorrect checksum validation range breaks 4-lane FRL error parsing.
- [High] Redefinition of `SCDC_CH_LOCK_MASK` breaks TMDS lock checking for HDMI 2.0 sinks.
- [Medium] Stale state leakage for `rs_corrections` across debugfs reads.
- [Medium] Reserved FRL rates cause debugfs read failures and unreachable dead code.
- [Medium] Concurrent debugfs reads cause a data race on `priv->state`.
--

--- Patch [3]: [PATCH v7 3/5] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields ---
commit c31217f1c2c58d659c6532a48bf298ea495b08b1
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

HDMI 2.1 redefines previously reserved fields in SCDC for various new uses.
Implement reading and outputting these fields over debugfs.

> diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> index 5871fc101815d..7e17272d52f23 100644
> --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> +++ b/drivers/gpu/drm/display/drm_scdc_helper.c

[ ... ]

> -static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[3])
> +static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[4],
> +					 unsigned int num_lanes)
>  {
> +	u8 end_reg;
>  	u8 sum = 0;
>  	int i;
>  
> -	for (i = SCDC_ERR_DET_0_L; i <= SCDC_ERR_DET_CHECKSUM ; i++)
> +	switch (num_lanes) {
> +	case 3:
> +		end_reg = SCDC_ERR_DET_CHECKSUM;
> +		break;
> +	case 4:
> +		end_reg = SCDC_ERR_DET_3_H;
> +		break;
> +	default:
> +		return -EINVAL;
> +	}
> +
> +	for (i = SCDC_ERR_DET_0_L; i <= end_reg; i++)
>  		sum = wrapping_add(u8, sum, scdc[i]);

[Severity: High]
Does this break the checksum validation for 4-lane FRL links?

The standard HDMI checksum is designed to sum registers 0x50 through 0x56
(SCDC_ERR_DET_CHECKSUM) to zero.

By extending the loop's end_reg to SCDC_ERR_DET_3_H (0x58) for 4-lane mode,
won't this incorporate non-checksum data into the sum and incorrectly return
-EPROTO whenever Lane 3 has errors or its valid bit is set?

[ ... ]

> @@ -356,11 +423,26 @@ int drm_scdc_read_state(struct drm_connector *connector, struct drm_scdc_state *
>  
>  	state->scrambling_detected = buf[SCDC_SCRAMBLER_STATUS] & SCDC_SCRAMBLING_STATUS;
>  
> +	state->rate = FIELD_GET(SCDC_FRL_RATE, buf[SCDC_CONFIG_1]);
> +	num_lanes = drm_scdc_num_frl_lanes(state->rate);
> +	if (num_lanes < 0)
> +		return num_lanes;

[Severity: Medium]
If a sink reports a reserved FRL rate (between 7 and 15), does this cause
debugfs reads to fail entirely?

drm_scdc_num_frl_lanes() returns -EINVAL for these reserved rates, causing
drm_scdc_read_state() to immediately abort. This makes the reserved rate
strings in drm_scdc_frl_rate_str() unreachable dead code.

> +	if (!num_lanes)
> +		num_lanes = 3;
> +
> +	state->ffe_levels = FIELD_GET(SCDC_FFE_LEVELS, buf[SCDC_CONFIG_1]);
> +
>  	drm_scdc_parse_status0_flags(buf[SCDC_STATUS_FLAGS_0], &state->stf);
> -	ret = drm_scdc_parse_error_counters(buf, state->error_count);
> +	drm_scdc_parse_status1_2_flags(buf[SCDC_STATUS_FLAGS_1],
> +				       buf[SCDC_STATUS_FLAGS_2], &state->stf);
> +	ret = drm_scdc_parse_error_counters(buf, state->error_count, num_lanes);
>  	if (ret)
>  		return ret;
>  
> +	if (num_lanes == 4 && (buf[SCDC_ERR_DET_RS_H] & SCDC_CHANNEL_VALID))
> +		state->rs_corrections = (buf[SCDC_ERR_DET_RS_H] & ~SCDC_CHANNEL_VALID) << 8 |
> +					buf[SCDC_ERR_DET_RS_L];
> +
>  	return 0;
>  }

[Severity: Medium]
Does this leak stale state across debugfs reads?

If a 4-lane link transitions to a state where the RS channel valid bit is
cleared, or if it drops to 3 lanes, the missing else branch here leaves the
old rs_corrections value intact.

Since the state struct is reused across reads, could this expose stale
correction counts to userspace?

[ ... ]

> @@ -406,6 +488,8 @@ static int scdc_status_show(struct seq_file *m, void *data)
>  {
>  	struct scdc_debugfs_priv *priv = m->private;
>  	struct drm_scdc_state *st = &priv->state;
>  	struct drm_connector *connector = priv->connector;
>  	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
>  	int i, ret;
>  
>  	drm_connector_get(connector);
>  
>  	if (connector->status != connector_status_connected) {
>  		ret = -ENODEV;
>  		goto err_conn_put;
>  	}
>  
>  	if (scdc->supported) {
>  		ret = drm_scdc_read_state(connector, st);

[Severity: Medium]
Can concurrent debugfs reads cause a data race on the shared state structure?

Multiple threads reading the scdc_status debugfs file share the same private
context. Calling drm_scdc_read_state() mutates the shared I2C buffer
(state->scdc) and its bitfields directly without serialization, which could
corrupt I2C reads and mangle the debugfs output.

[ ... ]

> diff --git a/include/drm/display/drm_scdc.h b/include/drm/display/drm_scdc.h
> index 3d58f37e8ed8e..7f0b05b2f2800 100644
> --- a/include/drm/display/drm_scdc.h
> +++ b/include/drm/display/drm_scdc.h
> @@ -46,14 +48,25 @@
>  #define SCDC_CONFIG_0 0x30
>  #define  SCDC_READ_REQUEST_ENABLE (1 << 0)
>  
> +#define SCDC_CONFIG_1 0x31
> +#define  SCDC_FRL_RATE   0x0f
> +#define  SCDC_FFE_LEVELS 0xf0
> +
>  #define SCDC_STATUS_FLAGS_0 0x40
> +#define  SCDC_DSC_FAIL (1 << 7)
> +#define  SCDC_FLT_READY (1 << 6)
> +#define  SCDC_LN3_LOCK (1 << 4)
>  #define  SCDC_CH2_LOCK (1 << 3)
>  #define  SCDC_CH1_LOCK (1 << 2)
>  #define  SCDC_CH0_LOCK (1 << 1)
> -#define  SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK)
> +#define  SCDC_CH_LOCK_MASK (SCDC_LN3_LOCK | SCDC_CH2_LOCK | SCDC_CH1_LOCK | \
> +			    SCDC_CH0_LOCK)

[Severity: High]
Can this redefinition break TMDS lock checking for HDMI 2.0 sinks?

Since HDMI 2.0 sinks only have 3 lanes, the SCDC_LN3_LOCK bit will always
be 0. Existing drivers performing an equality check against this mask
will systematically fail to detect the lock.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-scdc-link-health-v7-0-92df1b6ad5fc@collabora.com?part=3

Re: [PATCH v7 3/5] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

[Nicolas Frattaroli]

On Tuesday, 21 July 2026 11:10:43 Central European Summer Time sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
> - [High] Incorrect checksum validation range breaks 4-lane FRL error parsing.
> - [High] Redefinition of `SCDC_CH_LOCK_MASK` breaks TMDS lock checking for HDMI 2.0 sinks.
> - [Medium] Stale state leakage for `rs_corrections` across debugfs reads.
> - [Medium] Reserved FRL rates cause debugfs read failures and unreachable dead code.
> - [Medium] Concurrent debugfs reads cause a data race on `priv->state`.
> --
> 
> --- Patch [3]: [PATCH v7 3/5] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields ---
> commit c31217f1c2c58d659c6532a48bf298ea495b08b1
> Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields
> 
> HDMI 2.1 redefines previously reserved fields in SCDC for various new uses.
> Implement reading and outputting these fields over debugfs.
> 
> > diff --git a/drivers/gpu/drm/display/drm_scdc_helper.c b/drivers/gpu/drm/display/drm_scdc_helper.c
> > index 5871fc101815d..7e17272d52f23 100644
> > --- a/drivers/gpu/drm/display/drm_scdc_helper.c
> > +++ b/drivers/gpu/drm/display/drm_scdc_helper.c
> 
> [ ... ]
> 
> > -static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[3])
> > +static int drm_scdc_parse_error_counters(const u8 scdc[256], u16 counter[4],
> > +					 unsigned int num_lanes)
> >  {
> > +	u8 end_reg;
> >  	u8 sum = 0;
> >  	int i;
> >  
> > -	for (i = SCDC_ERR_DET_0_L; i <= SCDC_ERR_DET_CHECKSUM ; i++)
> > +	switch (num_lanes) {
> > +	case 3:
> > +		end_reg = SCDC_ERR_DET_CHECKSUM;
> > +		break;
> > +	case 4:
> > +		end_reg = SCDC_ERR_DET_3_H;
> > +		break;
> > +	default:
> > +		return -EINVAL;
> > +	}
> > +
> > +	for (i = SCDC_ERR_DET_0_L; i <= end_reg; i++)
> >  		sum = wrapping_add(u8, sum, scdc[i]);
> 
> [Severity: High]
> Does this break the checksum validation for 4-lane FRL links?
> 
> The standard HDMI checksum is designed to sum registers 0x50 through 0x56
> (SCDC_ERR_DET_CHECKSUM) to zero.
> 
> By extending the loop's end_reg to SCDC_ERR_DET_3_H (0x58) for 4-lane mode,
> won't this incorporate non-checksum data into the sum and incorrectly return
> -EPROTO whenever Lane 3 has errors or its valid bit is set?
> 
> [ ... ]
> 
> > @@ -356,11 +423,26 @@ int drm_scdc_read_state(struct drm_connector *connector, struct drm_scdc_state *
> >  
> >  	state->scrambling_detected = buf[SCDC_SCRAMBLER_STATUS] & SCDC_SCRAMBLING_STATUS;
> >  
> > +	state->rate = FIELD_GET(SCDC_FRL_RATE, buf[SCDC_CONFIG_1]);
> > +	num_lanes = drm_scdc_num_frl_lanes(state->rate);
> > +	if (num_lanes < 0)
> > +		return num_lanes;
> 
> [Severity: Medium]
> If a sink reports a reserved FRL rate (between 7 and 15), does this cause
> debugfs reads to fail entirely?
> 
> drm_scdc_num_frl_lanes() returns -EINVAL for these reserved rates, causing
> drm_scdc_read_state() to immediately abort. This makes the reserved rate
> strings in drm_scdc_frl_rate_str() unreachable dead code.
> 
> > +	if (!num_lanes)
> > +		num_lanes = 3;
> > +
> > +	state->ffe_levels = FIELD_GET(SCDC_FFE_LEVELS, buf[SCDC_CONFIG_1]);
> > +
> >  	drm_scdc_parse_status0_flags(buf[SCDC_STATUS_FLAGS_0], &state->stf);
> > -	ret = drm_scdc_parse_error_counters(buf, state->error_count);
> > +	drm_scdc_parse_status1_2_flags(buf[SCDC_STATUS_FLAGS_1],
> > +				       buf[SCDC_STATUS_FLAGS_2], &state->stf);
> > +	ret = drm_scdc_parse_error_counters(buf, state->error_count, num_lanes);
> >  	if (ret)
> >  		return ret;
> >  
> > +	if (num_lanes == 4 && (buf[SCDC_ERR_DET_RS_H] & SCDC_CHANNEL_VALID))
> > +		state->rs_corrections = (buf[SCDC_ERR_DET_RS_H] & ~SCDC_CHANNEL_VALID) << 8 |
> > +					buf[SCDC_ERR_DET_RS_L];
> > +
> >  	return 0;
> >  }
> 
> [Severity: Medium]
> Does this leak stale state across debugfs reads?
> 
> If a 4-lane link transitions to a state where the RS channel valid bit is
> cleared, or if it drops to 3 lanes, the missing else branch here leaves the
> old rs_corrections value intact.
> 
> Since the state struct is reused across reads, could this expose stale
> correction counts to userspace?
> 
> [ ... ]
> 
> > @@ -406,6 +488,8 @@ static int scdc_status_show(struct seq_file *m, void *data)
> >  {
> >  	struct scdc_debugfs_priv *priv = m->private;
> >  	struct drm_scdc_state *st = &priv->state;
> >  	struct drm_connector *connector = priv->connector;
> >  	struct drm_scdc *scdc = &connector->display_info.hdmi.scdc;
> >  	int i, ret;
> >  
> >  	drm_connector_get(connector);
> >  
> >  	if (connector->status != connector_status_connected) {
> >  		ret = -ENODEV;
> >  		goto err_conn_put;
> >  	}
> >  
> >  	if (scdc->supported) {
> >  		ret = drm_scdc_read_state(connector, st);
> 
> [Severity: Medium]
> Can concurrent debugfs reads cause a data race on the shared state structure?
> 
> Multiple threads reading the scdc_status debugfs file share the same private
> context. Calling drm_scdc_read_state() mutates the shared I2C buffer
> (state->scdc) and its bitfields directly without serialization, which could
> corrupt I2C reads and mangle the debugfs output.
> 
> [ ... ]
> 
> > diff --git a/include/drm/display/drm_scdc.h b/include/drm/display/drm_scdc.h
> > index 3d58f37e8ed8e..7f0b05b2f2800 100644
> > --- a/include/drm/display/drm_scdc.h
> > +++ b/include/drm/display/drm_scdc.h
> > @@ -46,14 +48,25 @@
> >  #define SCDC_CONFIG_0 0x30
> >  #define  SCDC_READ_REQUEST_ENABLE (1 << 0)
> >  
> > +#define SCDC_CONFIG_1 0x31
> > +#define  SCDC_FRL_RATE   0x0f
> > +#define  SCDC_FFE_LEVELS 0xf0
> > +
> >  #define SCDC_STATUS_FLAGS_0 0x40
> > +#define  SCDC_DSC_FAIL (1 << 7)
> > +#define  SCDC_FLT_READY (1 << 6)
> > +#define  SCDC_LN3_LOCK (1 << 4)
> >  #define  SCDC_CH2_LOCK (1 << 3)
> >  #define  SCDC_CH1_LOCK (1 << 2)
> >  #define  SCDC_CH0_LOCK (1 << 1)
> > -#define  SCDC_CH_LOCK_MASK (SCDC_CH2_LOCK | SCDC_CH1_LOCK | SCDC_CH0_LOCK)
> > +#define  SCDC_CH_LOCK_MASK (SCDC_LN3_LOCK | SCDC_CH2_LOCK | SCDC_CH1_LOCK | \
> > +			    SCDC_CH0_LOCK)
> 
> [Severity: High]
> Can this redefinition break TMDS lock checking for HDMI 2.0 sinks?
> 
> Since HDMI 2.0 sinks only have 3 lanes, the SCDC_LN3_LOCK bit will always
> be 0. Existing drivers performing an equality check against this mask
> will systematically fail to detect the lock.
> 
> 

One of those (state race) is already in the previous patch's review,
the rest are all delirious nonsense.

Re: [PATCH v7 3/5] drm/scdc-helper: Implement parsing and printing HDMI 2.1 fields

[Maxime Ripard]

On Tue, 21 Jul 2026 10:58:59 +0200, Nicolas Frattaroli wrote:
> HDMI 2.1 redefines previously reserved fields in SCDC for various new
> uses. No version check needs to be performed, as an HDMI 2.0 sink's
> reserved SCDC fields are well-defined to be 0, and any zero-ness of
> these fields for an HDMI 2.0 sink is not a surprise for SCDC parsers for
> HDMI 2.1.
> 
> [ ... ]

Reviewed-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

[PATCH v7 4/5] drm/debugfs: Move HDMI debugfs registration to state helper

[Nicolas Frattaroli]

The HDMI debugfs files are only useful for connectors that already make
use of the HDMI state helpers. In addition, relying on HDMI-specific
functions in drm_debugfs.c can lead to circular linking problems later
down the line.

Move hdmi_debugfs_add() and friends to a new helper in
drm_hdmi_state_helper.c, and make all current users (vc4, sun4i, bridge)
either use it as a debugfs_init func, or call it directly in its
debugfs_init func.

Suggested-by: Maxime Ripard <mripard@kernel.org>
Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_bridge_connector.c  |   3 +
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 154 +++++++++++++++++++++++
 drivers/gpu/drm/drm_debugfs.c                   | 157 ------------------------
 drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c          |   1 +
 drivers/gpu/drm/vc4/vc4_hdmi.c                  |   1 +
 include/drm/display/drm_hdmi_state_helper.h     |   3 +
 6 files changed, 162 insertions(+), 157 deletions(-)

diff --git a/drivers/gpu/drm/display/drm_bridge_connector.c b/drivers/gpu/drm/display/drm_bridge_connector.c
index 8b54069fa53a..00feb73e63d7 100644
--- a/drivers/gpu/drm/display/drm_bridge_connector.c
+++ b/drivers/gpu/drm/display/drm_bridge_connector.c
@@ -263,6 +263,9 @@ static void drm_bridge_connector_debugfs_init(struct drm_connector *connector,
 		if (bridge->funcs->debugfs_init)
 			bridge->funcs->debugfs_init(bridge, root);
 	}
+
+	if (bridge_connector->bridge_hdmi)
+		drm_hdmi_connector_debugfs_init(connector, root);
 }
 
 static struct drm_connector_state *
diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index ce17eeefc2da..8d5ee10ceb87 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: MIT
 
+#include <linux/debugfs.h>
 #include <linux/export.h>
 
 #include <drm/drm_atomic.h>
@@ -965,6 +966,159 @@ drm_hdmi_connector_mode_valid(struct drm_connector *connector,
 }
 EXPORT_SYMBOL(drm_hdmi_connector_mode_valid);
 
+static ssize_t
+audio_infoframe_read(struct file *filp, char __user *ubuf, size_t count, loff_t *ppos)
+{
+	struct drm_connector_hdmi_infoframe *infoframe;
+	struct drm_connector *connector;
+	union hdmi_infoframe *frame;
+	u8 buf[HDMI_INFOFRAME_SIZE(AUDIO)];
+	ssize_t len = 0;
+
+	connector = filp->private_data;
+	mutex_lock(&connector->hdmi.infoframes.lock);
+
+	infoframe = &connector->hdmi.infoframes.audio;
+	if (!infoframe->set)
+		goto out;
+
+	frame = &infoframe->data;
+	len = hdmi_infoframe_pack(frame, buf, sizeof(buf));
+	if (len < 0)
+		goto out;
+
+	len = simple_read_from_buffer(ubuf, count, ppos, buf, len);
+
+out:
+	mutex_unlock(&connector->hdmi.infoframes.lock);
+	return len;
+}
+
+static const struct file_operations audio_infoframe_fops = {
+	.owner   = THIS_MODULE,
+	.open    = simple_open,
+	.read    = audio_infoframe_read,
+};
+
+static int create_hdmi_audio_infoframe_file(struct drm_connector *connector,
+					    struct dentry *parent)
+{
+	struct dentry *file;
+
+	if (!connector->hdmi.funcs ||
+	    !connector->hdmi.funcs->audio.write_infoframe)
+		return 0;
+
+	file = debugfs_create_file("audio", 0400, parent, connector, &audio_infoframe_fops);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	return 0;
+}
+
+#define DEFINE_INFOFRAME_FILE(_f) \
+static ssize_t _f##_read_infoframe(struct file *filp, \
+				   char __user *ubuf, \
+				   size_t count,      \
+				   loff_t *ppos)      \
+{ \
+	struct drm_connector_hdmi_infoframe *infoframe; \
+	struct drm_connector_state *conn_state; \
+	struct drm_connector *connector; \
+	union hdmi_infoframe *frame; \
+	struct drm_device *dev; \
+	u8 buf[HDMI_INFOFRAME_SIZE(MAX)]; \
+	ssize_t len = 0; \
+	\
+	connector = filp->private_data; \
+	dev = connector->dev; \
+	\
+	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); \
+	\
+	conn_state = connector->state; \
+	infoframe = &conn_state->hdmi.infoframes._f; \
+	if (!infoframe->set) \
+		goto out; \
+	\
+	frame = &infoframe->data; \
+	len = hdmi_infoframe_pack(frame, buf, sizeof(buf)); \
+	if (len < 0) \
+		goto out; \
+	\
+	len = simple_read_from_buffer(ubuf, count, ppos, buf, len); \
+	\
+out: \
+	drm_modeset_unlock(&dev->mode_config.connection_mutex); \
+	return len; \
+} \
+\
+static const struct file_operations _f##_infoframe_fops = { \
+	.owner = THIS_MODULE, \
+	.open = simple_open, \
+	.read = _f##_read_infoframe, \
+}; \
+\
+static int create_hdmi_## _f ## _infoframe_file(struct drm_connector *connector, \
+						struct dentry *parent) \
+{ \
+	struct dentry *file; \
+	\
+	if (!connector->hdmi.funcs || \
+	    !connector->hdmi.funcs->_f.write_infoframe) \
+		return 0; \
+	file = debugfs_create_file(#_f, 0400, parent, connector, &_f ## _infoframe_fops); \
+	if (IS_ERR(file)) \
+		return PTR_ERR(file); \
+	\
+	return 0; \
+}
+
+DEFINE_INFOFRAME_FILE(avi);
+DEFINE_INFOFRAME_FILE(hdmi);
+DEFINE_INFOFRAME_FILE(hdr_drm);
+DEFINE_INFOFRAME_FILE(spd);
+
+static int create_hdmi_infoframe_files(struct drm_connector *connector,
+				       struct dentry *parent)
+{
+	int ret;
+
+	ret = create_hdmi_audio_infoframe_file(connector, parent);
+	if (ret)
+		return ret;
+
+	ret = create_hdmi_avi_infoframe_file(connector, parent);
+	if (ret)
+		return ret;
+
+	ret = create_hdmi_hdmi_infoframe_file(connector, parent);
+	if (ret)
+		return ret;
+
+	ret = create_hdmi_hdr_drm_infoframe_file(connector, parent);
+	if (ret)
+		return ret;
+
+	ret = create_hdmi_spd_infoframe_file(connector, parent);
+	if (ret)
+		return ret;
+
+	return 0;
+}
+
+void drm_hdmi_connector_debugfs_init(struct drm_connector *connector,
+				     struct dentry *root)
+{
+	struct dentry *dir;
+
+	dir = debugfs_create_dir("infoframes", root);
+	if (IS_ERR(dir))
+		return;
+
+	create_hdmi_infoframe_files(connector, dir);
+}
+EXPORT_SYMBOL(drm_hdmi_connector_debugfs_init);
+
 static int clear_infoframe(struct drm_connector *connector,
 			   const struct drm_connector_infoframe_funcs *funcs,
 			   const char *type)
diff --git a/drivers/gpu/drm/drm_debugfs.c b/drivers/gpu/drm/drm_debugfs.c
index ae1c6126c2c5..a72f9723d3a9 100644
--- a/drivers/gpu/drm/drm_debugfs.c
+++ b/drivers/gpu/drm/drm_debugfs.c
@@ -633,161 +633,6 @@ static const struct file_operations drm_connector_fops = {
 	.write = connector_write
 };
 
-static ssize_t
-audio_infoframe_read(struct file *filp, char __user *ubuf, size_t count, loff_t *ppos)
-{
-	struct drm_connector_hdmi_infoframe *infoframe;
-	struct drm_connector *connector;
-	union hdmi_infoframe *frame;
-	u8 buf[HDMI_INFOFRAME_SIZE(AUDIO)];
-	ssize_t len = 0;
-
-	connector = filp->private_data;
-	mutex_lock(&connector->hdmi.infoframes.lock);
-
-	infoframe = &connector->hdmi.infoframes.audio;
-	if (!infoframe->set)
-		goto out;
-
-	frame = &infoframe->data;
-	len = hdmi_infoframe_pack(frame, buf, sizeof(buf));
-	if (len < 0)
-		goto out;
-
-	len = simple_read_from_buffer(ubuf, count, ppos, buf, len);
-
-out:
-	mutex_unlock(&connector->hdmi.infoframes.lock);
-	return len;
-}
-
-static const struct file_operations audio_infoframe_fops = {
-	.owner   = THIS_MODULE,
-	.open    = simple_open,
-	.read    = audio_infoframe_read,
-};
-
-static int create_hdmi_audio_infoframe_file(struct drm_connector *connector,
-					    struct dentry *parent)
-{
-	struct dentry *file;
-
-	if (!connector->hdmi.funcs ||
-	    !connector->hdmi.funcs->audio.write_infoframe)
-		return 0;
-
-	file = debugfs_create_file("audio", 0400, parent, connector, &audio_infoframe_fops);
-	if (IS_ERR(file))
-		return PTR_ERR(file);
-
-	return 0;
-}
-
-#define DEFINE_INFOFRAME_FILE(_f) \
-static ssize_t _f##_read_infoframe(struct file *filp, \
-				   char __user *ubuf, \
-				   size_t count,      \
-				   loff_t *ppos)      \
-{ \
-	struct drm_connector_hdmi_infoframe *infoframe; \
-	struct drm_connector_state *conn_state; \
-	struct drm_connector *connector; \
-	union hdmi_infoframe *frame; \
-	struct drm_device *dev; \
-	u8 buf[HDMI_INFOFRAME_SIZE(MAX)]; \
-	ssize_t len = 0; \
-	\
-	connector = filp->private_data; \
-	dev = connector->dev; \
-	\
-	drm_modeset_lock(&dev->mode_config.connection_mutex, NULL); \
-	\
-	conn_state = connector->state; \
-	infoframe = &conn_state->hdmi.infoframes._f; \
-	if (!infoframe->set) \
-		goto out; \
-	\
-	frame = &infoframe->data; \
-	len = hdmi_infoframe_pack(frame, buf, sizeof(buf)); \
-	if (len < 0) \
-		goto out; \
-	\
-	len = simple_read_from_buffer(ubuf, count, ppos, buf, len); \
-	\
-out: \
-	drm_modeset_unlock(&dev->mode_config.connection_mutex); \
-	return len; \
-} \
-\
-static const struct file_operations _f##_infoframe_fops = { \
-	.owner = THIS_MODULE, \
-	.open = simple_open, \
-	.read = _f##_read_infoframe, \
-}; \
-\
-static int create_hdmi_## _f ## _infoframe_file(struct drm_connector *connector, \
-						struct dentry *parent) \
-{ \
-	struct dentry *file; \
-	\
-	if (!connector->hdmi.funcs || \
-	    !connector->hdmi.funcs->_f.write_infoframe) \
-		return 0; \
-	file = debugfs_create_file(#_f, 0400, parent, connector, &_f ## _infoframe_fops); \
-	if (IS_ERR(file)) \
-		return PTR_ERR(file); \
-	\
-	return 0; \
-}
-
-DEFINE_INFOFRAME_FILE(avi);
-DEFINE_INFOFRAME_FILE(hdmi);
-DEFINE_INFOFRAME_FILE(hdr_drm);
-DEFINE_INFOFRAME_FILE(spd);
-
-static int create_hdmi_infoframe_files(struct drm_connector *connector,
-				       struct dentry *parent)
-{
-	int ret;
-
-	ret = create_hdmi_audio_infoframe_file(connector, parent);
-	if (ret)
-		return ret;
-
-	ret = create_hdmi_avi_infoframe_file(connector, parent);
-	if (ret)
-		return ret;
-
-	ret = create_hdmi_hdmi_infoframe_file(connector, parent);
-	if (ret)
-		return ret;
-
-	ret = create_hdmi_hdr_drm_infoframe_file(connector, parent);
-	if (ret)
-		return ret;
-
-	ret = create_hdmi_spd_infoframe_file(connector, parent);
-	if (ret)
-		return ret;
-
-	return 0;
-}
-
-static void hdmi_debugfs_add(struct drm_connector *connector)
-{
-	struct dentry *dir;
-
-	if (!(connector->connector_type == DRM_MODE_CONNECTOR_HDMIA ||
-	      connector->connector_type == DRM_MODE_CONNECTOR_HDMIB))
-		return;
-
-	dir = debugfs_create_dir("infoframes", connector->debugfs_entry);
-	if (IS_ERR(dir))
-		return;
-
-	create_hdmi_infoframe_files(connector, dir);
-}
-
 void drm_debugfs_connector_add(struct drm_connector *connector)
 {
 	struct drm_device *dev = connector->dev;
@@ -815,8 +660,6 @@ void drm_debugfs_connector_add(struct drm_connector *connector)
 	debugfs_create_file("output_bpc", 0444, root, connector,
 			    &output_bpc_fops);
 
-	hdmi_debugfs_add(connector);
-
 	if (connector->funcs->debugfs_init)
 		connector->funcs->debugfs_init(connector, root);
 }
diff --git a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
index e99f52ebb26f..e6c4f121ad0c 100644
--- a/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
+++ b/drivers/gpu/drm/sun4i/sun4i_hdmi_enc.c
@@ -292,6 +292,7 @@ static void sun4i_hdmi_connector_reset(struct drm_connector *connector)
 }
 
 static const struct drm_connector_funcs sun4i_hdmi_connector_funcs = {
+	.debugfs_init		= drm_hdmi_connector_debugfs_init,
 	.detect			= sun4i_hdmi_connector_detect,
 	.fill_modes		= drm_helper_probe_single_connector_modes,
 	.reset			= sun4i_hdmi_connector_reset,
diff --git a/drivers/gpu/drm/vc4/vc4_hdmi.c b/drivers/gpu/drm/vc4/vc4_hdmi.c
index 17c8635c5afa..ea1f2013b20e 100644
--- a/drivers/gpu/drm/vc4/vc4_hdmi.c
+++ b/drivers/gpu/drm/vc4/vc4_hdmi.c
@@ -513,6 +513,7 @@ static void vc4_hdmi_connector_reset(struct drm_connector *connector)
 }
 
 static const struct drm_connector_funcs vc4_hdmi_connector_funcs = {
+	.debugfs_init = drm_hdmi_connector_debugfs_init,
 	.force = drm_atomic_helper_connector_hdmi_force,
 	.fill_modes = drm_helper_probe_single_connector_modes,
 	.reset = vc4_hdmi_connector_reset,
diff --git a/include/drm/display/drm_hdmi_state_helper.h b/include/drm/display/drm_hdmi_state_helper.h
index 13375bd0f4ae..596c57d9d250 100644
--- a/include/drm/display/drm_hdmi_state_helper.h
+++ b/include/drm/display/drm_hdmi_state_helper.h
@@ -3,6 +3,7 @@
 #ifndef DRM_HDMI_STATE_HELPER_H_
 #define DRM_HDMI_STATE_HELPER_H_
 
+struct dentry;
 struct drm_atomic_commit;
 struct drm_connector;
 struct drm_connector_state;
@@ -29,5 +30,7 @@ void drm_atomic_helper_connector_hdmi_force(struct drm_connector *connector);
 enum drm_mode_status
 drm_hdmi_connector_mode_valid(struct drm_connector *connector,
 			      const struct drm_display_mode *mode);
+void drm_hdmi_connector_debugfs_init(struct drm_connector *connector,
+				     struct dentry *root);
 
 #endif // DRM_HDMI_STATE_HELPER_H_

-- 
2.55.0

Re: [PATCH v7 4/5] drm/debugfs: Move HDMI debugfs registration to state helper

[Maxime Ripard]

On Tue, 21 Jul 2026 10:59:00 +0200, Nicolas Frattaroli wrote:
> The HDMI debugfs files are only useful for connectors that already make
> use of the HDMI state helpers. In addition, relying on HDMI-specific
> functions in drm_debugfs.c can lead to circular linking problems later
> down the line.
> 
> 
> [ ... ]

Acked-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime

[PATCH v7 5/5] drm/display: hdmi-state-helper: Init SCDC debugfs for HDMI

[Nicolas Frattaroli]

Initialise the SCDC debugfs file for connectors that call the HDMI state
helper's debugfs_init helper. This makes all connectors that use the
helper expose the scdc_status file in their connector debugfs.

Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
---
 drivers/gpu/drm/display/drm_hdmi_state_helper.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
index 8d5ee10ceb87..709524b40c26 100644
--- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
+++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
@@ -13,6 +13,7 @@
 #include <drm/display/drm_hdmi_cec_helper.h>
 #include <drm/display/drm_hdmi_helper.h>
 #include <drm/display/drm_hdmi_state_helper.h>
+#include <drm/display/drm_scdc_helper.h>
 
 /**
  * DOC: hdmi helpers
@@ -1116,6 +1117,7 @@ void drm_hdmi_connector_debugfs_init(struct drm_connector *connector,
 		return;
 
 	create_hdmi_infoframe_files(connector, dir);
+	drm_scdc_debugfs_init(connector, root);
 }
 EXPORT_SYMBOL(drm_hdmi_connector_debugfs_init);
 

-- 
2.55.0

Re: [PATCH v7 5/5] drm/display: hdmi-state-helper: Init SCDC debugfs for HDMI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Reading the `scdc_status` debugfs file causes a kernel panic if the HDMI connector lacks an I2C DDC adapter (`connector->ddc == NULL`).
- [Medium] Concurrent reads of the `scdc_status` debugfs file cause a data race on the shared per-connector `priv->state` buffer.
- [Low] SCDC debugfs initialization is incorrectly skipped if the creation of the `infoframes` debugfs directory fails.
--

--- Patch [5]: [PATCH v7 5/5] drm/display: hdmi-state-helper: Init SCDC debugfs for HDMI ---
commit 4d2f5fcf6dba954346bc26d1af8d8e70d0cf5de0
Author: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>

drm/display: hdmi-state-helper: Init SCDC debugfs for HDMI

Initialise the SCDC debugfs file for connectors that call the HDMI state
helper's debugfs_init helper. This makes all connectors that use the
helper expose the scdc_status file in their connector debugfs.

> diff --git a/drivers/gpu/drm/display/drm_hdmi_state_helper.c b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> index 8d5ee10ceb874..709524b40c26f 100644
> --- a/drivers/gpu/drm/display/drm_hdmi_state_helper.c
> +++ b/drivers/gpu/drm/display/drm_hdmi_state_helper.c
[ ... ]
> @@ -1116,6 +1117,7 @@ void drm_hdmi_connector_debugfs_init(struct drm_connector *connector,
>  		return;

[Severity: Low]
Is it intended that drm_scdc_debugfs_init() is skipped if the infoframes
directory creation fails?

Right above this line, the code checks:

	dir = debugfs_create_dir("infoframes", root);
	if (IS_ERR(dir))
		return;

If debugfs_create_dir() fails, the early return skips the independent
drm_scdc_debugfs_init() call entirely.

>  
>  	create_hdmi_infoframe_files(connector, dir);
> +	drm_scdc_debugfs_init(connector, root);
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

[Severity: High]
Does exposing this to all HDMI connectors cause a regression with a NULL
pointer dereference?

If an HDMI connector lacks an I2C adapter (connector->ddc is NULL) but
indicates SCDC support, reading the newly exposed scdc_status debugfs
file leads to a panic:

drivers/gpu/drm/display/drm_scdc_helper.c:scdc_status_show() {
	if (scdc->supported) {
		ret = drm_scdc_read_state(connector, st);
...
}

drivers/gpu/drm/display/drm_scdc_helper.c:drm_scdc_read_state() {
	ddc = connector->ddc;
	if (!scdc->supported)
		return -EOPNOTSUPP;
	/* Read in 128-byte chunks, to work around DP<->HDMI converters with issues. */
	ret = drm_scdc_read(ddc, 0, buf, 128);
...
}

drm_scdc_read() unconditionally passes the NULL pointer to i2c_transfer(),
causing a panic.

[Severity: Medium]
Could concurrent reads of the scdc_status debugfs file cause a regression
due to a data race?

drm_scdc_debugfs_init() allocates a single, shared struct scdc_debugfs_priv
per connector:

drivers/gpu/drm/display/drm_scdc_helper.c:drm_scdc_debugfs_init() {
	priv = drmm_kzalloc(connector->dev, sizeof(*priv), GFP_KERNEL);
...
}

When multiple processes concurrently read the debugfs file, they extract this
shared pointer:

drivers/gpu/drm/display/drm_scdc_helper.c:scdc_status_show() {
	struct scdc_debugfs_priv *priv = m->private;
	struct drm_scdc_state *st = &priv->state;
...
}

Since there is no locking across the sequential I2C reads and data parsing
in drm_scdc_read_state(), concurrent threads could overwrite priv->state
simultaneously.

>  }
>  EXPORT_SYMBOL(drm_hdmi_connector_debugfs_init);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260721-scdc-link-health-v7-0-92df1b6ad5fc@collabora.com?part=5

Re: [PATCH v7 5/5] drm/display: hdmi-state-helper: Init SCDC debugfs for HDMI

[Maxime Ripard]

On Tue, 21 Jul 2026 10:59:01 +0200, Nicolas Frattaroli wrote:
> Initialise the SCDC debugfs file for connectors that call the HDMI state
> helper's debugfs_init helper. This makes all connectors that use the
> helper expose the scdc_status file in their connector debugfs.
> 
> Signed-off-by: Nicolas Frattaroli <nicolas.frattaroli@collabora.com>
> 
> [ ... ]

Acked-by: Maxime Ripard <mripard@kernel.org>

Thanks!
Maxime