Re: [PATCH i-g-t 2/3] lib/igt_kms: Add support to check joiner mode limit

[Karthik B S <karthik.b.s@intel.com>]

Hi,

On 2/4/2025 11:57 PM, Jeevan B wrote:
> Added library changes to find the non-joiner mode by selecting
> the top threshold mode for joiner mode. This helps in determining
The first sentence seems a little confusing. Could this be simplified.
> the maximum resolution or clock frequency that does not require a
> big joiner configuration.
>
> Signed-off-by: Jeevan B <jeevan.b@intel.com>
> ---
>   lib/igt_kms.c | 35 +++++++++++++++++++++++++++++++++++
>   lib/igt_kms.h |  2 ++
>   2 files changed, 37 insertions(+)
>
> diff --git a/lib/igt_kms.c b/lib/igt_kms.c
> index d4980d2ac..3c73f57fc 100644
> --- a/lib/igt_kms.c
> +++ b/lib/igt_kms.c
> @@ -6406,6 +6406,41 @@ bool bigjoiner_mode_found(int drm_fd, drmModeConnector *connector,
>   	return found;
>   }
>   
> +/**
> + * non_joiner_mode_found:
> + * @drm_fd: drm file descriptor
> + * @connector: libdrm connector
> + * @max_dot_clock: max dot clock frequency
> + * @mode: libdrm mode to be filled
> + *
> + * Non-Joiner will come in to the picture when the
> + * resolution > 5K/6K or clock < max-dot-clock.
Please fix/update this.  There is no ROI testing any random non-joiner 
mode. It should/will return the max mode that can be supported without 
joiner.
> + *
> + * Returns: True if big joiner found in connector modes
> + */
> +bool non_joiner_mode_found(int drm_fd, drmModeConnector *connector,
> +			   int max_dotclock, drmModeModeInfo *mode)
> +{
> +	bool found = false;
> +	int max_hdisplay, dev_id;
> +
> +	dev_id = intel_get_drm_devid(drm_fd);
> +	max_hdisplay = (intel_display_ver(dev_id) >= 30) ? HDISPLAY_6K_PER_PIPE :
> +			HDISPLAY_5K_PER_PIPE;
This will be used in other places as well. Can we have a helper for this?
> +
> +	for (int i = 0; i < connector->count_modes; i++) {
> +		drmModeModeInfo *current_mode = &connector->modes[i];
> +
> +		if ((current_mode->hdisplay == max_hdisplay &&
> +		    current_mode->vrefresh == 30) || current_mode->clock < max_dotclock) {

We shouldn't be having an OR for 'current_mode->clock < max_dotclock', 
as this will return any lower mode.

Also we shouldn't be hardcoding vrefresh to 30, we need to find the max 
mode which will be just less than max_dotclock.

Thanks,
Karthik.B.S
> +			*mode = connector->modes[i];
> +			found = true;
> +			break;
> +		}
> +	}
> +	return found;
> +}
> +
>   /**
>    * igt_is_joiner_enabled:
>    * @drmfd: A drm file descriptor
> diff --git a/lib/igt_kms.h b/lib/igt_kms.h
> index 7df53ec9c..069444667 100644
> --- a/lib/igt_kms.h
> +++ b/lib/igt_kms.h
> @@ -1250,6 +1250,8 @@ bool igt_ultrajoiner_possible(drmModeModeInfo *mode, int max_dotclock);
>   bool ultrajoiner_mode_found(int drm_fd, drmModeConnector *connector,
>   			  int max_dotclock, drmModeModeInfo *mode);
>   bool igt_has_force_joiner_debugfs(int drmfd, char *conn_name);
> +bool non_joiner_mode_found(int drm_fd, drmModeConnector *connector,
> +			   int max_dotclock, drmModeModeInfo *mode);
>   bool is_joiner_mode(int drm_fd, igt_output_t *output);
>   bool igt_check_force_joiner_status(int drmfd, char *connector_name);
>   bool igt_check_bigjoiner_support(igt_display_t *display);