Re: [PATCH v4 11/13] drm/i915/display: Evade scanline 0 as well if PSR1 or PSR2 is enabled

["Ville Syrjälä" <ville.syrjala@linux.intel.com>]

On Fri, Jan 24, 2025 at 12:56:22PM +0200, Jouni Högander wrote:
> PIPEDSL is reading as 0 when in SRDENT(PSR1) or DEEP_SLEEP(PSR2). On
> wake-up scanline counting starts from vblank_start - 1. We don't know if
> wake-up is already ongoing when evasion starts. In worst case PIPEDSL could
> start reading valid value right after checking the scanline. In this
> scenario we wouldn't have enough time to write all registers. To tackle
> this evade scanline 0 as well. As a drawback we have 1 frame delay in flip
> when waking up.
> 
> Signed-off-by: Jouni Högander <jouni.hogander@intel.com>
> ---
>  drivers/gpu/drm/i915/display/intel_dsb.c | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
> 
> diff --git a/drivers/gpu/drm/i915/display/intel_dsb.c b/drivers/gpu/drm/i915/display/intel_dsb.c
> index bb77ded8bf726..914f0be4491c4 100644
> --- a/drivers/gpu/drm/i915/display/intel_dsb.c
> +++ b/drivers/gpu/drm/i915/display/intel_dsb.c
> @@ -528,6 +528,18 @@ void intel_dsb_vblank_evade(struct intel_atomic_state *state,
>  	int latency = intel_usecs_to_scanlines(&crtc_state->hw.adjusted_mode, 20);
>  	int start, end;
>  
> +	/*
> +	 * PIPEDSL is reading as 0 when in SRDENT(PSR1) or DEEP_SLEEP(PSR2). On
> +	 * wake-up scanline counting starts from vblank_start - 1. We don't know
> +	 * if wake-up is already ongoing when evasion starts. In worst case
> +	 * PIPEDSL could start reading valid value right after checking the
> +	 * scanline. In this scenario we wouldn't have enough time to write all
> +	 * registers. To tackle this evade scanline 0 as well. As a drawback we
> +	 * have 1 frame delay in flip when waking up.
> +	 */
> +	if (crtc_state->has_psr && !crtc_state->has_panel_replay)

What's the deal with panel replay here?

> +		intel_dsb_wait_scanline_out(state, dsb, 0, 0);

This needs to be a raw
intel_dsb_emit_wait_dsl(dsb, DSB_OPCODE_WAIT_DSL_OUT, 0, 0)
because we need to evade the hw scanline 0. What the software
thinks is scanline 0 is a bit different (see scanline_offset).

> +
>  	if (pre_commit_is_vrr_active(state, crtc)) {
>  		int vblank_delay = intel_vrr_vblank_delay(crtc_state);
>  
> -- 
> 2.43.0

-- 
Ville Syrjälä
Intel