[igt-dev] [PATCH i-g-t v3] tests/kms_concurrent: Skip the subtest if the resolution is not supported

[Karthik B S]

On 5/16/2022 4:11 PM, Nidhi Gupta wrote:
> The kms_concurrent is about testing mode setting with
> reducing the resolution and then again increasing it,
> for this the test will take the high resolution supported
> by the connector and then calculate the lowest resolution,
> if the calculated resolution is not supported by the connector
> it will assign the default resolution of 1024x 768 without
> checking.
>
> Added check to skip the subtest if the default resolution is not supported
> by the connector, to achieve that we are skipping if, mode_default->vdisplay < 1024"
> becuase if the default resolution is greater then the highest
> resolution supported by the connector then the test should skip.

Hi,

Please update the commit message to something like, "Added check to skip 
the subtest if the current mode is smaller than 1024 mode". We're 
basically skipping the test because we couldn't find a mode lower than 
the current mode.
In most cases it would be true that current mode would be the highest, 
but need not be always true. So better to make it clear in the commit 
message.

With this updated, the patch LGTM.

Reviewed-by: Karthik B S <karthik.b.s at intel.com>

Thanks,
Karthik.B.S
>
> Signed-off-by: Nidhi Gupta <nidhi1.gupta at intel.com>
> ---
>   tests/kms_concurrent.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/tests/kms_concurrent.c b/tests/kms_concurrent.c
> index 82b2021e..0b696e63 100644
> --- a/tests/kms_concurrent.c
> +++ b/tests/kms_concurrent.c
> @@ -252,6 +252,7 @@ get_lowres_mode(data_t *data, const drmModeModeInfo *mode_default,
>   			return igt_memdup(mode, sizeof(*mode));
>   	}
>   
> +	igt_skip_on_f(mode_default->vdisplay < 1024, "No suitable resolution was found\n");
>   	return igt_std_1024_mode_get(igt_output_preferred_vrefresh(output));
>   }
>