Hi Ville,
> -----Original Message-----
> From: Intel-gfx <intel-gfx-bounces@xxxxxxxxxxxxxxxxxxxxx> On Behalf Of Ville
> Syrjala
> Sent: 04 April 2023 23:24
> To: intel-gfx@xxxxxxxxxxxxxxxxxxxxx
> Subject: [PATCH 1/3] drm/i915: Allow arbitrary refresh rates with
> VRR eDP panels
>
> From: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
>
> 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@xxxxxxxxxxxxxxx>
> ---
> 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@xxxxxxxxx>
> drm_mode_set_crtcinfo(adjusted_mode, 0);
>
> return 0;
> --
> 2.39.2
