Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH i-g-t] lib/igt_amd: parse link_settings records by label
@ 2026-08-31 20:56 Roman.Li
  2026-08-31 22:34 ` Alex Hung
                   ` (4 more replies)
  0 siblings, 5 replies; 6+ messages in thread
From: Roman.Li @ 2026-08-31 20:56 UTC (permalink / raw)
  To: igt-dev; +Cc: alex.hung, Roman Li, Harry Wentland

From: Roman Li <Roman.Li@amd.com>

[Why]
igt_amd_read_link_settings() depends on an artifact of a kernel bug.

dp_link_settings_read() passes strlen() of each format string as the
size argument to snprintf() and advances its output pointer by that
same fixed amount, so each of the Current, Verified, Reported and
Preferred records is truncated at a NUL written inside the buffer.
The parser rewrites those NULs to ';' and uses them as strtok
delimiters.

A kernel fix for that truncation is pending [1]. It makes the node
emit a single, properly terminated string, at which point the whole
buffer collapses into one strtok token: only Current is parsed and
the remaining records stay zero. amd_ilr@ilr-link-training-configs
then writes a lane count of 0 taken from the Reported record, the
driver rejects it as invalid, the link is left at its previous
configuration, and the trained lane count no longer matches the
requested one:

  Write training setting - lane count:0, supported link rate idx:0
  Actual link result - lane count:4, link rate:0x1E
  Failed assertion: reported_lc == data->lane_count[CURRENT]

Parsing by label is correct either way, so this can land ahead of
the kernel change.

[How]
Locate each record by its label with memmem() over the number of
bytes actually read, then parse it with sscanf(). Searching the raw
buffer rather than treating it as a string keeps the parser working
whether or not the records are separated by NUL bytes, so both old
and fixed kernels are handled.

Tested on eDP with both kernel variants: amd_ilr passes in both
cases.

[1] "drm/amd/display: fix malformed link_settings debugfs output"

Cc: Harry Wentland <harry.wentland@amd.com>
Assisted-by: Copilot:Claude-Opus-5
Signed-off-by: Roman Li <Roman.Li@amd.com>
---
 lib/igt_amd.c | 46 +++++++++++++++++++++++++---------------------
 1 file changed, 25 insertions(+), 21 deletions(-)

diff --git a/lib/igt_amd.c b/lib/igt_amd.c
index 47e587e54..79052979c 100644
--- a/lib/igt_amd.c
+++ b/lib/igt_amd.c
@@ -833,10 +833,11 @@ int igt_amd_trigger_hotplug(int drm_fd, char *connector_name)
 void igt_amd_read_link_settings(
 	int drm_fd, const char *connector_name, int *lane_count, int *link_rate, int *link_spread)
 {
-	int fd, ret;
+	static const char * const labels[] = {
+		"Current:", "Verified:", "Reported:", "Preferred:"
+	};
+	int fd, ret, i;
 	char buf[101];
-	int i = 0;
-	char *token_end, *val_token;
 
 	fd = igt_debugfs_connector_dir(drm_fd, connector_name, O_RDONLY);
 	if (fd < 0) {
@@ -844,29 +845,32 @@ void igt_amd_read_link_settings(
 			 connector_name);
 		return;
 	}
-	ret = igt_debugfs_simple_read(fd, DEBUGFS_DP_LINK_SETTINGS, buf, sizeof(buf));
+
+	ret = igt_debugfs_simple_read(fd, DEBUGFS_DP_LINK_SETTINGS, buf,
+				      sizeof(buf));
 	igt_assert_f(ret >= 0, "Reading %s for connector %s failed.\n",
 		     DEBUGFS_DP_LINK_SETTINGS, connector_name);
 
 	close(fd);
 
-	/* Between current, verified, reported, and preferred are null terminators,
-	 * replace them with ';' to use as the delimiter for strtok. */
-	while (strlen(buf) < sizeof(buf) - 1 && buf[strlen(buf)] == '\0')
-		buf[strlen(buf)] = ';';
-
-	/* Parse values read from file. */
-	for (char *token = strtok_r(buf, ";", &token_end);
-	     token != NULL;
-	     token = strtok_r(NULL, ";", &token_end))
-	{
-		strtok_r(token, ": ", &val_token);
-		lane_count[i] = strtol(val_token, &val_token, 10);
-		link_rate[i] = strtol(val_token, &val_token, 16);
-		link_spread[i] = strtol(val_token, &val_token, 10);
-		i++;
-
-		if (i > 3) return;
+	/*
+	 * Some kernels separate the records with NUL bytes, so locate each
+	 * one in the raw buffer rather than treating it as a single string.
+	 */
+	for (i = 0; i < 4; i++) {
+		const char *label = labels[i];
+		char *rec = memmem(buf, ret, label, strlen(label));
+		unsigned int rate;
+		int lanes, spread;
+
+		igt_assert_f(rec, "Missing %s record in %s for connector %s\n",
+			     label, DEBUGFS_DP_LINK_SETTINGS, connector_name);
+		igt_assert_eq(sscanf(rec + strlen(label), "%d %x %d",
+				     &lanes, &rate, &spread), 3);
+
+		lane_count[i] = lanes;
+		link_rate[i] = rate;
+		link_spread[i] = spread;
 	}
 }
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-02 12:24 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 20:56 [PATCH i-g-t] lib/igt_amd: parse link_settings records by label Roman.Li
2026-08-31 22:34 ` Alex Hung
2026-09-01 21:55 ` ✓ Xe.CI.BAT: success for " Patchwork
2026-09-01 22:11 ` ✓ i915.CI.BAT: " Patchwork
2026-09-02 11:29 ` ✓ Xe.CI.FULL: " Patchwork
2026-09-02 12:24 ` ✗ i915.CI.Full: failure " Patchwork

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox