[PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode

Igt-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed

* [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode
@ 2025-09-19  3:17 Jeevan B
  2025-09-19  3:17 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
                   ` (3 more replies)
  0 siblings, 4 replies; 11+ messages in thread
From: Jeevan B @ 2025-09-19  3:17 UTC (permalink / raw)
  To: igt-dev; +Cc: swati2.sharma, Jeevan B

If an eDP connector is present, prefer selecting its default mode for
cloning. Some eDP panels support only high refresh rate (HRR) modes,
which may not be compatible with external displays. To avoid invalid
clone configurations and test failures, we choose the default mode
from the eDP connector when available.

Signed-off-by: Jeevan B <jeevan.b@intel.com>

Jeevan B (2):
  tests/kms_setmode: simplify mode selection by removing goto
  tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode

 tests/kms_setmode.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto
  2025-09-19  3:17 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
@ 2025-09-19  3:17 ` Jeevan B
  2025-09-19  5:34   ` Karthik B S
  2025-09-19  3:17 ` [PATCH i-g-t 2/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 11+ messages in thread
From: Jeevan B @ 2025-09-19  3:17 UTC (permalink / raw)
  To: igt-dev; +Cc: swati2.sharma, Jeevan B

Replaced 'goto found' with early returns in get_mode_for_crtc() and
added braces around the fallback loop. This makes the logic cleaner
and easier to follow without changing behavior.

v2: write to *mode_ret before returning from function.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 tests/kms_setmode.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 484c3a95f..48d0c3e1e 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -238,8 +238,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->connector_count; i++) {
 		mode = &crtc->cconfs[i].default_mode;
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -248,8 +250,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->cconfs[0].connector->count_modes; i++) {
 		mode = &crtc->cconfs[0].connector->modes[i];
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -258,10 +262,11 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 * scaling etc.
 	 */
 	mode = &crtc->cconfs[0].default_mode;
-	for (i = 1; i < crtc->connector_count; i++)
+	for (i = 1; i < crtc->connector_count; i++) {
 		if (crtc->cconfs[i].default_mode.clock < mode->clock)
 			mode = &crtc->cconfs[i].default_mode;
-found:
+	}
+
 	*mode_ret = *mode;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto
  2025-09-19  3:17 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
@ 2025-09-19  5:34   ` Karthik B S
  0 siblings, 0 replies; 11+ messages in thread
From: Karthik B S @ 2025-09-19  5:34 UTC (permalink / raw)
  To: Jeevan B, igt-dev; +Cc: swati2.sharma


On 9/19/2025 8:47 AM, Jeevan B wrote:
> Replaced 'goto found' with early returns in get_mode_for_crtc() and
> added braces around the fallback loop. This makes the logic cleaner
> and easier to follow without changing behavior.
>
> v2: write to *mode_ret before returning from function.
>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
Reviewed-by: Karthik B S <karthik.b.s@intel.com>
> ---
>   tests/kms_setmode.c | 17 +++++++++++------
>   1 file changed, 11 insertions(+), 6 deletions(-)
>
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 484c3a95f..48d0c3e1e 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -238,8 +238,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>   	 */
>   	for (i = 0; i < crtc->connector_count; i++) {
>   		mode = &crtc->cconfs[i].default_mode;
> -		if (crtc_supports_mode(crtc, mode))
> -			goto found;
> +		if (crtc_supports_mode(crtc, mode)) {
> +			*mode_ret = *mode;
> +			return;
> +		}
>   	}
>   
>   	/*
> @@ -248,8 +250,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>   	 */
>   	for (i = 0; i < crtc->cconfs[0].connector->count_modes; i++) {
>   		mode = &crtc->cconfs[0].connector->modes[i];
> -		if (crtc_supports_mode(crtc, mode))
> -			goto found;
> +		if (crtc_supports_mode(crtc, mode)) {
> +			*mode_ret = *mode;
> +			return;
> +		}
>   	}
>   
>   	/*
> @@ -258,10 +262,11 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>   	 * scaling etc.
>   	 */
>   	mode = &crtc->cconfs[0].default_mode;
> -	for (i = 1; i < crtc->connector_count; i++)
> +	for (i = 1; i < crtc->connector_count; i++) {
>   		if (crtc->cconfs[i].default_mode.clock < mode->clock)
>   			mode = &crtc->cconfs[i].default_mode;
> -found:
> +	}
> +
>   	*mode_ret = *mode;
>   }
>   

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 2/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode
  2025-09-19  3:17 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
  2025-09-19  3:17 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
@ 2025-09-19  3:17 ` Jeevan B
  2025-09-19  6:20   ` Karthik B S
  2025-09-19  5:11 ` ✗ i915.CI.BAT: failure for tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5) Patchwork
  2025-09-19  5:12 ` ✓ Xe.CI.BAT: success " Patchwork
  3 siblings, 1 reply; 11+ messages in thread
From: Jeevan B @ 2025-09-19  3:17 UTC (permalink / raw)
  To: igt-dev; +Cc: swati2.sharma, Jeevan B

If an eDP connector is present, prefer selecting its default mode for
cloning. Some eDP panels support only high refresh rate (HRR) modes,
which may not be compatible with external displays. To avoid invalid
clone configurations and test failures, we choose the default mode
from the eDP connector when available.

v2: Fix logic for eDP fallback.
v3: Fix comment to say ‘lowest clock mode selected’ instead of
    ‘default mode’.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 tests/kms_setmode.c | 19 +++++++++++++++++++
 1 file changed, 19 insertions(+)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 48d0c3e1e..135918007 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -256,6 +256,25 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 		}
 	}
 
+	/* If an eDP connector is present, iterate through its modes and
+	 * pick a mode with lowest clock, since internal panels typically
+	 * dictate the clone mode and may have HRR support, making them
+	 * incompatible with modes supported by external displays.
+	 */
+	for (i = 0; i < crtc->connector_count; i++) {
+		drmModeConnector *conn = crtc->cconfs[i].connector;
+
+		if (conn->connector_type == DRM_MODE_CONNECTOR_eDP) {
+			mode = &conn->modes[0];
+			for (int j = 1; j < conn->count_modes; j++) {
+				if (conn->modes[j].clock < mode->clock)
+					mode = &conn->modes[j];
+			}
+			*mode_ret = *mode;
+			return;
+		}
+	}
+
 	/*
 	 * If none is found then just pick the default mode from all connectors
 	 * with the smallest clock, hope the other connectors can support it by
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t 2/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode
  2025-09-19  3:17 ` [PATCH i-g-t 2/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
@ 2025-09-19  6:20   ` Karthik B S
  0 siblings, 0 replies; 11+ messages in thread
From: Karthik B S @ 2025-09-19  6:20 UTC (permalink / raw)
  To: Jeevan B, igt-dev; +Cc: swati2.sharma

Hi Jeevan,

On 9/19/2025 8:47 AM, Jeevan B wrote:
> If an eDP connector is present, prefer selecting its default mode for
> cloning. Some eDP panels support only high refresh rate (HRR) modes,
> which may not be compatible with external displays. To avoid invalid
Please rephrase "which may not be compatible with external displays", 
with something similar to the comment used in the test. As the current 
sentence is slightly misleading IMHO.
> clone configurations and test failures, we choose the default mode
> from the eDP connector when available.
>
> v2: Fix logic for eDP fallback.
> v3: Fix comment to say ‘lowest clock mode selected’ instead of
>      ‘default mode’.

Please add reference to the gitlab issue with closes tag. I assume this 
one? https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13780

Regards,
Karthik.B.S
>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>   tests/kms_setmode.c | 19 +++++++++++++++++++
>   1 file changed, 19 insertions(+)
>
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 48d0c3e1e..135918007 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -256,6 +256,25 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>   		}
>   	}
>   
> +	/* If an eDP connector is present, iterate through its modes and
> +	 * pick a mode with lowest clock, since internal panels typically
> +	 * dictate the clone mode and may have HRR support, making them
> +	 * incompatible with modes supported by external displays.
> +	 */
> +	for (i = 0; i < crtc->connector_count; i++) {
> +		drmModeConnector *conn = crtc->cconfs[i].connector;
> +
> +		if (conn->connector_type == DRM_MODE_CONNECTOR_eDP) {
> +			mode = &conn->modes[0];
> +			for (int j = 1; j < conn->count_modes; j++) {
> +				if (conn->modes[j].clock < mode->clock)
> +					mode = &conn->modes[j];
> +			}
> +			*mode_ret = *mode;
> +			return;
> +		}
> +	}
> +
>   	/*
>   	 * If none is found then just pick the default mode from all connectors
>   	 * with the smallest clock, hope the other connectors can support it by

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✗ i915.CI.BAT: failure for tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5)
  2025-09-19  3:17 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
  2025-09-19  3:17 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
  2025-09-19  3:17 ` [PATCH i-g-t 2/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
@ 2025-09-19  5:11 ` Patchwork
  2025-09-19  5:12 ` ✓ Xe.CI.BAT: success " Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2025-09-19  5:11 UTC (permalink / raw)
  To: Jeevan B; +Cc: igt-dev

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

== Series Details ==

Series: tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5)
URL   : https://patchwork.freedesktop.org/series/151429/
State : failure

== Summary ==

CI Bug Log - changes from IGT_8543 -> IGTPW_13784
====================================================

Summary
-------

  **FAILURE**

  Serious unknown changes coming with IGTPW_13784 absolutely need to be
  verified manually.
  
  If you think the reported changes have nothing to do with the changes
  introduced in IGTPW_13784, 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_13784/index.html

Participating hosts (43 -> 42)
------------------------------

  Missing    (1): fi-snb-2520m 

Possible new issues
-------------------

  Here are the unknown changes that may have been introduced in IGTPW_13784:

### IGT changes ###

#### Possible regressions ####

  * igt@i915_selftest@live@coherency:
    - bat-arlh-3:         [PASS][1] -> [INCOMPLETE][2]
   [1]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/bat-arlh-3/igt@i915_selftest@live@coherency.html
   [2]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/bat-arlh-3/igt@i915_selftest@live@coherency.html

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

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

### IGT changes ###

#### Issues hit ####

  * igt@dmabuf@all-tests@dma_fence_chain:
    - fi-bsw-nick:        [PASS][3] -> [ABORT][4] ([i915#12904]) +1 other test abort
   [3]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html
   [4]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/fi-bsw-nick/igt@dmabuf@all-tests@dma_fence_chain.html

  * igt@i915_selftest@live:
    - bat-arlh-3:         [PASS][5] -> [INCOMPLETE][6] ([i915#14764] / [i915#14837])
   [5]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/bat-arlh-3/igt@i915_selftest@live.html
   [6]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/bat-arlh-3/igt@i915_selftest@live.html

  * igt@i915_selftest@live@sanitycheck:
    - bat-apl-1:          [PASS][7] -> [DMESG-WARN][8] ([i915#13735]) +77 other tests dmesg-warn
   [7]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/bat-apl-1/igt@i915_selftest@live@sanitycheck.html
   [8]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/bat-apl-1/igt@i915_selftest@live@sanitycheck.html

  * igt@i915_selftest@live@workarounds:
    - bat-arls-5:         [PASS][9] -> [DMESG-FAIL][10] ([i915#12061]) +1 other test dmesg-fail
   [9]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/bat-arls-5/igt@i915_selftest@live@workarounds.html
   [10]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/bat-arls-5/igt@i915_selftest@live@workarounds.html

  * igt@kms_pm_rpm@basic-pci-d3-state:
    - bat-apl-1:          [PASS][11] -> [DMESG-WARN][12] ([i915#13735] / [i915#180]) +49 other tests dmesg-warn
   [11]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/bat-apl-1/igt@kms_pm_rpm@basic-pci-d3-state.html
   [12]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/bat-apl-1/igt@kms_pm_rpm@basic-pci-d3-state.html

  
#### Possible fixes ####

  * igt@i915_selftest@live:
    - bat-mtlp-8:         [DMESG-FAIL][13] ([i915#12061]) -> [PASS][14] +1 other test pass
   [13]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/bat-mtlp-8/igt@i915_selftest@live.html
   [14]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/bat-mtlp-8/igt@i915_selftest@live.html

  * igt@kms_cursor_legacy@basic-flip-after-cursor-atomic:
    - fi-skl-6600u:       [DMESG-WARN][15] ([i915#13736]) -> [PASS][16]
   [15]: https://intel-gfx-ci.01.org/tree/drm-tip/IGT_8543/fi-skl-6600u/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html
   [16]: https://intel-gfx-ci.01.org/tree/drm-tip/IGTPW_13784/fi-skl-6600u/igt@kms_cursor_legacy@basic-flip-after-cursor-atomic.html

  
  [i915#12061]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12061
  [i915#12904]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/12904
  [i915#13735]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13735
  [i915#13736]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/13736
  [i915#14764]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14764
  [i915#14837]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/14837
  [i915#180]: https://gitlab.freedesktop.org/drm/i915/kernel/-/issues/180


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

  * CI: CI-20190529 -> None
  * IGT: IGT_8543 -> IGTPW_13784
  * Linux: CI_DRM_17227 -> CI_DRM_17238

  CI-20190529: 20190529
  CI_DRM_17227: 9fab6e66b0f21d7175cd687e62dedd1b2d5d2781 @ git://anongit.freedesktop.org/gfx-ci/linux
  CI_DRM_17238: ec61e29b21d4e69adea87f61061a9a68c2f94555 @ git://anongit.freedesktop.org/gfx-ci/linux
  IGTPW_13784: b11d44092d67ac70e4a5b441c32adb51c693e67e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  IGT_8543: 8543

== Logs ==

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

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* ✓ Xe.CI.BAT: success for tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5)
  2025-09-19  3:17 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
                   ` (2 preceding siblings ...)
  2025-09-19  5:11 ` ✗ i915.CI.BAT: failure for tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5) Patchwork
@ 2025-09-19  5:12 ` Patchwork
  3 siblings, 0 replies; 11+ messages in thread
From: Patchwork @ 2025-09-19  5:12 UTC (permalink / raw)
  To: Jeevan B; +Cc: igt-dev

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

== Series Details ==

Series: tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5)
URL   : https://patchwork.freedesktop.org/series/151429/
State : success

== Summary ==

CI Bug Log - changes from XEIGT_8543_BAT -> XEIGTPW_13784_BAT
====================================================

Summary
-------

  **SUCCESS**

  No regressions found.

  

Participating hosts (11 -> 11)
------------------------------

  No changes in participating hosts

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

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

### IGT changes ###

#### Possible fixes ####

  * igt@kms_flip@basic-plain-flip@a-edp1:
    - bat-adlp-7:         [DMESG-WARN][1] ([Intel XE#4543]) -> [PASS][2] +1 other test pass
   [1]: https://intel-gfx-ci.01.org/tree/intel-xe/IGT_8543/bat-adlp-7/igt@kms_flip@basic-plain-flip@a-edp1.html
   [2]: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_13784/bat-adlp-7/igt@kms_flip@basic-plain-flip@a-edp1.html

  
  [Intel XE#4543]: https://gitlab.freedesktop.org/drm/xe/kernel/issues/4543


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

  * IGT: IGT_8543 -> IGTPW_13784
  * Linux: xe-3784-d84b3514a845885db8cf928cc246c8efbd80a00e -> xe-3796-ec61e29b21d4e69adea87f61061a9a68c2f94555

  IGTPW_13784: b11d44092d67ac70e4a5b441c32adb51c693e67e @ https://gitlab.freedesktop.org/drm/igt-gpu-tools.git
  IGT_8543: 8543
  xe-3784-d84b3514a845885db8cf928cc246c8efbd80a00e: d84b3514a845885db8cf928cc246c8efbd80a00e
  xe-3796-ec61e29b21d4e69adea87f61061a9a68c2f94555: ec61e29b21d4e69adea87f61061a9a68c2f94555

== Logs ==

For more details see: https://intel-gfx-ci.01.org/tree/intel-xe/IGTPW_13784/index.html

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode
@ 2025-09-22  4:42 Jeevan B
  2025-09-22  4:42 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
  0 siblings, 1 reply; 11+ messages in thread
From: Jeevan B @ 2025-09-22  4:42 UTC (permalink / raw)
  To: igt-dev; +Cc: karthik.b.s, Jeevan B

If an eDP connector is present, prefer selecting its default mode for
cloning. Some eDP panels support only high refresh rate (HRR) modes,
which may not be compatible with external displays. To avoid invalid
clone configurations and test failures, we choose the default mode
from the eDP connector when available.

Signed-off-by: Jeevan B <jeevan.b@intel.com>

Jeevan B (2):
  tests/kms_setmode: simplify mode selection by removing goto
  tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode

 tests/kms_setmode.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto
  2025-09-22  4:42 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
@ 2025-09-22  4:42 ` Jeevan B
  0 siblings, 0 replies; 11+ messages in thread
From: Jeevan B @ 2025-09-22  4:42 UTC (permalink / raw)
  To: igt-dev; +Cc: karthik.b.s, Jeevan B

Replaced 'goto found' with early returns in get_mode_for_crtc() and
added braces around the fallback loop. This makes the logic cleaner
and easier to follow without changing behavior.

v2: write to *mode_ret before returning from function.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
Reviewed-by: Karthik B S <karthik.b.s@intel.com>
---
 tests/kms_setmode.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 484c3a95f..48d0c3e1e 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -238,8 +238,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->connector_count; i++) {
 		mode = &crtc->cconfs[i].default_mode;
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -248,8 +250,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->cconfs[0].connector->count_modes; i++) {
 		mode = &crtc->cconfs[0].connector->modes[i];
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -258,10 +262,11 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 * scaling etc.
 	 */
 	mode = &crtc->cconfs[0].default_mode;
-	for (i = 1; i < crtc->connector_count; i++)
+	for (i = 1; i < crtc->connector_count; i++) {
 		if (crtc->cconfs[i].default_mode.clock < mode->clock)
 			mode = &crtc->cconfs[i].default_mode;
-found:
+	}
+
 	*mode_ret = *mode;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode
@ 2025-09-22  4:01 Jeevan B
  2025-09-22  4:01 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
  0 siblings, 1 reply; 11+ messages in thread
From: Jeevan B @ 2025-09-22  4:01 UTC (permalink / raw)
  To: igt-dev; +Cc: karthik.b.s, Jeevan B

If an eDP connector is present, prefer selecting its default mode for
cloning. Some eDP panels support only high refresh rate (HRR) modes,
which may not be compatible with external displays. To avoid invalid
clone configurations and test failures, we choose the default mode
from the eDP connector when available.

Signed-off-by: Jeevan B <jeevan.b@intel.com>

Jeevan B (2):
  tests/kms_setmode: simplify mode selection by removing goto
  tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode

 tests/kms_setmode.c | 36 ++++++++++++++++++++++++++++++------
 1 file changed, 30 insertions(+), 6 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto
  2025-09-22  4:01 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
@ 2025-09-22  4:01 ` Jeevan B
  0 siblings, 0 replies; 11+ messages in thread
From: Jeevan B @ 2025-09-22  4:01 UTC (permalink / raw)
  To: igt-dev; +Cc: karthik.b.s, Jeevan B

Replaced 'goto found' with early returns in get_mode_for_crtc() and
added braces around the fallback loop. This makes the logic cleaner
and easier to follow without changing behavior.

v2: write to *mode_ret before returning from function.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
Reviewed-by: Karthik B S <karthik.b.s@intel.com>
---
 tests/kms_setmode.c | 17 +++++++++++------
 1 file changed, 11 insertions(+), 6 deletions(-)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 484c3a95f..48d0c3e1e 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -238,8 +238,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->connector_count; i++) {
 		mode = &crtc->cconfs[i].default_mode;
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -248,8 +250,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->cconfs[0].connector->count_modes; i++) {
 		mode = &crtc->cconfs[0].connector->modes[i];
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -258,10 +262,11 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 * scaling etc.
 	 */
 	mode = &crtc->cconfs[0].default_mode;
-	for (i = 1; i < crtc->connector_count; i++)
+	for (i = 1; i < crtc->connector_count; i++) {
 		if (crtc->cconfs[i].default_mode.clock < mode->clock)
 			mode = &crtc->cconfs[i].default_mode;
-found:
+	}
+
 	*mode_ret = *mode;
 }
 
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode
@ 2025-09-17 14:43 Jeevan B
  2025-09-17 14:43 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
  0 siblings, 1 reply; 11+ messages in thread
From: Jeevan B @ 2025-09-17 14:43 UTC (permalink / raw)
  To: igt-dev; +Cc: swati2.sharma, Jeevan B

If an eDP connector is present, prefer selecting its default mode for
cloning. Some eDP panels support only high refresh rate (HRR) modes,
which may not be compatible with external displays. To avoid invalid
clone configurations and test failures, we choose the default mode
from the eDP connector when available.

Signed-off-by: Jeevan B <jeevan.b@intel.com>

Jeevan B (2):
  tests/kms_setmode: simplify mode selection by removing goto
  tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode

 tests/kms_setmode.c | 36 +++++++++++++++++++++++++++++-------
 1 file changed, 29 insertions(+), 7 deletions(-)

-- 
2.43.0


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto
  2025-09-17 14:43 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
@ 2025-09-17 14:43 ` Jeevan B
  2025-09-18 15:37   ` Kamil Konieczny
  0 siblings, 1 reply; 11+ messages in thread
From: Jeevan B @ 2025-09-17 14:43 UTC (permalink / raw)
  To: igt-dev; +Cc: swati2.sharma, Jeevan B

replaced 'goto found' with early returns in get_mode_for_crtc() and
added braces around the fallback loop. This makes the logic cleaner
and easier to follow without changing behavior.

Signed-off-by: Jeevan B <jeevan.b@intel.com>
---
 tests/kms_setmode.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
index 484c3a95f..49fc45043 100644
--- a/tests/kms_setmode.c
+++ b/tests/kms_setmode.c
@@ -238,8 +238,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->connector_count; i++) {
 		mode = &crtc->cconfs[i].default_mode;
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -248,8 +250,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 */
 	for (i = 0; i < crtc->cconfs[0].connector->count_modes; i++) {
 		mode = &crtc->cconfs[0].connector->modes[i];
-		if (crtc_supports_mode(crtc, mode))
-			goto found;
+		if (crtc_supports_mode(crtc, mode)) {
+			*mode_ret = *mode;
+			return;
+		}
 	}
 
 	/*
@@ -258,11 +262,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
 	 * scaling etc.
 	 */
 	mode = &crtc->cconfs[0].default_mode;
-	for (i = 1; i < crtc->connector_count; i++)
+	for (i = 1; i < crtc->connector_count; i++) {
 		if (crtc->cconfs[i].default_mode.clock < mode->clock)
 			mode = &crtc->cconfs[i].default_mode;
-found:
-	*mode_ret = *mode;
+	}
 }
 
 static int get_encoder_idx(drmModeRes *resources, drmModeEncoder *encoder)
-- 
2.43.0


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto
  2025-09-17 14:43 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
@ 2025-09-18 15:37   ` Kamil Konieczny
  0 siblings, 0 replies; 11+ messages in thread
From: Kamil Konieczny @ 2025-09-18 15:37 UTC (permalink / raw)
  To: Jeevan B; +Cc: igt-dev, swati2.sharma

Hi Jeevan,
On 2025-09-17 at 20:13:07 +0530, Jeevan B wrote:
> replaced 'goto found' with early returns in get_mode_for_crtc() and
> added braces around the fallback loop. This makes the logic cleaner
> and easier to follow without changing behavior.
> 
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>  tests/kms_setmode.c | 17 ++++++++++-------
>  1 file changed, 10 insertions(+), 7 deletions(-)
> 
> diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c
> index 484c3a95f..49fc45043 100644
> --- a/tests/kms_setmode.c
> +++ b/tests/kms_setmode.c
> @@ -238,8 +238,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>  	 */
>  	for (i = 0; i < crtc->connector_count; i++) {
>  		mode = &crtc->cconfs[i].default_mode;
> -		if (crtc_supports_mode(crtc, mode))
> -			goto found;
> +		if (crtc_supports_mode(crtc, mode)) {
> +			*mode_ret = *mode;
> +			return;
> +		}
>  	}
>  
>  	/*
> @@ -248,8 +250,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>  	 */
>  	for (i = 0; i < crtc->cconfs[0].connector->count_modes; i++) {
>  		mode = &crtc->cconfs[0].connector->modes[i];
> -		if (crtc_supports_mode(crtc, mode))
> -			goto found;
> +		if (crtc_supports_mode(crtc, mode)) {
> +			*mode_ret = *mode;
> +			return;
> +		}
>  	}
>  
>  	/*
> @@ -258,11 +262,10 @@ static void get_mode_for_crtc(struct crtc_config *crtc,
>  	 * scaling etc.
>  	 */
>  	mode = &crtc->cconfs[0].default_mode;
> -	for (i = 1; i < crtc->connector_count; i++)
> +	for (i = 1; i < crtc->connector_count; i++) {
>  		if (crtc->cconfs[i].default_mode.clock < mode->clock)
>  			mode = &crtc->cconfs[i].default_mode;
> -found:
> -	*mode_ret = *mode;
> +	}

You need write to *mode_ret before returning from function:

	for (i = 1; i < crtc->connector_count; i++)
 		if (crtc->cconfs[i].default_mode.clock < mode->clock)
  			mode = &crtc->cconfs[i].default_mode;

	*mode_ret = *mode;

Regards,
Kamil

>  }
>  
>  static int get_encoder_idx(drmModeRes *resources, drmModeEncoder *encoder)
> -- 
> 2.43.0
> 

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2025-09-22  4:43 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-19  3:17 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
2025-09-19  3:17 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
2025-09-19  5:34   ` Karthik B S
2025-09-19  3:17 ` [PATCH i-g-t 2/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
2025-09-19  6:20   ` Karthik B S
2025-09-19  5:11 ` ✗ i915.CI.BAT: failure for tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode (rev5) Patchwork
2025-09-19  5:12 ` ✓ Xe.CI.BAT: success " Patchwork
  -- strict thread matches above, loose matches on Subject: below --
2025-09-22  4:42 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
2025-09-22  4:42 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
2025-09-22  4:01 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
2025-09-22  4:01 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
2025-09-17 14:43 [PATCH i-g-t 0/2] tests/kms_setmode: Fix clone mode mismatch by preferring eDP mode Jeevan B
2025-09-17 14:43 ` [PATCH i-g-t 1/2] tests/kms_setmode: simplify mode selection by removing goto Jeevan B
2025-09-18 15:37   ` Kamil Konieczny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox