Re: [PATCH 1/5] drm/i915/psr: Add helper to get min psr guardband

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

On 10/17/2025 3:45 PM, Hogander, Jouni wrote:
> On Fri, 2025-10-17 at 12:58 +0300, Hogander, Jouni wrote:
>> On Fri, 2025-10-17 at 15:07 +0530, Nautiyal, Ankit K wrote:
>>> On 10/17/2025 2:37 PM, Hogander, Jouni wrote:
>>>> On Fri, 2025-10-17 at 10:31 +0530, Ankit Nautiyal wrote:
>>>>> Introduce a helper to compute the max link wake latency when
>>>>> using
>>>>> Auxless/Aux wake mechanism for PSR/Panel Replay/LOBF features.
>>>>>
>>>>> This will be used to compute the minimum guardband so that the
>>>>> link
>>>>> wake
>>>>> latencies are accounted and these features work smoothly for
>>>>> higher
>>>>> refresh rate panels.
>>>>>
>>>>> Bspec: 70151, 71477
>>>>> Signed-off-by: Ankit Nautiyal <ankit.k.nautiyal@intel.com>
>>>>> ---
>>>>>    drivers/gpu/drm/i915/display/intel_psr.c | 12 ++++++++++++
>>>>>    drivers/gpu/drm/i915/display/intel_psr.h |  1 +
>>>>>    2 files changed, 13 insertions(+)
>>>>>
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> b/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> index 703e5f6af04c..a8303b669853 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.c
>>>>> @@ -4416,3 +4416,15 @@ void
>>>>> intel_psr_compute_config_late(struct
>>>>> intel_dp *intel_dp,
>>>>>    
>>>>>    	intel_psr_set_non_psr_pipes(intel_dp, crtc_state);
>>>>>    }
>>>>> +
>>>>> +int intel_psr_min_guardband(struct intel_crtc_state
>>>>> *crtc_state)
>>>>> +{
>>>>> +	struct intel_display *display =
>>>>> to_intel_display(crtc_state);
>>>>> +	int auxless_wake_lines = crtc_state-
>>>>>> alpm_state.aux_less_wake_lines;
>>>>> +	int wake_lines = DISPLAY_VER(display) < 20 ?
>>>>> +			 psr2_block_count_lines(crtc_state-
>>>>>> alpm_state.io_wake_lines,
>>>>> +						crtc_state-
>>>>>> alpm_state.fast_wake_lines) :
>>>>> +			 crtc_state->alpm_state.io_wake_lines;
>>>>> +
>>>>> +	return max(auxless_wake_lines, wake_lines);
>>>> hmm, now if you add:
>>>>
>>>> if (crtc_state->req_psr2_sdp_prior_scanline)
>>>> 		psr_min_guardband++;
>>> I did not get this part. Do we need to account for 1 more wakelines
>>> if
>>> this flag is set?
>> If you look at how that flag affects vblank check in
>> intel_psr_compute_config_late:
>>
>> ...
>> static bool _wake_lines_fit_into_vblank(const struct intel_crtc_state
>> *crtc_state,
>> 					int vblank,
>> 					int wake_lines)
>> {
>> 	if (crtc_state->req_psr2_sdp_prior_scanline)
>> 		vblank -= 1;
>> ...
>>
>> So to take that into account when calculating minimum guardband
>> needed
>> by PSR you need to increase by one. Same goes with SCL:
>>
>> ...
>> 	int scl = _intel_psr_min_set_context_latency(crtc_state,
>> 						
>> needs_panel_replay,
>> 						
>> needs_sel_update);
>> 	vblank -= scl;
>> ...
>>
>> You are already partially taking into account PSR needs when
>> calculating optimized guardband except these two lines which are
>> needed
>> conditionally.
>>
>> Also intel_psr_compute_config is run at this point -> you know which
>> one to use: auxless wake time or aux wake time. no need to use max()
>> with them. Just choose the one which is relevant.
>>
>> With these changes you could drop intel_psr_compute_config_late as
>> vblank would be long enough for PSR mode computed by
>> intel_psr_compute_config, no?
> Ok, noticed now this in the last patch:
>
> ...
> crtc_state->vrr.guardband = min(guardband, intel_vrr_max_guardband(crtc_state));
> ...
>
> So if we need to fall back to intel_vrr_max_guardband we need to have
> that intel_psr_compute_config_late.
>
> Anyways I think you need to take into account that
> req_psr2_sdp_prior_scanline and _intel_psr_min_set_context_latency in
> intel_psr_min_guardband.


Hmm I think you are right, we need to account for 
req_psr2_sdp_prior_scanline here.

But for SCL I still think we do not need to account in wakelines as we 
are already accounting in intel_vrr_max_guardband() which calls :

intel_vrr_max_vblank_guardband(const struct intel_crtc_state 
*crtc_state) { struct intel_display *display = 
to_intel_display(crtc_state); const struct drm_display_mode 
*adjusted_mode = &crtc_state->hw.adjusted_mode; return 
crtc_state->vrr.vmin - adjusted_mode->crtc_vdisplay - 
crtc_state->set_context_latency - intel_vrr_extra_vblank_delay(display); }

Regards,

Ankit

>
> BR,
>
> Jouni Högander
>> BR,
>>
>> Jouni Högander
>>
>>
>>> What we want to do is to check for min guardband for
>>> panel_replay/sel_update based on the required wakelines.
>>>
>>> Whether we can use the auxless_wake_lines and wake_lines as
>>> computed
>>> above to estimate the max wakelines or instead we should use
>>> functions
>>> from alpm.c :
>>>
>>> io_buffer_wake_time() and _lnl_compute_aux_less_wake_time() to get
>>> the
>>> worst case wakelines.
>>>
>>>
>>>> Whatever is the PSR mode it can be enabled what comes to vblank
>>>> restrictions and you can drop psr_compute_config_late?
>>>
>>> I think we cannot drop the psr_compute_config_late as it checks
>>> whether
>>> the actual guardband is enough for PSR features.
>>>
>>> intel_psr_min_guardband() is used along with other features to have
>>> an estimate on the guardband that works for all cases, without a
>>> need
>>> to change the guardband.
>>> It is bounded by the vblank length available
>>>
>>> Regards,
>>>
>>> Ankit
>>>
>>>> BR,
>>>>
>>>> Jouni Högander
>>>>
>>>>> +}
>>>>> diff --git a/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> b/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> index b17ce312dc37..620b35928832 100644
>>>>> --- a/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> +++ b/drivers/gpu/drm/i915/display/intel_psr.h
>>>>> @@ -85,5 +85,6 @@ bool intel_psr_needs_alpm_aux_less(struct
>>>>> intel_dp
>>>>> *intel_dp,
>>>>>    				   const struct
>>>>> intel_crtc_state
>>>>> *crtc_state);
>>>>>    void intel_psr_compute_config_late(struct intel_dp *intel_dp,
>>>>>    				   struct intel_crtc_state
>>>>> *crtc_state);
>>>>> +int intel_psr_min_guardband(struct intel_crtc_state
>>>>> *crtc_state);
>>>>>    
>>>>>    #endif /* __INTEL_PSR_H__ */