Re: [PATCH 15/15] drm/i915/display: Use optimized guardband to set vblank start

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

On 10/1/2025 5:42 PM, Ville Syrjälä wrote:
> On Wed, Oct 01, 2025 at 04:11:13PM +0530, Nautiyal, Ankit K wrote:
>> On 9/29/2025 2:30 PM, Ville Syrjälä wrote:
>>> On Sun, Sep 28, 2025 at 12:35:40PM +0530, Ankit Nautiyal wrote:
>>>> As we move towards using a shorter, optimized guardband, we need to adjust
>>>> how the delayed vblank start is computed.
>>>>
>>>> Use the helper intel_vrr_compute_guardband() to calculate the optimized
>>>> guardband. Since this is measured from the vblank end, we shift the
>>>> vblank-start accordingly.
>>>>
>>>> Calculate the minimum delay required based on the guardband and apply it in
>>>> intel_crtc_vblank_delay() to update crtc_vblank_start.
>>>>
>>>> Additionally, introduce a new allow_vblank_delay_fastset() helper that
>>>> combines the existing LRR-based logic with an additional check for the
>>>> optimized guardband usage.
>>>>
>>>> v2:
>>>> - Check if optimized guardband is more than vblank length and add debug
>>>>     print.
>>>> - Extend vblank delay fastset logic to cover optimized guardband.
>>>>
>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>> ---
>>>>    drivers/gpu/drm/i915/display/intel_display.c | 79 +++++++++++++++++++-
>>>>    1 file changed, 76 insertions(+), 3 deletions(-)
>>>>
>>>> diff --git a/drivers/gpu/drm/i915/display/intel_display.c b/drivers/gpu/drm/i915/display/intel_display.c
>>>> index 4135f9be53fd..97a3121a204f 100644
>>>> --- a/drivers/gpu/drm/i915/display/intel_display.c
>>>> +++ b/drivers/gpu/drm/i915/display/intel_display.c
>>>> @@ -2361,6 +2361,67 @@ static int intel_crtc_compute_pipe_mode(struct intel_crtc_state *crtc_state)
>>>>    	return 0;
>>>>    }
>>>>    
>>>> +static
>>>> +int intel_crtc_min_guardband_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);
>>>> +	const struct drm_display_mode *adjusted_mode =
>>>> +		&crtc_state->hw.adjusted_mode;
>>>> +	struct drm_connector_state *conn_state;
>>>> +	struct drm_connector *drm_connector;
>>>> +	int vblank_length;
>>>> +	int i;
>>>> +
>>>> +	if (!intel_vrr_use_optimized_guardband(crtc_state))
>>>> +		return 0;
>>>> +
>>>> +	vblank_length = crtc_state->vrr.vmin -
>>>> +			adjusted_mode->crtc_vdisplay;
>>>> +
>>>> +	for_each_new_connector_in_state(&state->base,
>>>> +					drm_connector,
>>>> +					conn_state, i) {
>>>> +		int guardband;
>>>> +		struct intel_connector *connector;
>>>> +
>>>> +		if (conn_state->crtc != &crtc->base)
>>>> +			continue;
>>>> +
>>>> +		connector = to_intel_connector(drm_connector);
>>>> +		guardband = intel_vrr_compute_guardband(crtc_state,
>>>> +							connector);
>>>> +		if (guardband > vblank_length) {
>>>> +			drm_dbg_kms(display->drm,
>>>> +				    "[CRTC:%d:%s] Cannot optimize guardband (%d) exceeds max (%d)\n",
>>>> +				    crtc->base.base.id, crtc->base.name,
>>>> +				    guardband,
>>>> +				    vblank_length);
>>>> +			return 0;
>>>> +		}
>>>> +
>>>> +		return vblank_length - guardband;
>>>> +	}
>>>> +
>>>> +	return 0;
>>>> +}
>>>> +
>>>> +static void intel_crtc_vblank_delay(struct intel_atomic_state *state,
>>>> +				    struct intel_crtc *crtc)
>>>> +{
>>>> +	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 = 0;
>>>> +
>>>> +	vblank_delay = intel_crtc_min_guardband_delay(state, crtc);
>>>> +
>>>> +	adjusted_mode->crtc_vblank_start += vblank_delay;
>>>> +}
>>>> +
>>>>    static int intel_crtc_set_context_latency(struct intel_crtc_state *crtc_state)
>>>>    {
>>>>    	struct intel_display *display = to_intel_display(crtc_state);
>>>> @@ -2413,6 +2474,7 @@ static int intel_crtc_compute_config(struct intel_atomic_state *state,
>>>>    	ret = intel_crtc_compute_set_context_latency(state, crtc);
>>>>    	if (ret)
>>>>    		return ret;
>>>> +	intel_crtc_vblank_delay(state, crtc);
>>> IMO we should get rid of all this vblank_delay terminology here.
>>> This one I think should just be our current
>>> intel_vrr_compute_config_late() (renamed to eg.
>>> intel_vrr_compute_guardband()).
>> Hmm ok so will rename this and call from intel_modeset_pipe_config_late().
> I meant you should move the intel_vrr_compute_config_late() call
> from intel_modeset_pipe_config_late() to here, and rename it to
> eg. intel_vrr_compute_guardband().

Oh ok sorry I got it other way round. Will do as suggested.

Regards,

Ankit

>