On Mon, Feb 24, 2025 at 11:46:59AM +0530, Ankit Nautiyal wrote:
> Make helpers to compute vmin and vmax.
>
> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@xxxxxxxxx>
> ---
> drivers/gpu/drm/i915/display/intel_vrr.c | 39 +++++++++++++++++++-----
> 1 file changed, 31 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_vrr.c b/drivers/gpu/drm/i915/display/intel_vrr.c
> index 106bfaf6649b..a435b8d5b631 100644
> --- a/drivers/gpu/drm/i915/display/intel_vrr.c
> +++ b/drivers/gpu/drm/i915/display/intel_vrr.c
> @@ -222,6 +222,35 @@ cmrr_get_vtotal(struct intel_crtc_state *crtc_state, bool video_mode_required)
> return vtotal;
> }
>
> +static
> +int intel_vrr_compute_vmin(struct intel_connector *connector,
> + struct drm_display_mode *adjusted_mode)
Make the adjusted mode const
> +{
> + int vmin;
> + const struct drm_display_info *info = &connector->base.display_info;
I generally prefer to order these approximately by the line length
in descending order. So swapping these would look better to me.
Same in the vmax counterpart.
> +
> + vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> + adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
> + vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
> +
> + return vmin;
> +}
> +
> +static
> +int intel_vrr_compute_vmax(struct intel_connector *connector,
> + struct drm_display_mode *adjusted_mode)
adjusted_mode should be const here as well
> +{
> + int vmax;
> + const struct drm_display_info *info = &connector->base.display_info;
> +
> + vmax = adjusted_mode->crtc_clock * 1000 /
> + (adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
> +
extra newline here but not in the vmin counterpart
> + vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
> +
> + return vmax;
> +}
> +
With those sorted this is
Reviewed-by: Ville Syrjälä <ville.syrjala@xxxxxxxxxxxxxxx>
> void
> intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> struct drm_connector_state *conn_state)
> @@ -232,7 +261,6 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> struct intel_dp *intel_dp = intel_attached_dp(connector);
> bool is_edp = intel_dp_is_edp(intel_dp);
> struct drm_display_mode *adjusted_mode = &crtc_state->hw.adjusted_mode;
> - const struct drm_display_info *info = &connector->base.display_info;
> int vmin, vmax;
>
> /*
> @@ -253,13 +281,8 @@ intel_vrr_compute_config(struct intel_crtc_state *crtc_state,
> if (HAS_LRR(display))
> crtc_state->update_lrr = true;
>
> - vmin = DIV_ROUND_UP(adjusted_mode->crtc_clock * 1000,
> - adjusted_mode->crtc_htotal * info->monitor_range.max_vfreq);
> - vmax = adjusted_mode->crtc_clock * 1000 /
> - (adjusted_mode->crtc_htotal * info->monitor_range.min_vfreq);
> -
> - vmin = max_t(int, vmin, adjusted_mode->crtc_vtotal);
> - vmax = max_t(int, vmax, adjusted_mode->crtc_vtotal);
> + vmin = intel_vrr_compute_vmin(connector, adjusted_mode);
> + vmax = intel_vrr_compute_vmax(connector, adjusted_mode);
>
> if (vmin >= vmax)
> return;
> --
> 2.45.2
--
Ville Syrjälä
Intel
