[PATCH i-g-t 0/7] add test to validate fallback

[PATCH i-g-t 0/7] add test to validate fallback

[Kunal Joshi]

[1] provides debugfs interfaces to force link training
failures, retrain link, set/get current/max link rate/lane count.

add new test using newly exposed interfaces to validate fallback.

[1] https://patchwork.freedesktop.org/series/133624/

Kunal Joshi (7):
  lib/igt_kms: add DP link management helper functions
  lib/igt_kms: add helper to set connector link status
  lib/igt_kms: allow set and reset value to be same
  lib/igt_kms: add function to reset link params
  lib/igt_kms.c: refactor parse_path_connector
  tests/intel/kms_dp_linktrain_fallback: add test for validating
    fallback
  HAX: Do not merge

 lib/igt_kms.c                            | 405 ++++++++++++++++++++++-
 lib/igt_kms.h                            |  13 +
 tests/intel-ci/fast-feedback.testlist    |   1 +
 tests/intel-ci/xe-fast-feedback.testlist |   2 +
 tests/intel/kms_dp_linktrain_fallback.c  | 397 ++++++++++++++++++++++
 tests/meson.build                        |   1 +
 6 files changed, 806 insertions(+), 13 deletions(-)
 create mode 100644 tests/intel/kms_dp_linktrain_fallback.c

-- 
2.43.0

[PATCH i-g-t 1/7] lib/igt_kms: add DP link management helper functions

[Kunal Joshi]

Added helper functions for below
- read current/max link_rate/lane_count
- forcing link retraining
- forcing link training failures
- read pending retrain
- read pending link training failures
- checking output supports forcing lt failures
- force link_rate/lane_count

v2: combine all link training debugfs in one patch (Imre)
    remove unwanted valid output check (Imre)
    return link_rate/lane_count marked with '*' or an error (Imre)

v3: add debugfs_connector_read/write helpers (Imre)

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 307 ++++++++++++++++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  10 ++
 2 files changed, 317 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index b40470c02..c79d800eb 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -47,6 +47,7 @@
 #include <poll.h>
 #include <errno.h>
 #include <time.h>
+#include <ctype.h>
 
 #include <i915_drm.h>
 
@@ -6713,3 +6714,309 @@ int get_num_scalers(igt_display_t *display, enum pipe pipe)
 
 	return num_scalers;
 }
+
+/**
+ * igt_parse_marked_value:
+ * @buf: Buffer containing the content to parse
+ * @marked_char: The character marking the value to parse
+ * @result: Pointer to store the parsed value
+ *
+ * Finds the integer value in the buffer that is marked by the given character.
+ *
+ * Returns: 0 on success, -1 on failure
+ */
+static int igt_parse_marked_value(const char *buf, char marked_char, int *result)
+{
+	char *star_ptr, *val_ptr;
+
+	/*
+	 * Look for the marked character
+	 */
+	star_ptr = strchr(buf, marked_char);
+
+	if (star_ptr) {
+		val_ptr = star_ptr - 1;
+		while (val_ptr > buf && isdigit(*val_ptr))
+			val_ptr--;
+		val_ptr++;
+		if (sscanf(val_ptr, "%d", result) == 1)
+			return 0;
+	}
+	return -1;
+}
+
+/**
+ * igt_debugfs_read_connector_file:
+ * @drm_fd: A drm file descriptor
+ * @conn_name: Name of the output connector
+ * @filename: The file to read from in the connector's directory
+ * @buf: Buffer to store the read content
+ * @buf_size: Size of the buffer
+ *
+ * Reads from a specific file in the connector's debugfs directory.
+ *
+ * Returns: 0 on success, -1 on failure.
+ */
+static int igt_debugfs_read_connector_file(int drm_fd, char *conn_name,
+				    const char *filename, char *buf,
+				    size_t buf_size)
+{
+	int dir, res;
+
+	dir = igt_debugfs_connector_dir(drm_fd, conn_name, O_RDONLY);
+	igt_assert_f(dir >= 0, "Failed to open debugfs dir for connector %s\n", conn_name);
+
+	res = igt_debugfs_simple_read(dir, filename, buf, buf_size);
+	close(dir);
+
+	if (res < 0)
+		return -1;
+
+	return 0;
+}
+
+/**
+ * igt_debugfs_write_connector_file:
+ * @drm_fd: A drm file descriptor
+ * @conn_name: Name of the output connector
+ * @filename: The file to write to in the connector's directory
+ * @data: Data to write to the file
+ * @data_size: Size of the data to write
+ *
+ * Writes to a specific file in the connector's debugfs directory.
+ *
+ * Returns: 0 on success, -1 on failure.
+ */
+static int igt_debugfs_write_connector_file(int drm_fd, char *conn_name,
+				     const char *filename, const char *data,
+				     size_t data_size)
+{
+	int dir, res;
+
+	dir = igt_debugfs_connector_dir(drm_fd, conn_name, O_DIRECTORY);
+	igt_assert_f(dir >= 0, "Failed to open debugfs dir for connector %s\n",
+		     conn_name);
+
+	res = igt_sysfs_write(dir, filename, data, data_size);
+	close(dir);
+
+	if (res < 0)
+		return -1;
+
+	return 0;
+}
+
+/**
+ * igt_get_current_link_rate:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: link_rate if set for output else -1
+ */
+int igt_get_current_link_rate(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res, ret;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					       "i915_dp_force_link_rate",
+					       buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_link_rate\n",
+			       output->name);
+	res = igt_parse_marked_value(buf, '*', &ret);
+	igt_assert_f(res == 0, "Output %s not enabled\n", output->name);
+	return ret;
+}
+
+/**
+ * igt_get_current_lane_count:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: lane_count if set for output else -1
+ */
+int igt_get_current_lane_count(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res, ret;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					      "i915_dp_force_lane_count",
+					      buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_lane_count\n",
+			       output->name);
+	res = igt_parse_marked_value(buf, '*', &ret);
+	igt_assert_f(res == 0, "Output %s not enabled\n", output->name);
+	return ret;
+}
+
+/**
+ * igt_get_max_link_rate:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: max_link_rate
+ */
+int igt_get_max_link_rate(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res, ret;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					       "i915_dp_max_link_rate",
+					       buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_max_link_rate\n",
+		     output->name);
+
+	sscanf(buf, "%d", &ret);
+	return ret;
+}
+
+/**
+ * igt_get_max_link_rate:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: max_link_rate
+ */
+int igt_get_max_lane_count(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res, ret;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					       "i915_dp_max_lane_count",
+					       buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_max_lane_count\n",
+		     output->name);
+
+	sscanf(buf, "%d", &ret);
+	return ret;
+}
+
+/**
+ * igt_force_link_retrain:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ * @retrain_count: number of retraining required
+ *
+ * Force link retrain on the output.
+ */
+void igt_force_link_retrain(int drm_fd, igt_output_t *output, int retrain_count)
+{
+	char value[2];
+	int res;
+
+	snprintf(value, sizeof(value), "%d", retrain_count);
+	res = igt_debugfs_write_connector_file(drm_fd, output->name,
+					       "i915_dp_force_link_retrain",
+					       value, sizeof(value));
+	igt_assert_f(res == 0, "Unable to write to %s/i915_dp_force_link_retrain\n",
+			  output->name);
+}
+
+/**
+ * igt_force_lt_failure:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ * @failure_count: 1 for same link param and
+ *		   2 for reduced link params
+ *
+ * Force link training failure on the output.
+ * @failure_count: 1 for retraining with same link params
+ *		   2 for retraining with reduced link params
+ */
+void igt_force_lt_failure(int drm_fd, igt_output_t *output, int failure_count)
+{
+	char value[2];
+	int res;
+
+	snprintf(value, sizeof(value), "%d", failure_count);
+	res = igt_debugfs_write_connector_file(drm_fd, output->name,
+					       "i915_dp_force_link_training_failure",
+					       value, sizeof(value));
+	igt_assert_f(res == 0, "Unable to write to %s/i915_dp_force_link_training_failure\n",
+			  output->name);
+}
+
+/**
+ * igt_get_dp_link_retrain_disabled:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: True if link retrain disabled, false otherwise
+ */
+bool igt_get_dp_link_retrain_disabled(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					      "i915_dp_link_retrain_disabled",
+					      buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_link_retrain_disabled\n",
+			       output->name);
+	return strstr(buf, "yes");
+}
+
+/**
+ * Checks if the force link training failure debugfs
+ * is available for a specific output.
+ *
+ * @drmfd: file descriptor of the DRM device.
+ * @output: output to check.
+ * Returns:
+ *  true if the debugfs is available, false otherwise.
+ */
+bool igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t *output)
+{
+	char buf[512];
+	int res;
+
+	res = igt_debugfs_read_connector_file(drmfd, output->name,
+					      "i915_dp_link_retrain_disabled",
+					      buf, sizeof(buf));
+	return res == 0;
+}
+
+/**
+ * igt_get_dp_pending_lt_failures:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: Number of pending link training failures.
+ */
+int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res, ret;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					      "i915_dp_force_link_training_failure",
+					      buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_link_training_failure\n",
+			       output->name);
+	sscanf(buf, "%d", &ret);
+	return ret;
+}
+
+/**
+ * igt_dp_pending_retrain:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Returns: Number of pending link retrains.
+ */
+int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output)
+{
+	char buf[512];
+	int res, ret;
+
+	res = igt_debugfs_read_connector_file(drm_fd, output->name,
+					      "i915_dp_force_link_retrain",
+					      buf, sizeof(buf));
+	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_link_retrain\n",
+			       output->name);
+	sscanf(buf, "%d", &ret);
+	return ret;
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 25ba50916..7d9c28d81 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1224,5 +1224,15 @@ bool intel_pipe_output_combo_valid(igt_display_t *display);
 bool igt_check_output_is_dp_mst(igt_output_t *output);
 int igt_get_dp_mst_connector_id(igt_output_t *output);
 int get_num_scalers(igt_display_t *display, enum pipe pipe);
+int igt_get_current_lane_count(int drm_fd, igt_output_t *output);
+int igt_get_current_link_rate(int drm_fd, igt_output_t *output);
+int igt_get_max_link_rate(int drm_fd, igt_output_t *output);
+int igt_get_max_lane_count(int drm_fd, igt_output_t *output);
+void igt_force_link_retrain(int drm_fd, igt_output_t *output, int retrain_count);
+void igt_force_lt_failure(int drm_fd, igt_output_t *output, int failure_count);
+bool igt_get_dp_link_retrain_disabled(int drm_fd, igt_output_t *output);
+bool igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t *output);
+int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t *output);
+int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output);
 
 #endif /* __IGT_KMS_H__ */
-- 
2.43.0

[PATCH i-g-t 2/7] lib/igt_kms: add helper to set connector link status

[Kunal Joshi]

add helper to set connector's link status property

v2: property ID's type is uint32_t (Imre)

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 37 +++++++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  2 ++
 2 files changed, 39 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index c79d800eb..53c6db7f3 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -2214,6 +2214,43 @@ void kmstest_set_connector_dpms(int fd, drmModeConnector *connector, int mode)
 					       dpms, mode) == 0);
 }
 
+/**
+ * kmstest_set_connector_link_status
+ * @drm_fd: drm file descriptor
+ * @connector: libdrm connector
+ * @link_status: DRM link status value
+ *
+ * This function sets the link status of @connector to @link_status.
+ */
+void kmstest_set_connector_link_status(int drm_fd, drmModeConnector *connector,
+				       int link_status)
+{
+	bool found_it = false;
+	int i;
+	uint32_t link_status_prop;
+
+	for (i = 0; i < connector->count_props; i++) {
+		struct drm_mode_get_property prop = {
+			.prop_id = connector->props[i],
+		};
+
+		if (drmIoctl(drm_fd, DRM_IOCTL_MODE_GETPROPERTY, &prop))
+			continue;
+
+		if (strcmp(prop.name, "link-status"))
+			continue;
+
+		link_status_prop = prop.prop_id;
+		found_it = true;
+		break;
+	}
+	igt_assert_f(found_it, "LINK_STATUS property not found on %d\n",
+		     connector->connector_id);
+
+	igt_assert(drmModeConnectorSetProperty(drm_fd, connector->connector_id,
+					       link_status_prop, link_status) == 0);
+}
+
 /**
  * kmstest_get_property:
  * @drm_fd: drm file descriptor
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 7d9c28d81..34a671bce 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -278,6 +278,8 @@ bool kmstest_probe_connector_config(int drm_fd, uint32_t connector_id,
 void kmstest_free_connector_config(struct kmstest_connector_config *config);
 
 void kmstest_set_connector_dpms(int fd, drmModeConnector *connector, int mode);
+void kmstest_set_connector_link_status(int drm_fd, drmModeConnector *connector,
+				       int link_status);
 bool kmstest_get_property(int drm_fd, uint32_t object_id, uint32_t object_type,
 			  const char *name, uint32_t *prop_id, uint64_t *value,
 			  drmModePropertyPtr *prop);
-- 
2.43.0

[PATCH i-g-t 3/7] lib/igt_kms: allow set and reset value to be same

[Kunal Joshi]

allow set and reset value to be same, let the caller handle
this scenario instead.

example scenario where this is required.
i915_dp_force_link_rate should be auto before starting test
should be auto at exit

v2: handle the scenario at caller side

v3: rename allow_set_equals_reset -> force_reset (Imre)

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 21 +++++++++++++--------
 1 file changed, 13 insertions(+), 8 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 53c6db7f3..011f3768a 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1557,7 +1557,8 @@ static void connector_attr_free(struct igt_connector_attr *c)
 static bool connector_attr_set(int idx, drmModeConnector *connector,
 			       int dir, igt_connector_attr_set set,
 			       const char *attr, const char *value,
-			       const char *reset_value)
+			       const char *reset_value,
+			       bool force_reset)
 {
 	struct igt_connector_attr *c;
 
@@ -1573,7 +1574,7 @@ static bool connector_attr_set(int idx, drmModeConnector *connector,
 		return false;
 	}
 
-	if (!strcmp(c->value, c->reset_value))
+	if (!force_reset && !strcmp(c->value, c->reset_value))
 		connector_attr_free(c);
 
 	return true;
@@ -1582,7 +1583,8 @@ static bool connector_attr_set(int idx, drmModeConnector *connector,
 static bool connector_attr_set_sysfs(int drm_fd,
 				     drmModeConnector *connector,
 				     const char *attr, const char *value,
-				     const char *reset_value)
+				     const char *reset_value,
+				     bool force_reset)
 {
 	char name[80];
 	int idx, dir;
@@ -1600,7 +1602,8 @@ static bool connector_attr_set_sysfs(int drm_fd,
 		return false;
 
 	if (!connector_attr_set(idx, connector, dir,
-				igt_sysfs_set, attr, value, reset_value))
+				igt_sysfs_set, attr, value, reset_value,
+				force_reset))
 		return false;
 
 	igt_debug("Connector %s/%s is now %s\n", name, attr, value);
@@ -1611,7 +1614,8 @@ static bool connector_attr_set_sysfs(int drm_fd,
 static bool connector_attr_set_debugfs(int drm_fd,
 				       drmModeConnector *connector,
 				       const char *attr, const char *value,
-				       const char *reset_value)
+				       const char *reset_value,
+				       bool force_reset)
 {
 	char name[80];
 	int idx, dir;
@@ -1630,7 +1634,7 @@ static bool connector_attr_set_debugfs(int drm_fd,
 
 	if (!connector_attr_set(idx, connector, dir,
 				igt_sysfs_set, attr,
-				value, reset_value))
+				value, reset_value, force_reset))
 		return false;
 
 	igt_info("Connector %s/%s is now %s\n", name, attr, value);
@@ -1662,7 +1666,8 @@ static bool force_connector(int drm_fd,
 			    const char *value)
 {
 	return connector_attr_set_sysfs(drm_fd, connector,
-					"status", value, "detect");
+					"status", value, "detect",
+					false);
 }
 
 /**
@@ -1727,7 +1732,7 @@ static bool force_connector_bigjoiner(int drm_fd,
 {
 	return connector_attr_set_debugfs(drm_fd, connector,
 					  "i915_bigjoiner_force_enable",
-					  value, "0");
+					  value, "0", false);
 }
 
 /**
-- 
2.43.0

[PATCH i-g-t 4/7] lib/igt_kms: add function to reset link params

[Kunal Joshi]

Writing auto to i915_dp_force_(link_rate/lane_count) and retraing
afterwards sets max link param's supported by sink.Reset link rate
and lane count to auto, also installs exit handler to set link rate
and lane count to auto on exit

v2: no need to do link retraining (Imre)

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 32 ++++++++++++++++++++++++++++++++
 lib/igt_kms.h |  1 +
 2 files changed, 33 insertions(+)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 011f3768a..96da33a48 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -7062,3 +7062,35 @@ int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output)
 	sscanf(buf, "%d", &ret);
 	return ret;
 }
+
+/**
+ * igt_reset_link_params:
+ * @drm_fd: A drm file descriptor
+ * @output: Target output
+ *
+ * Reset link rate and lane count to auto, also installs exit handler
+ * to set link rate and lane count to auto on exit
+ */
+void igt_reset_link_params(int drm_fd, igt_output_t *output)
+{
+	bool valid;
+	drmModeConnector *temp;
+
+	valid = true;
+	valid = valid && connector_attr_set_debugfs(drm_fd, output->config.connector,
+						    "i915_dp_force_link_rate",
+						    "auto", "auto", true);
+	valid = valid && connector_attr_set_debugfs(drm_fd, output->config.connector,
+						    "i915_dp_force_lane_count",
+						    "auto", "auto", true);
+	igt_assert_f(valid, "Unable to set attr or install exit handler\n");
+	dump_connector_attrs();
+	igt_install_exit_handler(reset_connectors_at_exit);
+
+	/*
+	 * To allow callers to always use GetConnectorCurrent we need to force a
+	 * redetection here.
+	 */
+	temp = drmModeGetConnector(drm_fd, output->config.connector->connector_id);
+	drmModeFreeConnector(temp);
+}
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index 34a671bce..264de4bb9 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1236,5 +1236,6 @@ bool igt_get_dp_link_retrain_disabled(int drm_fd, igt_output_t *output);
 bool igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t *output);
 int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t *output);
 int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output);
+void igt_reset_link_params(int drm_fd, igt_output_t *output);
 
 #endif /* __IGT_KMS_H__ */
-- 
2.43.0

[PATCH i-g-t 5/7] lib/igt_kms.c: refactor parse_path_connector

[Kunal Joshi]

Use strdup() to make a copy of connector_path
before passing it to strtok(), so the original
string remains unmodified

Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 96da33a48..3d2a88b76 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -6670,10 +6670,10 @@ static int parse_path_connector(char *connector_path)
 {
 	int connector_id;
 	char *encoder;
+	char *connector_path_copy = strdup(connector_path);
 
-	encoder = strtok(connector_path, ":");
+	encoder = strtok(connector_path_copy, ":");
 	igt_assert_f(!strcmp(encoder, "mst"), "PATH connector property expected to have 'mst'\n");
-
 	connector_id = atoi(strtok(NULL, "-"));
 
 	return connector_id;
@@ -6688,13 +6688,11 @@ static int parse_path_connector(char *connector_path)
 int igt_get_dp_mst_connector_id(igt_output_t *output)
 {
 	int connector_id;
-	char *connector_path;
 
 	if (!igt_check_output_is_dp_mst(output))
 		return -EINVAL;
 
-	connector_path = output->config.connector_path;
-	connector_id = parse_path_connector(connector_path);
+	connector_id = parse_path_connector(output->config.connector_path);
 
 	return connector_id;
 }
-- 
2.43.0

[PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test for validating fallback

[Kunal Joshi]

add test to valdiate fallback for DP connector,
eDP subtest will be added later.

How does test validates fallback?
- test start by doing initial modeset on default mode
   (if connector is DP then we enable just that connector,
    if its DP-MST we enable all on the same topology)
- force link training failures and retrain until we reach
  lowest param or retrain is disabled
- expect hotplug and link-status to turn bad
- expect link params reduce after fallback

v2: add test for mst (imre)
    refresh mode list (imre)
    monitor got hotplugs (imre)
    check link parameter are reduced (imre)

v3: call check_fn (Santosh)

v4: handle buggy lg monitor (Imre)
    remove reset in between (Imre)

v5: fit modes wrt to bw in non-mst case as well

v6: remove LT_FAILURE_SAME_CAPS (Imre)
    explain LT_FAILURE_REDUCED_CAPS to be 2 (Imre)
    combine infra for mst and non-mst case (Imre)
    call igt_reset_link_params before setup (Imre)
    Avoid duplication setting prev_(link_rate/lane_count) (Imre)
    use the cached property name here instead of hard-coding it (Imre)
    move test logic to function (Imre)
    remove extra w/s (Imre)
    remove braces in one liners (Imre)
    enhance igt_info message (Pranay)

v7: rename kms_fallback -> kms_dp_linktrain_fallback (Imre)
    remove unused headers (Imre)
    fill mst outputs based on root id (Imre)
    check bounds for array (Imre)
    use same syntax across code (Imre)
    add todo for joined pipe (Imre)
    remove redundant commit (Imre)

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
Suggested-by: Imre Deak <imre.deak@intel.com>
---
 tests/intel/kms_dp_linktrain_fallback.c | 397 ++++++++++++++++++++++++
 tests/meson.build                       |   1 +
 2 files changed, 398 insertions(+)
 create mode 100644 tests/intel/kms_dp_linktrain_fallback.c

diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
new file mode 100644
index 000000000..06e61851e
--- /dev/null
+++ b/tests/intel/kms_dp_linktrain_fallback.c
@@ -0,0 +1,397 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+/**
+ * TEST: kms fallback
+ * Category: Display
+ * Description: Test link training fallback for eDP/DP connectors
+ * Driver requirement: i915, xe
+ * Functionality: link training
+ * Mega feature: General Display Features
+ * Test category: functionality test
+ */
+
+#include <sys/types.h>
+
+#include "igt.h"
+
+/**
+ * SUBTEST: dp-fallback
+ * Description: Test fallback on DP connectors
+ */
+
+#define RETRAIN_COUNT 1
+/*
+ * Two consecutives link training failures
+ * reduces link params (link rate, lane count)
+ */
+#define LT_FAILURE_REDUCED_CAPS 2
+#define SPURIOUS_HPD_RETRY 3
+
+static int traversed_mst_outputs[IGT_MAX_PIPES];
+static int traversed_mst_output_count;
+typedef struct {
+	int drm_fd;
+	igt_display_t display;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
+	enum pipe pipe;
+	struct igt_fb fb;
+	struct igt_plane *primary;
+	int n_pipes;
+} data_t;
+
+typedef int (*condition_check_fn)(int drm_fd, igt_output_t *output);
+
+IGT_TEST_DESCRIPTION("Test link training fallback");
+
+static void find_mst_outputs(int drm_fd, data_t *data,
+			     igt_output_t *output,
+			     igt_output_t *mst_outputs[],
+			     int *num_mst_outputs)
+{
+	int output_root_id, root_id;
+	igt_output_t *connector_output;
+
+	output_root_id = igt_get_dp_mst_connector_id(output);
+	/*
+	 * If output is MST check all other connected output which shares
+	 * same path and fill mst_outputs and num_mst_outputs
+	 */
+	for_each_connected_output(&data->display, connector_output) {
+		if (!igt_check_output_is_dp_mst(connector_output))
+			continue;
+		root_id = igt_get_dp_mst_connector_id(connector_output);
+		if (((*num_mst_outputs) < IGT_MAX_PIPES) && root_id == output_root_id)
+			mst_outputs[(*num_mst_outputs)++] = connector_output;
+	}
+}
+
+static bool setup_mst_outputs(data_t *data, igt_output_t *mst_output[],
+			      int *output_count)
+{
+	int i;
+	igt_output_t *output;
+
+	/*
+	 * Check if this is already traversed
+	 */
+	for (i = 0; i < traversed_mst_output_count; i++)
+		if (i < IGT_MAX_PIPES &&
+		    traversed_mst_outputs[i] == data->output->config.connector->connector_id)
+			return false;
+
+	find_mst_outputs(data->drm_fd, data, data->output,
+			 mst_output, output_count);
+
+	for (i = 0; i < *output_count; i++) {
+		output = mst_output[i];
+		if (traversed_mst_output_count < IGT_MAX_PIPES) {
+			traversed_mst_outputs[traversed_mst_output_count++] = output->config.connector->connector_id;
+			igt_info("Output %s is in same topology as %s\n",
+				 igt_output_name(output),
+				 igt_output_name(data->output));
+		}
+	}
+	return true;
+}
+
+static void setup_pipe_on_outputs(data_t *data,
+				      igt_output_t *outputs[],
+				      int *output_count)
+{
+	int i = 0;
+
+	igt_require_f(data->n_pipes >= *output_count,
+		      "Need %d pipes to assign to %d outputs\n",
+		      data->n_pipes, *output_count);
+
+	for_each_pipe(&data->display, data->pipe) {
+		if (i >= *output_count)
+			break;
+		/*
+		 * TODO: add support for modes requiring joined pipes
+		 */
+		igt_info("Setting pipe %s on output %s\n",
+			 kmstest_pipe_name(data->pipe),
+			 igt_output_name(outputs[i]));
+		igt_output_set_pipe(outputs[i++], data->pipe);
+	}
+}
+
+static void setup_modeset_on_outputs(data_t *data,
+				     igt_output_t *outputs[],
+				     int *output_count,
+				     drmModeModeInfo *mode[],
+				     struct igt_fb fb[],
+				     struct igt_plane *primary[])
+{
+	int i;
+
+	for (i = 0; i < *output_count; i++) {
+		outputs[i]->force_reprobe = true;
+		igt_output_refresh(outputs[i]);
+		mode[i] = igt_output_get_mode(outputs[i]);
+		igt_info("Mode %dx%d@%d on output %s\n",
+			 mode[i]->hdisplay, mode[i]->vdisplay,
+			 mode[i]->vrefresh,
+			 igt_output_name(outputs[i]));
+		primary[i] = igt_output_get_plane_type(outputs[i],
+						       DRM_PLANE_TYPE_PRIMARY);
+		igt_create_color_fb(data->drm_fd,
+				    mode[i]->hdisplay,
+				    mode[i]->vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    DRM_FORMAT_MOD_LINEAR, 0.0, 1.0, 0.0,
+				    &fb[i]);
+		igt_plane_set_fb(primary[i], &fb[i]);
+	}
+}
+
+static bool fit_modes_in_bw(data_t *data)
+{
+	bool found;
+	int ret;
+
+	if (!igt_display_try_commit2(&data->display, COMMIT_ATOMIC)) {
+		found = igt_override_all_active_output_modes_to_fit_bw(&data->display);
+		igt_require_f(found,
+			      "No valid mode combo found for MST modeset\n");
+		ret = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);
+		igt_require_f(ret == 0,
+			      "Commit failure during MST modeset\n");
+	}
+	return true;
+}
+
+static bool validate_modeset_for_outputs(data_t *data,
+					igt_output_t *outputs[],
+					int *output_count,
+					drmModeModeInfo *mode[],
+					struct igt_fb fb[],
+					struct igt_plane *primary[])
+{
+	igt_require_f(*output_count > 0, "Require at least 1 output\n");
+	setup_pipe_on_outputs(data, outputs, output_count);
+	setup_modeset_on_outputs(data, outputs,
+				 output_count,
+				 mode, fb, primary);
+	igt_assert_f(fit_modes_in_bw(data), "Unable to fit modes in bw\n");
+	return true;
+}
+
+static bool setup_outputs(data_t *data, bool is_mst,
+		      igt_output_t *outputs[],
+		      int *output_count, drmModeModeInfo *mode[],
+		      struct igt_fb fb[], struct igt_plane *primary[])
+{
+	bool ret;
+
+	*output_count = 0;
+
+	if (is_mst) {
+		ret = setup_mst_outputs(data, outputs, output_count);
+		if (!ret) {
+			igt_info("Skipping MST output %s as already tested\n",
+				 igt_output_name(data->output));
+			return false;
+		}
+	} else
+		if ((*output_count) < IGT_MAX_PIPES)
+			outputs[(*output_count)++] = data->output;
+
+	ret = validate_modeset_for_outputs(data, outputs,
+					   output_count, mode,
+					   fb, primary);
+
+	if (!ret) {
+		igt_info("Skipping output %s as valid pipe/output combo not found\n",
+			 igt_output_name(data->output));
+		return false;
+	}
+
+	igt_display_commit2(&data->display, COMMIT_ATOMIC);
+	return true;
+}
+
+static int check_condition_with_timeout(int drm_fd, igt_output_t *output,
+					condition_check_fn check_fn,
+					double interval, double timeout)
+{
+	struct timespec start_time, current_time;
+	double elapsed_time;
+
+	clock_gettime(CLOCK_MONOTONIC, &start_time);
+
+	while (1) {
+		if (check_fn(drm_fd, output) == 0)
+			return 0;
+
+		clock_gettime(CLOCK_MONOTONIC, &current_time);
+		elapsed_time = (current_time.tv_sec - start_time.tv_sec) +
+			(current_time.tv_nsec - start_time.tv_nsec) / 1e9;
+
+		if (elapsed_time >= timeout)
+			return -1;
+
+		usleep((useconds_t)(interval * 1000000));
+	}
+}
+
+static void test_fallback(data_t *data, bool is_mst)
+{
+	int output_count, retries;
+	int max_link_rate, curr_link_rate, prev_link_rate;
+	int max_lane_count, curr_lane_count, prev_lane_count;
+	igt_output_t *outputs[IGT_MAX_PIPES];
+	uint32_t link_status_prop_id;
+	uint64_t link_status_value;
+	drmModeModeInfo *modes[IGT_MAX_PIPES];
+	drmModePropertyPtr link_status_prop;
+	struct igt_fb fbs[IGT_MAX_PIPES];
+	struct igt_plane *primarys[IGT_MAX_PIPES];
+	struct udev_monitor *mon;
+
+	retries = SPURIOUS_HPD_RETRY;
+
+	igt_display_reset(&data->display);
+	igt_reset_link_params(data->drm_fd, data->output);
+	if (!setup_outputs(data, is_mst, outputs,
+			   &output_count, modes, fbs,
+			   primarys))
+		return;
+
+	igt_info("Testing link training fallback on %s\n",
+		 igt_output_name(data->output));
+	max_link_rate = igt_get_max_link_rate(data->drm_fd, data->output);
+	max_lane_count = igt_get_max_lane_count(data->drm_fd, data->output);
+	prev_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
+	prev_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
+
+	while (!igt_get_dp_link_retrain_disabled(data->drm_fd,
+						 data->output)) {
+		igt_info("Current link rate: %d, Current lane count: %d\n",
+			 prev_link_rate,
+			 prev_lane_count);
+		mon = igt_watch_uevents();
+		igt_force_lt_failure(data->drm_fd, data->output,
+				     LT_FAILURE_REDUCED_CAPS);
+		igt_force_link_retrain(data->drm_fd, data->output,
+				       RETRAIN_COUNT);
+
+		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
+							   data->output,
+							   igt_get_dp_pending_retrain,
+							   1.0, 20.0), 0);
+		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
+							   data->output,
+							   igt_get_dp_pending_lt_failures,
+							   1.0, 20.0), 0);
+
+		if (igt_get_dp_link_retrain_disabled(data->drm_fd,
+						     data->output)) {
+			igt_reset_connectors();
+			return;
+		}
+
+		igt_assert_f(igt_hotplug_detected(mon, 20),
+			     "Didn't get hotplug for force link training failure\n");
+
+		kmstest_get_property(data->drm_fd,
+				data->output->config.connector->connector_id,
+				DRM_MODE_OBJECT_CONNECTOR, "link-status",
+				&link_status_prop_id, &link_status_value,
+				&link_status_prop);
+		igt_assert_eq(link_status_value, DRM_MODE_LINK_STATUS_BAD);
+		igt_flush_uevents(mon);
+		igt_assert_f(validate_modeset_for_outputs(data,
+							  outputs,
+							  &output_count,
+							  modes,
+							  fbs,
+							  primarys),
+			     "modeset failed\n");
+
+		kmstest_set_connector_link_status(data->drm_fd,
+						  data->output->config.connector,
+						  DRM_MODE_LINK_STATUS_GOOD);
+
+		igt_assert_eq(data->output->values[IGT_CONNECTOR_LINK_STATUS], DRM_MODE_LINK_STATUS_GOOD);
+		curr_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
+		curr_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
+
+		igt_assert_f((curr_link_rate < prev_link_rate ||
+			     curr_lane_count < prev_lane_count) ||
+			     ((curr_link_rate == max_link_rate && curr_lane_count == max_lane_count) && --retries),
+			     "Fallback unsuccessful\n");
+
+		prev_link_rate = curr_link_rate;
+		prev_lane_count = curr_lane_count;
+	}
+}
+
+static bool run_test(data_t *data)
+{
+	bool ran = false;
+	igt_output_t *output;
+
+	for_each_connected_output(&data->display, output) {
+		data->output = output;
+
+		if (!igt_has_force_link_training_failure_debugfs(data->drm_fd,
+								 data->output)) {
+			igt_info("Output %s doesn't support forcing link training failure\n",
+				 igt_output_name(data->output));
+			continue;
+		}
+
+		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort) {
+			igt_info("Skipping output %s as it's not DP\n", output->name);
+				continue;
+		}
+
+		ran = true;
+
+		/*
+		 * Check output is MST
+		 */
+		if (igt_check_output_is_dp_mst(data->output)) {
+			igt_info("Testing MST output %s\n",
+				 igt_output_name(data->output));
+			test_fallback(data, true);
+		} else {
+			igt_info("Testing DP output %s\n",
+				 igt_output_name(data->output));
+			test_fallback(data, false);
+		}
+	}
+	return ran;
+}
+
+igt_main
+{
+	data_t data = {};
+
+	igt_fixture {
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
+						     DRIVER_XE);
+		kmstest_set_vt_graphics_mode();
+		igt_display_require(&data.display, data.drm_fd);
+		igt_display_require_output(&data.display);
+		for_each_pipe(&data.display, data.pipe)
+			data.n_pipes++;
+	}
+
+	igt_subtest("dp-fallback") {
+		igt_require_f(run_test(&data),
+			      "Skipping test as no output found or none supports fallback\n");
+	}
+
+	igt_fixture {
+		igt_remove_fb(data.drm_fd, &data.fb);
+		igt_display_fini(&data.display);
+		close(data.drm_fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 00556c9d6..4adaf34f2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -247,6 +247,7 @@ intel_kms_progs = [
 	'kms_ccs',
 	'kms_cdclk',
 	'kms_dirtyfb',
+        'kms_dp_linktrain_fallback',
 	'kms_draw_crc',
 	'kms_dsc',
 	'kms_fb_coherency',
-- 
2.43.0

[PATCH i-g-t 7/7] HAX: Do not merge

[Kunal Joshi]

HAX: Do not merge

Just for testing in CI
---
 tests/intel-ci/fast-feedback.testlist    | 1 +
 tests/intel-ci/xe-fast-feedback.testlist | 2 ++
 2 files changed, 3 insertions(+)

diff --git a/tests/intel-ci/fast-feedback.testlist b/tests/intel-ci/fast-feedback.testlist
index be0965110..7b84ccffc 100644
--- a/tests/intel-ci/fast-feedback.testlist
+++ b/tests/intel-ci/fast-feedback.testlist
@@ -1,6 +1,7 @@
 # Try to load the driver if it's not available yet.
 igt@i915_module_load@load
 
+igt@kms_fallback@dp-fallback
 # Keep alphabetically sorted by default
 igt@core_auth@basic-auth
 igt@debugfs_test@read_all_entries
diff --git a/tests/intel-ci/xe-fast-feedback.testlist b/tests/intel-ci/xe-fast-feedback.testlist
index 01b01dcf9..110675cd3 100644
--- a/tests/intel-ci/xe-fast-feedback.testlist
+++ b/tests/intel-ci/xe-fast-feedback.testlist
@@ -1,6 +1,8 @@
 # Should be the first test
 igt@xe_module_load@load
 
+igt@kms_fallback@dp-fallback
+
 igt@fbdev@eof
 igt@fbdev@info
 igt@fbdev@nullptr
-- 
2.43.0

Re: [PATCH i-g-t 1/7] lib/igt_kms: add DP link management helper functions

[Imre Deak]

On Mon, Sep 23, 2024 at 02:55:43AM +0530, Kunal Joshi wrote:
> Added helper functions for below
> - read current/max link_rate/lane_count
> - forcing link retraining
> - forcing link training failures
> - read pending retrain
> - read pending link training failures
> - checking output supports forcing lt failures
> - force link_rate/lane_count
> 
> v2: combine all link training debugfs in one patch (Imre)
>     remove unwanted valid output check (Imre)
>     return link_rate/lane_count marked with '*' or an error (Imre)
> 
> v3: add debugfs_connector_read/write helpers (Imre)
> 
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_kms.c | 307 ++++++++++++++++++++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  10 ++
>  2 files changed, 317 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index b40470c02..c79d800eb 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -47,6 +47,7 @@
>  #include <poll.h>
>  #include <errno.h>
>  #include <time.h>
> +#include <ctype.h>
>  
>  #include <i915_drm.h>
>  
> @@ -6713,3 +6714,309 @@ int get_num_scalers(igt_display_t *display, enum pipe pipe)
>  
>  	return num_scalers;
>  }
> +
> +/**
> + * igt_parse_marked_value:
> + * @buf: Buffer containing the content to parse
> + * @marked_char: The character marking the value to parse
> + * @result: Pointer to store the parsed value
> + *
> + * Finds the integer value in the buffer that is marked by the given character.
> + *
> + * Returns: 0 on success, -1 on failure
> + */
> +static int igt_parse_marked_value(const char *buf, char marked_char, int *result)
> +{
> +	char *star_ptr, *val_ptr;

It's not necessarily the '*' character, so the above should be called marked_ptr, or
remove the parameter, since always '*' is passed to the function.

> +
> +	/*
> +	 * Look for the marked character
> +	 */
> +	star_ptr = strchr(buf, marked_char);
> +
> +	if (star_ptr) {
> +		val_ptr = star_ptr - 1;
> +		while (val_ptr > buf && isdigit(*val_ptr))
> +			val_ptr--;
> +		val_ptr++;
> +		if (sscanf(val_ptr, "%d", result) == 1)
> +			return 0;
> +	}
> +	return -1;
> +}
> +
> +/**
> + * igt_debugfs_read_connector_file:
> + * @drm_fd: A drm file descriptor
> + * @conn_name: Name of the output connector
> + * @filename: The file to read from in the connector's directory
> + * @buf: Buffer to store the read content
> + * @buf_size: Size of the buffer
> + *
> + * Reads from a specific file in the connector's debugfs directory.
> + *
> + * Returns: 0 on success, -1 on failure.
> + */
> +static int igt_debugfs_read_connector_file(int drm_fd, char *conn_name,
> +				    const char *filename, char *buf,
> +				    size_t buf_size)
> +{
> +	int dir, res;
> +
> +	dir = igt_debugfs_connector_dir(drm_fd, conn_name, O_RDONLY);
> +	igt_assert_f(dir >= 0, "Failed to open debugfs dir for connector %s\n", conn_name);
> +
> +	res = igt_debugfs_simple_read(dir, filename, buf, buf_size);
> +	close(dir);
> +
> +	if (res < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +/**
> + * igt_debugfs_write_connector_file:
> + * @drm_fd: A drm file descriptor
> + * @conn_name: Name of the output connector
> + * @filename: The file to write to in the connector's directory
> + * @data: Data to write to the file
> + * @data_size: Size of the data to write
> + *
> + * Writes to a specific file in the connector's debugfs directory.
> + *
> + * Returns: 0 on success, -1 on failure.
> + */
> +static int igt_debugfs_write_connector_file(int drm_fd, char *conn_name,
> +				     const char *filename, const char *data,
> +				     size_t data_size)
> +{
> +	int dir, res;
> +
> +	dir = igt_debugfs_connector_dir(drm_fd, conn_name, O_DIRECTORY);

Using O_RDONLY here as well is enough, the directory does not change.

> +	igt_assert_f(dir >= 0, "Failed to open debugfs dir for connector %s\n",
> +		     conn_name);
> +
> +	res = igt_sysfs_write(dir, filename, data, data_size);
> +	close(dir);
> +
> +	if (res < 0)
> +		return -1;
> +
> +	return 0;
> +}
> +
> +/**
> + * igt_get_current_link_rate:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: link_rate if set for output else -1
> + */
> +int igt_get_current_link_rate(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res, ret;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					       "i915_dp_force_link_rate",
> +					       buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_link_rate\n",
> +			       output->name);
> +	res = igt_parse_marked_value(buf, '*', &ret);
> +	igt_assert_f(res == 0, "Output %s not enabled\n", output->name);
> +	return ret;
> +}
> +
> +/**
> + * igt_get_current_lane_count:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: lane_count if set for output else -1
> + */
> +int igt_get_current_lane_count(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res, ret;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					      "i915_dp_force_lane_count",
> +					      buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_lane_count\n",
> +			       output->name);
> +	res = igt_parse_marked_value(buf, '*', &ret);
> +	igt_assert_f(res == 0, "Output %s not enabled\n", output->name);
> +	return ret;
> +}
> +
> +/**
> + * igt_get_max_link_rate:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: max_link_rate
> + */
> +int igt_get_max_link_rate(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res, ret;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					       "i915_dp_max_link_rate",
> +					       buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_max_link_rate\n",
> +		     output->name);
> +
> +	sscanf(buf, "%d", &ret);
> +	return ret;
> +}
> +
> +/**
> + * igt_get_max_link_rate:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: max_link_rate
> + */
> +int igt_get_max_lane_count(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res, ret;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					       "i915_dp_max_lane_count",
> +					       buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_max_lane_count\n",
> +		     output->name);
> +
> +	sscanf(buf, "%d", &ret);
> +	return ret;
> +}
> +
> +/**
> + * igt_force_link_retrain:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + * @retrain_count: number of retraining required
> + *
> + * Force link retrain on the output.
> + */
> +void igt_force_link_retrain(int drm_fd, igt_output_t *output, int retrain_count)
> +{
> +	char value[2];
> +	int res;
> +
> +	snprintf(value, sizeof(value), "%d", retrain_count);
> +	res = igt_debugfs_write_connector_file(drm_fd, output->name,
> +					       "i915_dp_force_link_retrain",
> +					       value, sizeof(value));

This just happens to work, since sizeof(value) is 2. Please use
strlen() instead.

> +	igt_assert_f(res == 0, "Unable to write to %s/i915_dp_force_link_retrain\n",
> +			  output->name);
> +}
> +
> +/**
> + * igt_force_lt_failure:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + * @failure_count: 1 for same link param and
> + *		   2 for reduced link params
> + *
> + * Force link training failure on the output.
> + * @failure_count: 1 for retraining with same link params
> + *		   2 for retraining with reduced link params
> + */
> +void igt_force_lt_failure(int drm_fd, igt_output_t *output, int failure_count)
> +{
> +	char value[2];
> +	int res;
> +
> +	snprintf(value, sizeof(value), "%d", failure_count);
> +	res = igt_debugfs_write_connector_file(drm_fd, output->name,
> +					       "i915_dp_force_link_training_failure",
> +					       value, sizeof(value));

Use strlen().

With the above fixed this is:
Reviewed-by: Imre Deak <imre.deak@intel.com>

> +	igt_assert_f(res == 0, "Unable to write to %s/i915_dp_force_link_training_failure\n",
> +			  output->name);
> +}
> +
> +/**
> + * igt_get_dp_link_retrain_disabled:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: True if link retrain disabled, false otherwise
> + */
> +bool igt_get_dp_link_retrain_disabled(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					      "i915_dp_link_retrain_disabled",
> +					      buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_link_retrain_disabled\n",
> +			       output->name);
> +	return strstr(buf, "yes");
> +}
> +
> +/**
> + * Checks if the force link training failure debugfs
> + * is available for a specific output.
> + *
> + * @drmfd: file descriptor of the DRM device.
> + * @output: output to check.
> + * Returns:
> + *  true if the debugfs is available, false otherwise.
> + */
> +bool igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res;
> +
> +	res = igt_debugfs_read_connector_file(drmfd, output->name,
> +					      "i915_dp_link_retrain_disabled",
> +					      buf, sizeof(buf));
> +	return res == 0;
> +}
> +
> +/**
> + * igt_get_dp_pending_lt_failures:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: Number of pending link training failures.
> + */
> +int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res, ret;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					      "i915_dp_force_link_training_failure",
> +					      buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_link_training_failure\n",
> +			       output->name);
> +	sscanf(buf, "%d", &ret);
> +	return ret;
> +}
> +
> +/**
> + * igt_dp_pending_retrain:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Returns: Number of pending link retrains.
> + */
> +int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output)
> +{
> +	char buf[512];
> +	int res, ret;
> +
> +	res = igt_debugfs_read_connector_file(drm_fd, output->name,
> +					      "i915_dp_force_link_retrain",
> +					      buf, sizeof(buf));
> +	igt_assert_f(res == 0, "Unable to read %s/i915_dp_force_link_retrain\n",
> +			       output->name);
> +	sscanf(buf, "%d", &ret);
> +	return ret;
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 25ba50916..7d9c28d81 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1224,5 +1224,15 @@ bool intel_pipe_output_combo_valid(igt_display_t *display);
>  bool igt_check_output_is_dp_mst(igt_output_t *output);
>  int igt_get_dp_mst_connector_id(igt_output_t *output);
>  int get_num_scalers(igt_display_t *display, enum pipe pipe);
> +int igt_get_current_lane_count(int drm_fd, igt_output_t *output);
> +int igt_get_current_link_rate(int drm_fd, igt_output_t *output);
> +int igt_get_max_link_rate(int drm_fd, igt_output_t *output);
> +int igt_get_max_lane_count(int drm_fd, igt_output_t *output);
> +void igt_force_link_retrain(int drm_fd, igt_output_t *output, int retrain_count);
> +void igt_force_lt_failure(int drm_fd, igt_output_t *output, int failure_count);
> +bool igt_get_dp_link_retrain_disabled(int drm_fd, igt_output_t *output);
> +bool igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t *output);
> +int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t *output);
> +int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output);
>  
>  #endif /* __IGT_KMS_H__ */
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t 3/7] lib/igt_kms: allow set and reset value to be same

[Imre Deak]

On Mon, Sep 23, 2024 at 02:55:45AM +0530, Kunal Joshi wrote:
> allow set and reset value to be same, let the caller handle
> this scenario instead.
> 
> example scenario where this is required.
> i915_dp_force_link_rate should be auto before starting test
> should be auto at exit
> 
> v2: handle the scenario at caller side
> 
> v3: rename allow_set_equals_reset -> force_reset (Imre)
> 
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  lib/igt_kms.c | 21 +++++++++++++--------
>  1 file changed, 13 insertions(+), 8 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 53c6db7f3..011f3768a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1557,7 +1557,8 @@ static void connector_attr_free(struct igt_connector_attr *c)
>  static bool connector_attr_set(int idx, drmModeConnector *connector,
>  			       int dir, igt_connector_attr_set set,
>  			       const char *attr, const char *value,
> -			       const char *reset_value)
> +			       const char *reset_value,
> +			       bool force_reset)
>  {
>  	struct igt_connector_attr *c;
>  
> @@ -1573,7 +1574,7 @@ static bool connector_attr_set(int idx, drmModeConnector *connector,
>  		return false;
>  	}
>  
> -	if (!strcmp(c->value, c->reset_value))
> +	if (!force_reset && !strcmp(c->value, c->reset_value))
>  		connector_attr_free(c);
>  
>  	return true;
> @@ -1582,7 +1583,8 @@ static bool connector_attr_set(int idx, drmModeConnector *connector,
>  static bool connector_attr_set_sysfs(int drm_fd,
>  				     drmModeConnector *connector,
>  				     const char *attr, const char *value,
> -				     const char *reset_value)
> +				     const char *reset_value,
> +				     bool force_reset)
>  {
>  	char name[80];
>  	int idx, dir;
> @@ -1600,7 +1602,8 @@ static bool connector_attr_set_sysfs(int drm_fd,
>  		return false;
>  
>  	if (!connector_attr_set(idx, connector, dir,
> -				igt_sysfs_set, attr, value, reset_value))
> +				igt_sysfs_set, attr, value, reset_value,
> +				force_reset))
>  		return false;
>  
>  	igt_debug("Connector %s/%s is now %s\n", name, attr, value);
> @@ -1611,7 +1614,8 @@ static bool connector_attr_set_sysfs(int drm_fd,
>  static bool connector_attr_set_debugfs(int drm_fd,
>  				       drmModeConnector *connector,
>  				       const char *attr, const char *value,
> -				       const char *reset_value)
> +				       const char *reset_value,
> +				       bool force_reset)
>  {
>  	char name[80];
>  	int idx, dir;
> @@ -1630,7 +1634,7 @@ static bool connector_attr_set_debugfs(int drm_fd,
>  
>  	if (!connector_attr_set(idx, connector, dir,
>  				igt_sysfs_set, attr,
> -				value, reset_value))
> +				value, reset_value, force_reset))
>  		return false;
>  
>  	igt_info("Connector %s/%s is now %s\n", name, attr, value);
> @@ -1662,7 +1666,8 @@ static bool force_connector(int drm_fd,
>  			    const char *value)
>  {
>  	return connector_attr_set_sysfs(drm_fd, connector,
> -					"status", value, "detect");
> +					"status", value, "detect",
> +					false);
>  }
>  
>  /**
> @@ -1727,7 +1732,7 @@ static bool force_connector_bigjoiner(int drm_fd,
>  {
>  	return connector_attr_set_debugfs(drm_fd, connector,
>  					  "i915_bigjoiner_force_enable",
> -					  value, "0");
> +					  value, "0", false);
>  }
>  
>  /**
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t 4/7] lib/igt_kms: add function to reset link params

[Imre Deak]

On Mon, Sep 23, 2024 at 02:55:46AM +0530, Kunal Joshi wrote:
> Writing auto to i915_dp_force_(link_rate/lane_count) and retraing
> afterwards sets max link param's supported by sink.Reset link rate
> and lane count to auto, also installs exit handler to set link rate
> and lane count to auto on exit
> 
> v2: no need to do link retraining (Imre)
> 
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>

Reviewed-by: Imre Deak <imre.deak@intel.com>

> ---
>  lib/igt_kms.c | 32 ++++++++++++++++++++++++++++++++
>  lib/igt_kms.h |  1 +
>  2 files changed, 33 insertions(+)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 011f3768a..96da33a48 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -7062,3 +7062,35 @@ int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output)
>  	sscanf(buf, "%d", &ret);
>  	return ret;
>  }
> +
> +/**
> + * igt_reset_link_params:
> + * @drm_fd: A drm file descriptor
> + * @output: Target output
> + *
> + * Reset link rate and lane count to auto, also installs exit handler
> + * to set link rate and lane count to auto on exit
> + */
> +void igt_reset_link_params(int drm_fd, igt_output_t *output)
> +{
> +	bool valid;
> +	drmModeConnector *temp;
> +
> +	valid = true;
> +	valid = valid && connector_attr_set_debugfs(drm_fd, output->config.connector,
> +						    "i915_dp_force_link_rate",
> +						    "auto", "auto", true);
> +	valid = valid && connector_attr_set_debugfs(drm_fd, output->config.connector,
> +						    "i915_dp_force_lane_count",
> +						    "auto", "auto", true);
> +	igt_assert_f(valid, "Unable to set attr or install exit handler\n");
> +	dump_connector_attrs();
> +	igt_install_exit_handler(reset_connectors_at_exit);
> +
> +	/*
> +	 * To allow callers to always use GetConnectorCurrent we need to force a
> +	 * redetection here.
> +	 */
> +	temp = drmModeGetConnector(drm_fd, output->config.connector->connector_id);
> +	drmModeFreeConnector(temp);
> +}
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 34a671bce..264de4bb9 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1236,5 +1236,6 @@ bool igt_get_dp_link_retrain_disabled(int drm_fd, igt_output_t *output);
>  bool igt_has_force_link_training_failure_debugfs(int drmfd, igt_output_t *output);
>  int igt_get_dp_pending_lt_failures(int drm_fd, igt_output_t *output);
>  int igt_get_dp_pending_retrain(int drm_fd, igt_output_t *output);
> +void igt_reset_link_params(int drm_fd, igt_output_t *output);
>  
>  #endif /* __IGT_KMS_H__ */
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t 5/7] lib/igt_kms.c: refactor parse_path_connector

[Imre Deak]

On Mon, Sep 23, 2024 at 02:55:47AM +0530, Kunal Joshi wrote:
> Use strdup() to make a copy of connector_path
> before passing it to strtok(), so the original
> string remains unmodified
> 
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>  lib/igt_kms.c | 8 +++-----
>  1 file changed, 3 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 96da33a48..3d2a88b76 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6670,10 +6670,10 @@ static int parse_path_connector(char *connector_path)
>  {
>  	int connector_id;
>  	char *encoder;
> +	char *connector_path_copy = strdup(connector_path);
>  
> -	encoder = strtok(connector_path, ":");
> +	encoder = strtok(connector_path_copy, ":");
>  	igt_assert_f(!strcmp(encoder, "mst"), "PATH connector property expected to have 'mst'\n");

The copy is leaked, sould be freed. This doesn't look like a refactor,
rather a fix, the commit subject should reflect that. With these fixed:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> -
>  	connector_id = atoi(strtok(NULL, "-"));
>  
>  	return connector_id;
> @@ -6688,13 +6688,11 @@ static int parse_path_connector(char *connector_path)
>  int igt_get_dp_mst_connector_id(igt_output_t *output)
>  {
>  	int connector_id;
> -	char *connector_path;
>  
>  	if (!igt_check_output_is_dp_mst(output))
>  		return -EINVAL;
>  
> -	connector_path = output->config.connector_path;
> -	connector_id = parse_path_connector(connector_path);
> +	connector_id = parse_path_connector(output->config.connector_path);
>  
>  	return connector_id;
>  }
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test for validating fallback

[Imre Deak]

On Mon, Sep 23, 2024 at 02:55:48AM +0530, Kunal Joshi wrote:
> add test to valdiate fallback for DP connector,
> eDP subtest will be added later.
> 
> How does test validates fallback?
> - test start by doing initial modeset on default mode
>    (if connector is DP then we enable just that connector,
>     if its DP-MST we enable all on the same topology)
> - force link training failures and retrain until we reach
>   lowest param or retrain is disabled
> - expect hotplug and link-status to turn bad
> - expect link params reduce after fallback
> 
> v2: add test for mst (imre)
>     refresh mode list (imre)
>     monitor got hotplugs (imre)
>     check link parameter are reduced (imre)
> 
> v3: call check_fn (Santosh)
> 
> v4: handle buggy lg monitor (Imre)
>     remove reset in between (Imre)
> 
> v5: fit modes wrt to bw in non-mst case as well
> 
> v6: remove LT_FAILURE_SAME_CAPS (Imre)
>     explain LT_FAILURE_REDUCED_CAPS to be 2 (Imre)
>     combine infra for mst and non-mst case (Imre)
>     call igt_reset_link_params before setup (Imre)
>     Avoid duplication setting prev_(link_rate/lane_count) (Imre)
>     use the cached property name here instead of hard-coding it (Imre)
>     move test logic to function (Imre)
>     remove extra w/s (Imre)
>     remove braces in one liners (Imre)
>     enhance igt_info message (Pranay)
> 
> v7: rename kms_fallback -> kms_dp_linktrain_fallback (Imre)
>     remove unused headers (Imre)
>     fill mst outputs based on root id (Imre)
>     check bounds for array (Imre)
>     use same syntax across code (Imre)
>     add todo for joined pipe (Imre)
>     remove redundant commit (Imre)
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> Suggested-by: Imre Deak <imre.deak@intel.com>
> ---
>  tests/intel/kms_dp_linktrain_fallback.c | 397 ++++++++++++++++++++++++
>  tests/meson.build                       |   1 +
>  2 files changed, 398 insertions(+)
>  create mode 100644 tests/intel/kms_dp_linktrain_fallback.c
> 
> diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
> new file mode 100644
> index 000000000..06e61851e
> --- /dev/null
> +++ b/tests/intel/kms_dp_linktrain_fallback.c
> @@ -0,0 +1,397 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +/**
> + * TEST: kms fallback
> + * Category: Display
> + * Description: Test link training fallback for eDP/DP connectors
> + * Driver requirement: i915, xe
> + * Functionality: link training
> + * Mega feature: General Display Features
> + * Test category: functionality test
> + */
> +
> +#include <sys/types.h>
> +
> +#include "igt.h"
> +
> +/**
> + * SUBTEST: dp-fallback
> + * Description: Test fallback on DP connectors
> + */
> +
> +#define RETRAIN_COUNT 1
> +/*
> + * Two consecutives link training failures
> + * reduces link params (link rate, lane count)
> + */
> +#define LT_FAILURE_REDUCED_CAPS 2
> +#define SPURIOUS_HPD_RETRY 3
> +
> +static int traversed_mst_outputs[IGT_MAX_PIPES];
> +static int traversed_mst_output_count;
> +typedef struct {
> +	int drm_fd;
> +	igt_display_t display;
> +	drmModeModeInfo *mode;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +	struct igt_fb fb;
> +	struct igt_plane *primary;
> +	int n_pipes;
> +} data_t;
> +
> +typedef int (*condition_check_fn)(int drm_fd, igt_output_t *output);
> +
> +IGT_TEST_DESCRIPTION("Test link training fallback");
> +
> +static void find_mst_outputs(int drm_fd, data_t *data,
> +			     igt_output_t *output,
> +			     igt_output_t *mst_outputs[],
> +			     int *num_mst_outputs)
> +{
> +	int output_root_id, root_id;
> +	igt_output_t *connector_output;
> +
> +	output_root_id = igt_get_dp_mst_connector_id(output);
> +	/*
> +	 * If output is MST check all other connected output which shares
> +	 * same path and fill mst_outputs and num_mst_outputs
> +	 */
> +	for_each_connected_output(&data->display, connector_output) {
> +		if (!igt_check_output_is_dp_mst(connector_output))
> +			continue;
> +		root_id = igt_get_dp_mst_connector_id(connector_output);
> +		if (((*num_mst_outputs) < IGT_MAX_PIPES) && root_id == output_root_id)
> +			mst_outputs[(*num_mst_outputs)++] = connector_output;
> +	}
> +}
> +
> +static bool setup_mst_outputs(data_t *data, igt_output_t *mst_output[],
> +			      int *output_count)
> +{
> +	int i;
> +	igt_output_t *output;
> +
> +	/*
> +	 * Check if this is already traversed
> +	 */
> +	for (i = 0; i < traversed_mst_output_count; i++)
> +		if (i < IGT_MAX_PIPES &&
> +		    traversed_mst_outputs[i] == data->output->config.connector->connector_id)
> +			return false;
> +
> +	find_mst_outputs(data->drm_fd, data, data->output,
> +			 mst_output, output_count);
> +
> +	for (i = 0; i < *output_count; i++) {
> +		output = mst_output[i];
> +		if (traversed_mst_output_count < IGT_MAX_PIPES) {

I suppose the function should fail if the traversed output can't be
saved, since then later the above check for an already tested output
wouldn't work.

> +			traversed_mst_outputs[traversed_mst_output_count++] = output->config.connector->connector_id;
> +			igt_info("Output %s is in same topology as %s\n",
> +				 igt_output_name(output),
> +				 igt_output_name(data->output));
> +		}
> +	}
> +	return true;
> +}
> +
> +static void setup_pipe_on_outputs(data_t *data,
> +				      igt_output_t *outputs[],
> +				      int *output_count)
> +{
> +	int i = 0;
> +
> +	igt_require_f(data->n_pipes >= *output_count,
> +		      "Need %d pipes to assign to %d outputs\n",
> +		      data->n_pipes, *output_count);
> +
> +	for_each_pipe(&data->display, data->pipe) {
> +		if (i >= *output_count)
> +			break;
> +		/*
> +		 * TODO: add support for modes requiring joined pipes
> +		 */
> +		igt_info("Setting pipe %s on output %s\n",
> +			 kmstest_pipe_name(data->pipe),
> +			 igt_output_name(outputs[i]));
> +		igt_output_set_pipe(outputs[i++], data->pipe);
> +	}
> +}
> +
> +static void setup_modeset_on_outputs(data_t *data,
> +				     igt_output_t *outputs[],
> +				     int *output_count,
> +				     drmModeModeInfo *mode[],
> +				     struct igt_fb fb[],
> +				     struct igt_plane *primary[])
> +{
> +	int i;
> +
> +	for (i = 0; i < *output_count; i++) {
> +		outputs[i]->force_reprobe = true;
> +		igt_output_refresh(outputs[i]);
> +		mode[i] = igt_output_get_mode(outputs[i]);
> +		igt_info("Mode %dx%d@%d on output %s\n",
> +			 mode[i]->hdisplay, mode[i]->vdisplay,
> +			 mode[i]->vrefresh,
> +			 igt_output_name(outputs[i]));
> +		primary[i] = igt_output_get_plane_type(outputs[i],
> +						       DRM_PLANE_TYPE_PRIMARY);
> +		igt_create_color_fb(data->drm_fd,
> +				    mode[i]->hdisplay,
> +				    mode[i]->vdisplay,
> +				    DRM_FORMAT_XRGB8888,
> +				    DRM_FORMAT_MOD_LINEAR, 0.0, 1.0, 0.0,
> +				    &fb[i]);
> +		igt_plane_set_fb(primary[i], &fb[i]);
> +	}
> +}
> +
> +static bool fit_modes_in_bw(data_t *data)
> +{
> +	bool found;
> +	int ret;
> +
> +	if (!igt_display_try_commit2(&data->display, COMMIT_ATOMIC)) {

This should be a TEST_ONLY commit as I commented earlier, so something
like:

igt_display_try_commit_atomic(&data->display,
			      DRM_MODE_ATOMIC_TEST_ONLY |
			      DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);

The function returns 0 in case of success so the condition should be
also flipped. The commit also requires the link state property to be
updated to good already here, otherwise the commit assumes the mode
hasn't changed and so won't recompute the state for the mode, thus
ignoring the reduced link parameters (and so pass incorrectly). So
before this commit you should update the link status calling
igt_output_set_prop_value() for all outputs.

> +		found = igt_override_all_active_output_modes_to_fit_bw(&data->display);
> +		igt_require_f(found,
> +			      "No valid mode combo found for MST modeset\n");

The function is also called for SST outputs.

> +		ret = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);

This should be also just a TEST_ONLY commit as above.

> +		igt_require_f(ret == 0,
> +			      "Commit failure during MST modeset\n");
> +	}
> +	return true;
> +}
> +
> +static bool validate_modeset_for_outputs(data_t *data,
> +					igt_output_t *outputs[],
> +					int *output_count,
> +					drmModeModeInfo *mode[],
> +					struct igt_fb fb[],
> +					struct igt_plane *primary[])
> +{
> +	igt_require_f(*output_count > 0, "Require at least 1 output\n");
> +	setup_pipe_on_outputs(data, outputs, output_count);
> +	setup_modeset_on_outputs(data, outputs,
> +				 output_count,
> +				 mode, fb, primary);
> +	igt_assert_f(fit_modes_in_bw(data), "Unable to fit modes in bw\n");
> +	return true;
> +}
> +
> +static bool setup_outputs(data_t *data, bool is_mst,
> +		      igt_output_t *outputs[],
> +		      int *output_count, drmModeModeInfo *mode[],
> +		      struct igt_fb fb[], struct igt_plane *primary[])
> +{
> +	bool ret;
> +
> +	*output_count = 0;
> +
> +	if (is_mst) {
> +		ret = setup_mst_outputs(data, outputs, output_count);
> +		if (!ret) {
> +			igt_info("Skipping MST output %s as already tested\n",
> +				 igt_output_name(data->output));
> +			return false;
> +		}
> +	} else
> +		if ((*output_count) < IGT_MAX_PIPES)
> +			outputs[(*output_count)++] = data->output;
> +
> +	ret = validate_modeset_for_outputs(data, outputs,
> +					   output_count, mode,
> +					   fb, primary);
> +
> +	if (!ret) {
> +		igt_info("Skipping output %s as valid pipe/output combo not found\n",
> +			 igt_output_name(data->output));
> +		return false;
> +	}
> +
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +	return true;
> +}
> +
> +static int check_condition_with_timeout(int drm_fd, igt_output_t *output,
> +					condition_check_fn check_fn,
> +					double interval, double timeout)
> +{
> +	struct timespec start_time, current_time;
> +	double elapsed_time;
> +
> +	clock_gettime(CLOCK_MONOTONIC, &start_time);
> +
> +	while (1) {
> +		if (check_fn(drm_fd, output) == 0)
> +			return 0;
> +
> +		clock_gettime(CLOCK_MONOTONIC, &current_time);
> +		elapsed_time = (current_time.tv_sec - start_time.tv_sec) +
> +			(current_time.tv_nsec - start_time.tv_nsec) / 1e9;
> +
> +		if (elapsed_time >= timeout)
> +			return -1;
> +
> +		usleep((useconds_t)(interval * 1000000));
> +	}
> +}
> +
> +static void test_fallback(data_t *data, bool is_mst)
> +{
> +	int output_count, retries;
> +	int max_link_rate, curr_link_rate, prev_link_rate;
> +	int max_lane_count, curr_lane_count, prev_lane_count;
> +	igt_output_t *outputs[IGT_MAX_PIPES];
> +	uint32_t link_status_prop_id;
> +	uint64_t link_status_value;
> +	drmModeModeInfo *modes[IGT_MAX_PIPES];
> +	drmModePropertyPtr link_status_prop;
> +	struct igt_fb fbs[IGT_MAX_PIPES];
> +	struct igt_plane *primarys[IGT_MAX_PIPES];
> +	struct udev_monitor *mon;
> +
> +	retries = SPURIOUS_HPD_RETRY;
> +
> +	igt_display_reset(&data->display);
> +	igt_reset_link_params(data->drm_fd, data->output);
> +	if (!setup_outputs(data, is_mst, outputs,
> +			   &output_count, modes, fbs,
> +			   primarys))
> +		return;
> +
> +	igt_info("Testing link training fallback on %s\n",
> +		 igt_output_name(data->output));
> +	max_link_rate = igt_get_max_link_rate(data->drm_fd, data->output);
> +	max_lane_count = igt_get_max_lane_count(data->drm_fd, data->output);
> +	prev_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
> +	prev_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
> +
> +	while (!igt_get_dp_link_retrain_disabled(data->drm_fd,
> +						 data->output)) {
> +		igt_info("Current link rate: %d, Current lane count: %d\n",
> +			 prev_link_rate,
> +			 prev_lane_count);
> +		mon = igt_watch_uevents();
> +		igt_force_lt_failure(data->drm_fd, data->output,
> +				     LT_FAILURE_REDUCED_CAPS);
> +		igt_force_link_retrain(data->drm_fd, data->output,
> +				       RETRAIN_COUNT);
> +
> +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> +							   data->output,
> +							   igt_get_dp_pending_retrain,
> +							   1.0, 20.0), 0);
> +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> +							   data->output,
> +							   igt_get_dp_pending_lt_failures,
> +							   1.0, 20.0), 0);
> +
> +		if (igt_get_dp_link_retrain_disabled(data->drm_fd,
> +						     data->output)) {
> +			igt_reset_connectors();
> +			return;
> +		}
> +
> +		igt_assert_f(igt_hotplug_detected(mon, 20),
> +			     "Didn't get hotplug for force link training failure\n");
> +
> +		kmstest_get_property(data->drm_fd,
> +				data->output->config.connector->connector_id,
> +				DRM_MODE_OBJECT_CONNECTOR, "link-status",
> +				&link_status_prop_id, &link_status_value,
> +				&link_status_prop);
> +		igt_assert_eq(link_status_value, DRM_MODE_LINK_STATUS_BAD);
> +		igt_flush_uevents(mon);
> +		igt_assert_f(validate_modeset_for_outputs(data,
> +							  outputs,
> +							  &output_count,
> +							  modes,
> +							  fbs,
> +							  primarys),
> +			     "modeset failed\n");
> +
> +		kmstest_set_connector_link_status(data->drm_fd,
> +						  data->output->config.connector,
> +						  DRM_MODE_LINK_STATUS_GOOD);

Setting the property for only one connector on an MST link just happens
to work, it should be set for all connectors. That will be done already
at this point, as discussed under fit_modes_in_bw(), so the above should
be simply a igt_display_commit2() call.

> +
> +		igt_assert_eq(data->output->values[IGT_CONNECTOR_LINK_STATUS], DRM_MODE_LINK_STATUS_GOOD);
> +		curr_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
> +		curr_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
> +
> +		igt_assert_f((curr_link_rate < prev_link_rate ||
> +			     curr_lane_count < prev_lane_count) ||
> +			     ((curr_link_rate == max_link_rate && curr_lane_count == max_lane_count) && --retries),
> +			     "Fallback unsuccessful\n");
> +
> +		prev_link_rate = curr_link_rate;
> +		prev_lane_count = curr_lane_count;
> +	}
> +}
> +
> +static bool run_test(data_t *data)
> +{
> +	bool ran = false;
> +	igt_output_t *output;
> +
> +	for_each_connected_output(&data->display, output) {
> +		data->output = output;
> +
> +		if (!igt_has_force_link_training_failure_debugfs(data->drm_fd,
> +								 data->output)) {
> +			igt_info("Output %s doesn't support forcing link training failure\n",
> +				 igt_output_name(data->output));
> +			continue;
> +		}
> +
> +		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort) {
> +			igt_info("Skipping output %s as it's not DP\n", output->name);
> +				continue;
> +		}
> +
> +		ran = true;
> +
> +		/*
> +		 * Check output is MST
> +		 */
> +		if (igt_check_output_is_dp_mst(data->output)) {
> +			igt_info("Testing MST output %s\n",
> +				 igt_output_name(data->output));
> +			test_fallback(data, true);
> +		} else {
> +			igt_info("Testing DP output %s\n",
> +				 igt_output_name(data->output));
> +			test_fallback(data, false);
> +		}
> +	}
> +	return ran;
> +}
> +
> +igt_main
> +{
> +	data_t data = {};
> +
> +	igt_fixture {
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
> +						     DRIVER_XE);
> +		kmstest_set_vt_graphics_mode();
> +		igt_display_require(&data.display, data.drm_fd);
> +		igt_display_require_output(&data.display);
> +		for_each_pipe(&data.display, data.pipe)
> +			data.n_pipes++;
> +	}
> +
> +	igt_subtest("dp-fallback") {
> +		igt_require_f(run_test(&data),
> +			      "Skipping test as no output found or none supports fallback\n");
> +	}
> +
> +	igt_fixture {
> +		igt_remove_fb(data.drm_fd, &data.fb);
> +		igt_display_fini(&data.display);
> +		close(data.drm_fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 00556c9d6..4adaf34f2 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -247,6 +247,7 @@ intel_kms_progs = [
>  	'kms_ccs',
>  	'kms_cdclk',
>  	'kms_dirtyfb',
> +        'kms_dp_linktrain_fallback',
>  	'kms_draw_crc',
>  	'kms_dsc',
>  	'kms_fb_coherency',
> -- 
> 2.43.0
> 

Re: [PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test for validating fallback

[Imre Deak]

On Mon, Sep 23, 2024 at 03:21:43PM +0300, Imre Deak wrote:
> On Mon, Sep 23, 2024 at 02:55:48AM +0530, Kunal Joshi wrote:
> > add test to valdiate fallback for DP connector,
> > eDP subtest will be added later.
> > 
> > How does test validates fallback?
> > - test start by doing initial modeset on default mode
> >    (if connector is DP then we enable just that connector,
> >     if its DP-MST we enable all on the same topology)
> > - force link training failures and retrain until we reach
> >   lowest param or retrain is disabled
> > - expect hotplug and link-status to turn bad
> > - expect link params reduce after fallback
> > 
> > v2: add test for mst (imre)
> >     refresh mode list (imre)
> >     monitor got hotplugs (imre)
> >     check link parameter are reduced (imre)
> > 
> > v3: call check_fn (Santosh)
> > 
> > v4: handle buggy lg monitor (Imre)
> >     remove reset in between (Imre)
> > 
> > v5: fit modes wrt to bw in non-mst case as well
> > 
> > v6: remove LT_FAILURE_SAME_CAPS (Imre)
> >     explain LT_FAILURE_REDUCED_CAPS to be 2 (Imre)
> >     combine infra for mst and non-mst case (Imre)
> >     call igt_reset_link_params before setup (Imre)
> >     Avoid duplication setting prev_(link_rate/lane_count) (Imre)
> >     use the cached property name here instead of hard-coding it (Imre)
> >     move test logic to function (Imre)
> >     remove extra w/s (Imre)
> >     remove braces in one liners (Imre)
> >     enhance igt_info message (Pranay)
> > 
> > v7: rename kms_fallback -> kms_dp_linktrain_fallback (Imre)
> >     remove unused headers (Imre)
> >     fill mst outputs based on root id (Imre)
> >     check bounds for array (Imre)
> >     use same syntax across code (Imre)
> >     add todo for joined pipe (Imre)
> >     remove redundant commit (Imre)
> > 
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> > Suggested-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  tests/intel/kms_dp_linktrain_fallback.c | 397 ++++++++++++++++++++++++
> >  tests/meson.build                       |   1 +
> >  2 files changed, 398 insertions(+)
> >  create mode 100644 tests/intel/kms_dp_linktrain_fallback.c
> > 
> > diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
> > new file mode 100644
> > index 000000000..06e61851e
> > --- /dev/null
> > +++ b/tests/intel/kms_dp_linktrain_fallback.c
> > @@ -0,0 +1,397 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2024 Intel Corporation
> > + */
> > +
> > +/**
> > + * TEST: kms fallback
> > + * Category: Display
> > + * Description: Test link training fallback for eDP/DP connectors
> > + * Driver requirement: i915, xe
> > + * Functionality: link training
> > + * Mega feature: General Display Features
> > + * Test category: functionality test
> > + */
> > +
> > +#include <sys/types.h>
> > +
> > +#include "igt.h"
> > +
> > +/**
> > + * SUBTEST: dp-fallback
> > + * Description: Test fallback on DP connectors
> > + */
> > +
> > +#define RETRAIN_COUNT 1
> > +/*
> > + * Two consecutives link training failures
> > + * reduces link params (link rate, lane count)
> > + */
> > +#define LT_FAILURE_REDUCED_CAPS 2
> > +#define SPURIOUS_HPD_RETRY 3
> > +
> > +static int traversed_mst_outputs[IGT_MAX_PIPES];
> > +static int traversed_mst_output_count;
> > +typedef struct {
> > +	int drm_fd;
> > +	igt_display_t display;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +	enum pipe pipe;
> > +	struct igt_fb fb;
> > +	struct igt_plane *primary;
> > +	int n_pipes;
> > +} data_t;
> > +
> > +typedef int (*condition_check_fn)(int drm_fd, igt_output_t *output);
> > +
> > +IGT_TEST_DESCRIPTION("Test link training fallback");
> > +
> > +static void find_mst_outputs(int drm_fd, data_t *data,
> > +			     igt_output_t *output,
> > +			     igt_output_t *mst_outputs[],
> > +			     int *num_mst_outputs)
> > +{
> > +	int output_root_id, root_id;
> > +	igt_output_t *connector_output;
> > +
> > +	output_root_id = igt_get_dp_mst_connector_id(output);
> > +	/*
> > +	 * If output is MST check all other connected output which shares
> > +	 * same path and fill mst_outputs and num_mst_outputs
> > +	 */
> > +	for_each_connected_output(&data->display, connector_output) {
> > +		if (!igt_check_output_is_dp_mst(connector_output))
> > +			continue;
> > +		root_id = igt_get_dp_mst_connector_id(connector_output);
> > +		if (((*num_mst_outputs) < IGT_MAX_PIPES) && root_id == output_root_id)
> > +			mst_outputs[(*num_mst_outputs)++] = connector_output;
> > +	}
> > +}
> > +
> > +static bool setup_mst_outputs(data_t *data, igt_output_t *mst_output[],
> > +			      int *output_count)
> > +{
> > +	int i;
> > +	igt_output_t *output;
> > +
> > +	/*
> > +	 * Check if this is already traversed
> > +	 */
> > +	for (i = 0; i < traversed_mst_output_count; i++)
> > +		if (i < IGT_MAX_PIPES &&
> > +		    traversed_mst_outputs[i] == data->output->config.connector->connector_id)
> > +			return false;
> > +
> > +	find_mst_outputs(data->drm_fd, data, data->output,
> > +			 mst_output, output_count);
> > +
> > +	for (i = 0; i < *output_count; i++) {
> > +		output = mst_output[i];
> > +		if (traversed_mst_output_count < IGT_MAX_PIPES) {
> 
> I suppose the function should fail if the traversed output can't be
> saved, since then later the above check for an already tested output
> wouldn't work.
> 
> > +			traversed_mst_outputs[traversed_mst_output_count++] = output->config.connector->connector_id;
> > +			igt_info("Output %s is in same topology as %s\n",
> > +				 igt_output_name(output),
> > +				 igt_output_name(data->output));
> > +		}
> > +	}
> > +	return true;
> > +}
> > +
> > +static void setup_pipe_on_outputs(data_t *data,
> > +				      igt_output_t *outputs[],
> > +				      int *output_count)
> > +{
> > +	int i = 0;
> > +
> > +	igt_require_f(data->n_pipes >= *output_count,
> > +		      "Need %d pipes to assign to %d outputs\n",
> > +		      data->n_pipes, *output_count);
> > +
> > +	for_each_pipe(&data->display, data->pipe) {
> > +		if (i >= *output_count)
> > +			break;
> > +		/*
> > +		 * TODO: add support for modes requiring joined pipes
> > +		 */
> > +		igt_info("Setting pipe %s on output %s\n",
> > +			 kmstest_pipe_name(data->pipe),
> > +			 igt_output_name(outputs[i]));
> > +		igt_output_set_pipe(outputs[i++], data->pipe);
> > +	}
> > +}
> > +
> > +static void setup_modeset_on_outputs(data_t *data,
> > +				     igt_output_t *outputs[],
> > +				     int *output_count,
> > +				     drmModeModeInfo *mode[],
> > +				     struct igt_fb fb[],
> > +				     struct igt_plane *primary[])
> > +{
> > +	int i;
> > +
> > +	for (i = 0; i < *output_count; i++) {
> > +		outputs[i]->force_reprobe = true;
> > +		igt_output_refresh(outputs[i]);
> > +		mode[i] = igt_output_get_mode(outputs[i]);
> > +		igt_info("Mode %dx%d@%d on output %s\n",
> > +			 mode[i]->hdisplay, mode[i]->vdisplay,
> > +			 mode[i]->vrefresh,
> > +			 igt_output_name(outputs[i]));
> > +		primary[i] = igt_output_get_plane_type(outputs[i],
> > +						       DRM_PLANE_TYPE_PRIMARY);
> > +		igt_create_color_fb(data->drm_fd,
> > +				    mode[i]->hdisplay,
> > +				    mode[i]->vdisplay,
> > +				    DRM_FORMAT_XRGB8888,
> > +				    DRM_FORMAT_MOD_LINEAR, 0.0, 1.0, 0.0,
> > +				    &fb[i]);
> > +		igt_plane_set_fb(primary[i], &fb[i]);
> > +	}
> > +}
> > +
> > +static bool fit_modes_in_bw(data_t *data)
> > +{
> > +	bool found;
> > +	int ret;
> > +
> > +	if (!igt_display_try_commit2(&data->display, COMMIT_ATOMIC)) {
> 
> This should be a TEST_ONLY commit as I commented earlier, so something
> like:
> 
> igt_display_try_commit_atomic(&data->display,
> 			      DRM_MODE_ATOMIC_TEST_ONLY |
> 			      DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> 
> The function returns 0 in case of success so the condition should be
> also flipped. The commit also requires the link state property to be
> updated to good already here, otherwise the commit assumes the mode
> hasn't changed and so won't recompute the state for the mode, thus
> ignoring the reduced link parameters (and so pass incorrectly). So
> before this commit you should update the link status calling
> igt_output_set_prop_value() for all outputs.
> 
> > +		found = igt_override_all_active_output_modes_to_fit_bw(&data->display);
> > +		igt_require_f(found,
> > +			      "No valid mode combo found for MST modeset\n");
> 
> The function is also called for SST outputs.
> 
> > +		ret = igt_display_try_commit2(&data->display, COMMIT_ATOMIC);
> 
> This should be also just a TEST_ONLY commit as above.
> 
> > +		igt_require_f(ret == 0,
> > +			      "Commit failure during MST modeset\n");
> > +	}
> > +	return true;
> > +}
> > +
> > +static bool validate_modeset_for_outputs(data_t *data,
> > +					igt_output_t *outputs[],
> > +					int *output_count,
> > +					drmModeModeInfo *mode[],
> > +					struct igt_fb fb[],
> > +					struct igt_plane *primary[])
> > +{
> > +	igt_require_f(*output_count > 0, "Require at least 1 output\n");
> > +	setup_pipe_on_outputs(data, outputs, output_count);
> > +	setup_modeset_on_outputs(data, outputs,
> > +				 output_count,
> > +				 mode, fb, primary);

Another thing that came to mind: the above will create FBs of a certain
size, then git_modes_in_bw() below will possibly pick a mode with a
smaller size. Hence scaling would get enabled, which may fail - due to
some scaling specific limitation, which is not interesting in this test.
So I think the FB(s) should be created with a size matching the already
reduced resolution(s).

> > +	igt_assert_f(fit_modes_in_bw(data), "Unable to fit modes in bw\n");
> > +	return true;
> > +}
> > +
> > +static bool setup_outputs(data_t *data, bool is_mst,
> > +		      igt_output_t *outputs[],
> > +		      int *output_count, drmModeModeInfo *mode[],
> > +		      struct igt_fb fb[], struct igt_plane *primary[])
> > +{
> > +	bool ret;
> > +
> > +	*output_count = 0;
> > +
> > +	if (is_mst) {
> > +		ret = setup_mst_outputs(data, outputs, output_count);
> > +		if (!ret) {
> > +			igt_info("Skipping MST output %s as already tested\n",
> > +				 igt_output_name(data->output));
> > +			return false;
> > +		}
> > +	} else
> > +		if ((*output_count) < IGT_MAX_PIPES)
> > +			outputs[(*output_count)++] = data->output;
> > +
> > +	ret = validate_modeset_for_outputs(data, outputs,
> > +					   output_count, mode,
> > +					   fb, primary);
> > +
> > +	if (!ret) {
> > +		igt_info("Skipping output %s as valid pipe/output combo not found\n",
> > +			 igt_output_name(data->output));
> > +		return false;
> > +	}
> > +
> > +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +	return true;
> > +}
> > +
> > +static int check_condition_with_timeout(int drm_fd, igt_output_t *output,
> > +					condition_check_fn check_fn,
> > +					double interval, double timeout)
> > +{
> > +	struct timespec start_time, current_time;
> > +	double elapsed_time;
> > +
> > +	clock_gettime(CLOCK_MONOTONIC, &start_time);
> > +
> > +	while (1) {
> > +		if (check_fn(drm_fd, output) == 0)
> > +			return 0;
> > +
> > +		clock_gettime(CLOCK_MONOTONIC, &current_time);
> > +		elapsed_time = (current_time.tv_sec - start_time.tv_sec) +
> > +			(current_time.tv_nsec - start_time.tv_nsec) / 1e9;
> > +
> > +		if (elapsed_time >= timeout)
> > +			return -1;
> > +
> > +		usleep((useconds_t)(interval * 1000000));
> > +	}
> > +}
> > +
> > +static void test_fallback(data_t *data, bool is_mst)
> > +{
> > +	int output_count, retries;
> > +	int max_link_rate, curr_link_rate, prev_link_rate;
> > +	int max_lane_count, curr_lane_count, prev_lane_count;
> > +	igt_output_t *outputs[IGT_MAX_PIPES];
> > +	uint32_t link_status_prop_id;
> > +	uint64_t link_status_value;
> > +	drmModeModeInfo *modes[IGT_MAX_PIPES];
> > +	drmModePropertyPtr link_status_prop;
> > +	struct igt_fb fbs[IGT_MAX_PIPES];
> > +	struct igt_plane *primarys[IGT_MAX_PIPES];
> > +	struct udev_monitor *mon;
> > +
> > +	retries = SPURIOUS_HPD_RETRY;
> > +
> > +	igt_display_reset(&data->display);
> > +	igt_reset_link_params(data->drm_fd, data->output);
> > +	if (!setup_outputs(data, is_mst, outputs,
> > +			   &output_count, modes, fbs,
> > +			   primarys))
> > +		return;
> > +
> > +	igt_info("Testing link training fallback on %s\n",
> > +		 igt_output_name(data->output));
> > +	max_link_rate = igt_get_max_link_rate(data->drm_fd, data->output);
> > +	max_lane_count = igt_get_max_lane_count(data->drm_fd, data->output);
> > +	prev_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
> > +	prev_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
> > +
> > +	while (!igt_get_dp_link_retrain_disabled(data->drm_fd,
> > +						 data->output)) {
> > +		igt_info("Current link rate: %d, Current lane count: %d\n",
> > +			 prev_link_rate,
> > +			 prev_lane_count);
> > +		mon = igt_watch_uevents();
> > +		igt_force_lt_failure(data->drm_fd, data->output,
> > +				     LT_FAILURE_REDUCED_CAPS);
> > +		igt_force_link_retrain(data->drm_fd, data->output,
> > +				       RETRAIN_COUNT);
> > +
> > +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> > +							   data->output,
> > +							   igt_get_dp_pending_retrain,
> > +							   1.0, 20.0), 0);
> > +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> > +							   data->output,
> > +							   igt_get_dp_pending_lt_failures,
> > +							   1.0, 20.0), 0);
> > +
> > +		if (igt_get_dp_link_retrain_disabled(data->drm_fd,
> > +						     data->output)) {
> > +			igt_reset_connectors();
> > +			return;
> > +		}
> > +
> > +		igt_assert_f(igt_hotplug_detected(mon, 20),
> > +			     "Didn't get hotplug for force link training failure\n");
> > +
> > +		kmstest_get_property(data->drm_fd,
> > +				data->output->config.connector->connector_id,
> > +				DRM_MODE_OBJECT_CONNECTOR, "link-status",
> > +				&link_status_prop_id, &link_status_value,
> > +				&link_status_prop);
> > +		igt_assert_eq(link_status_value, DRM_MODE_LINK_STATUS_BAD);
> > +		igt_flush_uevents(mon);
> > +		igt_assert_f(validate_modeset_for_outputs(data,
> > +							  outputs,
> > +							  &output_count,
> > +							  modes,
> > +							  fbs,
> > +							  primarys),
> > +			     "modeset failed\n");
> > +
> > +		kmstest_set_connector_link_status(data->drm_fd,
> > +						  data->output->config.connector,
> > +						  DRM_MODE_LINK_STATUS_GOOD);
> 
> Setting the property for only one connector on an MST link just happens
> to work, it should be set for all connectors. That will be done already
> at this point, as discussed under fit_modes_in_bw(), so the above should
> be simply a igt_display_commit2() call.
> 
> > +
> > +		igt_assert_eq(data->output->values[IGT_CONNECTOR_LINK_STATUS], DRM_MODE_LINK_STATUS_GOOD);
> > +		curr_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
> > +		curr_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
> > +
> > +		igt_assert_f((curr_link_rate < prev_link_rate ||
> > +			     curr_lane_count < prev_lane_count) ||
> > +			     ((curr_link_rate == max_link_rate && curr_lane_count == max_lane_count) && --retries),
> > +			     "Fallback unsuccessful\n");
> > +
> > +		prev_link_rate = curr_link_rate;
> > +		prev_lane_count = curr_lane_count;
> > +	}
> > +}
> > +
> > +static bool run_test(data_t *data)
> > +{
> > +	bool ran = false;
> > +	igt_output_t *output;
> > +
> > +	for_each_connected_output(&data->display, output) {
> > +		data->output = output;
> > +
> > +		if (!igt_has_force_link_training_failure_debugfs(data->drm_fd,
> > +								 data->output)) {
> > +			igt_info("Output %s doesn't support forcing link training failure\n",
> > +				 igt_output_name(data->output));
> > +			continue;
> > +		}
> > +
> > +		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort) {
> > +			igt_info("Skipping output %s as it's not DP\n", output->name);
> > +				continue;
> > +		}
> > +
> > +		ran = true;
> > +
> > +		/*
> > +		 * Check output is MST
> > +		 */
> > +		if (igt_check_output_is_dp_mst(data->output)) {
> > +			igt_info("Testing MST output %s\n",
> > +				 igt_output_name(data->output));
> > +			test_fallback(data, true);
> > +		} else {
> > +			igt_info("Testing DP output %s\n",
> > +				 igt_output_name(data->output));
> > +			test_fallback(data, false);
> > +		}
> > +	}
> > +	return ran;
> > +}
> > +
> > +igt_main
> > +{
> > +	data_t data = {};
> > +
> > +	igt_fixture {
> > +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
> > +						     DRIVER_XE);
> > +		kmstest_set_vt_graphics_mode();
> > +		igt_display_require(&data.display, data.drm_fd);
> > +		igt_display_require_output(&data.display);
> > +		for_each_pipe(&data.display, data.pipe)
> > +			data.n_pipes++;
> > +	}
> > +
> > +	igt_subtest("dp-fallback") {
> > +		igt_require_f(run_test(&data),
> > +			      "Skipping test as no output found or none supports fallback\n");
> > +	}
> > +
> > +	igt_fixture {
> > +		igt_remove_fb(data.drm_fd, &data.fb);
> > +		igt_display_fini(&data.display);
> > +		close(data.drm_fd);
> > +	}
> > +}
> > diff --git a/tests/meson.build b/tests/meson.build
> > index 00556c9d6..4adaf34f2 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -247,6 +247,7 @@ intel_kms_progs = [
> >  	'kms_ccs',
> >  	'kms_cdclk',
> >  	'kms_dirtyfb',
> > +        'kms_dp_linktrain_fallback',
> >  	'kms_draw_crc',
> >  	'kms_dsc',
> >  	'kms_fb_coherency',
> > -- 
> > 2.43.0
> > 

[PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test for validating fallback

[Kunal Joshi]

add test to valdiate fallback for DP connector,
eDP subtest will be added later.

How does test validates fallback?
- test start by doing initial modeset on default mode
   (if connector is DP then we enable just that connector,
    if its DP-MST we enable all on the same topology)
- force link training failures and retrain until we reach
  lowest param or retrain is disabled
- expect hotplug and link-status to turn bad
- expect link params reduce after fallback

v2: add test for mst (imre)
    refresh mode list (imre)
    monitor got hotplugs (imre)
    check link parameter are reduced (imre)
v3: call check_fn (Santosh)
v4: handle buggy lg monitor (Imre)
    remove reset in between (Imre)
v5: fit modes wrt to bw in non-mst case as well
v6: remove LT_FAILURE_SAME_CAPS (Imre)
    explain LT_FAILURE_REDUCED_CAPS to be 2 (Imre)
    combine infra for mst and non-mst case (Imre)
    call igt_reset_link_params before setup (Imre)
    Avoid duplication setting prev_(link_rate/lane_count) (Imre)
    use the cached property name here instead of hard-coding it (Imre)
    move test logic to function (Imre)
    remove extra w/s (Imre)
    remove braces in one liners (Imre)
    enhance igt_info message (Pranay)
v7: rename kms_fallback -> kms_dp_linktrain_fallback (Imre)
    remove unused headers (Imre)
    fill mst outputs based on root id (Imre)
    check bounds for array (Imre)
    use same syntax across code (Imre)
    add todo for joined pipe (Imre)
    remove redundant commit (Imre)
v8: fail if the traversed output can't be saved (Imre)
    use TEST_ONLY commit (Imre)
    set link_status as good for all outputs in fit_modes_in_bw (Imre)
    prepare fb's with modes that fit bw (Imre)

Cc: Imre Deak <imre.deak@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
Suggested-by: Imre Deak <imre.deak@intel.com>
---
 tests/intel/kms_dp_linktrain_fallback.c | 409 ++++++++++++++++++++++++
 tests/meson.build                       |   1 +
 2 files changed, 410 insertions(+)
 create mode 100644 tests/intel/kms_dp_linktrain_fallback.c

diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
new file mode 100644
index 000000000..924da6d7c
--- /dev/null
+++ b/tests/intel/kms_dp_linktrain_fallback.c
@@ -0,0 +1,409 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Copyright © 2024 Intel Corporation
+ */
+
+/**
+ * TEST: kms dp linktrain fallback
+ * Category: Display
+ * Description: Test link training fallback for eDP/DP connectors
+ * Driver requirement: i915, xe
+ * Functionality: link training
+ * Mega feature: General Display Features
+ * Test category: functionality test
+ */
+
+#include <sys/types.h>
+
+#include "igt.h"
+
+/**
+ * SUBTEST: dp-fallback
+ * Description: Test fallback on DP connectors
+ */
+
+#define RETRAIN_COUNT 1
+/*
+ * Two consecutives link training failures
+ * reduces link params (link rate, lane count)
+ */
+#define LT_FAILURE_REDUCED_CAPS 2
+#define SPURIOUS_HPD_RETRY 3
+
+static int traversed_mst_outputs[IGT_MAX_PIPES];
+static int traversed_mst_output_count;
+typedef struct {
+	int drm_fd;
+	igt_display_t display;
+	drmModeModeInfo *mode;
+	igt_output_t *output;
+	enum pipe pipe;
+	struct igt_fb fb;
+	struct igt_plane *primary;
+	int n_pipes;
+} data_t;
+
+typedef int (*condition_check_fn)(int drm_fd, igt_output_t *output);
+
+IGT_TEST_DESCRIPTION("Test link training fallback");
+
+static void find_mst_outputs(int drm_fd, data_t *data,
+			     igt_output_t *output,
+			     igt_output_t *mst_outputs[],
+			     int *num_mst_outputs)
+{
+	int output_root_id, root_id;
+	igt_output_t *connector_output;
+
+	output_root_id = igt_get_dp_mst_connector_id(output);
+	/*
+	 * If output is MST check all other connected output which shares
+	 * same path and fill mst_outputs and num_mst_outputs
+	 */
+	for_each_connected_output(&data->display, connector_output) {
+		if (!igt_check_output_is_dp_mst(connector_output))
+			continue;
+		root_id = igt_get_dp_mst_connector_id(connector_output);
+		if (((*num_mst_outputs) < IGT_MAX_PIPES) && root_id == output_root_id)
+			mst_outputs[(*num_mst_outputs)++] = connector_output;
+	}
+}
+
+static bool setup_mst_outputs(data_t *data, igt_output_t *mst_output[],
+			      int *output_count)
+{
+	int i;
+	igt_output_t *output;
+
+	/*
+	 * Check if this is already traversed
+	 */
+	for (i = 0; i < traversed_mst_output_count; i++)
+		if (i < IGT_MAX_PIPES &&
+		    traversed_mst_outputs[i] == data->output->config.connector->connector_id)
+			return false;
+
+	find_mst_outputs(data->drm_fd, data, data->output,
+			 mst_output, output_count);
+
+	for (i = 0; i < *output_count; i++) {
+		output = mst_output[i];
+		if (traversed_mst_output_count < IGT_MAX_PIPES) {
+			traversed_mst_outputs[traversed_mst_output_count++] = output->config.connector->connector_id;
+			igt_info("Output %s is in same topology as %s\n",
+				 igt_output_name(output),
+				 igt_output_name(data->output));
+		} else {
+			igt_assert_f(false, "Unable to save traversed output\n");
+			return false;
+		}
+	}
+	return true;
+}
+
+static void setup_pipe_on_outputs(data_t *data,
+				      igt_output_t *outputs[],
+				      int *output_count)
+{
+	int i = 0;
+
+	igt_require_f(data->n_pipes >= *output_count,
+		      "Need %d pipes to assign to %d outputs\n",
+		      data->n_pipes, *output_count);
+
+	for_each_pipe(&data->display, data->pipe) {
+		if (i >= *output_count)
+			break;
+		/*
+		 * TODO: add support for modes requiring joined pipes
+		 */
+		igt_info("Setting pipe %s on output %s\n",
+			 kmstest_pipe_name(data->pipe),
+			 igt_output_name(outputs[i]));
+		igt_output_set_pipe(outputs[i++], data->pipe);
+	}
+}
+
+static void setup_modeset_on_outputs(data_t *data,
+				     igt_output_t *outputs[],
+				     int *output_count,
+				     drmModeModeInfo *mode[],
+				     struct igt_fb fb[],
+				     struct igt_plane *primary[])
+{
+	int i;
+
+	for (i = 0; i < *output_count; i++) {
+		mode[i] = igt_output_get_mode(outputs[i]);
+		igt_info("Mode %dx%d@%d on output %s\n",
+			 mode[i]->hdisplay, mode[i]->vdisplay,
+			 mode[i]->vrefresh,
+			 igt_output_name(outputs[i]));
+		primary[i] = igt_output_get_plane_type(outputs[i],
+						       DRM_PLANE_TYPE_PRIMARY);
+		igt_create_color_fb(data->drm_fd,
+				    mode[i]->hdisplay,
+				    mode[i]->vdisplay,
+				    DRM_FORMAT_XRGB8888,
+				    DRM_FORMAT_MOD_LINEAR, 0.0, 1.0, 0.0,
+				    &fb[i]);
+		igt_plane_set_fb(primary[i], &fb[i]);
+	}
+}
+
+static bool fit_modes_in_bw(data_t *data)
+{
+	bool found;
+	int ret;
+	igt_output_t *output;
+
+	/*
+	 * update the link status to good for all outputs
+	 */
+	for_each_connected_output(&data->display, output)
+		igt_output_set_prop_value(output,
+					  IGT_CONNECTOR_LINK_STATUS,
+					  DRM_MODE_LINK_STATUS_GOOD);
+
+	ret = igt_display_try_commit_atomic(&data->display,
+					    DRM_MODE_ATOMIC_TEST_ONLY |
+					    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+	if (ret != 0) {
+		found = igt_override_all_active_output_modes_to_fit_bw(&data->display);
+		igt_require_f(found,
+			      "No valid mode combo found for modeset\n");
+		ret = igt_display_try_commit_atomic(&data->display,
+						    DRM_MODE_ATOMIC_TEST_ONLY |
+						    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
+		igt_require_f(ret == 0,
+			      "Commit failure during MST modeset\n");
+	}
+	return true;
+}
+
+static bool validate_modeset_for_outputs(data_t *data,
+					igt_output_t *outputs[],
+					int *output_count,
+					drmModeModeInfo *mode[],
+					struct igt_fb fb[],
+					struct igt_plane *primary[])
+{
+	igt_require_f(*output_count > 0, "Require at least 1 output\n");
+	setup_pipe_on_outputs(data, outputs, output_count);
+	igt_assert_f(fit_modes_in_bw(data), "Unable to fit modes in bw\n");
+	setup_modeset_on_outputs(data, outputs,
+				 output_count,
+				 mode, fb, primary);
+	return true;
+}
+
+static bool setup_outputs(data_t *data, bool is_mst,
+		      igt_output_t *outputs[],
+		      int *output_count, drmModeModeInfo *mode[],
+		      struct igt_fb fb[], struct igt_plane *primary[])
+{
+	bool ret;
+
+	*output_count = 0;
+
+	if (is_mst) {
+		ret = setup_mst_outputs(data, outputs, output_count);
+		if (!ret) {
+			igt_info("Skipping MST output %s as already tested\n",
+				 igt_output_name(data->output));
+			return false;
+		}
+	} else
+		if ((*output_count) < IGT_MAX_PIPES)
+			outputs[(*output_count)++] = data->output;
+
+	ret = validate_modeset_for_outputs(data, outputs,
+					   output_count, mode,
+					   fb, primary);
+
+	if (!ret) {
+		igt_info("Skipping output %s as valid pipe/output combo not found\n",
+			 igt_output_name(data->output));
+		return false;
+	}
+
+	igt_display_commit2(&data->display, COMMIT_ATOMIC);
+	return true;
+}
+
+static int check_condition_with_timeout(int drm_fd, igt_output_t *output,
+					condition_check_fn check_fn,
+					double interval, double timeout)
+{
+	struct timespec start_time, current_time;
+	double elapsed_time;
+
+	clock_gettime(CLOCK_MONOTONIC, &start_time);
+
+	while (1) {
+		if (check_fn(drm_fd, output) == 0)
+			return 0;
+
+		clock_gettime(CLOCK_MONOTONIC, &current_time);
+		elapsed_time = (current_time.tv_sec - start_time.tv_sec) +
+			(current_time.tv_nsec - start_time.tv_nsec) / 1e9;
+
+		if (elapsed_time >= timeout)
+			return -1;
+
+		usleep((useconds_t)(interval * 1000000));
+	}
+}
+
+static void test_fallback(data_t *data, bool is_mst)
+{
+	int output_count, retries;
+	int max_link_rate, curr_link_rate, prev_link_rate;
+	int max_lane_count, curr_lane_count, prev_lane_count;
+	igt_output_t *outputs[IGT_MAX_PIPES];
+	uint32_t link_status_prop_id;
+	uint64_t link_status_value;
+	drmModeModeInfo *modes[IGT_MAX_PIPES];
+	drmModePropertyPtr link_status_prop;
+	struct igt_fb fbs[IGT_MAX_PIPES];
+	struct igt_plane *primarys[IGT_MAX_PIPES];
+	struct udev_monitor *mon;
+
+	retries = SPURIOUS_HPD_RETRY;
+
+	igt_display_reset(&data->display);
+	igt_reset_link_params(data->drm_fd, data->output);
+	if (!setup_outputs(data, is_mst, outputs,
+			   &output_count, modes, fbs,
+			   primarys))
+		return;
+
+	igt_info("Testing link training fallback on %s\n",
+		 igt_output_name(data->output));
+	max_link_rate = igt_get_max_link_rate(data->drm_fd, data->output);
+	max_lane_count = igt_get_max_lane_count(data->drm_fd, data->output);
+	prev_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
+	prev_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
+
+	while (!igt_get_dp_link_retrain_disabled(data->drm_fd,
+						 data->output)) {
+		igt_info("Current link rate: %d, Current lane count: %d\n",
+			 prev_link_rate,
+			 prev_lane_count);
+		mon = igt_watch_uevents();
+		igt_force_lt_failure(data->drm_fd, data->output,
+				     LT_FAILURE_REDUCED_CAPS);
+		igt_force_link_retrain(data->drm_fd, data->output,
+				       RETRAIN_COUNT);
+
+		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
+							   data->output,
+							   igt_get_dp_pending_retrain,
+							   1.0, 20.0), 0);
+		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
+							   data->output,
+							   igt_get_dp_pending_lt_failures,
+							   1.0, 20.0), 0);
+
+		if (igt_get_dp_link_retrain_disabled(data->drm_fd,
+						     data->output)) {
+			igt_reset_connectors();
+			return;
+		}
+
+		igt_assert_f(igt_hotplug_detected(mon, 20),
+			     "Didn't get hotplug for force link training failure\n");
+
+		kmstest_get_property(data->drm_fd,
+				data->output->config.connector->connector_id,
+				DRM_MODE_OBJECT_CONNECTOR, "link-status",
+				&link_status_prop_id, &link_status_value,
+				&link_status_prop);
+		igt_assert_eq(link_status_value, DRM_MODE_LINK_STATUS_BAD);
+		igt_flush_uevents(mon);
+		igt_assert_f(validate_modeset_for_outputs(data,
+							  outputs,
+							  &output_count,
+							  modes,
+							  fbs,
+							  primarys),
+			     "modeset failed\n");
+		igt_display_commit2(&data->display, COMMIT_ATOMIC);
+
+		igt_assert_eq(data->output->values[IGT_CONNECTOR_LINK_STATUS], DRM_MODE_LINK_STATUS_GOOD);
+		curr_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
+		curr_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
+
+		igt_assert_f((curr_link_rate < prev_link_rate ||
+			     curr_lane_count < prev_lane_count) ||
+			     ((curr_link_rate == max_link_rate && curr_lane_count == max_lane_count) && --retries),
+			     "Fallback unsuccessful\n");
+
+		prev_link_rate = curr_link_rate;
+		prev_lane_count = curr_lane_count;
+	}
+}
+
+static bool run_test(data_t *data)
+{
+	bool ran = false;
+	igt_output_t *output;
+
+	for_each_connected_output(&data->display, output) {
+		data->output = output;
+
+		if (!igt_has_force_link_training_failure_debugfs(data->drm_fd,
+								 data->output)) {
+			igt_info("Output %s doesn't support forcing link training failure\n",
+				 igt_output_name(data->output));
+			continue;
+		}
+
+		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort) {
+			igt_info("Skipping output %s as it's not DP\n", output->name);
+				continue;
+		}
+
+		ran = true;
+
+		/*
+		 * Check output is MST
+		 */
+		if (igt_check_output_is_dp_mst(data->output)) {
+			igt_info("Testing MST output %s\n",
+				 igt_output_name(data->output));
+			test_fallback(data, true);
+		} else {
+			igt_info("Testing DP output %s\n",
+				 igt_output_name(data->output));
+			test_fallback(data, false);
+		}
+	}
+	return ran;
+}
+
+igt_main
+{
+	data_t data = {};
+
+	igt_fixture {
+		data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
+						     DRIVER_XE);
+		kmstest_set_vt_graphics_mode();
+		igt_display_require(&data.display, data.drm_fd);
+		igt_display_require_output(&data.display);
+		for_each_pipe(&data.display, data.pipe)
+			data.n_pipes++;
+	}
+
+	igt_subtest("dp-fallback") {
+		igt_require_f(run_test(&data),
+			      "Skipping test as no output found or none supports fallback\n");
+	}
+
+	igt_fixture {
+		igt_remove_fb(data.drm_fd, &data.fb);
+		igt_display_fini(&data.display);
+		close(data.drm_fd);
+	}
+}
diff --git a/tests/meson.build b/tests/meson.build
index 00556c9d6..4adaf34f2 100644
--- a/tests/meson.build
+++ b/tests/meson.build
@@ -247,6 +247,7 @@ intel_kms_progs = [
 	'kms_ccs',
 	'kms_cdclk',
 	'kms_dirtyfb',
+        'kms_dp_linktrain_fallback',
 	'kms_draw_crc',
 	'kms_dsc',
 	'kms_fb_coherency',
-- 
2.43.0

Re: [PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test for validating fallback

[Imre Deak]

On Wed, Sep 25, 2024 at 11:59:43PM +0530, Kunal Joshi wrote:
> add test to valdiate fallback for DP connector,
> eDP subtest will be added later.
> 
> How does test validates fallback?
> - test start by doing initial modeset on default mode
>    (if connector is DP then we enable just that connector,
>     if its DP-MST we enable all on the same topology)
> - force link training failures and retrain until we reach
>   lowest param or retrain is disabled
> - expect hotplug and link-status to turn bad
> - expect link params reduce after fallback
> 
> v2: add test for mst (imre)
>     refresh mode list (imre)
>     monitor got hotplugs (imre)
>     check link parameter are reduced (imre)
> v3: call check_fn (Santosh)
> v4: handle buggy lg monitor (Imre)
>     remove reset in between (Imre)
> v5: fit modes wrt to bw in non-mst case as well
> v6: remove LT_FAILURE_SAME_CAPS (Imre)
>     explain LT_FAILURE_REDUCED_CAPS to be 2 (Imre)
>     combine infra for mst and non-mst case (Imre)
>     call igt_reset_link_params before setup (Imre)
>     Avoid duplication setting prev_(link_rate/lane_count) (Imre)
>     use the cached property name here instead of hard-coding it (Imre)
>     move test logic to function (Imre)
>     remove extra w/s (Imre)
>     remove braces in one liners (Imre)
>     enhance igt_info message (Pranay)
> v7: rename kms_fallback -> kms_dp_linktrain_fallback (Imre)
>     remove unused headers (Imre)
>     fill mst outputs based on root id (Imre)
>     check bounds for array (Imre)
>     use same syntax across code (Imre)
>     add todo for joined pipe (Imre)
>     remove redundant commit (Imre)
> v8: fail if the traversed output can't be saved (Imre)
>     use TEST_ONLY commit (Imre)
>     set link_status as good for all outputs in fit_modes_in_bw (Imre)
>     prepare fb's with modes that fit bw (Imre)
> 
> Cc: Imre Deak <imre.deak@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> Suggested-by: Imre Deak <imre.deak@intel.com>
> ---
>  tests/intel/kms_dp_linktrain_fallback.c | 409 ++++++++++++++++++++++++
>  tests/meson.build                       |   1 +
>  2 files changed, 410 insertions(+)
>  create mode 100644 tests/intel/kms_dp_linktrain_fallback.c
> 
> diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
> new file mode 100644
> index 000000000..924da6d7c
> --- /dev/null
> +++ b/tests/intel/kms_dp_linktrain_fallback.c
> @@ -0,0 +1,409 @@
> +/* SPDX-License-Identifier: MIT */
> +/*
> + * Copyright © 2024 Intel Corporation
> + */
> +
> +/**
> + * TEST: kms dp linktrain fallback
> + * Category: Display
> + * Description: Test link training fallback for eDP/DP connectors
> + * Driver requirement: i915, xe
> + * Functionality: link training
> + * Mega feature: General Display Features
> + * Test category: functionality test
> + */
> +
> +#include <sys/types.h>
> +
> +#include "igt.h"
> +
> +/**
> + * SUBTEST: dp-fallback
> + * Description: Test fallback on DP connectors
> + */
> +
> +#define RETRAIN_COUNT 1
> +/*
> + * Two consecutives link training failures
> + * reduces link params (link rate, lane count)
> + */
> +#define LT_FAILURE_REDUCED_CAPS 2
> +#define SPURIOUS_HPD_RETRY 3
> +
> +static int traversed_mst_outputs[IGT_MAX_PIPES];
> +static int traversed_mst_output_count;
> +typedef struct {
> +	int drm_fd;
> +	igt_display_t display;
> +	drmModeModeInfo *mode;
> +	igt_output_t *output;
> +	enum pipe pipe;
> +	struct igt_fb fb;
> +	struct igt_plane *primary;
> +	int n_pipes;
> +} data_t;
> +
> +typedef int (*condition_check_fn)(int drm_fd, igt_output_t *output);
> +
> +IGT_TEST_DESCRIPTION("Test link training fallback");
> +
> +static void find_mst_outputs(int drm_fd, data_t *data,
> +			     igt_output_t *output,
> +			     igt_output_t *mst_outputs[],
> +			     int *num_mst_outputs)
> +{
> +	int output_root_id, root_id;
> +	igt_output_t *connector_output;
> +
> +	output_root_id = igt_get_dp_mst_connector_id(output);
> +	/*
> +	 * If output is MST check all other connected output which shares
> +	 * same path and fill mst_outputs and num_mst_outputs
> +	 */
> +	for_each_connected_output(&data->display, connector_output) {
> +		if (!igt_check_output_is_dp_mst(connector_output))
> +			continue;
> +		root_id = igt_get_dp_mst_connector_id(connector_output);
> +		if (((*num_mst_outputs) < IGT_MAX_PIPES) && root_id == output_root_id)
> +			mst_outputs[(*num_mst_outputs)++] = connector_output;
> +	}
> +}
> +
> +static bool setup_mst_outputs(data_t *data, igt_output_t *mst_output[],
> +			      int *output_count)
> +{
> +	int i;
> +	igt_output_t *output;
> +
> +	/*
> +	 * Check if this is already traversed
> +	 */
> +	for (i = 0; i < traversed_mst_output_count; i++)
> +		if (i < IGT_MAX_PIPES &&
> +		    traversed_mst_outputs[i] == data->output->config.connector->connector_id)
> +			return false;
> +
> +	find_mst_outputs(data->drm_fd, data, data->output,
> +			 mst_output, output_count);
> +
> +	for (i = 0; i < *output_count; i++) {
> +		output = mst_output[i];
> +		if (traversed_mst_output_count < IGT_MAX_PIPES) {
> +			traversed_mst_outputs[traversed_mst_output_count++] = output->config.connector->connector_id;
> +			igt_info("Output %s is in same topology as %s\n",
> +				 igt_output_name(output),
> +				 igt_output_name(data->output));
> +		} else {
> +			igt_assert_f(false, "Unable to save traversed output\n");
> +			return false;
> +		}
> +	}
> +	return true;
> +}
> +
> +static void setup_pipe_on_outputs(data_t *data,
> +				      igt_output_t *outputs[],
> +				      int *output_count)
> +{
> +	int i = 0;
> +
> +	igt_require_f(data->n_pipes >= *output_count,
> +		      "Need %d pipes to assign to %d outputs\n",
> +		      data->n_pipes, *output_count);
> +
> +	for_each_pipe(&data->display, data->pipe) {
> +		if (i >= *output_count)
> +			break;
> +		/*
> +		 * TODO: add support for modes requiring joined pipes
> +		 */
> +		igt_info("Setting pipe %s on output %s\n",
> +			 kmstest_pipe_name(data->pipe),
> +			 igt_output_name(outputs[i]));
> +		igt_output_set_pipe(outputs[i++], data->pipe);
> +	}
> +}
> +
> +static void setup_modeset_on_outputs(data_t *data,
> +				     igt_output_t *outputs[],
> +				     int *output_count,
> +				     drmModeModeInfo *mode[],
> +				     struct igt_fb fb[],
> +				     struct igt_plane *primary[])
> +{
> +	int i;
> +
> +	for (i = 0; i < *output_count; i++) {
> +		mode[i] = igt_output_get_mode(outputs[i]);
> +		igt_info("Mode %dx%d@%d on output %s\n",
> +			 mode[i]->hdisplay, mode[i]->vdisplay,
> +			 mode[i]->vrefresh,
> +			 igt_output_name(outputs[i]));
> +		primary[i] = igt_output_get_plane_type(outputs[i],
> +						       DRM_PLANE_TYPE_PRIMARY);
> +		igt_create_color_fb(data->drm_fd,
> +				    mode[i]->hdisplay,
> +				    mode[i]->vdisplay,
> +				    DRM_FORMAT_XRGB8888,
> +				    DRM_FORMAT_MOD_LINEAR, 0.0, 1.0, 0.0,
> +				    &fb[i]);
> +		igt_plane_set_fb(primary[i], &fb[i]);
> +	}
> +}
> +
> +static bool fit_modes_in_bw(data_t *data)
> +{
> +	bool found;
> +	int ret;
> +	igt_output_t *output;
> +
> +	/*
> +	 * update the link status to good for all outputs
> +	 */
> +	for_each_connected_output(&data->display, output)
> +		igt_output_set_prop_value(output,
> +					  IGT_CONNECTOR_LINK_STATUS,
> +					  DRM_MODE_LINK_STATUS_GOOD);

It's an odd place to reset the link status, there should be a separate
function for it called from the test loop in test_fallback(). Also I
think it should only set the properties for the tested outputs - stored
in outputs[] in test_fallback() - instead of all the connected ones.

> +
> +	ret = igt_display_try_commit_atomic(&data->display,
> +					    DRM_MODE_ATOMIC_TEST_ONLY |
> +					    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> +	if (ret != 0) {
> +		found = igt_override_all_active_output_modes_to_fit_bw(&data->display);
> +		igt_require_f(found,
> +			      "No valid mode combo found for modeset\n");
> +		ret = igt_display_try_commit_atomic(&data->display,
> +						    DRM_MODE_ATOMIC_TEST_ONLY |
> +						    DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);

This commit could not really fail after
igt_override_all_active_output_modes_to_fit_bw() passed, so there's no
point in the commit.

With the above fixed:

Reviewed-by: Imre Deak <imre.deak@intel.com>

> +		igt_require_f(ret == 0,
> +			      "Commit failure during MST modeset\n");
> +	}
> +	return true;
> +}
> +
> +static bool validate_modeset_for_outputs(data_t *data,
> +					igt_output_t *outputs[],
> +					int *output_count,
> +					drmModeModeInfo *mode[],
> +					struct igt_fb fb[],
> +					struct igt_plane *primary[])
> +{
> +	igt_require_f(*output_count > 0, "Require at least 1 output\n");
> +	setup_pipe_on_outputs(data, outputs, output_count);
> +	igt_assert_f(fit_modes_in_bw(data), "Unable to fit modes in bw\n");
> +	setup_modeset_on_outputs(data, outputs,
> +				 output_count,
> +				 mode, fb, primary);
> +	return true;
> +}
> +
> +static bool setup_outputs(data_t *data, bool is_mst,
> +		      igt_output_t *outputs[],
> +		      int *output_count, drmModeModeInfo *mode[],
> +		      struct igt_fb fb[], struct igt_plane *primary[])
> +{
> +	bool ret;
> +
> +	*output_count = 0;
> +
> +	if (is_mst) {
> +		ret = setup_mst_outputs(data, outputs, output_count);
> +		if (!ret) {
> +			igt_info("Skipping MST output %s as already tested\n",
> +				 igt_output_name(data->output));
> +			return false;
> +		}
> +	} else
> +		if ((*output_count) < IGT_MAX_PIPES)
> +			outputs[(*output_count)++] = data->output;
> +
> +	ret = validate_modeset_for_outputs(data, outputs,
> +					   output_count, mode,
> +					   fb, primary);
> +
> +	if (!ret) {
> +		igt_info("Skipping output %s as valid pipe/output combo not found\n",
> +			 igt_output_name(data->output));
> +		return false;
> +	}
> +
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +	return true;
> +}
> +
> +static int check_condition_with_timeout(int drm_fd, igt_output_t *output,
> +					condition_check_fn check_fn,
> +					double interval, double timeout)
> +{
> +	struct timespec start_time, current_time;
> +	double elapsed_time;
> +
> +	clock_gettime(CLOCK_MONOTONIC, &start_time);
> +
> +	while (1) {
> +		if (check_fn(drm_fd, output) == 0)
> +			return 0;
> +
> +		clock_gettime(CLOCK_MONOTONIC, &current_time);
> +		elapsed_time = (current_time.tv_sec - start_time.tv_sec) +
> +			(current_time.tv_nsec - start_time.tv_nsec) / 1e9;
> +
> +		if (elapsed_time >= timeout)
> +			return -1;
> +
> +		usleep((useconds_t)(interval * 1000000));
> +	}
> +}
> +
> +static void test_fallback(data_t *data, bool is_mst)
> +{
> +	int output_count, retries;
> +	int max_link_rate, curr_link_rate, prev_link_rate;
> +	int max_lane_count, curr_lane_count, prev_lane_count;
> +	igt_output_t *outputs[IGT_MAX_PIPES];
> +	uint32_t link_status_prop_id;
> +	uint64_t link_status_value;
> +	drmModeModeInfo *modes[IGT_MAX_PIPES];
> +	drmModePropertyPtr link_status_prop;
> +	struct igt_fb fbs[IGT_MAX_PIPES];
> +	struct igt_plane *primarys[IGT_MAX_PIPES];
> +	struct udev_monitor *mon;
> +
> +	retries = SPURIOUS_HPD_RETRY;
> +
> +	igt_display_reset(&data->display);
> +	igt_reset_link_params(data->drm_fd, data->output);
> +	if (!setup_outputs(data, is_mst, outputs,
> +			   &output_count, modes, fbs,
> +			   primarys))
> +		return;
> +
> +	igt_info("Testing link training fallback on %s\n",
> +		 igt_output_name(data->output));
> +	max_link_rate = igt_get_max_link_rate(data->drm_fd, data->output);
> +	max_lane_count = igt_get_max_lane_count(data->drm_fd, data->output);
> +	prev_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
> +	prev_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
> +
> +	while (!igt_get_dp_link_retrain_disabled(data->drm_fd,
> +						 data->output)) {
> +		igt_info("Current link rate: %d, Current lane count: %d\n",
> +			 prev_link_rate,
> +			 prev_lane_count);
> +		mon = igt_watch_uevents();
> +		igt_force_lt_failure(data->drm_fd, data->output,
> +				     LT_FAILURE_REDUCED_CAPS);
> +		igt_force_link_retrain(data->drm_fd, data->output,
> +				       RETRAIN_COUNT);
> +
> +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> +							   data->output,
> +							   igt_get_dp_pending_retrain,
> +							   1.0, 20.0), 0);
> +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> +							   data->output,
> +							   igt_get_dp_pending_lt_failures,
> +							   1.0, 20.0), 0);
> +
> +		if (igt_get_dp_link_retrain_disabled(data->drm_fd,
> +						     data->output)) {
> +			igt_reset_connectors();
> +			return;
> +		}
> +
> +		igt_assert_f(igt_hotplug_detected(mon, 20),
> +			     "Didn't get hotplug for force link training failure\n");
> +
> +		kmstest_get_property(data->drm_fd,
> +				data->output->config.connector->connector_id,
> +				DRM_MODE_OBJECT_CONNECTOR, "link-status",
> +				&link_status_prop_id, &link_status_value,
> +				&link_status_prop);
> +		igt_assert_eq(link_status_value, DRM_MODE_LINK_STATUS_BAD);
> +		igt_flush_uevents(mon);
> +		igt_assert_f(validate_modeset_for_outputs(data,
> +							  outputs,
> +							  &output_count,
> +							  modes,
> +							  fbs,
> +							  primarys),
> +			     "modeset failed\n");
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +		igt_assert_eq(data->output->values[IGT_CONNECTOR_LINK_STATUS], DRM_MODE_LINK_STATUS_GOOD);
> +		curr_link_rate = igt_get_current_link_rate(data->drm_fd, data->output);
> +		curr_lane_count = igt_get_current_lane_count(data->drm_fd, data->output);
> +
> +		igt_assert_f((curr_link_rate < prev_link_rate ||
> +			     curr_lane_count < prev_lane_count) ||
> +			     ((curr_link_rate == max_link_rate && curr_lane_count == max_lane_count) && --retries),
> +			     "Fallback unsuccessful\n");
> +
> +		prev_link_rate = curr_link_rate;
> +		prev_lane_count = curr_lane_count;
> +	}
> +}
> +
> +static bool run_test(data_t *data)
> +{
> +	bool ran = false;
> +	igt_output_t *output;
> +
> +	for_each_connected_output(&data->display, output) {
> +		data->output = output;
> +
> +		if (!igt_has_force_link_training_failure_debugfs(data->drm_fd,
> +								 data->output)) {
> +			igt_info("Output %s doesn't support forcing link training failure\n",
> +				 igt_output_name(data->output));
> +			continue;
> +		}
> +
> +		if (output->config.connector->connector_type != DRM_MODE_CONNECTOR_DisplayPort) {
> +			igt_info("Skipping output %s as it's not DP\n", output->name);
> +				continue;
> +		}
> +
> +		ran = true;
> +
> +		/*
> +		 * Check output is MST
> +		 */
> +		if (igt_check_output_is_dp_mst(data->output)) {
> +			igt_info("Testing MST output %s\n",
> +				 igt_output_name(data->output));
> +			test_fallback(data, true);
> +		} else {
> +			igt_info("Testing DP output %s\n",
> +				 igt_output_name(data->output));
> +			test_fallback(data, false);
> +		}
> +	}
> +	return ran;
> +}
> +
> +igt_main
> +{
> +	data_t data = {};
> +
> +	igt_fixture {
> +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
> +						     DRIVER_XE);
> +		kmstest_set_vt_graphics_mode();
> +		igt_display_require(&data.display, data.drm_fd);
> +		igt_display_require_output(&data.display);
> +		for_each_pipe(&data.display, data.pipe)
> +			data.n_pipes++;
> +	}
> +
> +	igt_subtest("dp-fallback") {
> +		igt_require_f(run_test(&data),
> +			      "Skipping test as no output found or none supports fallback\n");
> +	}
> +
> +	igt_fixture {
> +		igt_remove_fb(data.drm_fd, &data.fb);
> +		igt_display_fini(&data.display);
> +		close(data.drm_fd);
> +	}
> +}
> diff --git a/tests/meson.build b/tests/meson.build
> index 00556c9d6..4adaf34f2 100644
> --- a/tests/meson.build
> +++ b/tests/meson.build
> @@ -247,6 +247,7 @@ intel_kms_progs = [
>  	'kms_ccs',
>  	'kms_cdclk',
>  	'kms_dirtyfb',
> +        'kms_dp_linktrain_fallback',
>  	'kms_draw_crc',
>  	'kms_dsc',
>  	'kms_fb_coherency',
> -- 
> 2.43.0
> 

RE: [PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test for validating fallback

[Joshi, Kunal1]

Hello,

Thanks Imre for the valuable feedback and reviews 😊.

Regards
Kunal Joshi

> -----Original Message-----
> From: Deak, Imre <imre.deak@intel.com>
> Sent: Thursday, September 26, 2024 3:55 PM
> To: Joshi, Kunal1 <kunal1.joshi@intel.com>
> Cc: igt-dev@lists.freedesktop.org
> Subject: Re: [PATCH i-g-t 6/7] tests/intel/kms_dp_linktrain_fallback: add test
> for validating fallback
> 
> On Wed, Sep 25, 2024 at 11:59:43PM +0530, Kunal Joshi wrote:
> > add test to valdiate fallback for DP connector, eDP subtest will be
> > added later.
> >
> > How does test validates fallback?
> > - test start by doing initial modeset on default mode
> >    (if connector is DP then we enable just that connector,
> >     if its DP-MST we enable all on the same topology)
> > - force link training failures and retrain until we reach
> >   lowest param or retrain is disabled
> > - expect hotplug and link-status to turn bad
> > - expect link params reduce after fallback
> >
> > v2: add test for mst (imre)
> >     refresh mode list (imre)
> >     monitor got hotplugs (imre)
> >     check link parameter are reduced (imre)
> > v3: call check_fn (Santosh)
> > v4: handle buggy lg monitor (Imre)
> >     remove reset in between (Imre)
> > v5: fit modes wrt to bw in non-mst case as well
> > v6: remove LT_FAILURE_SAME_CAPS (Imre)
> >     explain LT_FAILURE_REDUCED_CAPS to be 2 (Imre)
> >     combine infra for mst and non-mst case (Imre)
> >     call igt_reset_link_params before setup (Imre)
> >     Avoid duplication setting prev_(link_rate/lane_count) (Imre)
> >     use the cached property name here instead of hard-coding it (Imre)
> >     move test logic to function (Imre)
> >     remove extra w/s (Imre)
> >     remove braces in one liners (Imre)
> >     enhance igt_info message (Pranay)
> > v7: rename kms_fallback -> kms_dp_linktrain_fallback (Imre)
> >     remove unused headers (Imre)
> >     fill mst outputs based on root id (Imre)
> >     check bounds for array (Imre)
> >     use same syntax across code (Imre)
> >     add todo for joined pipe (Imre)
> >     remove redundant commit (Imre)
> > v8: fail if the traversed output can't be saved (Imre)
> >     use TEST_ONLY commit (Imre)
> >     set link_status as good for all outputs in fit_modes_in_bw (Imre)
> >     prepare fb's with modes that fit bw (Imre)
> >
> > Cc: Imre Deak <imre.deak@intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> > Suggested-by: Imre Deak <imre.deak@intel.com>
> > ---
> >  tests/intel/kms_dp_linktrain_fallback.c | 409 ++++++++++++++++++++++++
> >  tests/meson.build                       |   1 +
> >  2 files changed, 410 insertions(+)
> >  create mode 100644 tests/intel/kms_dp_linktrain_fallback.c
> >
> > diff --git a/tests/intel/kms_dp_linktrain_fallback.c
> > b/tests/intel/kms_dp_linktrain_fallback.c
> > new file mode 100644
> > index 000000000..924da6d7c
> > --- /dev/null
> > +++ b/tests/intel/kms_dp_linktrain_fallback.c
> > @@ -0,0 +1,409 @@
> > +/* SPDX-License-Identifier: MIT */
> > +/*
> > + * Copyright © 2024 Intel Corporation  */
> > +
> > +/**
> > + * TEST: kms dp linktrain fallback
> > + * Category: Display
> > + * Description: Test link training fallback for eDP/DP connectors
> > + * Driver requirement: i915, xe
> > + * Functionality: link training
> > + * Mega feature: General Display Features
> > + * Test category: functionality test
> > + */
> > +
> > +#include <sys/types.h>
> > +
> > +#include "igt.h"
> > +
> > +/**
> > + * SUBTEST: dp-fallback
> > + * Description: Test fallback on DP connectors  */
> > +
> > +#define RETRAIN_COUNT 1
> > +/*
> > + * Two consecutives link training failures
> > + * reduces link params (link rate, lane count)  */ #define
> > +LT_FAILURE_REDUCED_CAPS 2 #define SPURIOUS_HPD_RETRY 3
> > +
> > +static int traversed_mst_outputs[IGT_MAX_PIPES];
> > +static int traversed_mst_output_count; typedef struct {
> > +	int drm_fd;
> > +	igt_display_t display;
> > +	drmModeModeInfo *mode;
> > +	igt_output_t *output;
> > +	enum pipe pipe;
> > +	struct igt_fb fb;
> > +	struct igt_plane *primary;
> > +	int n_pipes;
> > +} data_t;
> > +
> > +typedef int (*condition_check_fn)(int drm_fd, igt_output_t *output);
> > +
> > +IGT_TEST_DESCRIPTION("Test link training fallback");
> > +
> > +static void find_mst_outputs(int drm_fd, data_t *data,
> > +			     igt_output_t *output,
> > +			     igt_output_t *mst_outputs[],
> > +			     int *num_mst_outputs)
> > +{
> > +	int output_root_id, root_id;
> > +	igt_output_t *connector_output;
> > +
> > +	output_root_id = igt_get_dp_mst_connector_id(output);
> > +	/*
> > +	 * If output is MST check all other connected output which shares
> > +	 * same path and fill mst_outputs and num_mst_outputs
> > +	 */
> > +	for_each_connected_output(&data->display, connector_output) {
> > +		if (!igt_check_output_is_dp_mst(connector_output))
> > +			continue;
> > +		root_id = igt_get_dp_mst_connector_id(connector_output);
> > +		if (((*num_mst_outputs) < IGT_MAX_PIPES) && root_id ==
> output_root_id)
> > +			mst_outputs[(*num_mst_outputs)++] =
> connector_output;
> > +	}
> > +}
> > +
> > +static bool setup_mst_outputs(data_t *data, igt_output_t *mst_output[],
> > +			      int *output_count)
> > +{
> > +	int i;
> > +	igt_output_t *output;
> > +
> > +	/*
> > +	 * Check if this is already traversed
> > +	 */
> > +	for (i = 0; i < traversed_mst_output_count; i++)
> > +		if (i < IGT_MAX_PIPES &&
> > +		    traversed_mst_outputs[i] == data->output-
> >config.connector->connector_id)
> > +			return false;
> > +
> > +	find_mst_outputs(data->drm_fd, data, data->output,
> > +			 mst_output, output_count);
> > +
> > +	for (i = 0; i < *output_count; i++) {
> > +		output = mst_output[i];
> > +		if (traversed_mst_output_count < IGT_MAX_PIPES) {
> > +
> 	traversed_mst_outputs[traversed_mst_output_count++] = output-
> >config.connector->connector_id;
> > +			igt_info("Output %s is in same topology as %s\n",
> > +				 igt_output_name(output),
> > +				 igt_output_name(data->output));
> > +		} else {
> > +			igt_assert_f(false, "Unable to save traversed
> output\n");
> > +			return false;
> > +		}
> > +	}
> > +	return true;
> > +}
> > +
> > +static void setup_pipe_on_outputs(data_t *data,
> > +				      igt_output_t *outputs[],
> > +				      int *output_count)
> > +{
> > +	int i = 0;
> > +
> > +	igt_require_f(data->n_pipes >= *output_count,
> > +		      "Need %d pipes to assign to %d outputs\n",
> > +		      data->n_pipes, *output_count);
> > +
> > +	for_each_pipe(&data->display, data->pipe) {
> > +		if (i >= *output_count)
> > +			break;
> > +		/*
> > +		 * TODO: add support for modes requiring joined pipes
> > +		 */
> > +		igt_info("Setting pipe %s on output %s\n",
> > +			 kmstest_pipe_name(data->pipe),
> > +			 igt_output_name(outputs[i]));
> > +		igt_output_set_pipe(outputs[i++], data->pipe);
> > +	}
> > +}
> > +
> > +static void setup_modeset_on_outputs(data_t *data,
> > +				     igt_output_t *outputs[],
> > +				     int *output_count,
> > +				     drmModeModeInfo *mode[],
> > +				     struct igt_fb fb[],
> > +				     struct igt_plane *primary[]) {
> > +	int i;
> > +
> > +	for (i = 0; i < *output_count; i++) {
> > +		mode[i] = igt_output_get_mode(outputs[i]);
> > +		igt_info("Mode %dx%d@%d on output %s\n",
> > +			 mode[i]->hdisplay, mode[i]->vdisplay,
> > +			 mode[i]->vrefresh,
> > +			 igt_output_name(outputs[i]));
> > +		primary[i] = igt_output_get_plane_type(outputs[i],
> > +
> DRM_PLANE_TYPE_PRIMARY);
> > +		igt_create_color_fb(data->drm_fd,
> > +				    mode[i]->hdisplay,
> > +				    mode[i]->vdisplay,
> > +				    DRM_FORMAT_XRGB8888,
> > +				    DRM_FORMAT_MOD_LINEAR, 0.0, 1.0, 0.0,
> > +				    &fb[i]);
> > +		igt_plane_set_fb(primary[i], &fb[i]);
> > +	}
> > +}
> > +
> > +static bool fit_modes_in_bw(data_t *data) {
> > +	bool found;
> > +	int ret;
> > +	igt_output_t *output;
> > +
> > +	/*
> > +	 * update the link status to good for all outputs
> > +	 */
> > +	for_each_connected_output(&data->display, output)
> > +		igt_output_set_prop_value(output,
> > +					  IGT_CONNECTOR_LINK_STATUS,
> > +					  DRM_MODE_LINK_STATUS_GOOD);
> 
> It's an odd place to reset the link status, there should be a separate function
> for it called from the test loop in test_fallback(). Also I think it should only set
> the properties for the tested outputs - stored in outputs[] in test_fallback() -
> instead of all the connected ones.
> 
> > +
> > +	ret = igt_display_try_commit_atomic(&data->display,
> > +					    DRM_MODE_ATOMIC_TEST_ONLY
> |
> > +
> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> > +	if (ret != 0) {
> > +		found =
> igt_override_all_active_output_modes_to_fit_bw(&data->display);
> > +		igt_require_f(found,
> > +			      "No valid mode combo found for modeset\n");
> > +		ret = igt_display_try_commit_atomic(&data->display,
> > +
> DRM_MODE_ATOMIC_TEST_ONLY |
> > +
> DRM_MODE_ATOMIC_ALLOW_MODESET, NULL);
> 
> This commit could not really fail after
> igt_override_all_active_output_modes_to_fit_bw() passed, so there's no
> point in the commit.
> 
> With the above fixed:
> 
> Reviewed-by: Imre Deak <imre.deak@intel.com>
> 
> > +		igt_require_f(ret == 0,
> > +			      "Commit failure during MST modeset\n");
> > +	}
> > +	return true;
> > +}
> > +
> > +static bool validate_modeset_for_outputs(data_t *data,
> > +					igt_output_t *outputs[],
> > +					int *output_count,
> > +					drmModeModeInfo *mode[],
> > +					struct igt_fb fb[],
> > +					struct igt_plane *primary[])
> > +{
> > +	igt_require_f(*output_count > 0, "Require at least 1 output\n");
> > +	setup_pipe_on_outputs(data, outputs, output_count);
> > +	igt_assert_f(fit_modes_in_bw(data), "Unable to fit modes in bw\n");
> > +	setup_modeset_on_outputs(data, outputs,
> > +				 output_count,
> > +				 mode, fb, primary);
> > +	return true;
> > +}
> > +
> > +static bool setup_outputs(data_t *data, bool is_mst,
> > +		      igt_output_t *outputs[],
> > +		      int *output_count, drmModeModeInfo *mode[],
> > +		      struct igt_fb fb[], struct igt_plane *primary[]) {
> > +	bool ret;
> > +
> > +	*output_count = 0;
> > +
> > +	if (is_mst) {
> > +		ret = setup_mst_outputs(data, outputs, output_count);
> > +		if (!ret) {
> > +			igt_info("Skipping MST output %s as already
> tested\n",
> > +				 igt_output_name(data->output));
> > +			return false;
> > +		}
> > +	} else
> > +		if ((*output_count) < IGT_MAX_PIPES)
> > +			outputs[(*output_count)++] = data->output;
> > +
> > +	ret = validate_modeset_for_outputs(data, outputs,
> > +					   output_count, mode,
> > +					   fb, primary);
> > +
> > +	if (!ret) {
> > +		igt_info("Skipping output %s as valid pipe/output combo not
> found\n",
> > +			 igt_output_name(data->output));
> > +		return false;
> > +	}
> > +
> > +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +	return true;
> > +}
> > +
> > +static int check_condition_with_timeout(int drm_fd, igt_output_t *output,
> > +					condition_check_fn check_fn,
> > +					double interval, double timeout) {
> > +	struct timespec start_time, current_time;
> > +	double elapsed_time;
> > +
> > +	clock_gettime(CLOCK_MONOTONIC, &start_time);
> > +
> > +	while (1) {
> > +		if (check_fn(drm_fd, output) == 0)
> > +			return 0;
> > +
> > +		clock_gettime(CLOCK_MONOTONIC, &current_time);
> > +		elapsed_time = (current_time.tv_sec - start_time.tv_sec) +
> > +			(current_time.tv_nsec - start_time.tv_nsec) / 1e9;
> > +
> > +		if (elapsed_time >= timeout)
> > +			return -1;
> > +
> > +		usleep((useconds_t)(interval * 1000000));
> > +	}
> > +}
> > +
> > +static void test_fallback(data_t *data, bool is_mst) {
> > +	int output_count, retries;
> > +	int max_link_rate, curr_link_rate, prev_link_rate;
> > +	int max_lane_count, curr_lane_count, prev_lane_count;
> > +	igt_output_t *outputs[IGT_MAX_PIPES];
> > +	uint32_t link_status_prop_id;
> > +	uint64_t link_status_value;
> > +	drmModeModeInfo *modes[IGT_MAX_PIPES];
> > +	drmModePropertyPtr link_status_prop;
> > +	struct igt_fb fbs[IGT_MAX_PIPES];
> > +	struct igt_plane *primarys[IGT_MAX_PIPES];
> > +	struct udev_monitor *mon;
> > +
> > +	retries = SPURIOUS_HPD_RETRY;
> > +
> > +	igt_display_reset(&data->display);
> > +	igt_reset_link_params(data->drm_fd, data->output);
> > +	if (!setup_outputs(data, is_mst, outputs,
> > +			   &output_count, modes, fbs,
> > +			   primarys))
> > +		return;
> > +
> > +	igt_info("Testing link training fallback on %s\n",
> > +		 igt_output_name(data->output));
> > +	max_link_rate = igt_get_max_link_rate(data->drm_fd, data->output);
> > +	max_lane_count = igt_get_max_lane_count(data->drm_fd, data-
> >output);
> > +	prev_link_rate = igt_get_current_link_rate(data->drm_fd, data-
> >output);
> > +	prev_lane_count = igt_get_current_lane_count(data->drm_fd,
> > +data->output);
> > +
> > +	while (!igt_get_dp_link_retrain_disabled(data->drm_fd,
> > +						 data->output)) {
> > +		igt_info("Current link rate: %d, Current lane count: %d\n",
> > +			 prev_link_rate,
> > +			 prev_lane_count);
> > +		mon = igt_watch_uevents();
> > +		igt_force_lt_failure(data->drm_fd, data->output,
> > +				     LT_FAILURE_REDUCED_CAPS);
> > +		igt_force_link_retrain(data->drm_fd, data->output,
> > +				       RETRAIN_COUNT);
> > +
> > +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> > +							   data->output,
> > +
> igt_get_dp_pending_retrain,
> > +							   1.0, 20.0), 0);
> > +		igt_assert_eq(check_condition_with_timeout(data->drm_fd,
> > +							   data->output,
> > +
> igt_get_dp_pending_lt_failures,
> > +							   1.0, 20.0), 0);
> > +
> > +		if (igt_get_dp_link_retrain_disabled(data->drm_fd,
> > +						     data->output)) {
> > +			igt_reset_connectors();
> > +			return;
> > +		}
> > +
> > +		igt_assert_f(igt_hotplug_detected(mon, 20),
> > +			     "Didn't get hotplug for force link training
> failure\n");
> > +
> > +		kmstest_get_property(data->drm_fd,
> > +				data->output->config.connector-
> >connector_id,
> > +				DRM_MODE_OBJECT_CONNECTOR, "link-
> status",
> > +				&link_status_prop_id, &link_status_value,
> > +				&link_status_prop);
> > +		igt_assert_eq(link_status_value,
> DRM_MODE_LINK_STATUS_BAD);
> > +		igt_flush_uevents(mon);
> > +		igt_assert_f(validate_modeset_for_outputs(data,
> > +							  outputs,
> > +							  &output_count,
> > +							  modes,
> > +							  fbs,
> > +							  primarys),
> > +			     "modeset failed\n");
> > +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
> > +
> > +		igt_assert_eq(data->output-
> >values[IGT_CONNECTOR_LINK_STATUS], DRM_MODE_LINK_STATUS_GOOD);
> > +		curr_link_rate = igt_get_current_link_rate(data->drm_fd,
> data->output);
> > +		curr_lane_count = igt_get_current_lane_count(data->drm_fd,
> > +data->output);
> > +
> > +		igt_assert_f((curr_link_rate < prev_link_rate ||
> > +			     curr_lane_count < prev_lane_count) ||
> > +			     ((curr_link_rate == max_link_rate &&
> curr_lane_count == max_lane_count) && --retries),
> > +			     "Fallback unsuccessful\n");
> > +
> > +		prev_link_rate = curr_link_rate;
> > +		prev_lane_count = curr_lane_count;
> > +	}
> > +}
> > +
> > +static bool run_test(data_t *data)
> > +{
> > +	bool ran = false;
> > +	igt_output_t *output;
> > +
> > +	for_each_connected_output(&data->display, output) {
> > +		data->output = output;
> > +
> > +		if (!igt_has_force_link_training_failure_debugfs(data-
> >drm_fd,
> > +								 data-
> >output)) {
> > +			igt_info("Output %s doesn't support forcing link
> training failure\n",
> > +				 igt_output_name(data->output));
> > +			continue;
> > +		}
> > +
> > +		if (output->config.connector->connector_type !=
> DRM_MODE_CONNECTOR_DisplayPort) {
> > +			igt_info("Skipping output %s as it's not DP\n", output-
> >name);
> > +				continue;
> > +		}
> > +
> > +		ran = true;
> > +
> > +		/*
> > +		 * Check output is MST
> > +		 */
> > +		if (igt_check_output_is_dp_mst(data->output)) {
> > +			igt_info("Testing MST output %s\n",
> > +				 igt_output_name(data->output));
> > +			test_fallback(data, true);
> > +		} else {
> > +			igt_info("Testing DP output %s\n",
> > +				 igt_output_name(data->output));
> > +			test_fallback(data, false);
> > +		}
> > +	}
> > +	return ran;
> > +}
> > +
> > +igt_main
> > +{
> > +	data_t data = {};
> > +
> > +	igt_fixture {
> > +		data.drm_fd = drm_open_driver_master(DRIVER_INTEL |
> > +						     DRIVER_XE);
> > +		kmstest_set_vt_graphics_mode();
> > +		igt_display_require(&data.display, data.drm_fd);
> > +		igt_display_require_output(&data.display);
> > +		for_each_pipe(&data.display, data.pipe)
> > +			data.n_pipes++;
> > +	}
> > +
> > +	igt_subtest("dp-fallback") {
> > +		igt_require_f(run_test(&data),
> > +			      "Skipping test as no output found or none
> supports fallback\n");
> > +	}
> > +
> > +	igt_fixture {
> > +		igt_remove_fb(data.drm_fd, &data.fb);
> > +		igt_display_fini(&data.display);
> > +		close(data.drm_fd);
> > +	}
> > +}
> > diff --git a/tests/meson.build b/tests/meson.build index
> > 00556c9d6..4adaf34f2 100644
> > --- a/tests/meson.build
> > +++ b/tests/meson.build
> > @@ -247,6 +247,7 @@ intel_kms_progs = [
> >  	'kms_ccs',
> >  	'kms_cdclk',
> >  	'kms_dirtyfb',
> > +        'kms_dp_linktrain_fallback',
> >  	'kms_draw_crc',
> >  	'kms_dsc',
> >  	'kms_fb_coherency',
> > --
> > 2.43.0
> >