Re: [PATCH i-g-t 3/6] lib/igt_kms: Fix max_non_joiner_mode_found

["Joshi, Kunal1" <kunal1.joshi@intel.com>]

Hello Jani,

On 01-04-2026 17:14, Jani Nikula wrote:
> On Wed, 01 Apr 2026, Kunal Joshi <kunal1.joshi@intel.com> wrote:
>> Previously the function, instead of giving the highest possible mode the
>> connector could support, started the check by looking if the mode was
>> equal to max_pipe_hdisplay. There may be corner cases where a connector never supports the highest mode our source can
>> drive (e.g. a 4K mode on a 5K-capable platform).
>>
>> Fix this by iterating all modes and tracking the best one (highest
>> hdisplay, then highest clock as a tiebreaker) that does not require a
>> big joiner, as determined by igt_bigjoiner_possible(). This correctly
>> handles connectors with any max resolution.
> I'm not requesting changes right now, but I keep wondering about having
> the joiner code in igt_kms.[ch] at all. It's an Intel implementation
> detail. Why is so much of that in the generic file instead of hidden in
> some Intel specific file or directory?
>
> BR,
> Jani.
Yeah right,
We actually have a helper for joiner related function (kms_joiner_helper),
Do you think that this approach makes sense to have feature helpers?

Thanks and Regards
Kunal Joshi
>
>
>> v2: Reword commit message (Suraj)
>>
>> Fixes: 3830ca6a5068 ("lib/igt_kms: Add support to check joiner mode limit")
>> Signed-off-by: Kunal Joshi <kunal1.joshi@intel.com>
>> Reviewed-by: Mohammed Thasleem <mohammed.thasleem@intel.com>
>> Reviewed-by: Suraj Kandpal <suraj.kandpal@intel.com>
>> ---
>>   lib/igt_kms.c | 17 +++++++++++------
>>   1 file changed, 11 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
>> index 04b1f3972..dbd419a9b 100644
>> --- a/lib/igt_kms.c
>> +++ b/lib/igt_kms.c
>> @@ -6979,21 +6979,26 @@ bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
>>    * false otherwise.
>>    */
>>   bool max_non_joiner_mode_found(int drm_fd, drmModeConnector *connector,
>> -			   int max_dotclock, drmModeModeInfo *mode)
>> +			       int max_dotclock, drmModeModeInfo *mode)
>>   {
>> -	int max_hdisplay = get_max_pipe_hdisplay(drm_fd);
>> +	bool found = false;
>>   
>>   	for (int i = 0; i < connector->count_modes; i++) {
>>   		drmModeModeInfo *current_mode = &connector->modes[i];
>>   
>> -		if (current_mode->hdisplay == max_hdisplay &&
>> -		    current_mode->clock < max_dotclock) {
>> +		if (igt_bigjoiner_possible(drm_fd, current_mode, max_dotclock))
>> +			continue;
>> +
>> +		if (!found ||
>> +		    current_mode->hdisplay > mode->hdisplay ||
>> +		    (current_mode->hdisplay == mode->hdisplay &&
>> +		     current_mode->clock > mode->clock)) {
>>   			*mode = *current_mode;
>> -			return true;
>> +			found = true;
>>   		}
>>   	}
>>   
>> -	return false;
>> +	return found;
>>   }
>>   
>>   /* TODO: Move these lib functions to the joiner-specific library file