Re: [PATCH v2] drm/bridge: analogix_dp: Make PSR-exit block less

[Sean Paul <sean@poorly.run>]

On Fri, Oct 29, 2021 at 04:50:55PM -0700, Brian Norris wrote:
> Prior to commit 6c836d965bad ("drm/rockchip: Use the helpers for PSR"),
> "PSR exit" used non-blocking analogix_dp_send_psr_spd(). The refactor
> started using the blocking variant, for a variety of reasons -- quoting
> Sean Paul's potentially-faulty memory:
> 
> """
>  - To avoid racing a subsequent PSR entry (if exit takes a long time)
>  - To avoid racing disable/modeset
>  - We're not displaying new content while exiting PSR anyways, so there
>    is minimal utility in allowing frames to be submitted
>  - We're lying to userspace telling them frames are on the screen when
>    we're just dropping them on the floor
> """
> 
> However, I'm finding that this blocking transition is causing upwards of
> 60+ ms of unneeded latency on PSR-exit, to the point that initial cursor
> movements when leaving PSR are unbearably jumpy.
> 
> It turns out that we need to meet in the middle somewhere: Sean is right
> that we were "lying to userspace" with a non-blocking PSR-exit, but the
> new blocking behavior is also waiting too long:
> 
> According to the eDP specification, the sink device must support PSR
> entry transitions from both state 4 (ACTIVE_RESYNC) and state 0
> (INACTIVE). It also states that in ACTIVE_RESYNC, "the Sink device must
> display the incoming active frames from the Source device with no
> visible glitches and/or artifacts."
> 
> Thus, for our purposes, we only need to wait for ACTIVE_RESYNC before
> moving on; we are ready to display video, and subsequent PSR-entry is
> safe.
> 
> Tested on a Samsung Chromebook Plus (i.e., Rockchip RK3399 Gru Kevin),
> where this saves about 60ms of latency, for PSR-exit that used to
> take about 80ms.
> 
> Fixes: 6c836d965bad ("drm/rockchip: Use the helpers for PSR")
> Cc: <stable@vger.kernel.org>
> Cc: Zain Wang <wzz@rock-chips.com>
> Cc: Tomasz Figa <tfiga@chromium.org>
> Cc: Heiko Stuebner <heiko@sntech.de>
> Cc: Sean Paul <seanpaul@chromium.org>

Thank you for revising this!

Reviewed-by: Sean Paul <seanpaul@chromium.org>

> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> CC list is partially constructed from the commit message of the Fixed
> commit
> 
> Changes in v2:
>  - retitled subject (previous: "drm/bridge: analogix_dp: Make
>    PSR-disable non-blocking")
>  - instead of completely non-blocking, make this "less"-blocking
>  - more background (thanks Sean!)
>  - more specification details
> 
>  drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> index cab6c8b92efd..f8e119e84ae2 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_reg.c
> @@ -998,11 +998,21 @@ int analogix_dp_send_psr_spd(struct analogix_dp_device *dp,
>  	if (!blocking)
>  		return 0;
>  
> +	/*
> +	 * db[1]==0: entering PSR, wait for fully active remote frame buffer.
> +	 * db[1]!=0: exiting PSR, wait for either
> +	 *  (a) ACTIVE_RESYNC - the sink "must display the
> +	 *      incoming active frames from the Source device with no visible
> +	 *      glitches and/or artifacts", even though timings may still be
> +	 *      re-synchronizing; or
> +	 *  (b) INACTIVE - the transition is fully complete.
> +	 */
>  	ret = readx_poll_timeout(analogix_dp_get_psr_status, dp, psr_status,
>  		psr_status >= 0 &&
>  		((vsc->db[1] && psr_status == DP_PSR_SINK_ACTIVE_RFB) ||
> -		(!vsc->db[1] && psr_status == DP_PSR_SINK_INACTIVE)), 1500,
> -		DP_TIMEOUT_PSR_LOOP_MS * 1000);
> +		(!vsc->db[1] && (psr_status == DP_PSR_SINK_ACTIVE_RESYNC ||
> +				 psr_status == DP_PSR_SINK_INACTIVE))),
> +		1500, DP_TIMEOUT_PSR_LOOP_MS * 1000);
>  	if (ret) {
>  		dev_warn(dp->dev, "Failed to apply PSR %d\n", ret);
>  		return ret;
> -- 
> 2.33.1.1089.g2158813163f-goog
> 

-- 
Sean Paul, Software Engineer, Google / Chromium OS