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

Imre Deak imre.deak at intel.com
Mon Sep 23 13:43:52 UTC 2024


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 at intel.com>
> > Signed-off-by: Kunal Joshi <kunal1.joshi at intel.com>
> > Suggested-by: Imre Deak <imre.deak at 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
> > 


More information about the igt-dev mailing list