[PATCH i-g-t 0/2] add support for choosing big joiner mode

[PATCH i-g-t 0/2] add support for choosing big joiner mode

[Kunal Joshi]

add support to choose big joiner mode with environment
variable, use mode with highest clock if no mode with big joiner
found.

Kunal Joshi (2):
  lib/igt_kms: move bigjoiner_mode_found to lib.
  lib/igt_kms: add support for choosing big joiner mode

 lib/igt_kms.c                | 25 ++++++++++++++++++++++++-
 lib/igt_kms.h                |  3 +++
 tests/intel/kms_big_joiner.c | 14 ++------------
 3 files changed, 29 insertions(+), 13 deletions(-)

-- 
2.25.1

[PATCH i-g-t 1/2] lib/igt_kms: move bigjoiner_mode_found to lib.

[Kunal Joshi]

move bigjoiner_mode_found to lib/igt_kms with some modification

Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c                | 19 +++++++++++++++++++
 lib/igt_kms.h                |  3 +++
 tests/intel/kms_big_joiner.c | 14 ++------------
 3 files changed, 24 insertions(+), 12 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index e4dea1a60..2c55af05f 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -6142,6 +6142,25 @@ bool igt_bigjoiner_possible(drmModeModeInfo *mode, int max_dotclock)
 		mode->clock > max_dotclock);
 }
 
+/**
+ * bigjoiner_mode_found:
+ * @connector: libdrm connector
+ * @sort_method: comparator method
+ * @mode: libdrm mode
+ *
+ * Returns: True if big joiner found in connector modes
+ */
+bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
+			  int (*sort_method)(const void *, const void*),
+			  drmModeModeInfo *mode)
+{
+        igt_sort_connector_modes(connector, sort_method);
+        *mode = connector->modes[0];
+
+        return igt_bigjoiner_possible(mode,
+				      igt_get_max_dotclock(drm_fd));
+}
+
 /**
  * igt_check_bigjoiner_support:
  * @display: a pointer to an #igt_display_t structure
diff --git a/lib/igt_kms.h b/lib/igt_kms.h
index b3882808b..9f5676d35 100644
--- a/lib/igt_kms.h
+++ b/lib/igt_kms.h
@@ -1212,6 +1212,9 @@ bool igt_max_bpc_constraint(igt_display_t *display, enum pipe pipe,
 		igt_output_t *output, int bpc);
 int igt_get_max_dotclock(int fd);
 bool igt_bigjoiner_possible(drmModeModeInfo *mode, int max_dotclock);
+bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
+                                 int (*sort_method)(const void *, const void*),
+                                 drmModeModeInfo *mode);
 bool igt_check_bigjoiner_support(igt_display_t *display);
 bool igt_parse_mode_string(const char *mode_string, drmModeModeInfo *mode);
 bool intel_pipe_output_combo_valid(igt_display_t *display);
diff --git a/tests/intel/kms_big_joiner.c b/tests/intel/kms_big_joiner.c
index aba2adfbe..1858c6362 100644
--- a/tests/intel/kms_big_joiner.c
+++ b/tests/intel/kms_big_joiner.c
@@ -199,16 +199,6 @@ static void test_dual_display(data_t *data)
 	igt_display_commit2(display, COMMIT_ATOMIC);
 }
 
-static bool bigjoiner_mode_found(drmModeConnector *connector,
-				 int (*sort_method)(const void *, const void*),
-				 drmModeModeInfo *mode)
-{
-	igt_sort_connector_modes(connector, sort_method);
-	*mode = connector->modes[0];
-
-	return igt_bigjoiner_possible(mode, max_dotclock);
-}
-
 igt_main
 {
 	data_t data;
@@ -235,8 +225,8 @@ igt_main
 			 * Bigjoiner will come in to the picture when the
 			 * resolution > 5K or clock > max-dot-clock.
 			 */
-			found = (bigjoiner_mode_found(connector, sort_drm_modes_by_res_dsc, &mode) ||
-				 bigjoiner_mode_found(connector, sort_drm_modes_by_clk_dsc, &mode)) ?
+			found = (bigjoiner_mode_found(data.drm_fd, connector, sort_drm_modes_by_res_dsc, &mode) ||
+				 bigjoiner_mode_found(data.drm_fd, connector, sort_drm_modes_by_clk_dsc, &mode)) ?
 					true : false;
 
 			if (found) {
-- 
2.25.1

[PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Kunal Joshi]

add support to choose big joiner mode with environment
variable, use mode with highest clock if no mode with big joiner
found.

v2: reuse bigjoiner_mode_found (Bhanu)

Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 2c55af05f..c4ecb0907 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1788,7 +1788,11 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 		 * 0/lowest: Choose connector mode with lowest possible resolution.
 		 * 1/highest: Choose connector mode with highest possible resolution.
 		 */
-		if (!strcmp(env, "highest") || !strcmp(env, "1"))
+		if (!strcmp(env, "joiner") || !strcmp(env, "2"))
+			return bigjoiner_mode_found(drm_fd, connector, sort_drm_modes_by_clk_dsc, mode) ||
+			       bigjoiner_mode_found(drm_fd, connector, sort_drm_modes_by_res_dsc, mode) ||
+			       bigjoiner_mode_found(drm_fd, connector, sort_drm_modes_by_clk_dsc, mode);
+		else 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);
-- 
2.25.1

✗ Fi.CI.BAT: failure for add support for choosing big joiner mode

[Patchwork]

[-- Attachment #1: Type: text/plain, Size: 2627 bytes --]

== Series Details ==

Series: add support for choosing big joiner mode
URL   : https://patchwork.freedesktop.org/series/128706/
State : failure

== Summary ==

CI Bug Log - changes from IGT_7671 -> IGTPW_10522
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_10522 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_10522, please notify your bug team (I915-ci-infra@lists.freedesktop.org) to allow them
  to document this new failure mode, which will reduce false positives in CI.

  External URL: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10522/index.html

Participating hosts (39 -> 14)
------------------------------

  ERROR: It appears as if the changes made in IGTPW_10522 prevented too many machines from booting.

  Missing    (25): fi-rkl-11600 fi-snb-2520m bat-rpls-3 fi-pnv-d510 fi-blb-e6850 fi-skl-6600u fi-bsw-n3050 bat-adlm-1 fi-ilk-650 fi-ivb-3770 fi-elk-e7500 fi-bsw-nick fi-kbl-7567u bat-dg1-7 bat-kbl-2 bat-adlp-9 fi-cfl-8700k bat-mtlp-8 bat-jsl-1 fi-tgl-1115g4 fi-cfl-guc fi-kbl-guc fi-cfl-8109u bat-dg2-14 bat-dg2-13 

Known issues
------------

  Here are the changes found in IGTPW_10522 that come from known issues:

### IGT changes ###

#### Issues hit ####

  * igt@i915_selftest@live@reset:
    - fi-apl-guc:         [PASS][1] -> [DMESG-WARN][2] ([i915#9730]) +31 other tests dmesg-warn
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7671/fi-apl-guc/igt@i915_selftest@live@reset.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10522/fi-apl-guc/igt@i915_selftest@live@reset.html

  * igt@i915_suspend@basic-s2idle-without-i915:
    - fi-apl-guc:         [PASS][3] -> [DMESG-WARN][4] ([i915#180] / [i915#1982])
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_7671/fi-apl-guc/igt@i915_suspend@basic-s2idle-without-i915.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10522/fi-apl-guc/igt@i915_suspend@basic-s2idle-without-i915.html

  
  [i915#180]: https://gitlab.freedesktop.org/drm/intel/issues/180
  [i915#1982]: https://gitlab.freedesktop.org/drm/intel/issues/1982
  [i915#9730]: https://gitlab.freedesktop.org/drm/intel/issues/9730


Build changes
-------------

  * CI: CI-20190529 -> None
  * IGT: IGT_7671 -> IGTPW_10522

  CI-20190529: 20190529
  CI_DRM_14116: 3b92a66f4bc89f4fa6e9e9369ac8243e23670030 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_10522: 10522
  IGT_7671: 7671

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_10522/index.html

[-- Attachment #2: Type: text/html, Size: 3274 bytes --]

[PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Kunal Joshi]

add support to choose big joiner mode with environment
variable, use mode with highest clock if no mode with big joiner
found.

v2: reuse bigjoiner_mode_found (Bhanu)
v3: avoid returning from multiple places (Bhanu)
    avoid frequent debugfs reads (Bhanu)

Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 24 +++++++++++++++++++-----
 1 file changed, 19 insertions(+), 5 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index cb6d57c2d..2c4210d4b 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1771,8 +1771,9 @@ void igt_sort_connector_modes(drmModeConnector *connector,
 bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 					drmModeModeInfo *mode)
 {
+	bool found;
 	char *env;
-	int i;
+	int i, max_dotclock;
 
 	if (!connector->count_modes) {
 		igt_warn("no modes for connector %d\n",
@@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 	}
 
 	env = getenv("IGT_KMS_RESOLUTION");
+	max_dotclock = igt_get_max_dotclock(drm_fd);
 	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 bigjoiner support or with
+			     highest clock if can't support  big joiner
 		 */
-		if (!strcmp(env, "highest") || !strcmp(env, "1"))
+		if (!strcmp(env, "joiner") || !strcmp(env, "2"))
+			found = bigjoiner_mode_found(drm_fd, connector,
+						     sort_drm_modes_by_clk_dsc,mode,
+						     max_dotclock) ||
+				bigjoiner_mode_found(drm_fd, connector,
+						     sort_drm_modes_by_res_dsc, mode,
+						     max_dotclock) ||
+				bigjoiner_mode_found(drm_fd, connector,
+						     sort_drm_modes_by_clk_dsc, mode,
+						     max_dotclock);
+		else 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
 			goto default_mode;
-
-		*mode = connector->modes[0];
+		if (!found)
+			*mode = connector->modes[0];
 		return true;
 	}
 
-- 
2.25.1

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Sharma, Swati2]

Hi Kunal,

On 15-Jan-24 4:28 PM, 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.
> 
> v2: reuse bigjoiner_mode_found (Bhanu)
> v3: avoid returning from multiple places (Bhanu)
>      avoid frequent debugfs reads (Bhanu)
> 
> Cc: Karthik B S <karthik.b.s@intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>   1 file changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index cb6d57c2d..2c4210d4b 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -1771,8 +1771,9 @@ void igt_sort_connector_modes(drmModeConnector *connector,
>   bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
>   					drmModeModeInfo *mode)
>   {
> +	bool found;
>   	char *env;
> -	int i;
> +	int i, max_dotclock;
>   
>   	if (!connector->count_modes) {
>   		igt_warn("no modes for connector %d\n",
> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
>   	}
>   
>   	env = getenv("IGT_KMS_RESOLUTION");
> +	max_dotclock = igt_get_max_dotclock(drm_fd);
>   	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 bigjoiner support or with
> +			     highest clock if can't support  big joiner

Shouldn't this be only joiner mode?

>   		 */
> -		if (!strcmp(env, "highest") || !strcmp(env, "1"))
> +		if (!strcmp(env, "joiner") || !strcmp(env, "2"))
> +			found = bigjoiner_mode_found(drm_fd, connector,
> +						     sort_drm_modes_by_clk_dsc,mode,
> +						     max_dotclock) ||
> +				bigjoiner_mode_found(drm_fd, connector,
> +						     sort_drm_modes_by_res_dsc, mode,
> +						     max_dotclock) ||
> +				bigjoiner_mode_found(drm_fd, connector,
> +						     sort_drm_modes_by_clk_dsc, mode,
> +						     max_dotclock);
> +		else 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
>   			goto default_mode;
> -
> -		*mode = connector->modes[0];
> +		if (!found)
> +			*mode = connector->modes[0];
>   		return true;
>   	}
>   

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Swati,

On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
> Hi Kunal,
>
> On 15-Jan-24 4:28 PM, 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.
>>
>> v2: reuse bigjoiner_mode_found (Bhanu)
>> v3: avoid returning from multiple places (Bhanu)
>>      avoid frequent debugfs reads (Bhanu)
>>
>> Cc: Karthik B S <karthik.b.s@intel.com>
>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> ---
>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index cb6d57c2d..2c4210d4b 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -1771,8 +1771,9 @@ void igt_sort_connector_modes(drmModeConnector 
>> *connector,
>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>> drmModeConnector *connector,
>>                       drmModeModeInfo *mode)
>>   {
>> +    bool found;
>>       char *env;
>> -    int i;
>> +    int i, max_dotclock;
>>         if (!connector->count_modes) {
>>           igt_warn("no modes for connector %d\n",
>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>> drm_fd, drmModeConnector *connector,
>>       }
>>         env = getenv("IGT_KMS_RESOLUTION");
>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>       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 bigjoiner support or 
>> with
>> +                 highest clock if can't support  big joiner
>
> Shouldn't this be only joiner mode?
>
>
You mean can be named as joiner mode because same can be used for big 
joiner / ultra joiner?
or
We are returning the highest clock mode if no big joiner mode found?
>>            */
>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>> +            found = bigjoiner_mode_found(drm_fd, connector,
>> +                             sort_drm_modes_by_clk_dsc,mode,
>> +                             max_dotclock) ||
>> +                bigjoiner_mode_found(drm_fd, connector,
>> +                             sort_drm_modes_by_res_dsc, mode,
>> +                             max_dotclock) ||
>> +                bigjoiner_mode_found(drm_fd, connector,
>> +                             sort_drm_modes_by_clk_dsc, mode,
>> +                             max_dotclock);
>> +        else 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
>>               goto default_mode;
>> -
>> -        *mode = connector->modes[0];
>> +        if (!found)
>> +            *mode = connector->modes[0];
>>           return true;
>>       }

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Swati,

On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
> Hi Kunal,
>
> On 15-Jan-24 4:28 PM, 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.
>>
>> v2: reuse bigjoiner_mode_found (Bhanu)
>> v3: avoid returning from multiple places (Bhanu)
>>      avoid frequent debugfs reads (Bhanu)
>>
>> Cc: Karthik B S <karthik.b.s@intel.com>
>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> ---
>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index cb6d57c2d..2c4210d4b 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -1771,8 +1771,9 @@ void igt_sort_connector_modes(drmModeConnector 
>> *connector,
>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>> drmModeConnector *connector,
>>                       drmModeModeInfo *mode)
>>   {
>> +    bool found;
>>       char *env;
>> -    int i;
>> +    int i, max_dotclock;
>>         if (!connector->count_modes) {
>>           igt_warn("no modes for connector %d\n",
>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>> drm_fd, drmModeConnector *connector,
>>       }
>>         env = getenv("IGT_KMS_RESOLUTION");
>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>       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 bigjoiner support or 
>> with
>> +                 highest clock if can't support  big joiner
>
> Shouldn't this be only joiner mode?
>
>
You mean can be named as joiner mode because same can be used for big 
joiner / ultra joiner?
or
We are returning the highest clock mode if no big joiner mode found?
>>            */
>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>> +            found = bigjoiner_mode_found(drm_fd, connector,
>> +                             sort_drm_modes_by_clk_dsc,mode,
>> +                             max_dotclock) ||
>> +                bigjoiner_mode_found(drm_fd, connector,
>> +                             sort_drm_modes_by_res_dsc, mode,
>> +                             max_dotclock) ||
>> +                bigjoiner_mode_found(drm_fd, connector,
>> +                             sort_drm_modes_by_clk_dsc, mode,
>> +                             max_dotclock);
>> +        else 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
>>               goto default_mode;
>> -
>> -        *mode = connector->modes[0];
>> +        if (!found)
>> +            *mode = connector->modes[0];
>>           return true;
>>       }

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Sharma, Swati2]

Hi Kunal,

On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
> Hello Swati,
> 
> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>> Hi Kunal,
>>
>> On 15-Jan-24 4:28 PM, 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.
>>>
>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>> v3: avoid returning from multiple places (Bhanu)
>>>      avoid frequent debugfs reads (Bhanu)
>>>
>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>> ---
>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>> index cb6d57c2d..2c4210d4b 100644
>>> --- a/lib/igt_kms.c
>>> +++ b/lib/igt_kms.c
>>> @@ -1771,8 +1771,9 @@ void igt_sort_connector_modes(drmModeConnector 
>>> *connector,
>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>> drmModeConnector *connector,
>>>                       drmModeModeInfo *mode)
>>>   {
>>> +    bool found;
>>>       char *env;
>>> -    int i;
>>> +    int i, max_dotclock;
>>>         if (!connector->count_modes) {
>>>           igt_warn("no modes for connector %d\n",
>>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>>> drm_fd, drmModeConnector *connector,
>>>       }
>>>         env = getenv("IGT_KMS_RESOLUTION");
>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>       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 bigjoiner support or 
>>> with
>>> +                 highest clock if can't support  big joiner
>>
>> Shouldn't this be only joiner mode?
>>
>>
> You mean can be named as joiner mode because same can be used for big 
> joiner / ultra joiner?

I guess this should be bigjoiner mode only. For ultra there might be 
other restrictions which we need to look at.

> or
> We are returning the highest clock mode if no big joiner mode found?

Yes, this is my query here. Why are we returning highest clock mode if 
no big joiner mode found?

>>>            */
>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>> +                             sort_drm_modes_by_clk_dsc,mode,
>>> +                             max_dotclock) ||
>>> +                bigjoiner_mode_found(drm_fd, connector,
>>> +                             sort_drm_modes_by_res_dsc, mode,
>>> +                             max_dotclock) ||
>>> +                bigjoiner_mode_found(drm_fd, connector,
>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>> +                             max_dotclock);
>>> +        else 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
>>>               goto default_mode;
>>> -
>>> -        *mode = connector->modes[0];
>>> +        if (!found)
>>> +            *mode = connector->modes[0];
>>>           return true;
>>>       }

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Swati,

On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
> Hi Kunal,
>
> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>> Hello Swati,
>>
>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>> Hi Kunal,
>>>
>>> On 15-Jan-24 4:28 PM, 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.
>>>>
>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>> v3: avoid returning from multiple places (Bhanu)
>>>>      avoid frequent debugfs reads (Bhanu)
>>>>
>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>> ---
>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>> index cb6d57c2d..2c4210d4b 100644
>>>> --- a/lib/igt_kms.c
>>>> +++ b/lib/igt_kms.c
>>>> @@ -1771,8 +1771,9 @@ void 
>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>> drmModeConnector *connector,
>>>>                       drmModeModeInfo *mode)
>>>>   {
>>>> +    bool found;
>>>>       char *env;
>>>> -    int i;
>>>> +    int i, max_dotclock;
>>>>         if (!connector->count_modes) {
>>>>           igt_warn("no modes for connector %d\n",
>>>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>>>> drm_fd, drmModeConnector *connector,
>>>>       }
>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>       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 bigjoiner support 
>>>> or with
>>>> +                 highest clock if can't support  big joiner
>>>
>>> Shouldn't this be only joiner mode?
>>>
>>>
>> You mean can be named as joiner mode because same can be used for big 
>> joiner / ultra joiner?
>
> I guess this should be bigjoiner mode only. For ultra there might be 
> other restrictions which we need to look at.
>
>> or
>> We are returning the highest clock mode if no big joiner mode found?
>
> Yes, this is my query here. Why are we returning highest clock mode if 
> no big joiner mode found?
>
We can return either with highest clock or highest resolution or default 
mode,
Since we have the other two already covered used highest clock.
>>>>            */
>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>> +                             max_dotclock) ||
>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>> +                             max_dotclock) ||
>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>> +                             max_dotclock);
>>>> +        else 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
>>>>               goto default_mode;
>>>> -
>>>> -        *mode = connector->modes[0];
>>>> +        if (!found)
>>>> +            *mode = connector->modes[0];
>>>>           return true;
>>>>       }

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Swati,

On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
> Hi Kunal,
>
> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>> Hello Swati,
>>
>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>> Hi Kunal,
>>>
>>> On 15-Jan-24 4:28 PM, 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.
>>>>
>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>> v3: avoid returning from multiple places (Bhanu)
>>>>      avoid frequent debugfs reads (Bhanu)
>>>>
>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>> ---
>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>> index cb6d57c2d..2c4210d4b 100644
>>>> --- a/lib/igt_kms.c
>>>> +++ b/lib/igt_kms.c
>>>> @@ -1771,8 +1771,9 @@ void 
>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>> drmModeConnector *connector,
>>>>                       drmModeModeInfo *mode)
>>>>   {
>>>> +    bool found;
>>>>       char *env;
>>>> -    int i;
>>>> +    int i, max_dotclock;
>>>>         if (!connector->count_modes) {
>>>>           igt_warn("no modes for connector %d\n",
>>>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>>>> drm_fd, drmModeConnector *connector,
>>>>       }
>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>       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 bigjoiner support 
>>>> or with
>>>> +                 highest clock if can't support  big joiner
>>>
>>> Shouldn't this be only joiner mode?
>>>
>>>
>> You mean can be named as joiner mode because same can be used for big 
>> joiner / ultra joiner?
>
> I guess this should be bigjoiner mode only. For ultra there might be 
> other restrictions which we need to look at.
>
>> or
>> We are returning the highest clock mode if no big joiner mode found?
>
> Yes, this is my query here. Why are we returning highest clock mode if 
> no big joiner mode found?
>
We can return either with highest clock or highest resolution or default 
mode,
Since we have the other two already covered used highest clock.
>>>>            */
>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>> +                             max_dotclock) ||
>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>> +                             max_dotclock) ||
>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>> +                             max_dotclock);
>>>> +        else 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
>>>>               goto default_mode;
>>>> -
>>>> -        *mode = connector->modes[0];
>>>> +        if (!found)
>>>> +            *mode = connector->modes[0];
>>>>           return true;
>>>>       }

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Sharma, Swati2]

Hi Kunal,

On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
> Hello Swati,
> 
> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>> Hi Kunal,
>>
>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>> Hello Swati,
>>>
>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>> Hi Kunal,
>>>>
>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>
>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>
>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>> ---
>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>> --- a/lib/igt_kms.c
>>>>> +++ b/lib/igt_kms.c
>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>> drmModeConnector *connector,
>>>>>                       drmModeModeInfo *mode)
>>>>>   {
>>>>> +    bool found;
>>>>>       char *env;
>>>>> -    int i;
>>>>> +    int i, max_dotclock;
>>>>>         if (!connector->count_modes) {
>>>>>           igt_warn("no modes for connector %d\n",
>>>>> @@ -1781,21 +1782,34 @@ bool kmstest_get_connector_default_mode(int 
>>>>> drm_fd, drmModeConnector *connector,
>>>>>       }
>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>       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 bigjoiner support 
>>>>> or with
>>>>> +                 highest clock if can't support  big joiner
>>>>
>>>> Shouldn't this be only joiner mode?
>>>>
>>>>
>>> You mean can be named as joiner mode because same can be used for big 
>>> joiner / ultra joiner?
>>
>> I guess this should be bigjoiner mode only. For ultra there might be 
>> other restrictions which we need to look at.
>>
>>> or
>>> We are returning the highest clock mode if no big joiner mode found?
>>
>> Yes, this is my query here. Why are we returning highest clock mode if 
>> no big joiner mode found?
>>
> We can return either with highest clock or highest resolution or default 
> mode,
> Since we have the other two already covered used highest clock.

But why we need to return something if bigjoiner mode not found?
If its not found, then we won't get to know only bigjoiner mode not 
found and we are falling back to default mode.
@bhanu what you suggest here?

>>>>>            */
>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>> +                             max_dotclock) ||
>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>>> +                             max_dotclock) ||
>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>>> +                             max_dotclock);
>>>>> +        else 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
>>>>>               goto default_mode;
>>>>> -
>>>>> -        *mode = connector->modes[0];
>>>>> +        if (!found)
>>>>> +            *mode = connector->modes[0];
>>>>>           return true;
>>>>>       }

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Swati and Bhanu,

On 1/17/2024 2:25 PM, Sharma, Swati2 wrote:
> Hi Kunal,
>
> On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
>> Hello Swati,
>>
>> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>>> Hi Kunal,
>>>
>>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>>> Hello Swati,
>>>>
>>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>>> Hi Kunal,
>>>>>
>>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>>
>>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>>
>>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>>> ---
>>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>>
>>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>>> --- a/lib/igt_kms.c
>>>>>> +++ b/lib/igt_kms.c
>>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>>> drmModeConnector *connector,
>>>>>>                       drmModeModeInfo *mode)
>>>>>>   {
>>>>>> +    bool found;
>>>>>>       char *env;
>>>>>> -    int i;
>>>>>> +    int i, max_dotclock;
>>>>>>         if (!connector->count_modes) {
>>>>>>           igt_warn("no modes for connector %d\n",
>>>>>> @@ -1781,21 +1782,34 @@ bool 
>>>>>> kmstest_get_connector_default_mode(int drm_fd, drmModeConnector 
>>>>>> *connector,
>>>>>>       }
>>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>>       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 bigjoiner 
>>>>>> support or with
>>>>>> +                 highest clock if can't support  big joiner
>>>>>
>>>>> Shouldn't this be only joiner mode?
>>>>>
>>>>>
>>>> You mean can be named as joiner mode because same can be used for 
>>>> big joiner / ultra joiner?
>>>
>>> I guess this should be bigjoiner mode only. For ultra there might be 
>>> other restrictions which we need to look at.
>>>
>>>> or
>>>> We are returning the highest clock mode if no big joiner mode found?
>>>
>>> Yes, this is my query here. Why are we returning highest clock mode 
>>> if no big joiner mode found?
>>>
>> We can return either with highest clock or highest resolution or 
>> default mode,
>> Since we have the other two already covered used highest clock.
>
> But why we need to return something if bigjoiner mode not found?
> If its not found, then we won't get to know only bigjoiner mode not 
> found and we are falling back to default mode.
> @bhanu what you suggest here?
>
Consider a setup with two display's with one having mode with big joiner 
support and other doesn't have any mode
for enabling big joiner.

In such case we expect the tests to run.
We can discuss on what mode to use on the display (highest_clock, 
highest_res, default) which doesn't support big joiner, but we need to 
have a mode there is
what i understand.
>>>>>>            */
>>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>>> +                             max_dotclock) ||
>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>>>> +                             max_dotclock) ||
>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>>>> +                             max_dotclock);
>>>>>> +        else 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
>>>>>>               goto default_mode;
>>>>>> -
>>>>>> -        *mode = connector->modes[0];
>>>>>> +        if (!found)
>>>>>> +            *mode = connector->modes[0];
>>>>>>           return true;
>>>>>>       }
Thanks and Regards
Kunal Joshi

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Modem, Bhanuprakash]

On 17-01-2024 02:44 pm, Joshi, Kunal1 wrote:
> Hello Swati and Bhanu,
> 
> On 1/17/2024 2:25 PM, Sharma, Swati2 wrote:
>> Hi Kunal,
>>
>> On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
>>> Hello Swati,
>>>
>>> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>>>> Hi Kunal,
>>>>
>>>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>>>> Hello Swati,
>>>>>
>>>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>>>> Hi Kunal,
>>>>>>
>>>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>>>
>>>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>>>
>>>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>>>> ---
>>>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>>>
>>>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>>>> --- a/lib/igt_kms.c
>>>>>>> +++ b/lib/igt_kms.c
>>>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>> drmModeConnector *connector,
>>>>>>>                       drmModeModeInfo *mode)
>>>>>>>   {
>>>>>>> +    bool found;
>>>>>>>       char *env;
>>>>>>> -    int i;
>>>>>>> +    int i, max_dotclock;
>>>>>>>         if (!connector->count_modes) {
>>>>>>>           igt_warn("no modes for connector %d\n",
>>>>>>> @@ -1781,21 +1782,34 @@ bool 
>>>>>>> kmstest_get_connector_default_mode(int drm_fd, drmModeConnector 
>>>>>>> *connector,
>>>>>>>       }
>>>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>>>       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 bigjoiner 
>>>>>>> support or with
>>>>>>> +                 highest clock if can't support  big joiner
>>>>>>
>>>>>> Shouldn't this be only joiner mode?
>>>>>>
>>>>>>
>>>>> You mean can be named as joiner mode because same can be used for 
>>>>> big joiner / ultra joiner?
>>>>
>>>> I guess this should be bigjoiner mode only. For ultra there might be 
>>>> other restrictions which we need to look at.
>>>>
>>>>> or
>>>>> We are returning the highest clock mode if no big joiner mode found?
>>>>
>>>> Yes, this is my query here. Why are we returning highest clock mode 
>>>> if no big joiner mode found?
>>>>
>>> We can return either with highest clock or highest resolution or 
>>> default mode,
>>> Since we have the other two already covered used highest clock.
>>
>> But why we need to return something if bigjoiner mode not found?
>> If its not found, then we won't get to know only bigjoiner mode not 
>> found and we are falling back to default mode.
>> @bhanu what you suggest here?
>>
> Consider a setup with two display's with one having mode with big joiner 
> support and other doesn't have any mode
> for enabling big joiner.
> 
> In such case we expect the tests to run.
> We can discuss on what mode to use on the display (highest_clock, 
> highest_res, default) which doesn't support big joiner, but we need to 
> have a mode there is
> what i understand.

IMHO, We should fall back to default mode if there is no bigjoiner 
supported mode available.

As per my understanding, Pick the mode that exercises the max bandwidth 
if there is no bigjoiner mode available is the expectation from Kunal.

- Bhanu

>>>>>>>            */
>>>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>>>> +                             max_dotclock) ||
>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>>>>> +                             max_dotclock) ||
>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>>>>> +                             max_dotclock);
>>>>>>> +        else 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
>>>>>>>               goto default_mode;
>>>>>>> -
>>>>>>> -        *mode = connector->modes[0];
>>>>>>> +        if (!found)
>>>>>>> +            *mode = connector->modes[0];
>>>>>>>           return true;
>>>>>>>       }
> Thanks and Regards
> Kunal Joshi

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Bhanu,

On 1/18/2024 2:40 PM, Modem, Bhanuprakash wrote:
>
> On 17-01-2024 02:44 pm, Joshi, Kunal1 wrote:
>> Hello Swati and Bhanu,
>>
>> On 1/17/2024 2:25 PM, Sharma, Swati2 wrote:
>>> Hi Kunal,
>>>
>>> On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
>>>> Hello Swati,
>>>>
>>>> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>>>>> Hi Kunal,
>>>>>
>>>>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>>>>> Hello Swati,
>>>>>>
>>>>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>>>>> Hi Kunal,
>>>>>>>
>>>>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>>>>
>>>>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>>>>
>>>>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>>>>> ---
>>>>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>>>>> --- a/lib/igt_kms.c
>>>>>>>> +++ b/lib/igt_kms.c
>>>>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>>> drmModeConnector *connector,
>>>>>>>>                       drmModeModeInfo *mode)
>>>>>>>>   {
>>>>>>>> +    bool found;
>>>>>>>>       char *env;
>>>>>>>> -    int i;
>>>>>>>> +    int i, max_dotclock;
>>>>>>>>         if (!connector->count_modes) {
>>>>>>>>           igt_warn("no modes for connector %d\n",
>>>>>>>> @@ -1781,21 +1782,34 @@ bool 
>>>>>>>> kmstest_get_connector_default_mode(int drm_fd, drmModeConnector 
>>>>>>>> *connector,
>>>>>>>>       }
>>>>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>>>>       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 bigjoiner 
>>>>>>>> support or with
>>>>>>>> +                 highest clock if can't support big joiner
>>>>>>>
>>>>>>> Shouldn't this be only joiner mode?
>>>>>>>
>>>>>>>
>>>>>> You mean can be named as joiner mode because same can be used for 
>>>>>> big joiner / ultra joiner?
>>>>>
>>>>> I guess this should be bigjoiner mode only. For ultra there might 
>>>>> be other restrictions which we need to look at.
>>>>>
>>>>>> or
>>>>>> We are returning the highest clock mode if no big joiner mode found?
>>>>>
>>>>> Yes, this is my query here. Why are we returning highest clock 
>>>>> mode if no big joiner mode found?
>>>>>
>>>> We can return either with highest clock or highest resolution or 
>>>> default mode,
>>>> Since we have the other two already covered used highest clock.
>>>
>>> But why we need to return something if bigjoiner mode not found?
>>> If its not found, then we won't get to know only bigjoiner mode not 
>>> found and we are falling back to default mode.
>>> @bhanu what you suggest here?
>>>
>> Consider a setup with two display's with one having mode with big 
>> joiner support and other doesn't have any mode
>> for enabling big joiner.
>>
>> In such case we expect the tests to run.
>> We can discuss on what mode to use on the display (highest_clock, 
>> highest_res, default) which doesn't support big joiner, but we need 
>> to have a mode there is
>> what i understand.
>
> IMHO, We should fall back to default mode if there is no bigjoiner 
> supported mode available.
>
> As per my understanding, Pick the mode that exercises the max 
> bandwidth if there is no bigjoiner mode available is the expectation 
> from Kunal.
>
> - Bhanu
Will select default_mode in case of no big joiner mode found.
Will have separate parameter for the bandwidth.

>
>
>>>>>>>>            */
>>>>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>>>>> +                             max_dotclock) ||
>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>> + sort_drm_modes_by_res_dsc, mode,
>>>>>>>> +                             max_dotclock) ||
>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>> + sort_drm_modes_by_clk_dsc, mode,
>>>>>>>> +                             max_dotclock);
>>>>>>>> +        else 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
>>>>>>>>               goto default_mode;
>>>>>>>> -
>>>>>>>> -        *mode = connector->modes[0];
>>>>>>>> +        if (!found)
>>>>>>>> +            *mode = connector->modes[0];
>>>>>>>>           return true;
>>>>>>>>       }
>> Thanks and Regards
>> Kunal Joshi

Re: [PATCH i-g-t 1/2] lib/igt_kms: move bigjoiner_mode_found to lib.

[Modem, Bhanuprakash]

Hi Kunal,

On 12-01-2024 02:25 pm, Kunal Joshi wrote:
> move bigjoiner_mode_found to lib/igt_kms with some modification
> 
> Cc: Karthik B S <karthik.b.s@intel.com>
> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
> ---
>   lib/igt_kms.c                | 19 +++++++++++++++++++
>   lib/igt_kms.h                |  3 +++
>   tests/intel/kms_big_joiner.c | 14 ++------------
>   3 files changed, 24 insertions(+), 12 deletions(-)
> 
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index e4dea1a60..2c55af05f 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6142,6 +6142,25 @@ bool igt_bigjoiner_possible(drmModeModeInfo *mode, int max_dotclock)
>   		mode->clock > max_dotclock);
>   }
>   
> +/**
> + * bigjoiner_mode_found:
> + * @connector: libdrm connector
> + * @sort_method: comparator method
> + * @mode: libdrm mode
> + *
> + * Returns: True if big joiner found in connector modes
> + */
> +bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
> +			  int (*sort_method)(const void *, const void*),
> +			  drmModeModeInfo *mode)
> +{
> +        igt_sort_connector_modes(connector, sort_method);
> +        *mode = connector->modes[0];

IMHO, Instead of calling this helper multiple times with different sort 
methods, let's consolidate here only. Example below:

bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
                          int max_dotclock)
{
   igt_sort_connector_modes(connector, sort_drm_modes_by_res_dsc);
   if (igt_bigjoiner_possible(&connector->modes[0], max_dotclock))
     return true;

   igt_sort_connector_modes(connector, sort_drm_modes_by_clk_dsc);
   return igt_bigjoiner_possible(&connector->modes[0], max_dotclock);
}

> +
> +        return igt_bigjoiner_possible(mode,
> +				      igt_get_max_dotclock(drm_fd));
> +}
> +
>   /**
>    * igt_check_bigjoiner_support:
>    * @display: a pointer to an #igt_display_t structure
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index b3882808b..9f5676d35 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1212,6 +1212,9 @@ bool igt_max_bpc_constraint(igt_display_t *display, enum pipe pipe,
>   		igt_output_t *output, int bpc);
>   int igt_get_max_dotclock(int fd);
>   bool igt_bigjoiner_possible(drmModeModeInfo *mode, int max_dotclock);
> +bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
> +                                 int (*sort_method)(const void *, const void*),
> +                                 drmModeModeInfo *mode);
>   bool igt_check_bigjoiner_support(igt_display_t *display);
>   bool igt_parse_mode_string(const char *mode_string, drmModeModeInfo *mode);
>   bool intel_pipe_output_combo_valid(igt_display_t *display);
> diff --git a/tests/intel/kms_big_joiner.c b/tests/intel/kms_big_joiner.c
> index aba2adfbe..1858c6362 100644
> --- a/tests/intel/kms_big_joiner.c
> +++ b/tests/intel/kms_big_joiner.c
> @@ -199,16 +199,6 @@ static void test_dual_display(data_t *data)
>   	igt_display_commit2(display, COMMIT_ATOMIC);
>   }
>   
> -static bool bigjoiner_mode_found(drmModeConnector *connector,
> -				 int (*sort_method)(const void *, const void*),
> -				 drmModeModeInfo *mode)
> -{
> -	igt_sort_connector_modes(connector, sort_method);
> -	*mode = connector->modes[0];
> -
> -	return igt_bigjoiner_possible(mode, max_dotclock);
> -}
> -
>   igt_main
>   {
>   	data_t data;
> @@ -235,8 +225,8 @@ igt_main
>   			 * Bigjoiner will come in to the picture when the
>   			 * resolution > 5K or clock > max-dot-clock.
>   			 */
> -			found = (bigjoiner_mode_found(connector, sort_drm_modes_by_res_dsc, &mode) ||
> -				 bigjoiner_mode_found(connector, sort_drm_modes_by_clk_dsc, &mode)) ?
> +			found = (bigjoiner_mode_found(data.drm_fd, connector, sort_drm_modes_by_res_dsc, &mode) ||
> +				 bigjoiner_mode_found(data.drm_fd, connector, sort_drm_modes_by_clk_dsc, &mode)) ?
>   					true : false;
>   
>   			if (found) {

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Nautiyal, Ankit K]

On 1/18/2024 2:44 PM, Joshi, Kunal1 wrote:
> Hello Bhanu,
>
> On 1/18/2024 2:40 PM, Modem, Bhanuprakash wrote:
>>
>> On 17-01-2024 02:44 pm, Joshi, Kunal1 wrote:
>>> Hello Swati and Bhanu,
>>>
>>> On 1/17/2024 2:25 PM, Sharma, Swati2 wrote:
>>>> Hi Kunal,
>>>>
>>>> On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
>>>>> Hello Swati,
>>>>>
>>>>> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>>>>>> Hi Kunal,
>>>>>>
>>>>>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>>>>>> Hello Swati,
>>>>>>>
>>>>>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>>>>>> Hi Kunal,
>>>>>>>>
>>>>>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>>>>>
>>>>>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>>>>>
>>>>>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>>>>>> ---
>>>>>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>>>>>
>>>>>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>>>>>> --- a/lib/igt_kms.c
>>>>>>>>> +++ b/lib/igt_kms.c
>>>>>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>>>> drmModeConnector *connector,
>>>>>>>>>                       drmModeModeInfo *mode)
>>>>>>>>>   {
>>>>>>>>> +    bool found;
>>>>>>>>>       char *env;
>>>>>>>>> -    int i;
>>>>>>>>> +    int i, max_dotclock;
>>>>>>>>>         if (!connector->count_modes) {
>>>>>>>>>           igt_warn("no modes for connector %d\n",
>>>>>>>>> @@ -1781,21 +1782,34 @@ bool 
>>>>>>>>> kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>>>> drmModeConnector *connector,
>>>>>>>>>       }
>>>>>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>>>>>       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 bigjoiner 
>>>>>>>>> support or with
>>>>>>>>> +                 highest clock if can't support big joiner
>>>>>>>>
>>>>>>>> Shouldn't this be only joiner mode?
>>>>>>>>
>>>>>>>>
>>>>>>> You mean can be named as joiner mode because same can be used 
>>>>>>> for big joiner / ultra joiner?
>>>>>>
>>>>>> I guess this should be bigjoiner mode only. For ultra there might 
>>>>>> be other restrictions which we need to look at.
>>>>>>
>>>>>>> or
>>>>>>> We are returning the highest clock mode if no big joiner mode 
>>>>>>> found?
>>>>>>
>>>>>> Yes, this is my query here. Why are we returning highest clock 
>>>>>> mode if no big joiner mode found?
>>>>>>
>>>>> We can return either with highest clock or highest resolution or 
>>>>> default mode,
>>>>> Since we have the other two already covered used highest clock.
>>>>
>>>> But why we need to return something if bigjoiner mode not found?
>>>> If its not found, then we won't get to know only bigjoiner mode not 
>>>> found and we are falling back to default mode.
>>>> @bhanu what you suggest here?
>>>>
>>> Consider a setup with two display's with one having mode with big 
>>> joiner support and other doesn't have any mode
>>> for enabling big joiner.
>>>
>>> In such case we expect the tests to run.
>>> We can discuss on what mode to use on the display (highest_clock, 
>>> highest_res, default) which doesn't support big joiner, but we need 
>>> to have a mode there is
>>> what i understand.
>>
>> IMHO, We should fall back to default mode if there is no bigjoiner 
>> supported mode available.
>>
>> As per my understanding, Pick the mode that exercises the max 
>> bandwidth if there is no bigjoiner mode available is the expectation 
>> from Kunal.
>>
>> - Bhanu
> Will select default_mode in case of no big joiner mode found.
> Will have separate parameter for the bandwidth.

'Highest' and 'lowest' make sense for any driver, but IMHO bigjoiner is 
Intel-specific terminology, do we really need to have an environment 
variable for that.

For most purpose, highest itself should be sufficient, if the test-setup 
has a panel that needs bigjoiner.


Regards,

Ankit


>
>>
>>
>>>>>>>>>            */
>>>>>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>>>>>> +                             max_dotclock) ||
>>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>>> + sort_drm_modes_by_res_dsc, mode,
>>>>>>>>> +                             max_dotclock) ||
>>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>>> + sort_drm_modes_by_clk_dsc, mode,
>>>>>>>>> +                             max_dotclock);
>>>>>>>>> +        else 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
>>>>>>>>>               goto default_mode;
>>>>>>>>> -
>>>>>>>>> -        *mode = connector->modes[0];
>>>>>>>>> +        if (!found)
>>>>>>>>> +            *mode = connector->modes[0];
>>>>>>>>>           return true;
>>>>>>>>>       }
>>> Thanks and Regards
>>> Kunal Joshi

[PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Kunal Joshi]

add support to choose big joiner mode with environment
variable, use mode with highest clock if no mode with big joiner
found.

v2: reuse bigjoiner_mode_found (Bhanu)
v3: avoid returning from multiple places (Bhanu)
    avoid frequent debugfs reads (Bhanu)
v4: use default mode (Bhanu)

Cc: Swati Sharma <swati2.sharma@intel.com>
Cc: Karthik B S <karthik.b.s@intel.com>
Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
---
 lib/igt_kms.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/lib/igt_kms.c b/lib/igt_kms.c
index 7a07e62f8..77059dd34 100644
--- a/lib/igt_kms.c
+++ b/lib/igt_kms.c
@@ -1771,8 +1771,9 @@ void igt_sort_connector_modes(drmModeConnector *connector,
 bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 					drmModeModeInfo *mode)
 {
+	bool found;
 	char *env;
-	int i;
+	int i, max_dotclock;
 
 	if (!connector->count_modes) {
 		igt_warn("no modes for connector %d\n",
@@ -1781,14 +1782,23 @@ bool kmstest_get_connector_default_mode(int drm_fd, drmModeConnector *connector,
 	}
 
 	env = getenv("IGT_KMS_RESOLUTION");
+	max_dotclock = igt_get_max_dotclock(drm_fd);
 	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 bigjoiner support or default
+		 *	     mode if can't support  big joiner
 		 */
-		if (!strcmp(env, "highest") || !strcmp(env, "1"))
+		if (!strcmp(env, "joiner") || !strcmp(env, "2")) {
+			found = bigjoiner_mode_found(drm_fd, connector,
+						     max_dotclock);
+			if (!found)
+				goto default_mode;
+		}
+		else 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);
-- 
2.25.1

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Joshi, Kunal1]

Hello Ankit,

On 1/18/2024 4:08 PM, Nautiyal, Ankit K wrote:
>
> On 1/18/2024 2:44 PM, Joshi, Kunal1 wrote:
>> Hello Bhanu,
>>
>> On 1/18/2024 2:40 PM, Modem, Bhanuprakash wrote:
>>>
>>> On 17-01-2024 02:44 pm, Joshi, Kunal1 wrote:
>>>> Hello Swati and Bhanu,
>>>>
>>>> On 1/17/2024 2:25 PM, Sharma, Swati2 wrote:
>>>>> Hi Kunal,
>>>>>
>>>>> On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
>>>>>> Hello Swati,
>>>>>>
>>>>>> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>>>>>>> Hi Kunal,
>>>>>>>
>>>>>>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>>>>>>> Hello Swati,
>>>>>>>>
>>>>>>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>>>>>>> Hi Kunal,
>>>>>>>>>
>>>>>>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>>>>>>
>>>>>>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>>>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>>>>>>
>>>>>>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>>>>>>> ---
>>>>>>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>>>>>>
>>>>>>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>>>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>>>>>>> --- a/lib/igt_kms.c
>>>>>>>>>> +++ b/lib/igt_kms.c
>>>>>>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>>>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>>>>> drmModeConnector *connector,
>>>>>>>>>>                       drmModeModeInfo *mode)
>>>>>>>>>>   {
>>>>>>>>>> +    bool found;
>>>>>>>>>>       char *env;
>>>>>>>>>> -    int i;
>>>>>>>>>> +    int i, max_dotclock;
>>>>>>>>>>         if (!connector->count_modes) {
>>>>>>>>>>           igt_warn("no modes for connector %d\n",
>>>>>>>>>> @@ -1781,21 +1782,34 @@ bool 
>>>>>>>>>> kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>>>>> drmModeConnector *connector,
>>>>>>>>>>       }
>>>>>>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>>>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>>>>>>       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 bigjoiner 
>>>>>>>>>> support or with
>>>>>>>>>> +                 highest clock if can't support big joiner
>>>>>>>>>
>>>>>>>>> Shouldn't this be only joiner mode?
>>>>>>>>>
>>>>>>>>>
>>>>>>>> You mean can be named as joiner mode because same can be used 
>>>>>>>> for big joiner / ultra joiner?
>>>>>>>
>>>>>>> I guess this should be bigjoiner mode only. For ultra there 
>>>>>>> might be other restrictions which we need to look at.
>>>>>>>
>>>>>>>> or
>>>>>>>> We are returning the highest clock mode if no big joiner mode 
>>>>>>>> found?
>>>>>>>
>>>>>>> Yes, this is my query here. Why are we returning highest clock 
>>>>>>> mode if no big joiner mode found?
>>>>>>>
>>>>>> We can return either with highest clock or highest resolution or 
>>>>>> default mode,
>>>>>> Since we have the other two already covered used highest clock.
>>>>>
>>>>> But why we need to return something if bigjoiner mode not found?
>>>>> If its not found, then we won't get to know only bigjoiner mode 
>>>>> not found and we are falling back to default mode.
>>>>> @bhanu what you suggest here?
>>>>>
>>>> Consider a setup with two display's with one having mode with big 
>>>> joiner support and other doesn't have any mode
>>>> for enabling big joiner.
>>>>
>>>> In such case we expect the tests to run.
>>>> We can discuss on what mode to use on the display (highest_clock, 
>>>> highest_res, default) which doesn't support big joiner, but we need 
>>>> to have a mode there is
>>>> what i understand.
>>>
>>> IMHO, We should fall back to default mode if there is no bigjoiner 
>>> supported mode available.
>>>
>>> As per my understanding, Pick the mode that exercises the max 
>>> bandwidth if there is no bigjoiner mode available is the expectation 
>>> from Kunal.
>>>
>>> - Bhanu
>> Will select default_mode in case of no big joiner mode found.
>> Will have separate parameter for the bandwidth.
>
> 'Highest' and 'lowest' make sense for any driver, but IMHO bigjoiner 
> is Intel-specific terminology, do we really need to have an 
> environment variable for that.
>
> For most purpose, highest itself should be sufficient, if the 
> test-setup has a panel that needs bigjoiner.
>
>
> Regards,
>
> Ankit
>
>
>>
>>>
>>>
>>>>>>>>>>            */
>>>>>>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
How about
if (is_intel_device(drm_fd) && !strcmp(env, "joiner") || !strcmp(env, "2")
>>>>>>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>>>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>>>>>>> +                             max_dotclock) ||
>>>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>>>> + sort_drm_modes_by_res_dsc, mode,
>>>>>>>>>> +                             max_dotclock) ||
>>>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>>>> + sort_drm_modes_by_clk_dsc, mode,
>>>>>>>>>> +                             max_dotclock);
>>>>>>>>>> +        else 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
>>>>>>>>>>               goto default_mode;
>>>>>>>>>> -
>>>>>>>>>> -        *mode = connector->modes[0];
>>>>>>>>>> +        if (!found)
>>>>>>>>>> +            *mode = connector->modes[0];
>>>>>>>>>>           return true;
>>>>>>>>>>       }
>>>> Thanks and Regards
>>>> Kunal Joshi
Thanks and Regards
Kunal Joshi

Re: [PATCH i-g-t 2/2] lib/igt_kms: add support for choosing big joiner mode

[Sharma, Swati2]

Hi Bhanu,

On 18-Jan-24 2:40 PM, Modem, Bhanuprakash wrote:
> 
> On 17-01-2024 02:44 pm, Joshi, Kunal1 wrote:
>> Hello Swati and Bhanu,
>>
>> On 1/17/2024 2:25 PM, Sharma, Swati2 wrote:
>>> Hi Kunal,
>>>
>>> On 17-Jan-24 11:14 AM, Joshi, Kunal1 wrote:
>>>> Hello Swati,
>>>>
>>>> On 1/16/2024 5:33 PM, Sharma, Swati2 wrote:
>>>>> Hi Kunal,
>>>>>
>>>>> On 16-Jan-24 5:29 PM, Joshi, Kunal1 wrote:
>>>>>> Hello Swati,
>>>>>>
>>>>>> On 1/16/2024 4:34 PM, Sharma, Swati2 wrote:
>>>>>>> Hi Kunal,
>>>>>>>
>>>>>>> On 15-Jan-24 4:28 PM, 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.
>>>>>>>>
>>>>>>>> v2: reuse bigjoiner_mode_found (Bhanu)
>>>>>>>> v3: avoid returning from multiple places (Bhanu)
>>>>>>>>      avoid frequent debugfs reads (Bhanu)
>>>>>>>>
>>>>>>>> Cc: Karthik B S <karthik.b.s@intel.com>
>>>>>>>> Cc: Bhanuprakash Modem <bhanuprakash.modem@intel.com>
>>>>>>>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>>>>>>>> ---
>>>>>>>>   lib/igt_kms.c | 24 +++++++++++++++++++-----
>>>>>>>>   1 file changed, 19 insertions(+), 5 deletions(-)
>>>>>>>>
>>>>>>>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>>>>>>>> index cb6d57c2d..2c4210d4b 100644
>>>>>>>> --- a/lib/igt_kms.c
>>>>>>>> +++ b/lib/igt_kms.c
>>>>>>>> @@ -1771,8 +1771,9 @@ void 
>>>>>>>> igt_sort_connector_modes(drmModeConnector *connector,
>>>>>>>>   bool kmstest_get_connector_default_mode(int drm_fd, 
>>>>>>>> drmModeConnector *connector,
>>>>>>>>                       drmModeModeInfo *mode)
>>>>>>>>   {
>>>>>>>> +    bool found;
>>>>>>>>       char *env;
>>>>>>>> -    int i;
>>>>>>>> +    int i, max_dotclock;
>>>>>>>>         if (!connector->count_modes) {
>>>>>>>>           igt_warn("no modes for connector %d\n",
>>>>>>>> @@ -1781,21 +1782,34 @@ bool 
>>>>>>>> kmstest_get_connector_default_mode(int drm_fd, drmModeConnector 
>>>>>>>> *connector,
>>>>>>>>       }
>>>>>>>>         env = getenv("IGT_KMS_RESOLUTION");
>>>>>>>> +    max_dotclock = igt_get_max_dotclock(drm_fd);
>>>>>>>>       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 bigjoiner 
>>>>>>>> support or with
>>>>>>>> +                 highest clock if can't support  big joiner
>>>>>>>
>>>>>>> Shouldn't this be only joiner mode?
>>>>>>>
>>>>>>>
>>>>>> You mean can be named as joiner mode because same can be used for 
>>>>>> big joiner / ultra joiner?
>>>>>
>>>>> I guess this should be bigjoiner mode only. For ultra there might 
>>>>> be other restrictions which we need to look at.
>>>>>
>>>>>> or
>>>>>> We are returning the highest clock mode if no big joiner mode found?
>>>>>
>>>>> Yes, this is my query here. Why are we returning highest clock mode 
>>>>> if no big joiner mode found?
>>>>>
>>>> We can return either with highest clock or highest resolution or 
>>>> default mode,
>>>> Since we have the other two already covered used highest clock.
>>>
>>> But why we need to return something if bigjoiner mode not found?
>>> If its not found, then we won't get to know only bigjoiner mode not 
>>> found and we are falling back to default mode.
>>> @bhanu what you suggest here?
>>>
>> Consider a setup with two display's with one having mode with big 
>> joiner support and other doesn't have any mode
>> for enabling big joiner.
>>
>> In such case we expect the tests to run.
>> We can discuss on what mode to use on the display (highest_clock, 
>> highest_res, default) which doesn't support big joiner, but we need to 
>> have a mode there is
>> what i understand.
> 
> IMHO, We should fall back to default mode if there is no bigjoiner 
> supported mode available.

Makes sense.

Should we have
igt_info("Using IGT_KMS_RESOLUTION=%d as default mode\n", env);
like something we have for IGT_SRANDOM env var.
igt_info("Using IGT_SRANDOM=%d for randomisation\n", seed);
> 
> As per my understanding, Pick the mode that exercises the max bandwidth 
> if there is no bigjoiner mode available is the expectation from Kunal.
> 
> - Bhanu
> 
>>>>>>>>            */
>>>>>>>> -        if (!strcmp(env, "highest") || !strcmp(env, "1"))
>>>>>>>> +        if (!strcmp(env, "joiner") || !strcmp(env, "2"))
>>>>>>>> +            found = bigjoiner_mode_found(drm_fd, connector,
>>>>>>>> + sort_drm_modes_by_clk_dsc,mode,
>>>>>>>> +                             max_dotclock) ||
>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>> +                             sort_drm_modes_by_res_dsc, mode,
>>>>>>>> +                             max_dotclock) ||
>>>>>>>> +                bigjoiner_mode_found(drm_fd, connector,
>>>>>>>> +                             sort_drm_modes_by_clk_dsc, mode,
>>>>>>>> +                             max_dotclock);
>>>>>>>> +        else 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
>>>>>>>>               goto default_mode;
>>>>>>>> -
>>>>>>>> -        *mode = connector->modes[0];
>>>>>>>> +        if (!found)
>>>>>>>> +            *mode = connector->modes[0];
>>>>>>>>           return true;
>>>>>>>>       }
>> Thanks and Regards
>> Kunal Joshi