Re: [PATCH 2/2] drm/i915/intel_panel: Use highest refresh rate mode for seamless VRR changes

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Tue, Apr 28, 2026 at 02:03:23PM +0530, Ankit Nautiyal wrote:
> intel_panel_fixed_mode() intends to choose a fixed mode at or above the
> requested refresh rate for VRR panels, so the requested refresh can be
> reached by extending vblank.
> 
> This is called in compute config to find the 'best' fixed mode for a
> requested mode and also called during mode valid phase to prune the
> unsupported fixed modes.
> 
> For Non-VRR panels there is no issue:
> - When a user wants to do a full modeset (sets DRM_MODE_ALLOW_MODESET
>   flag) with a custom mode, the intel_panel_fixed_mode() gives the mode
>   with refresh rate nearest to the requested mode.
> - Seamless switch to a lower mode is only possible if the platform has
>   support for Double buffered MN and the sink is Seamless-DRRS capable.
>   In this case the user sets a lower refresh rate mode and doesn't set
>   the DRM_MODE_ALLOW_MODESET flag the intel_panel_fixed_mode() returns
>   again the fixed mode whose refresh rate is nearest to the requested
>   mode.
> - Since Duble buffered MN is not supported on LNL+, the seamless switch
>   is not possible for such panels from LNL+.
> 
> For VRR panels the current logic has some issues:
> - For VRR panels intel_panel_fixed_mode() intends to choose a fixed mode
>   at or above the requested refresh rate so the requested refresh can be
>   reached by extending vblank.
> - However, as per the current logic the helper can return a lower refresh
>   rate mode, if the lower refresh rate mode is first in the list of fixed
>   mode. Later, if the selected fixed mode's refresh rate < the requested
>   mode's refresh rate, then the requested rate is matched by extending
>   the vblank. 
> - In case of a full modeset request with a custom mode this is not a
>   problem. But for the seamless switch features like LRR (Lower Refresh
>   Rate) and the Seamless-DRRS this creates a problem as this results in
>   change in vsync_start/end and resulting in a full modeset.
> - Furthermore, as with the Non-VRR panel case, the Seamless-DRRS is not
>   supported on LNL+, but for VRR panels, the vblank can be extended
>   similar to LRR case. But due to the above mentioned problem in the
>   intel_panel_fixed_mode() this also results in full modeset.
> 
> To solve these problems for the VRR panels, identify if the user wants a
> full modeset or expects seamless switch. If seamless switch to a lower
> mode is desired, make intel_panel_fixed_mode() return the highest
> refresh rate mode, provided the requested rate is in vrr range. This
> will then be modified to extend the vblank to provide the desired
> refresh rate.
> 
> This is particularly needed for DRRS panels on platforms without the
> double buffered M/N support (display version 20+), where seamless clock
> changes are not possible.
> 
> To understand the user requirement for full modeset/seamless switch, the
> intel_panel_fixed_mode() and intel_panel_compute_config() need access to
> the connector state to check the allow_modeset flag.
> 
> Add a nullable conn_state parameter to both. The mode_valid callers pass
> NULL since they have no atomic state and the compute_config callers pass
> their conn_state.
> 
> Also remove the VRR check from is_best_fixed_mode() since the selection is
> handled upfront in intel_panel_fixed_mode().
> 
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
> ---
>  drivers/gpu/drm/i915/display/icl_dsi.c     |  2 +-
>  drivers/gpu/drm/i915/display/intel_dp.c    |  6 ++--
>  drivers/gpu/drm/i915/display/intel_dsi.c   |  2 +-
>  drivers/gpu/drm/i915/display/intel_dvo.c   |  6 ++--
>  drivers/gpu/drm/i915/display/intel_lvds.c  |  4 +--
>  drivers/gpu/drm/i915/display/intel_panel.c | 35 +++++++++++-----------
>  drivers/gpu/drm/i915/display/intel_panel.h |  6 ++--
>  drivers/gpu/drm/i915/display/intel_sdvo.c  |  8 ++---
>  drivers/gpu/drm/i915/display/vlv_dsi.c     |  2 +-
>  9 files changed, 37 insertions(+), 34 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/icl_dsi.c b/drivers/gpu/drm/i915/display/icl_dsi.c
> index afbaa0465842..1efe81404d01 100644
> --- a/drivers/gpu/drm/i915/display/icl_dsi.c
> +++ b/drivers/gpu/drm/i915/display/icl_dsi.c
> @@ -1671,7 +1671,7 @@ static int gen11_dsi_compute_config(struct intel_encoder *encoder,
>  	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
> -	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
> +	ret = intel_panel_compute_config(intel_connector, adjusted_mode, conn_state);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dp.c b/drivers/gpu/drm/i915/display/intel_dp.c
> index 35b8fb5740aa..f014ce28e69f 100644
> --- a/drivers/gpu/drm/i915/display/intel_dp.c
> +++ b/drivers/gpu/drm/i915/display/intel_dp.c
> @@ -1588,7 +1588,7 @@ intel_dp_mode_valid(struct drm_connector *_connector,
>  	if (intel_dp_hdisplay_bad(display, mode->hdisplay))
>  		return MODE_H_ILLEGAL;
>  
> -	fixed_mode = intel_panel_fixed_mode(connector, mode);
> +	fixed_mode = intel_panel_fixed_mode(connector, mode, NULL);
>  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
>  		status = intel_panel_mode_valid(connector, mode);
>  		if (status != MODE_OK)
> @@ -3577,9 +3577,9 @@ intel_dp_compute_config(struct intel_encoder *encoder,
>  	struct intel_connector *connector = intel_dp->attached_connector;
>  	int ret = 0, link_bpp_x16;
>  
> -	fixed_mode = intel_panel_fixed_mode(connector, adjusted_mode);
> +	fixed_mode = intel_panel_fixed_mode(connector, adjusted_mode, conn_state);
>  	if (intel_dp_is_edp(intel_dp) && fixed_mode) {
> -		ret = intel_panel_compute_config(connector, adjusted_mode);
> +		ret = intel_panel_compute_config(connector, adjusted_mode, conn_state);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_dsi.c b/drivers/gpu/drm/i915/display/intel_dsi.c
> index 9005c1f5d857..a480bb79dca7 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsi.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsi.c
> @@ -64,7 +64,7 @@ enum drm_mode_status intel_dsi_mode_valid(struct drm_connector *connector,
>  	struct intel_display *display = to_intel_display(connector->dev);
>  	struct intel_connector *intel_connector = to_intel_connector(connector);
>  	const struct drm_display_mode *fixed_mode =
> -		intel_panel_fixed_mode(intel_connector, mode);
> +		intel_panel_fixed_mode(intel_connector, mode, NULL);
>  	int max_dotclk = display->cdclk.max_dotclk_freq;
>  	enum drm_mode_status status;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_dvo.c b/drivers/gpu/drm/i915/display/intel_dvo.c
> index 405b33aca9dd..0ec25d895777 100644
> --- a/drivers/gpu/drm/i915/display/intel_dvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_dvo.c
> @@ -224,7 +224,7 @@ intel_dvo_mode_valid(struct drm_connector *_connector,
>  	struct intel_connector *connector = to_intel_connector(_connector);
>  	struct intel_dvo *intel_dvo = intel_attached_dvo(connector);
>  	const struct drm_display_mode *fixed_mode =
> -		intel_panel_fixed_mode(connector, mode);
> +		intel_panel_fixed_mode(connector, mode, NULL);
>  	int max_dotclk = display->cdclk.max_dotclk_freq;
>  	int target_clock = mode->clock;
>  	enum drm_mode_status status;
> @@ -259,7 +259,7 @@ static int intel_dvo_compute_config(struct intel_encoder *encoder,
>  	struct intel_connector *connector = to_intel_connector(conn_state->connector);
>  	struct drm_display_mode *adjusted_mode = &pipe_config->hw.adjusted_mode;
>  	const struct drm_display_mode *fixed_mode =
> -		intel_panel_fixed_mode(intel_dvo->attached_connector, adjusted_mode);
> +		intel_panel_fixed_mode(intel_dvo->attached_connector, adjusted_mode, conn_state);
>  
>  	/*
>  	 * If we have timings from the BIOS for the panel, put them in
> @@ -270,7 +270,7 @@ static int intel_dvo_compute_config(struct intel_encoder *encoder,
>  	if (fixed_mode) {
>  		int ret;
>  
> -		ret = intel_panel_compute_config(connector, adjusted_mode);
> +		ret = intel_panel_compute_config(connector, adjusted_mode, conn_state);
>  		if (ret)
>  			return ret;
>  	}
> diff --git a/drivers/gpu/drm/i915/display/intel_lvds.c b/drivers/gpu/drm/i915/display/intel_lvds.c
> index cc6d4bfcff10..2e30bc3f1e62 100644
> --- a/drivers/gpu/drm/i915/display/intel_lvds.c
> +++ b/drivers/gpu/drm/i915/display/intel_lvds.c
> @@ -396,7 +396,7 @@ intel_lvds_mode_valid(struct drm_connector *_connector,
>  	struct intel_display *display = to_intel_display(_connector->dev);
>  	struct intel_connector *connector = to_intel_connector(_connector);
>  	const struct drm_display_mode *fixed_mode =
> -		intel_panel_fixed_mode(connector, mode);
> +		intel_panel_fixed_mode(connector, mode, NULL);
>  	int max_pixclk = display->cdclk.max_dotclk_freq;
>  	enum drm_mode_status status;
>  
> @@ -460,7 +460,7 @@ static int intel_lvds_compute_config(struct intel_encoder *encoder,
>  	 * with the panel scaling set up to source from the H/VDisplay
>  	 * of the original mode.
>  	 */
> -	ret = intel_panel_compute_config(connector, adjusted_mode);
> +	ret = intel_panel_compute_config(connector, adjusted_mode, conn_state);
>  	if (ret)
>  		return ret;
>  
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c b/drivers/gpu/drm/i915/display/intel_panel.c
> index 5e918ee0c8ea..65d017371d78 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -59,24 +59,13 @@ intel_panel_preferred_fixed_mode(struct intel_connector *connector)
>  					struct drm_display_mode, head);
>  }
>  
> -static bool is_best_fixed_mode(struct intel_connector *connector,
> -			       int vrefresh, int fixed_mode_vrefresh,
> +static bool is_best_fixed_mode(int vrefresh, int fixed_mode_vrefresh,
>  			       const struct drm_display_mode *best_mode)
>  {
>  	/* we want to always return something */
>  	if (!best_mode)
>  		return true;
>  
> -	/*
> -	 * With VRR always pick a mode with equal/higher than requested
> -	 * vrefresh, which we can then reduce to match the requested
> -	 * vrefresh by extending the vblank length.
> -	 */
> -	if (intel_vrr_is_in_range(connector, vrefresh) &&
> -	    intel_vrr_is_in_range(connector, fixed_mode_vrefresh) &&
> -	    fixed_mode_vrefresh < vrefresh)
> -		return false;
> -
>  	/* pick the fixed_mode that is closest in terms of vrefresh */
>  	return abs(fixed_mode_vrefresh - vrefresh) <
>  		abs(drm_mode_vrefresh(best_mode) - vrefresh);
> @@ -84,15 +73,26 @@ static bool is_best_fixed_mode(struct intel_connector *connector,
>  
>  const struct drm_display_mode *
>  intel_panel_fixed_mode(struct intel_connector *connector,
> -		       const struct drm_display_mode *mode)
> +		       const struct drm_display_mode *mode,
> +		       const struct drm_connector_state *conn_state)
>  {
>  	const struct drm_display_mode *fixed_mode, *best_mode = NULL;
>  	int vrefresh = drm_mode_vrefresh(mode);
>  
> +	/*
> +	 * With VRR always pick the highest refresh rate mode,
> +	 * which we can then reduce to match the requested
> +	 * vrefresh by extending the vblank length.
> +	 */
> +	if (conn_state && !conn_state->state->allow_modeset &&

The foo_state->state pointer should never be used. If you need the full
atomic state then plumb it through from the top.

> +	    intel_vrr_is_capable(connector) &&
> +	    intel_vrr_is_in_range(connector, vrefresh))
> +		return intel_panel_highest_vrefresh_mode(connector);

What we want is the fixed mode that matches the current adjusted mode
exactly except for vtotal, and I think we also want to maintain the
vsync pulse location relative to vtotal.

We should in fact fix the vsync_start/end mess first. We need to add
TRANS_VSYNC handling to the LRR codepaths and allow that to change
during fastsets. And intel_panel_compute_config() needs to preserve
the vtotal-vsync distance when adjusting vtotal. I think that should
all be fine for DP since it doesn't really use TRANS_VSYNC for
anything.

>
+
>  	list_for_each_entry(fixed_mode, &connector->panel.fixed_modes, head) {
>  		int fixed_mode_vrefresh = drm_mode_vrefresh(fixed_mode);
>  
> -		if (is_best_fixed_mode(connector, vrefresh,
> +		if (is_best_fixed_mode(vrefresh,
>  				       fixed_mode_vrefresh, best_mode))
>  			best_mode = fixed_mode;
>  	}
> @@ -213,10 +213,11 @@ enum drrs_type intel_panel_drrs_type(struct intel_connector *connector)
>  }
>  
>  int intel_panel_compute_config(struct intel_connector *connector,
> -			       struct drm_display_mode *adjusted_mode)
> +			       struct drm_display_mode *adjusted_mode,
> +			       const struct drm_connector_state *conn_state)
>  {
>  	const struct drm_display_mode *fixed_mode =
> -		intel_panel_fixed_mode(connector, adjusted_mode);
> +		intel_panel_fixed_mode(connector, adjusted_mode, conn_state);
>  	int vrefresh, fixed_mode_vrefresh;
>  	bool is_vrr;
>  
> @@ -414,7 +415,7 @@ intel_panel_mode_valid(struct intel_connector *connector,
>  		       const struct drm_display_mode *mode)
>  {
>  	const struct drm_display_mode *fixed_mode =
> -		intel_panel_fixed_mode(connector, mode);
> +		intel_panel_fixed_mode(connector, mode, NULL);
>  
>  	if (!fixed_mode)
>  		return MODE_OK;
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.h b/drivers/gpu/drm/i915/display/intel_panel.h
> index 2f7a317995ea..c1189a20c8b2 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.h
> +++ b/drivers/gpu/drm/i915/display/intel_panel.h
> @@ -32,7 +32,8 @@ const struct drm_display_mode *
>  intel_panel_preferred_fixed_mode(struct intel_connector *connector);
>  const struct drm_display_mode *
>  intel_panel_fixed_mode(struct intel_connector *connector,
> -		       const struct drm_display_mode *mode);
> +		       const struct drm_display_mode *mode,
> +		       const struct drm_connector_state *conn_state);
>  const struct drm_display_mode *
>  intel_panel_downclock_mode(struct intel_connector *connector,
>  			   const struct drm_display_mode *adjusted_mode);
> @@ -47,7 +48,8 @@ enum drm_mode_status
>  intel_panel_mode_valid(struct intel_connector *connector,
>  		       const struct drm_display_mode *mode);
>  int intel_panel_compute_config(struct intel_connector *connector,
> -			       struct drm_display_mode *adjusted_mode);
> +			       struct drm_display_mode *adjusted_mode,
> +			       const struct drm_connector_state *conn_state);
>  void intel_panel_add_edid_fixed_modes(struct intel_connector *connector,
>  				      bool use_alt_fixed_modes);
>  void intel_panel_add_vbt_lfp_fixed_mode(struct intel_connector *connector);
> diff --git a/drivers/gpu/drm/i915/display/intel_sdvo.c b/drivers/gpu/drm/i915/display/intel_sdvo.c
> index 2e1af9e869de..e07c1070a3ec 100644
> --- a/drivers/gpu/drm/i915/display/intel_sdvo.c
> +++ b/drivers/gpu/drm/i915/display/intel_sdvo.c
> @@ -797,7 +797,7 @@ intel_sdvo_create_preferred_input_timing(struct intel_sdvo *intel_sdvo,
>  
>  	if (IS_LVDS(intel_sdvo_connector)) {
>  		const struct drm_display_mode *fixed_mode =
> -			intel_panel_fixed_mode(&intel_sdvo_connector->base, mode);
> +			intel_panel_fixed_mode(&intel_sdvo_connector->base, mode, NULL);
>  
>  		if (fixed_mode->hdisplay != args.width ||
>  		    fixed_mode->vdisplay != args.height)
> @@ -1396,11 +1396,11 @@ static int intel_sdvo_compute_config(struct intel_encoder *encoder,
>  		pipe_config->sdvo_tv_clock = true;
>  	} else if (IS_LVDS(intel_sdvo_connector)) {
>  		const struct drm_display_mode *fixed_mode =
> -			intel_panel_fixed_mode(&intel_sdvo_connector->base, mode);
> +			intel_panel_fixed_mode(&intel_sdvo_connector->base, mode, conn_state);
>  		int ret;
>  
>  		ret = intel_panel_compute_config(&intel_sdvo_connector->base,
> -						 adjusted_mode);
> +						 adjusted_mode, conn_state);
>  		if (ret)
>  			return ret;
>  
> @@ -1562,7 +1562,7 @@ static void intel_sdvo_pre_enable(struct intel_atomic_state *state,
>  	/* lvds has a special fixed output timing. */
>  	if (IS_LVDS(intel_sdvo_connector)) {
>  		const struct drm_display_mode *fixed_mode =
> -			intel_panel_fixed_mode(&intel_sdvo_connector->base, mode);
> +			intel_panel_fixed_mode(&intel_sdvo_connector->base, mode, conn_state);
>  
>  		intel_sdvo_get_dtd_from_mode(&output_dtd, fixed_mode);
>  	} else {
> diff --git a/drivers/gpu/drm/i915/display/vlv_dsi.c b/drivers/gpu/drm/i915/display/vlv_dsi.c
> index 76e8cd0f65a4..bfe465443d20 100644
> --- a/drivers/gpu/drm/i915/display/vlv_dsi.c
> +++ b/drivers/gpu/drm/i915/display/vlv_dsi.c
> @@ -280,7 +280,7 @@ static int intel_dsi_compute_config(struct intel_encoder *encoder,
>  	pipe_config->sink_format = INTEL_OUTPUT_FORMAT_RGB;
>  	pipe_config->output_format = INTEL_OUTPUT_FORMAT_RGB;
>  
> -	ret = intel_panel_compute_config(intel_connector, adjusted_mode);
> +	ret = intel_panel_compute_config(intel_connector, adjusted_mode, conn_state);
>  	if (ret)
>  		return ret;
>  
> -- 
> 2.45.2

-- 
Ville Syrjälä
Intel