Re: [PATCH i-g-t v5 1/1] tests/intel/kms_joiner: switch modeset between uj and bj

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

On 10/22/2024 7:59 AM, Santhosh Reddy Guddati wrote:
> Add a subtest to validate switching from ultra joiner to big joiner
> and vice-versa.
>
> v2: Add new subtests for switching without force joiner (Karthik)
> v3: start with uj to bj switch, if not available switch to force mode
> v4: check for uj, bj support before starting dynamic tests (karthik)
>      call reset_connectors after forcing to uj
> v5: Add a separate function to switch modes and execute test for each
>      connected supported output (karthik)
>
> Signed-off-by: Santhosh Reddy Guddati <santhosh.reddy.guddati@intel.com>
> ---
>   tests/intel/kms_joiner.c | 94 ++++++++++++++++++++++++++++++++++++++++
>   1 file changed, 94 insertions(+)
>
> diff --git a/tests/intel/kms_joiner.c b/tests/intel/kms_joiner.c
> index 9a353ee1b..88d1567d7 100644
> --- a/tests/intel/kms_joiner.c
> +++ b/tests/intel/kms_joiner.c
> @@ -71,6 +71,9 @@
>    * 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: switch-modeset-ultra-joiner-big-joiner
> + * Description: Verify switching between ultra joiner and big joiner modeset.
>    */
>   IGT_TEST_DESCRIPTION("Test joiner / force joiner");
>   
> @@ -161,6 +164,82 @@ static enum pipe setup_pipe(data_t *data, igt_output_t *output, enum pipe pipe,
>   	return master_pipe;
>   }
>   
> +static void set_joiner_mode(data_t *data, igt_output_t *output, drmModeModeInfo *mode)
> +{
> +	igt_plane_t *primary;
> +	igt_fb_t fb;
> +
> +	igt_output_set_pipe(output, PIPE_A);
> +	igt_output_override_mode(output, mode);
> +	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_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	igt_display_reset(&data->display);
> +	igt_reset_connectors();
> +	igt_plane_set_fb(primary, NULL);
> +	igt_remove_fb(data->drm_fd, &fb);
> +}
> +
> +static void switch_modeset_ultra_joiner_big_joiner(data_t *data, igt_output_t *output)
> +{
> +	drmModeModeInfo bj_mode;
> +	drmModeModeInfo uj_mode;
> +	int status;
> +	bool ultrajoiner_found;
> +	enum pipe pipe;
> +
> +	drmModeConnector *connector = output->config.connector;
> +
> +	igt_display_reset(&data->display);
> +	igt_display_commit2(&data->display, COMMIT_ATOMIC);
> +
> +	ultrajoiner_found = ultrajoiner_mode_found(data->drm_fd, connector, max_dotclock, &uj_mode);
> +
> +	if (ultrajoiner_found) {
Hi Santhosh,
> +		uj_mode = *igt_output_get_mode(output);
This is redundant as this is already being populated in the previous 
function. Please remove this.
> +	} else if (data->non_ultra_joiner_output_count > 0) {
> +		status = kmstest_force_connector_joiner(data->drm_fd, output->config.connector,
> +							JOINED_PIPES_ULTRA_JOINER);
> +		igt_assert_f(status, "Failed to toggle force joiner\n");
> +		uj_mode = *igt_output_get_mode(output);
This sequence will lead to test fail whenever we have an output which 
doesn't support either UJ or force UJ. Please use 
"force_joiner_supported && is_dsc_supported_by_sink" check before 
toggling the joiner debugfs.
> +	} else {
> +		igt_skip("No ultra joiner mode found on output %s\n", output->name);
> +	}
> +
> +	igt_dynamic_f("pipe-%s-%s", kmstest_pipe_name(pipe), output->name) {
> +		set_joiner_mode(data, output, &uj_mode);
> +		/* Switch to big joiner mode */
> +		if (bigjoiner_mode_found(data->drm_fd, output->config.connector,
> +					 max_dotclock, &bj_mode)) {
> +			bj_mode = *igt_output_get_mode(output);
> +			igt_output_override_mode(output, &bj_mode);
Same as above.
> +		} else {
> +			status = kmstest_force_connector_joiner(data->drm_fd,
> +								output->config.connector,
> +								JOINED_PIPES_BIG_JOINER);
> +			igt_assert_f(status, "Failed to toggle force joiner\n");
> +			bj_mode = *igt_output_get_mode(output);

Same as above.

Regards,
Karthik.B.S
> +		}
> +
> +		set_joiner_mode(data, output, &bj_mode);
> +
> +		/* Switch back to ultra joiner*/
> +		if (ultrajoiner_found) {
> +			igt_output_override_mode(output, &uj_mode);
> +		} else {
> +			status = kmstest_force_connector_joiner(data->drm_fd,
> +								output->config.connector,
> +								JOINED_PIPES_ULTRA_JOINER);
> +			igt_assert_f(status, "Failed to toggle force joiner\n");
> +		}
> +
> +		set_joiner_mode(data, output, &uj_mode);
> +	}
> +}
> +
>   static void test_single_joiner(data_t *data, int output_count, bool force_joiner)
>   {
>   	int i;
> @@ -592,6 +671,21 @@ igt_main
>   		}
>   	}
>   
> +	igt_describe("Verify modeset switch between ultra joiner and big joiner");
> +	igt_subtest_with_dynamic("switch-modeset-ultra-joiner-big-joiner") {
> +		igt_require_f(ultra_joiner_supported,
> +			      "Ultra joiner not supported on this platform\n");
> +		igt_require_f(data.ultra_joiner_output_count > 0 ||
> +			      data.non_ultra_joiner_output_count > 0,
> +				  "No ultra joiner or force ultra joiner output found\n");
> +		igt_require_f(data.n_pipes > 3,
> +			      "Minimum 4 pipes required\n");
> +
> +		for_each_connected_output(&data.display, output) {
> +			switch_modeset_ultra_joiner_big_joiner(&data, output);
> +		}
> +	}
> +
>   	igt_subtest_with_dynamic("invalid-modeset-force-ultra-joiner") {
>   		igt_require_f(ultra_joiner_supported,
>   			      "Ultra joiner not supported on this platform\n");