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

[B, Jeevan]

> -----Original Message-----
> From: Kamil Konieczny <kamil.konieczny at linux.intel.com>
> Sent: Monday, July 14, 2025 5:16 PM
> To: B, Jeevan <jeevan.b at intel.com>
> Cc: 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: Fix clone mode mismatch by
> preferring eDP mode
> 
> Hi Jeevan,
> On 2025-07-10 at 12:35:27 +0530, 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
> > clone configurations and test failures, we choose the default mode
> > from the eDP connector when available.
> >
> > Signed-off-by: Jeevan B <jeevan.b at intel.com>
> > ---
> >  tests/kms_setmode.c | 18 +++++++++++++++++-
> >  1 file changed, 17 insertions(+), 1 deletion(-)
> >
> > diff --git a/tests/kms_setmode.c b/tests/kms_setmode.c index
> > 484c3a95f..462313e5a 100644
> > --- a/tests/kms_setmode.c
> > +++ b/tests/kms_setmode.c
> > @@ -252,12 +252,28 @@ static void get_mode_for_crtc(struct crtc_config
> *crtc,
> >  			goto found;
> >  	}
> >
> > +	/* If an eDP connector is present, prefer selecting its default mode
> > +	 * for cloning, since internal panels typically dictate the clone mode
> > +	 * and may have HRR support, making them incompatible with modes
> > +	 * supported by external displays.
> > +	 */
> > +	mode = &crtc->cconfs[0].default_mode;
> > +	for (i = 1; i < crtc->connector_count; i++) {
> > +		drmModeConnector *conn = crtc->cconfs[i].connector;
> > +
> > +		if (conn->connector_type == DRM_MODE_CONNECTOR_eDP) {
> > +			if (crtc->cconfs[i].default_mode.clock < mode->clock)
> > +				mode = &crtc->cconfs[i].default_mode;
> > +		}
> > +		goto found;
> 
> I missed this but imho 'goto' should be done only when you actually found fitting
> mode, so here:
> 
> 		if (conn->connector_type == DRM_MODE_CONNECTOR_eDP)
> &&
> 		    crtc->cconfs[i].default_mode.clock < mode->clock) {
> 			mode = &crtc->cconfs[i].default_mode;
> 			goto found;
> 		}
Yea, so the initial 2 checks we try to find the compatible mode, If not found then we select the mode based on smallest clock logic. 
https://gitlab.freedesktop.org/drm/igt-gpu-tools/-/blob/master/tests/kms_setmode.c?ref_type=heads#L255
	/*
	 * 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 when any eDP panel is present we make sure eDP gets priority as we might pass invalid argument if eDP doesn’t support the mode that is selected by the below logic
So we call goto when we have eDP. 

Thanks 
Jeevan B 
> 
> Best regards,
> Kamil
> 
> > +	}
> > +
> >  	/*
> >  	 * 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.
> >  	 */
> > -	mode = &crtc->cconfs[0].default_mode;
> > +
> >  	for (i = 1; i < crtc->connector_count; i++)
> >  		if (crtc->cconfs[i].default_mode.clock < mode->clock)
> >  			mode = &crtc->cconfs[i].default_mode;
> > --
> > 2.43.0
> >