[PATCH] lib/igt_kms: add support for choosing big joiner mode

[Modem, Bhanuprakash]

Hi Kunal,

On 12-01-2024 11:09 am, Kunal Joshi wrote:
> add support to choose big joiner mode with environment
> variable, use mode with highest clock if no mode with big joiner
> found.
> 
> Cc: Karthik B S <karthik.b.s at intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem at intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi at intel.com>
> ---
>   lib/igt_kms.c | 17 ++++++++++++++++-
>   1 file changed, 16 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e4dea1a60..3d0fe635c 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1783,15 +1783,30 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
>   	env = getenv("IGT_KMS_RESOLUTION");
>   	if (env) {
>   		/*
> -		 * Only (0 or 1) and (lowest or highest) are allowed.
> +		 * Only (0 or 1 or 2) and (lowest or highest or joiner) are allowed.
>   		 *
>   		 * 0/lowest: Choose connector mode with lowest possible resolution.
>   		 * 1/highest: Choose connector mode with highest possible resolution.
> +		 * 2/joiner: Choose connector mode with big joiner output or output
> +		 *           with max clock requirement

Please re-write this to choose bigjoiner mode if found any.
>   		 */
>   		if (!strcmp(env, "highest") || !strcmp(env, "1"))
>   			igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
>   		else if (!strcmp(env, "lowest") || !strcmp(env, "0"))
>   			igt_sort_connector_modes(connector, sort_drm_modes_by_res_asc);
> +		else if (!strcmp(env, "joiner") || !strcmp(env, "2")) {

Let's sort the if-else ladder as "0->1->2" or "2->1->0".

> +			igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
> +			if (igt_bigjoiner_possible(&connector->modes[0], igt_get_max_dotclock(drm_fd))) {
> +				*mode = connector->modes[0];
> +				return true;
> +			}
> +			igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
------------------------------------------------------------^
typo: s/sort_drm_modes_by_res_dsc/sort_modes_by_clk_dsc/

> +			if(igt_bigjoiner_possible(&connector->modes[0], igt_get_max_dotclock(drm_fd))) {
> +				*mode = connector->modes[0];
> +				return true;
> +			}
> +			igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);

Please drop this sorting.

IMHO, Instead of returning from multiple places, try to adopt this [*] 
cleaner way.

[*]: 
https://cgit.freedesktop.org/drm/igt-gpu-tools/tree/tests/intel/kms_big_joiner.c#n235

- Bhanu

> +		}
>   		else
>   			goto default_mode;
>