Re: [PATCH i-g-t] tests/kms_display_modes: Fit modes within MST bandwidth constraints

["Reddy Guddati, Santhosh" <santhosh.reddy.guddati@intel.com>]

Hi Jeevan,


On 03-04-2025 16:23, Jeevan B wrote:
> Instead of checking only the current modes, find a combination of
> modes in the MST topology that can be accommodated within the
> available link bandwidth.
> 
> v2: move fit_modes_in_bw to lib/igt_kms
> v3: update tag from fixes to closes
> 
> Closes: https://gitlab.freedesktop.org/drm/xe/kernel/-/issues/1542
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>   lib/igt_kms.c                           | 24 ++++++++++++++++++++++++
>   lib/igt_kms.h                           |  1 +
>   tests/intel/kms_dp_link_training.c      | 24 +-----------------------
>   tests/intel/kms_dp_linktrain_fallback.c | 18 +-----------------
>   tests/kms_display_modes.c               |  4 ++++
>   5 files changed, 31 insertions(+), 40 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index 99c8707c7..f8274f49a 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -5046,6 +5046,30 @@ bool igt_override_all_active_output_modes_to_fit_bw(igt_display_t *display)
>   	return __override_all_active_output_modes_to_fit_bw(display, outputs, n_outputs, 0);
>   }
>   
> +/*
> + * fit_modes_in_bw :
> + * @display: a pointer to an #igt_display_t structure
> + *
> + * Tries atomic TEST_ONLY commit; if it fails, overrides
> + * output modes to fit bandwidth.
> + */
> +bool fit_modes_in_bw(igt_display_t *display)
> +{
> +	bool found;
> +	int ret;
> +
> +	ret = igt_display_try_commit_atomic(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(display);
> +		igt_require_f(found,
> +			      "No valid mode combo found for modeset\n");
> +	}
> +
> +	return true;
> +}
> +
>   /**
>    * igt_pipe_refresh:
>    * @display: a pointer to an #igt_display_t structure
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 0381c82ad..fe01eaf3f 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1226,6 +1226,7 @@ void igt_require_pipe(igt_display_t *display,
>   void igt_dump_connectors_fd(int drmfd);
>   void igt_dump_crtcs_fd(int drmfd);
>   bool igt_override_all_active_output_modes_to_fit_bw(igt_display_t *display);
> +bool fit_modes_in_bw(igt_display_t *display);
>   bool igt_get_i915_edp_lobf_status(int drmfd, char *connector_name);
>   unsigned int igt_get_output_max_bpc(int drmfd, char *connector_name);
>   unsigned int igt_get_pipe_current_bpc(int drmfd, enum pipe pipe);
> diff --git a/tests/intel/kms_dp_link_training.c b/tests/intel/kms_dp_link_training.c
> index 9e9b1e6db..9b6248c6f 100644
> --- a/tests/intel/kms_dp_link_training.c
> +++ b/tests/intel/kms_dp_link_training.c
> @@ -140,28 +140,6 @@ static void setup_planes_fbs(data_t *data, igt_output_t *outs[],
>   	}
>   }
>   
> -/*
> - * fit_modes_in_bw - Tries atomic TEST_ONLY commit; if it fails, overrides
> - * output modes to fit bandwidth.
> - */
> -static bool fit_modes_in_bw(data_t *data)
> -{
> -	int ret;
> -
> -	ret = igt_display_try_commit_atomic(&data->display,
> -					    DRM_MODE_ATOMIC_TEST_ONLY |
> -					    DRM_MODE_ATOMIC_ALLOW_MODESET,
> -					    NULL);
> -	if (ret != 0) {
> -		bool found;
> -
> -		found = igt_override_all_active_output_modes_to_fit_bw(&data->display);
> -		igt_require_f(found, "No valid mode combo found for modeset\n");
> -	}
> -
> -	return true;
> -}
> -
>   static void do_modeset(data_t *data, bool mst)
>   {
>   	uint32_t master_pipes_mask = 0;
> @@ -204,7 +182,7 @@ static void do_modeset(data_t *data, bool mst)
>   						  "Unable to assign pipes for outputs\n");
>   
>   	setup_planes_fbs(data, outs, out_count, modes, fbs, planes);
> -	fit_modes_in_bw(data);
> +	fit_modes_in_bw(&data->display);
>   	igt_display_commit2(&data->display, COMMIT_ATOMIC);
>   }
>   
> diff --git a/tests/intel/kms_dp_linktrain_fallback.c b/tests/intel/kms_dp_linktrain_fallback.c
> index 6a872efd2..64f21ec6f 100644
> --- a/tests/intel/kms_dp_linktrain_fallback.c
> +++ b/tests/intel/kms_dp_linktrain_fallback.c
> @@ -151,22 +151,6 @@ static void set_connector_link_status_good(data_t *data, igt_output_t *outputs[]
>   							  DRM_MODE_LINK_STATUS_GOOD);
>   }
>   
> -static bool fit_modes_in_bw(data_t *data)
> -{
> -	bool found;
> -	int ret;
> -
> -	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");
> -	}
> -	return true;
> -}
> -
>   static bool validate_modeset_for_outputs(data_t *data,
>   					igt_output_t *outputs[],
>   					int *output_count,
> @@ -176,7 +160,7 @@ static bool validate_modeset_for_outputs(data_t *data,
>   {
>   	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");
> +	igt_assert_f(fit_modes_in_bw(&data->display), "Unable to fit modes in bw\n");
The function fit_modes_in_bw causes the test to skip if valid modes is 
not found, but here we are intending to assert the test. Are both 
contradicting?

>   	setup_modeset_on_outputs(data, outputs,
>   				 output_count,
>   				 mode, fb, primary);
> diff --git a/tests/kms_display_modes.c b/tests/kms_display_modes.c
> index 588820170..f2bbfafe2 100644
> --- a/tests/kms_display_modes.c
> +++ b/tests/kms_display_modes.c
> @@ -93,6 +93,8 @@ static void run_extendedmode_basic(data_t *data,
>   	mode[0] = igt_output_get_mode(output1);
>   	mode[1] = igt_output_get_mode(output2);
>   
> +	igt_assert(fit_modes_in_bw(display));
> +
>   	pipe_crc[0] = igt_pipe_crc_new(data->drm_fd, pipe1, IGT_PIPE_CRC_SOURCE_AUTO);
>   	pipe_crc[1] = igt_pipe_crc_new(data->drm_fd, pipe2, IGT_PIPE_CRC_SOURCE_AUTO);
>   
> @@ -138,6 +140,7 @@ static void run_extendedmode_basic(data_t *data,
>   	igt_fb_set_size(&fb, plane[1], mode[1]->hdisplay, mode[1]->vdisplay);
>   	igt_plane_set_size(plane[1], mode[1]->hdisplay, mode[1]->vdisplay);
>   
> +	igt_assert(fit_modes_in_bw(display));
>   	igt_display_commit2(display, COMMIT_ATOMIC);
>   
>   	igt_pipe_crc_collect_crc(pipe_crc[0], &crc[0]);
> @@ -158,6 +161,7 @@ static void run_extendedmode_basic(data_t *data,
>   			  DRM_PLANE_TYPE_PRIMARY), NULL);
>   	igt_plane_set_fb(igt_pipe_get_plane_type(&display->pipes[pipe2],
>   			  DRM_PLANE_TYPE_PRIMARY), NULL);
> +	igt_assert(fit_modes_in_bw(display));
>   	igt_display_commit2(display, COMMIT_ATOMIC);
>   
>   	/*Compare CRC*/