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

[Grzelak, Michal]

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 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ł