Re: [PATCH i-g-t 2/3] tests/kms_bw: allow physically connected only

[Aurabindo Pillai <aurabindo.pillai@amd.com>]

Hi Kunal,

On 5/23/24 6:02 AM, Kunal Joshi wrote:
> modify kms_bw test to only run on physically connected displays,
> avoiding issues with forcing a 4K EDID on disconnected connectors.
> 
> For instance, consider a system with 2 DP and 2 HDMI connectors
> (DP-1, DP-2, HDMI-1, HDMI-2). Forcing EDID on DP is currently problematic,
> and we only support forcing for HDMI. However, if there's only one

Is this limitation intel specific?

> TMDS encoder, we can only drive one HDMI display, not two.
> 
> For now, the test will only run on physically connected displays.
> Future improvements may consider the encoder count to handle such
> scenarios more gracefully.
> 

Making it run only on physical connected displays limits us from 
catching any issues in backend programming related to bandwidth 
calculation, so this change isnt helpful.

How about adding a subtest that can be tested only on physically 
connected ones ?

> Cc: Aurabindo Pillai <aurabindo.pillai@amd.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>   tests/kms_bw.c | 43 ++++++++++++++++++-------------------------
>   1 file changed, 18 insertions(+), 25 deletions(-)
> 
> diff --git a/tests/kms_bw.c b/tests/kms_bw.c
> index b50f324af..4aabb02b7 100644
> --- a/tests/kms_bw.c
> +++ b/tests/kms_bw.c
> @@ -61,6 +61,7 @@ typedef struct data {
>           int h[IGT_MAX_PIPES];
>           int fd;
>   	int num_pipes;
> +	int num_outputs;
>   } data_t;
>   
>   static drmModeModeInfo test_mode[] = {
> @@ -105,8 +106,9 @@ static drmModeModeInfo test_mode[] = {
>   static void test_init(data_t *data)
>   {
>   	igt_display_t *display = &data->display;
> -	int i, max_pipes = display->n_pipes;
> +	int i;
>   	igt_output_t *output;
> +	data->num_outputs = 0;
>   
>   	for_each_pipe(display, i) {
>   		data->pipe_id[i] = i;
> @@ -118,23 +120,22 @@ static void test_init(data_t *data)
>   					 IGT_PIPE_CRC_SOURCE_AUTO);
>   	}
>   
> -	for (i = 0; i < display->n_outputs && i < max_pipes; i++) {
> -		if (!data->pipe[i])
> -			continue;
> -
> -		output = &display->outputs[i];
> -
> -		data->output[i] = output;
> +	i = 0;
>   
> +	for_each_output(display, output) {
>   		/* Only allow physically connected displays for the tests. */
>   		if (!igt_output_is_connected(output))
>   			continue;
>   
> +		data->output[i] = output;
> +		data->num_outputs++;
> +
>   		igt_assert(kmstest_get_connector_default_mode(
>   			data->fd, output->config.connector, &data->mode[i]));
>   
>   		data->w[i] = data->mode[i].hdisplay;
>   		data->h[i] = data->mode[i].vdisplay;
> +		i++;
>   	}
>   
>   
> @@ -156,27 +157,12 @@ static void test_fini(data_t *data)
>   	igt_display_commit_atomic(display, DRM_MODE_ATOMIC_ALLOW_MODESET, 0);
>   }
>   
> -/* Forces a mode for a connector. */
> -static void force_output_mode(data_t *d, igt_output_t *output,
> -			      const drmModeModeInfo *mode)
> -{
> -	/* This allows us to create a virtual sink. */
> -	if (!igt_output_is_connected(output)) {
> -		kmstest_force_edid(d->fd, output->config.connector,
> -				   igt_kms_get_4k_edid());
> -
> -		kmstest_force_connector(d->fd, output->config.connector,
> -					FORCE_CONNECTOR_DIGITAL);
> -	}
> -
> -	igt_output_override_mode(output, mode);
> -}
> -
>   static void run_test_linear_tiling(data_t *data, int pipe, const drmModeModeInfo *mode) {
>   	igt_display_t *display = &data->display;
>   	igt_output_t *output;
>   	struct igt_fb buffer[IGT_MAX_PIPES];
>   	igt_crc_t zero, captured[IGT_MAX_PIPES];
> +	int i = 0;
>   	int ret;
>   
>   	igt_skip_on_f(pipe >= data->num_pipes,
> @@ -184,6 +170,10 @@ static void run_test_linear_tiling(data_t *data, int pipe, const drmModeModeInfo
>   
>   	test_init(data);
>   
> +	igt_skip_on_f(pipe >= data->num_outputs,
> +				  "%d connected outputs required but found %d\n",
> +				  pipe+1, data->num_outputs+1);
> +
>   	/* create buffers */
>   	for (i = 0; i <= pipe; i++) {
>   		output = data->output[i];
> @@ -191,7 +181,7 @@ static void run_test_linear_tiling(data_t *data, int pipe, const drmModeModeInfo
>   			continue;
>   		}
>   
> -		force_output_mode(data, output, mode);
> +		igt_output_override_mode(output, mode);
>   
>   		igt_create_color_fb(display->drm_fd, mode->hdisplay,
>   				    mode->vdisplay, DRM_FORMAT_XRGB8888,
> @@ -199,6 +189,9 @@ static void run_test_linear_tiling(data_t *data, int pipe, const drmModeModeInfo
>   				    &buffer[i]);
>   
>   		igt_output_set_pipe(output, i);
> +		igt_info("Assigned output %s to pipe %s with mode %dx%d@%d\n",
> +				 igt_output_name(output), kmstest_pipe_name(i),
> +				 mode->hdisplay, mode->vdisplay, mode->vrefresh);
>   
>   		igt_plane_set_fb(data->primary[i], &buffer[i]);
>   	}

-- 
--

Thanks & Regards,
Aurabindo Pillai