Re: [PATCH 02/18] drm/i915: Check vblank delay validity

["Nautiyal, Ankit K" <ankit.k.nautiyal@intel.com>]

On 12/11/2024 2:39 AM, Ville Syrjala wrote:
> From: Ville Syrjälä <ville.syrjala@linux.intel.com>
>
> Make sure we have enough vblank for the computed vblank delay.
> Supposedly we'd reject things anyway later if this gets violated,
> but it seems niver to do some basic sanity checks early just
typo: nicer
> so we can't be sure the basic relationship vblank_end > vblank_start
typo: can
> always holds.
>
> Signed-off-by: Ville Syrjälä <ville.syrjala@linux.intel.com>

Patch LGTM.

Reviewed-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>

> ---
>   drivers/gpu/drm/i915/display/intel_display.c | 31 +++++++++++++++++---
>   1 file changed, 27 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
> index ff907afa6451..22b5eacda0f0 100644
> --- a/drivers/gpu/drm/i915/display/intel_display.c
> +++ b/drivers/gpu/drm/i915/display/intel_display.c
> @@ -2633,17 +2633,40 @@ static int intel_crtc_vblank_delay(const struct intel_crtc_state *crtc_state)
>   	return vblank_delay;
>   }
>   
> -static int intel_crtc_compute_config(struct intel_atomic_state *state,
> -				     struct intel_crtc *crtc)
> +static int intel_crtc_compute_vblank_delay(struct intel_atomic_state *state,
> +					   struct intel_crtc *crtc)
>   {
> +	struct intel_display *display = to_intel_display(state);
>   	struct intel_crtc_state *crtc_state =
>   		intel_atomic_get_new_crtc_state(state, crtc);
>   	struct drm_display_mode *adjusted_mode =
>   		&crtc_state->hw.adjusted_mode;
> +	int vblank_delay, max_vblank_delay;
> +
> +	vblank_delay = intel_crtc_vblank_delay(crtc_state);
> +	max_vblank_delay = adjusted_mode->crtc_vblank_end - adjusted_mode->crtc_vblank_start - 1;
> +
> +	if (vblank_delay > max_vblank_delay) {
> +		drm_dbg_kms(display->drm, "[CRTC:%d:%s] vblank delay (%d) exceeds max (%d)\n",
> +			    crtc->base.base.id, crtc->base.name, vblank_delay, max_vblank_delay);
> +		return -EINVAL;
> +	}
> +
> +	adjusted_mode->crtc_vblank_start += vblank_delay;
> +
> +	return 0;
> +}
> +
> +static int intel_crtc_compute_config(struct intel_atomic_state *state,
> +				     struct intel_crtc *crtc)
> +{
> +	struct intel_crtc_state *crtc_state =
> +		intel_atomic_get_new_crtc_state(state, crtc);
>   	int ret;
>   
> -	adjusted_mode->crtc_vblank_start +=
> -		intel_crtc_vblank_delay(crtc_state);
> +	ret = intel_crtc_compute_vblank_delay(state, crtc);
> +	if (ret)
> +		return ret;
>   
>   	ret = intel_dpll_crtc_compute_clock(state, crtc);
>   	if (ret)