Re: [PATCH i-g-t] RFC: tests/intel/kms_joiner: Add a new test to validate non-joiner mode

[Karthik B S <karthik.b.s@intel.com>]

Hi,

On 12/18/2024 12:46 AM, Jeevan B wrote:
> We need to ensure that the system does not use a joiner for modes that do
> not require it. This test will validate that the correct non-joiner mode
> is selected, and then forcing a modeset and flip on the last pipe. If the
> joiner is mistakenly enabled for a non-joiner mode, the test should fail.
> otherwise, the commit should proceed as expected.
>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>   tests/intel/kms_joiner.c | 83 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 83 insertions(+)
>
> diff --git a/tests/intel/kms_joiner.c b/tests/intel/kms_joiner.c
> index 9a353ee1b..a41f201d1 100644
> --- a/tests/intel/kms_joiner.c
> +++ b/tests/intel/kms_joiner.c
> @@ -71,10 +71,15 @@
>    * SUBTEST: invalid-modeset-force-ultra-joiner
>    * Description: Verify if the modeset on the other pipes are rejected when
>    *              the pipe A is active with force ultra joiner modeset.
> + *
> + * SUBTEST: basic-non-joiner
> + * Description:
>    */
>   IGT_TEST_DESCRIPTION("Test joiner / force joiner");
>   
>   #define INVALID_TEST_OUTPUT 2
> +#define HDISPLAY_6K_PER_PIPE 6144
> +#define HDISPLAY_5K_PER_PIPE 5120
>   
>   typedef struct {
>   	int drm_fd;
> @@ -82,6 +87,7 @@ typedef struct {
>   	int ultra_joiner_output_count;
>   	int non_big_joiner_output_count;
>   	int non_ultra_joiner_output_count;
> +	int nonjoiner_output_count;
>   	int mixed_output_count;
>   	int output_count;
>   	int n_pipes;
> @@ -91,6 +97,7 @@ typedef struct {
>   	igt_output_t *non_big_joiner_output[IGT_MAX_PIPES];
>   	igt_output_t *non_ultra_joiner_output[IGT_MAX_PIPES];
>   	igt_output_t *mixed_output[IGT_MAX_PIPES];
> +	igt_output_t *nonjoiner_output[IGT_MAX_PIPES];
>   	enum pipe pipe_seq[IGT_MAX_PIPES];
>   	igt_display_t display;
>   } data_t;
> @@ -161,6 +168,25 @@ static enum pipe setup_pipe(data_t *data, igt_output_t *output, enum pipe pipe,
>   	return master_pipe;
>   }
>   
> +static bool nonjoiner_mode_found(int drm_fd, drmModeConnector *connector, drmModeModeInfo *mode)
> +{
> +	igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
> +	igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);
The second sort immediately following the first, nullifies the first 
one. Please check. Also with below loop is this sorting adding any value?
> +
> +	for (int i = 0; i < connector->count_modes; i++) {
> +		drmModeModeInfo *current_mode = &connector->modes[i];
> +
> +		if ((current_mode->hdisplay <= HDISPLAY_6K_PER_PIPE &&
> +		    current_mode->clock <= max_dotclock) ||
> +		    (current_mode->hdisplay <= HDISPLAY_5K_PER_PIPE)) {
Although this will give a non joiner mode, do we really want to test the 
first non-joiner mode we get. I feel this test would add value only if 
check the corner cases.
> +			*mode = *current_mode;
> +			return true;
> +		}
> +	}
> +
> +	return false;
> +}
> +
>   static void test_single_joiner(data_t *data, int output_count, bool force_joiner)
>   {
>   	int i;
> @@ -409,6 +435,49 @@ static void test_ultra_joiner(data_t *data, bool invalid_pipe, bool two_display,
>   	}
>   }
>   
> +static void test_single_non_joiner(data_t *data)
> +{
> +	int count;
> +	igt_output_t **outputs, *output;
> +	igt_fb_t fb;
> +	igt_plane_t *primary;
> +	drmModeModeInfo mode;
> +	drmModeConnector *con;
> +
> +	count = data->nonjoiner_output_count;
> +	outputs = data->nonjoiner_output;
> +
> +	for (int i = 0; i < count; i++) {
> +		igt_display_reset(&data->display);
> +		igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +		output = outputs[i];
> +		con = output->config.connector;
> +
> +		if (nonjoiner_mode_found(data->drm_fd, con, &mode)) {
> +			igt_output_override_mode(output, &mode);
> +			igt_info("Assigning pipe %s to %s with mode %dx%d@%d\n",
> +				 kmstest_pipe_name(data->pipe_seq[data->n_pipes - 1]),
> +				 igt_output_name(output), mode.hdisplay,
> +				 mode.vdisplay, mode.vrefresh);
> +
> +			igt_output_set_pipe(output, data->pipe_seq[data->n_pipes - 1]);
> +
> +			primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> +
> +			igt_create_pattern_fb(data->drm_fd, mode.hdisplay, mode.vdisplay, DRM_FORMAT_XRGB8888,
> +					      DRM_FORMAT_MOD_LINEAR, &fb);
> +
> +			igt_plane_set_fb(primary, &fb);
> +			igt_assert(igt_display_try_commit2(&data->display, COMMIT_ATOMIC));

Commit failure is not the right way to confirm if joiner is being used 
IMHO, as commit could fail for even reasons other than joiner being 
enabled. Please check if there is a better way to validate/confirm this.

Thanks,
Karthik.B.S
> +			igt_plane_set_fb(primary, NULL);
> +			igt_remove_fb(data->drm_fd, &fb);
> +		}
> +		else {
> +			igt_warn("No valid non-joiner mode found for output %s\n", igt_output_name(output));
> +		}
> +	}
> +}
> +
>   igt_main
>   {
>   	bool ultra_joiner_supported, is_dgfx;
> @@ -423,6 +492,7 @@ igt_main
>   		data.ultra_joiner_output_count = 0;
>   		data.non_big_joiner_output_count = 0;
>   		data.non_ultra_joiner_output_count = 0;
> +		data.nonjoiner_output_count = 0;
>   		data.mixed_output_count = 0;
>   		data.output_count = 0;
>   		j = 0;
> @@ -441,6 +511,7 @@ igt_main
>   
>   		for_each_connected_output(&data.display, output) {
>   			bool ultrajoiner_found = false, bigjoiner_found = false, force_joiner_supported = false;
> +			bool nonjoiner_found = false;
>   			drmModeConnector *connector = output->config.connector;
>   
>   			/*
> @@ -451,6 +522,7 @@ igt_main
>   			 */
>   			bigjoiner_found = bigjoiner_mode_found(data.drm_fd, connector, max_dotclock, &mode);
>   			ultrajoiner_found = ultrajoiner_mode_found(data.drm_fd, connector, max_dotclock, &mode);
> +			nonjoiner_found = nonjoiner_mode_found(data.drm_fd, connector, &mode);
>   
>   			if (igt_has_force_joiner_debugfs(data.drm_fd, output->name))
>   				force_joiner_supported = true;
> @@ -465,6 +537,9 @@ igt_main
>   			else if (force_joiner_supported)
>   				data.non_big_joiner_output[data.non_big_joiner_output_count++] = output;
>   
> +			if (nonjoiner_found)
> +				data.nonjoiner_output[data.nonjoiner_output_count++] = output;
> +
>   			data.output_count++;
>   		}
>   		if (data.big_joiner_output_count == 1 && data.non_big_joiner_output_count >= 1) {
> @@ -615,6 +690,14 @@ igt_main
>   		}
>   	}
>   
> +	igt_describe("Verify the basic modeset on big joiner mode on all pipes");
> +	igt_subtest_with_dynamic("basic-non-joiner") {
> +		igt_require_f(data.n_pipes >= 1,
> +			      "Minimum of 1 pipes are required\n");
> +		igt_dynamic_f("non-joiner")
> +			test_single_non_joiner(&data);
> +        }
> +
>   	igt_fixture {
>   		igt_display_fini(&data.display);
>   		drm_close_driver(data.drm_fd);