On Thu, Sep 11, 2025 at 08:15:53AM +0530, Ankit Nautiyal wrote:
> Refactor intel_panel_highest_mode() to return the fixed mode with the
> highest pixel clock, removing the fallback to the adjusted mode. This makes
> the function semantics clearer and better suited for future use cases where
> fallback is not desirable.
>
> Update the caller in intel_dp_mode_clock() to handle the NULL case
> explicitly by falling back to the adjusted mode's crtc_clock. This also
> addresses the existing FIXME comment about ambiguity between clock and
> crtc_clock, by using mode->clock for fixed modes and mode->crtc_clock for
> adjusted modes.
>
> v2: Avoid introducing a new function and refactor existing one instead.
> (Jani).
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nauti...@intel.com>
> Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.gol...@intel.com>
> ---
> drivers/gpu/drm/i915/display/intel_dp.c | 14 +++++++++-----
> drivers/gpu/drm/i915/display/intel_panel.c | 11 +++++------
> drivers/gpu/drm/i915/display/intel_panel.h | 3 +--
> 3 files changed, 15 insertions(+), 13 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c
> b/drivers/gpu/drm/i915/display/intel_dp.c
> index 83c46e4680b3..f74ac98062d4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1760,11 +1760,15 @@ static int intel_dp_mode_clock(const struct
> intel_crtc_state *crtc_state,
> struct intel_connector *connector =
> to_intel_connector(conn_state->connector);
> const struct drm_display_mode *adjusted_mode =
> &crtc_state->hw.adjusted_mode;
>
> - /* FIXME a bit of a mess wrt clock vs. crtc_clock */
> - if (has_seamless_m_n(connector))
> - return intel_panel_highest_mode(connector,
> adjusted_mode)->clock;
> - else
> - return adjusted_mode->crtc_clock;
> + if (has_seamless_m_n(connector)) {
> + const struct drm_display_mode *highest_mode;
> +
> + highest_mode = intel_panel_highest_mode(connector);
> + if (highest_mode)
> + return highest_mode->clock;
> + }
> +
> + return adjusted_mode->crtc_clock;
> }
>
> /* Optimize link config in order: max bpp, min clock, min lanes */
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index 2a20aaaaac39..ac0f04073ecb 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -144,18 +144,17 @@ intel_panel_downclock_mode(struct intel_connector
> *connector,
> }
>
> const struct drm_display_mode *
> -intel_panel_highest_mode(struct intel_connector *connector,
> - const struct drm_display_mode *adjusted_mode)
> +intel_panel_highest_mode(struct intel_connector *connector)
> {
> - const struct drm_display_mode *fixed_mode, *best_mode = adjusted_mode;
> + const struct drm_display_mode *fixed_mode, *highest_mode = NULL;
>
> /* pick the fixed_mode that has the highest clock */
> list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
> - if (fixed_mode->clock > best_mode->clock)
> - best_mode = fixed_mode;
> + if (!highest_mode || fixed_mode->clock > highest_mode->clock)
> + highest_mode = fixed_mode;
This looks broken now as it will always return some kind of mode
from the list, but whether or not it's better than the adjusted_mode
is no logner guaranteed.
> }
>
> - return best_mode;
> + return highest_mode;
> }
>
> int intel_panel_get_modes(struct intel_connector *connector)
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h
> b/drivers/gpu/drm/i915/display/intel_panel.h
> index 56a6412cf0fb..8a17600e46a3 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -37,8 +37,7 @@ const struct drm_display_mode *
> intel_panel_downclock_mode(struct intel_connector *connector,
> const struct drm_display_mode *adjusted_mode);
> const struct drm_display_mode *
> -intel_panel_highest_mode(struct intel_connector *connector,
> - const struct drm_display_mode *adjusted_mode);
> +intel_panel_highest_mode(struct intel_connector *connector);
> int intel_panel_get_modes(struct intel_connector *connector);
> enum drrs_type intel_panel_drrs_type(struct intel_connector *connector);
> enum drm_mode_status
> --
> 2.45.2
--
Ville Syrjälä
Intel
