[PATCH i-g-t v2] lib/igt_amd: parse link_settings records by label

[PATCH i-g-t v2] lib/igt_amd: parse link_settings records by label

[Roman.Li]

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. 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.

Link: https://lore.kernel.org/amd-gfx/20260624180829.4775-12-george.zhang@amd.com/
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 | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

diff --git a/lib/igt_amd.c b/lib/igt_amd.c
index 47e587e54..e7e8def58 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) {
@@ -850,23 +851,24 @@ void igt_amd_read_link_settings(
 
 	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 < ARRAY_SIZE(labels); 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

✗ Fi.CI.BUILD: failure for lib/igt_amd: parse link_settings records by label (rev2)

[Patchwork]

== Series Details ==

Series: lib/igt_amd: parse link_settings records by label (rev2)
URL   : https://patchwork.freedesktop.org/series/173095/
State : failure

== Summary ==

Applying: lib/igt_amd: parse link_settings records by label
Using index info to reconstruct a base tree...
M	lib/igt_amd.c
Falling back to patching base and 3-way merge...
Auto-merging lib/igt_amd.c
CONFLICT (content): Merge conflict in lib/igt_amd.c
Patch failed at 0001 lib/igt_amd: parse link_settings records by label
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".