Re: [PATCH i-g-t 2/5] tests/intel/kms_cdclk: Add conditions to filter valid outputs

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 2/13/2025 4:38 PM, Swati Sharma wrote:
> Add vrefresh as an additional parameter to check if highest and
> lowest refresh rates are identical.
> Skip the test, if highres and lowres are the same, as no cdclk
> transition will occur.
> Store highres and lowres of valid outputs in an array for reuse later.
>
> v2:-fixed description (Ankit)
>     -highres lowres stored in previous loop (Ankit)
>     -removed comments (Ankit)
>
> Signed-off-by: Swati Sharma <swati2.sharma@intel.com>
> ---
>   tests/intel/kms_cdclk.c | 50 ++++++++++++++++++++++++++++-------------
>   1 file changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/tests/intel/kms_cdclk.c b/tests/intel/kms_cdclk.c
> index e78d22e1d..75542e21b 100644
> --- a/tests/intel/kms_cdclk.c
> +++ b/tests/intel/kms_cdclk.c
> @@ -223,7 +223,9 @@ static void test_mode_transition(data_t *data, enum pipe pipe, igt_output_t *out
>   		      mode_hi.vrefresh >= VREFRESH, "Mode >= 4K not found on output %s.\n",
>   		      igt_output_name(output));
>   
> -	igt_skip_on_f(mode_hi.hdisplay == mode_lo.hdisplay && mode_hi.vdisplay == mode_lo.vdisplay,
> +	igt_skip_on_f(mode_hi.hdisplay == mode_lo.hdisplay &&
> +		      mode_hi.vdisplay == mode_lo.vdisplay &&
> +		      mode_hi.vrefresh == mode_lo.vrefresh,
>   		      "Highest and lowest mode resolutions are same; no transition\n");
>   
>   	primary = igt_output_get_plane_type(output, DRM_PLANE_TYPE_PRIMARY);
> @@ -261,8 +263,10 @@ static void test_mode_transition_on_all_outputs(data_t *data)
>   	igt_display_t *display = &data->display;
>   	int debugfs_fd = data->debugfs_fd;
>   	drmModeModeInfo *mode, mode_hi, mode_lo;
> +	drmModeModeInfo mode_highres[IGT_MAX_PIPES] = {0}, mode_lowres[IGT_MAX_PIPES] = {0};
> +	igt_output_t *valid_outputs[IGT_MAX_PIPES] = {NULL};
>   	igt_output_t *output;
> -	int valid_outputs = 0;
> +	int count = 0;
>   	int cdclk_ref, cdclk_new;
>   	uint16_t width = 0, height = 0;
>   	struct igt_fb fb;
> @@ -273,31 +277,45 @@ static void test_mode_transition_on_all_outputs(data_t *data)
>   	do_cleanup_display(display);
>   	igt_display_reset(display);
>   
> -	for_each_connected_output(&data->display, output)
> -		valid_outputs++;
> -
> -	i = 0;
>   	for_each_connected_output(display, output) {
> -		mode = igt_output_get_mode(output);
> +		mode_highres[count] = *igt_output_get_highres_mode(output);
> +		igt_require_f(mode_highres[count].hdisplay >= HDISPLAY_4K && mode_highres[count].vdisplay >= VDISPLAY_4K &&
> +			      mode_highres[count].vrefresh >= VREFRESH, "Mode >= 4K not found on output %s.\n",
> +			      igt_output_name(output));
> +
> +		mode_lowres[count] = *get_lowres_mode(output);
> +
> +		if (mode_highres[count].hdisplay == mode_lowres[count].hdisplay &&
> +		    mode_highres[count].vdisplay == mode_lowres[count].vdisplay &&
> +		    mode_highres[count].vrefresh == mode_lowres[count].vrefresh) {
> +			igt_info("Highest and lowest mode resolutions are same on output %s; no transition will occur, skipping\n",
> +				  igt_output_name(output));
> +			continue;
> +		}
> +
> +		valid_outputs[count] = output;
> +		count++;
> +	}
> +
> +	igt_skip_on_f(count < 2,
> +		      "Number of valid outputs (%d) must be greater than or equal to 2\n", count);
> +
> +	for (int k = 0; k < count; k++) {

Just use i for iterator.


> +		mode = igt_output_get_mode(valid_outputs[k]);
>   		igt_assert(mode);
>   
>   		width = max(width, mode->hdisplay);
>   		height = max(height, mode->vdisplay);
>   
> -		mode_hi = *igt_output_get_highres_mode(output);
> -		igt_require_f(mode_hi.hdisplay >= HDISPLAY_4K && mode_hi.vdisplay >= VDISPLAY_4K &&
> -			      mode_hi.vrefresh >= VREFRESH, "Mode >= 4K not found on output %s.\n",
> -			      igt_output_name(output));
> -
> -		igt_output_set_pipe(output, i);
> -		igt_output_override_mode(output, &mode_hi);
> -		i++;
> +		igt_output_set_pipe(valid_outputs[k], k);
> +		igt_output_override_mode(valid_outputs[k], &mode_highres[k]);
>   	}
> +
>   	igt_require(intel_pipe_output_combo_valid(display));
> -	igt_display_reset(display);
>   
>   	igt_create_pattern_fb(data->drm_fd, width, height, DRM_FORMAT_XRGB8888,
>   			      DRM_FORMAT_MOD_LINEAR, &fb);
> +

This line is extra, not part of this patch.

With above things fixed:

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>


>   	i = 0;
>   	for_each_connected_output(display, output) {
>   		pipe = &display->pipes[i];