Re: [PATCH i-g-t] tests/kms_bw: Allow custom modes for external panels

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

Hi Pranay,

On 4/9/2026 2:27 PM, Samala, Pranay wrote:
> Hi Karthik,
>
>> -----Original Message-----
>> From: igt-dev <igt-dev-bounces@lists.freedesktop.org> On Behalf Of Karthik
>> B S
>> Sent: Tuesday, April 7, 2026 3:23 PM
>> To: igt-dev@lists.freedesktop.org
>> Cc: B S, Karthik <karthik.b.s@intel.com>; Jason-JH Lin <jason-
>> jh.lin@mediatek.com>; Joshi, Kunal1 <kunal1.joshi@intel.com>
>> Subject: [PATCH i-g-t] tests/kms_bw: Allow custom modes for external panels
>>
>> Currently the test is rejecting all the outputs where the mode is not
>> matching. This is leading to excessive skips on configs wherer the tests were
>> passing previouly.
>>
>> Instead of this, use default mode for fixed mode panels if the required
>> custom mode is not found. And use custom mode for external panels as
> The current code always uses default mode on internal panels even
> if the custom mode is missing.
> Imho, if this behavior is intentional, please update the commit message.
> Remove this " if the required custom mode is not found".

Thank you for the review.

Even though we use the default mode, we are checking if that is the same 
as requested mode and if that is true, then test doesn't throw any 
warnings as we're using the requested mode itself on all available panels.

>
>> these are allowed and have also been passing previously.
>>
>> Cc: Jason-JH Lin <jason-jh.lin@mediatek.com>
>> Cc: Kunal Joshi <kunal1.joshi@intel.com>
>> Signed-off-by: Karthik B S <karthik.b.s@intel.com>
>> ---
>>   tests/kms_bw.c | 57 ++++++++++++++++++++++++++++++++------------------
>>   1 file changed, 37 insertions(+), 20 deletions(-)
>>
>> diff --git a/tests/kms_bw.c b/tests/kms_bw.c index 4df5c2cee..4a32ec811
>> 100644
>> --- a/tests/kms_bw.c
>> +++ b/tests/kms_bw.c
>> @@ -186,14 +186,16 @@ static void force_output_mode(data_t *d,
>> igt_output_t *output,
>>   	igt_output_override_mode(output, mode);  }
>>
>> -static bool output_mode_supported(igt_output_t *output, const
>> drmModeModeInfo *mode)
>> +/* Check if output has a matching mode and call it out if mode is being
>> +forced */ static void output_mode_supported(igt_output_t *output, const
>> +drmModeModeInfo *mode)
>>   {
>>   	drmModeConnector *connector = output->config.connector;
>> +	drmModeModeInfo *default_mode;
>>   	int i;
>>
>>   	/* Virtual/forced sinks support all modes */
>>   	if (!igt_output_is_connected(output))
>> -		return true;
>> +		return;
>>
>>   	for (i = 0; i < connector->count_modes; i++) {
>>   		drmModeModeInfo *conn_mode = &connector->modes[i];
>> @@ -204,26 +206,32 @@ static bool output_mode_supported(igt_output_t
>> *output, const drmModeModeInfo *m
>>   			igt_debug("Found matching mode for %dx%d@%dHz
>> on %s\n",
>>   				  mode->hdisplay, mode->vdisplay, mode-
>>> vrefresh,
>>   				  igt_output_name(output));
>> -			return true;
>>   		}
>>   	}
>>
>> -	igt_info("Mode %dx%d@%dHz not supported by %s (has %d
>> modes)\n",
>> -		 mode->hdisplay, mode->vdisplay, mode->vrefresh,
>> -		 igt_output_name(output), connector->count_modes);
>> -
>> -	return false;
>> +	if (output_is_internal_panel(output)) {
>> +		default_mode = igt_output_get_mode(output);
>> +		igt_info("Mode %dx%d@%dHz not supported by %s (has %d
>> modes).\n"
>> +			 "%s Default mode: %dx%d@%dHz\n",
>> +			 mode->hdisplay, mode->vdisplay, mode->vrefresh,
>> +			 igt_output_name(output), connector-
>>> count_modes, igt_output_name(output),
>> +			 default_mode->hdisplay, default_mode->vdisplay,
>> default_mode->vrefresh);
>> +	} else {
>> +		igt_info("Mode %dx%d@%dHz not supported by %s (has %d
>> modes). Forcing mode.\n",
>> +			 mode->hdisplay, mode->vdisplay, mode->vrefresh,
>> +			 igt_output_name(output), connector-
>>> count_modes);
>> +	}
> Here since we are anyhow never using the requested mode even if we found it,
> A suggestion, can we move this internal panel "if check" before scanning for the
> requested mode for loop earlier.
>
> For eg:
> Void output_mode_supported() {
> 	If (internal_panel)
> 		Get default mode and return
> 	Else
> 		Scan and get the requested mode if exists and return
> 	Log & force it.
> }
Same reasoning as above.
>
>>   }
>>
>>   static void run_test_linear_tiling(data_t *data, int n_crtcs, const
>> drmModeModeInfo *mode, bool physical) {
>>   	igt_display_t *display = &data->display;
>>   	igt_output_t *output;
>> +	drmModeModeInfo fb_mode;
>>   	struct igt_fb buffer[IGT_MAX_PIPES];
>>   	igt_crc_t zero, captured[IGT_MAX_PIPES];
>>   	int i = 0, num_pipes = 0;
>>   	igt_crtc_t *crtc;
>>   	int ret;
>> -	bool has_supported_mode = false;
>>
>>   	/* Cannot use igt_display_n_crtcs() due to fused pipes on i915 where
>> they do
>>   	 * not give the numver of valid crtcs and always return
>> IGT_MAX_PIPES */ @@ -242,14 +250,26 @@ static void
>> run_test_linear_tiling(data_t *data, int n_crtcs, const drmModeModeI
>>   		crtc = data->crtc[i];
>>
>>   		output = physical ? data->connected_output[i] : data-
>>> output[i];
>> -		if (!output || !output_mode_supported(output, mode)) {
>> +		if (!output)
>>   			continue;
>> -		}
>>
>> -		force_output_mode(data, output, mode);
>> +		output_mode_supported(output, mode);
>> +
>> +		/*
>> +		 * On fixed mode panels trying to force a custom mode can
>> lead to failures or
>> +		 * implicit handling where the default mode is used even
>> though custom mode is requested.
> Line length exceeds 100 columns.

Will fix this.

Thanks,
Karthik.B.S
>
> Regards,
> Pranay.
>
>> +		 * To avoid this use default mode on fixed mode panels and
>> +		 * use custom modes only on external panels.
>> +		 */
>> +		if (output_is_internal_panel(output)) {
>> +			fb_mode = *igt_output_get_mode(output);
>> +		} else {
>> +			force_output_mode(data, output, mode);
>> +			fb_mode = *mode;
>> +		}
>>
>> -		igt_create_color_fb(display->drm_fd, mode->hdisplay,
>> -				    mode->vdisplay, DRM_FORMAT_XRGB8888,
>> +		igt_create_color_fb(display->drm_fd, fb_mode.hdisplay,
>> +				    fb_mode.vdisplay,
>> DRM_FORMAT_XRGB8888,
>>   				    DRM_FORMAT_MOD_LINEAR, 1.f, 0.f, 0.f,
>>   				    &buffer[i]);
>>
>> @@ -258,10 +278,8 @@ static void run_test_linear_tiling(data_t *data, int
>> n_crtcs, const drmModeModeI
>>   		igt_plane_set_fb(data->primary[i], &buffer[i]);
>>   		igt_info("Assigning pipe %s to output %s with mode %s\n",
>>   			 igt_crtc_name(crtc), igt_output_name(output),
>> -			 mode->name);
>> -		has_supported_mode = true;
>> +			 fb_mode.name);
>>   	}
>> -	igt_skip_on_f(!has_supported_mode, "Unsupported mode for all
>> pipes\n");
>>
>>   	ret = igt_display_try_commit_atomic(display,
>>
>> DRM_MODE_ATOMIC_ALLOW_MODESET | @@ -273,9 +291,8 @@ static void
>> run_test_linear_tiling(data_t *data, int n_crtcs, const drmModeModeI
>>
>>   	for (i = 0; i < n_crtcs; i++) {
>>   		output = physical ? data->connected_output[i] : data-
>>> output[i];
>> -		if (!output || !output_mode_supported(output, mode)) {
>> +		if (!output)
>>   			continue;
>> -		}
>>
>>   		igt_pipe_crc_collect_crc(data->pipe_crc[i], &captured[i]);
>>   		igt_assert_f(!igt_check_crc_equal(&zero, &captured[i]), @@
>> -284,7 +301,7 @@ static void run_test_linear_tiling(data_t *data, int n_crtcs,
>> const drmModeModeI
>>
>>   	for (i = n_crtcs - 1; i >= 0; i--) {
>>   		output = physical ? data->connected_output[i] : data-
>>> output[i];
>> -		if (!output || !output_mode_supported(output, mode))
>> +		if (!output)
>>   			continue;
>>
>>   		igt_remove_fb(display->drm_fd, &buffer[i]);
>> --
>> 2.43.0