[Intel-gfx] [PATCH 1/3] drm/i915: Allow arbitrary refresh rates with VRR eDP panels

[Golani, Mitulkumar Ajitkumar]

Hi Ville,

> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces at lists.freedesktop.org> On Behalf Of Ville
> Syrjala
> Sent: 04 April 2023 23:24
> To: intel-gfx at lists.freedesktop.org
> Subject: [Intel-gfx] [PATCH 1/3] drm/i915: Allow arbitrary refresh rates with
> VRR eDP panels
> 
> From: Ville Syrjälä <ville.syrjala at linux.intel.com>
> 
> If the panel supports VRR it must be capable of accepting timings with
> arbitrary vblank length, within the valid VRR range. Use that fact to allow the
> user to request any refresh rate they like. We simply pick the next highest
> fixed mode from our list, and adjust the vblank to get the desired refresh
> rate in the end.
> 
> Of course currently everything to do with the vrefresh is using 1Hz precision,
> so might not be exact. But we can improve that in the future by just upping
> our vrefresh precision.
> 
> Signed-off-by: Ville Syrjälä <ville.syrjala at linux.intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_panel.c | 80 ++++++++++++++++++----
>  1 file changed, 66 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_panel.c
> b/drivers/gpu/drm/i915/display/intel_panel.c
> index ce2a34a25211..9acdd68b2dbc 100644
> --- a/drivers/gpu/drm/i915/display/intel_panel.c
> +++ b/drivers/gpu/drm/i915/display/intel_panel.c
> @@ -42,6 +42,7 @@
>  #include "intel_lvds_regs.h"
>  #include "intel_panel.h"
>  #include "intel_quirks.h"
> +#include "intel_vrr.h"
> 
>  bool intel_panel_use_ssc(struct drm_i915_private *i915)  { @@ -58,6 +59,38
> @@ intel_panel_preferred_fixed_mode(struct intel_connector *connector)
>  					struct drm_display_mode, head);
>  }
> 
> +static bool is_in_vrr_range(struct intel_connector *connector, int
> +vrefresh) {
> +	const struct drm_display_info *info = &connector-
> >base.display_info;
> +
> +	return intel_vrr_is_capable(connector) &&
> +		vrefresh >= info->monitor_range.min_vfreq &&
> +		vrefresh <= info->monitor_range.max_vfreq; }
> +
> +static bool is_best_fixed_mode(struct intel_connector *connector,
> +			       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 (is_in_vrr_range(connector, vrefresh) &&
> +	    is_in_vrr_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); }
> +
>  const struct drm_display_mode *
>  intel_panel_fixed_mode(struct intel_connector *connector,
>  		       const struct drm_display_mode *mode) @@ -65,11
> +98,11 @@ intel_panel_fixed_mode(struct intel_connector *connector,
>  	const struct drm_display_mode *fixed_mode, *best_mode = NULL;
>  	int vrefresh = drm_mode_vrefresh(mode);
> 
> -	/* pick the fixed_mode that is closest in terms of vrefresh */
>  	list_for_each_entry(fixed_mode, &connector->panel.fixed_modes,
> head) {
> -		if (!best_mode ||
> -		    abs(drm_mode_vrefresh(fixed_mode) - vrefresh) <
> -		    abs(drm_mode_vrefresh(best_mode) - vrefresh))
> +		int fixed_mode_vrefresh =
> drm_mode_vrefresh(fixed_mode);
> +
> +		if (is_best_fixed_mode(connector, vrefresh,
> +				       fixed_mode_vrefresh, best_mode))
>  			best_mode = fixed_mode;
>  	}
> 
> @@ -178,27 +211,46 @@ int intel_panel_compute_config(struct
> intel_connector *connector,  {
>  	const struct drm_display_mode *fixed_mode =
>  		intel_panel_fixed_mode(connector, adjusted_mode);
> +	int vrefresh, fixed_mode_vrefresh;
> +	bool is_vrr;
> 
>  	if (!fixed_mode)
>  		return 0;
> 
> +	vrefresh = drm_mode_vrefresh(adjusted_mode);
> +	fixed_mode_vrefresh = drm_mode_vrefresh(fixed_mode);
> +
>  	/*
> -	 * We don't want to lie too much to the user about the refresh
> -	 * rate they're going to get. But we have to allow a bit of latitude
> -	 * for Xorg since it likes to automagically cook up modes with slightly
> -	 * off refresh rates.
> +	 * Assume that we shouldn't muck about with the
> +	 * timings if they don't land in the VRR range.
>  	 */
> -	if (abs(drm_mode_vrefresh(adjusted_mode) -
> drm_mode_vrefresh(fixed_mode)) > 1) {
> -		drm_dbg_kms(connector->base.dev,
> -			    "[CONNECTOR:%d:%s] Requested mode vrefresh
> (%d Hz) does not match fixed mode vrefresh (%d Hz)\n",
> -			    connector->base.base.id, connector->base.name,
> -			    drm_mode_vrefresh(adjusted_mode),
> drm_mode_vrefresh(fixed_mode));
> +	is_vrr = is_in_vrr_range(connector, vrefresh) &&
> +		is_in_vrr_range(connector, fixed_mode_vrefresh);
> 
> -		return -EINVAL;
> +	if (!is_vrr) {
> +		/*
> +		 * We don't want to lie too much to the user about the
> refresh
> +		 * rate they're going to get. But we have to allow a bit of
> latitude
> +		 * for Xorg since it likes to automagically cook up modes with
> slightly
> +		 * off refresh rates.
> +		 */
> +		if (abs(vrefresh - fixed_mode_vrefresh) > 1) {
> +			drm_dbg_kms(connector->base.dev,
> +				    "[CONNECTOR:%d:%s] Requested mode
> vrefresh (%d Hz) does not match fixed mode vrefresh (%d Hz)\n",
> +				    connector->base.base.id, connector-
> >base.name,
> +				    vrefresh, fixed_mode_vrefresh);
> +
> +			return -EINVAL;
> +		}
>  	}
> 
>  	drm_mode_copy(adjusted_mode, fixed_mode);
> 
> +	if (is_vrr && fixed_mode_vrefresh != vrefresh)
> +		adjusted_mode->vtotal =
> +			DIV_ROUND_CLOSEST(adjusted_mode->clock * 1000,
> +					  adjusted_mode->htotal * vrefresh);
> +

Changes LGTM
Thanks

Reviewed-by: Mitul Golani <mitulkumar.ajitkumar.golani at intel.com>

>  	drm_mode_set_crtcinfo(adjusted_mode, 0);
> 
>  	return 0;
> --
> 2.39.2