[PATCH i-g-t] lib/igt_kms: Use get_max_pipe_hdisplay in joiner_possible fucntions

[B, Jeevan]

LGTM.

Reviewed-by: Jeevan B <jeevan.b at intel.com>

> -----Original Message-----
> From: B S, Karthik <karthik.b.s at intel.com>
> Sent: Thursday, March 20, 2025 11:32 AM
> To: B, Jeevan <jeevan.b at intel.com>; igt-dev at lists.freedesktop.org
> Cc: Reddy Guddati, Santhosh <santhosh.reddy.guddati at intel.com>
> Subject: Re: [PATCH i-g-t] lib/igt_kms: Use get_max_pipe_hdisplay in
> joiner_possible fucntions
> 
> Hi Jeevan,
> 
> On 3/19/2025 11:29 PM, B, Jeevan wrote:
> >> -----Original Message-----
> >> From: B S, Karthik <karthik.b.s at intel.com>
> >> Sent: Wednesday, March 19, 2025 11:43 AM
> >> To: igt-dev at lists.freedesktop.org
> >> Cc: Reddy Guddati, Santhosh <santhosh.reddy.guddati at intel.com>; B,
> >> Jeevan <jeevan.b at intel.com>; B S, Karthik <karthik.b.s at intel.com>
> >> Subject: [PATCH i-g-t] lib/igt_kms: Use get_max_pipe_hdisplay in
> >> joiner_possible fucntions
> >>
> >> Update 'joiner_possible' helper functions to use the existing
> >> get_max_pipe_hdisplay helper.
> >>
> >> Signed-off-by: Karthik B S <karthik.b.s at intel.com>
> >> ---
> >>   lib/igt_kms.c | 45 ++++++++++++++++++++-------------------------
> >>   lib/igt_kms.h |  2 +-
> >>   2 files changed, 21 insertions(+), 26 deletions(-)
> >>
> >> diff --git a/lib/igt_kms.c b/lib/igt_kms.c index cc3bb3ae7..99c8707c7
> >> 100644
> >> --- a/lib/igt_kms.c
> >> +++ b/lib/igt_kms.c
> >> @@ -6428,6 +6428,20 @@ int igt_get_current_cdclk(int fd)
> >>   	return read_and_parse_cdclk_debugfs(fd, "Current CD clock
> >> frequency:");  }
> >>
> >> +/**
> >> + * get_max_hdisplay:
> >> + * @drm_fd: drm file descriptor
> >> + *
> >> + * Returns: The maximum hdisplay supported per pipe.
> >> + */
> >> +static int get_max_pipe_hdisplay(int drm_fd) {
> >> +	int dev_id = intel_get_drm_devid(drm_fd);
> >> +
> >> +	return (intel_display_ver(dev_id) >= 30) ? HDISPLAY_6K_PER_PIPE :
> >> +						   HDISPLAY_5K_PER_PIPE;
> >> +}
> >> +
> >>   /**
> >>    * igt_bigjoiner_possible:
> >>    * @drm_fd: drm file descriptor
> >> @@ -6441,13 +6455,8 @@ int igt_get_current_cdclk(int fd)
> >>    */
> >>   bool igt_bigjoiner_possible(int drm_fd, drmModeModeInfo *mode, int
> >> max_dotclock)  {
> >> -	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;
> >> -
> >> -	return (mode->hdisplay > max_hdisplay || mode->clock >
> >> max_dotclock);
> >> +	return (mode->hdisplay > get_max_pipe_hdisplay(drm_fd) ||
> >> +		mode->clock > max_dotclock);
> >>   }
> >>
> >>   /**
> >> @@ -6469,7 +6478,7 @@ bool bigjoiner_mode_found(int drm_fd,
> >> drmModeConnector *connector,
> >>
> >>   	for (int i=0; i< connector->count_modes; i++) {
> >>   		if (igt_bigjoiner_possible(drm_fd, &connector->modes[i],
> >> max_dotclock) &&
> >> -		    !igt_ultrajoiner_possible(&connector->modes[i],
> >> max_dotclock)) {
> >> +		    !igt_ultrajoiner_possible(drm_fd, &connector->modes[i],
> >> +max_dotclock)) {
> >>   			*mode = connector->modes[i];
> >>   			found = true;
> >>   			break;
> >> @@ -6478,20 +6487,6 @@ bool bigjoiner_mode_found(int drm_fd,
> >> drmModeConnector *connector,
> >>   	return found;
> >>   }
> >>
> >> -/**
> >> - * get_max_hdisplay:
> >> - * @drm_fd: drm file descriptor
> >> - *
> >> - * Returns: The maximum hdisplay supported per pipe.
> >> - */
> >> -static int get_max_pipe_hdisplay(int drm_fd) -{
> >> -	int dev_id = intel_get_drm_devid(drm_fd);
> >> -
> >> -	return (intel_display_ver(dev_id) >= 30) ? HDISPLAY_6K_PER_PIPE :
> >> -						   HDISPLAY_5K_PER_PIPE;
> >> -}
> >> -
> >>   /**
> >>    * max_non_joiner_mode_found:
> >>    * @drm_fd: drm file descriptor
> >> @@ -6567,9 +6562,9 @@ bool igt_is_joiner_enabled_for_pipe(int drmfd,
> >> enum pipe pipe)
> >>    *
> >>    * Returns: True if mode requires Ultrajoiner, else False.
> >>    */
> >> -bool igt_ultrajoiner_possible(drmModeModeInfo *mode, int
> >> max_dotclock)
> >> +bool igt_ultrajoiner_possible(int drm_fd, drmModeModeInfo *mode, int
> >> +max_dotclock)
> >>   {
> >> -	return (mode->hdisplay > 2 * HDISPLAY_5K_PER_PIPE ||
> >> +	return (mode->hdisplay > 2 * get_max_pipe_hdisplay(drm_fd) ||
> >>   		mode->clock > 2 * max_dotclock);
> >>   }
> >>
> > For ultrajoiner hdisplay logic feels off, we need to correct this.
> > Need to check spec and update. Correct me if I am wrong
> >
> > Apart from these code LGTM.
> 
> Thank you for the review.
> 
> The logic for ultrajoiner is similar to what we've in bigjoiner and also we've the
> same logic in kernel as well.
> 
> https://gitlab.freedesktop.org/drm/tip/-/blob/drm-
> tip/drivers/gpu/drm/i915/display/intel_dp.c?ref_type=heads#L1333
> 
> Thanks,
> Karthik.B.S
> >> @@ -6591,7 +6586,7 @@ bool ultrajoiner_mode_found(int drm_fd,
> >> drmModeConnector *connector,
> >>   	bool found = false;
> >>
> >>   	for (int i = 0; i < connector->count_modes; i++) {
> >> -		if (igt_ultrajoiner_possible(&connector->modes[i],
> >> max_dotclock)) {
> >> +		if (igt_ultrajoiner_possible(drm_fd, &connector->modes[i],
> >> +max_dotclock)) {
> >>   			*mode = connector->modes[i];
> >>   			found = true;
> >>   			break;
> >> diff --git a/lib/igt_kms.h b/lib/igt_kms.h index 27b545f52..0381c82ad
> >> 100644
> >> --- a/lib/igt_kms.h
> >> +++ b/lib/igt_kms.h
> >> @@ -1252,7 +1252,7 @@ bool bigjoiner_mode_found(int drm_fd,
> >> drmModeConnector *connector,  bool max_non_joiner_mode_found(int
> >> drm_fd, drmModeConnector *connector,
> >>   			       int max_dotclock, drmModeModeInfo *mode);
> bool
> >> igt_is_joiner_enabled_for_pipe(int drmfd, enum pipe pipe); -bool
> >> igt_ultrajoiner_possible(drmModeModeInfo *mode, int max_dotclock);
> >> +bool igt_ultrajoiner_possible(int drmfd, 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);
> >> --
> >> 2.43.0