[PATCH i-g-t] tests/kms_setmode: Find compatible mode for eDP before selecting default mode

[B, Jeevan]

> -----Original Message-----
> From: Grzelak, Michal <michal.grzelak at intel.com>
> Sent: Tuesday, July 15, 2025 6:01 PM
> To: Kamil Konieczny <kamil.konieczny at linux.intel.com>; B, Jeevan
> <jeevan.b at intel.com>
> Cc: B, Jeevan <jeevan.b at intel.com>; igt-dev at lists.freedesktop.org; B S, Karthik
> <karthik.b.s at intel.com>; Sharma, Swati2 <swati2.sharma at intel.com>
> Subject: RE: [PATCH i-g-t] tests/kms_setmode: Find compatible mode for eDP
> before selecting default mode
> 
> Hi Kamil,
> 
> > From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> > Sent: Monday, July 14, 2025 1:56 PM
> > To: Grzelak, Michal <michal.grzelak at intel.com>
> > Cc: B, Jeevan <jeevan.b at intel.com>; igt-dev at lists.freedesktop.org; B
> > S, Karthik <karthik.b.s at intel.com>; Sharma, Swati2
> > <swati2.sharma at intel.com>
> > Subject: Re: [PATCH i-g-t] tests/kms_setmode: Find compatible mode for
> > eDP before selecting default mode
> >
> > Hi Grzelak,,
> > On 2025-06-16 at 18:35:26 +0000, Grzelak, Michal wrote:
> > > Hi Jeevan,
> > >
> > > >From: igt-dev <igt-dev-bounces at lists.freedesktop.org> On Behalf Of
> > > >Jeevan B
> > > >Sent: Friday, June 13, 2025 7:49 AM
> > > >To: igt-dev at lists.freedesktop.org
> > > >Cc: B, Jeevan <jeevan.b at intel.com>
> > > >Subject: [PATCH i-g-t] tests/kms_setmode: Find compatible mode for
> > > >eDP before selecting default mode
> > > >
> > > >This change finds a common mode for eDP before falling back to default
> mode.
> > > >It avoids unnecessary failures by not selecting an unsupported mode for eDP.
> > > >
> > > >Signed-off-by: Jeevan B <jeevan.b at intel.com>
> > > >---
> > > > tests/kms_setmode.c | 15 +++++++++++++++
> > > > 1 file changed, 15 insertions(+)
> > > >
> > > >diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c index
> > > >484c3a95f..6e7e5942a 100644
> > > >--- a/tests/kms_setmode.c
> > > >+++ b/tests/kms_setmode.c
> > > >@@ -252,6 +252,21 @@ static void get_mode_for_crtc(struct crtc_config
> *crtc,
> > > > 			goto found;
> > > > 	}
> > > >
> > > >+	/*
> > > >+	 * If eDP is present, try its modes to find a compatible one.
> > > >+	 */
> > > >+	for (int c = 0; c < crtc->connector_count; c++) {
> > > >+		drmModeConnector *conn = crtc->cconfs[c].connector;
> > > >+		if (conn->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > > >+			for (i = 0; i < conn->count_modes; i++) {
> > > >+				mode = &conn->modes[i];
> > > >+				if (crtc_supports_mode(crtc, mode))
> > > >+					goto found;
> > > >+			}
> > > >+		break;
> >
> > Ugh, sorry for not reading your code close enough, one nit here.
> > Why 'break' ? Could it have any more eDP connectors?
> 
> In my opinion, we should assume that we are dealing with multiple eDP
> connectors and test against that. But to be sure, let's ask Jeevan for input, as he is
> the author of the patch.
> 
> Best regards,
> Michał
If we check how the modes are selected in setup_crtcs, 
In clone mode we select 2 connectors for each combination :- 

1. First try to select a default mode that is supported by all connectors. 
2. Then just fall back to find any that is supported by all connectors.
3. 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 scaling etc.

So with this logic we used to get Invalid Argument. 22 error for the HRR panel on eDP. 
This is help to sort modes for such case and avoid invalid commit. 

Thanks 
Jeevan B 
> 
> > > >+		}
> > > >+	}
> > > >+
> > > > 	/*
> > > > 	 * 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.25.1
> > >
> > > This patch looks good to me.
> > > Reviewed-by: Michał Grzelak <michal.grzelak at intel.com>
> > >
> > > Best regards,
> > > Michał