[Intel-gfx] [PATCH] drm/i915: Wait for PSR exit before checking for vblank evasion for an atomic update

[Rodrigo Vivi]

On Sun, Apr 29, 2018 at 09:00:18PM -0700, Tarun Vyas wrote:
> From: Tarun <tarun.vyas at intel.com>
> 
> The PIPEDSL freezes on PSR entry and if PSR hasn't fully exited, then
> the pipe_update_start call schedules itself out to check back later.
> 
> On ChromeOS-4.4 kernel, which is fairly up-to-date w.r.t drm/i915 but
> lags w.r.t core kernel code, hot plugging an external display triggers
> tons of "potential atomic update errors" in the dmesg, on *pipe A*. A
> closer analysis reveals that we try to read the scanline 3 times and
> eventually timeout, b/c PSR hasn't exited fully leading to a PIPEDSL
> stuck @ 1599. This issue is not seen on upstream kernels, b/c for *some*
> reason we loop inside intel_pipe_update start for ~2+ msec which in this
> case is more than enough to exit PSR fully, hence an *unstuck* PIPEDSL
> counter, hence no error. On the other hand, the ChromeOS kernel spends
> ~1.1 msec looping inside intel_pipe_update_start and hence errors out
> b/c the source is still in PSR.
> 
> Regardless, we should wait for PSR exit (if PSR is supported and active
> on the current pipe) before reading the PIPEDSL, b/c if we haven't
> fully exited PSR, then checking for vblank evasion isn't actually
> applicable.
> 
> This scenario applies to a configuration with an additional pipe,
> as of now.

I honestly believe you picking the wrong culprit here. By "coincidence".
PSR will allow DC state with screen on and DC state will mess up with all
registers reads....

probably what you are missing you your kernel is some power domain
grab that would keep DC_OFF and consequently a sane read of these
registers.

Maybe Imre has a quick idea of what you could be missing on your kernel
that we already have on upstream one.

Thanks,
Rodrigo.

> 
> ---
>  drivers/gpu/drm/i915/intel_sprite.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/i915/intel_sprite.c b/drivers/gpu/drm/i915/intel_sprite.c
> index aa1dfaa692b9..135b41568503 100644
> --- a/drivers/gpu/drm/i915/intel_sprite.c
> +++ b/drivers/gpu/drm/i915/intel_sprite.c
> @@ -107,14 +107,17 @@ void intel_pipe_update_start(const struct intel_crtc_state *new_crtc_state)
>  						      VBLANK_EVASION_TIME_US);
>  	max = vblank_start - 1;
>  
> -	local_irq_disable();
> -
>  	if (min <= 0 || max <= 0)
>  		return;
>  
>  	if (WARN_ON(drm_crtc_vblank_get(&crtc->base)))
>  		return;
>  
> +	if(new_crtc_state->has_psr && dev_priv->psr.active)
> +		intel_wait_for_register(dev_priv, EDP_PSR_STATUS, EDP_PSR_STATUS_STATE_MASK, EDP_PSR_STATUS_STATE_IDLE, 5);
> +
> +	local_irq_disable();
> +
>  	crtc->debug.min_vbl = min;
>  	crtc->debug.max_vbl = max;
>  	trace_i915_pipe_update_start(crtc);
> -- 
> 2.13.5
> 
> _______________________________________________
> Intel-gfx mailing list
> Intel-gfx at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/intel-gfx