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@xxxxxxxxxxxxxxx>
> Cc: Zain Wang <wzz@xxxxxxxxxxxxxx>
> Cc: Tomasz Figa <tfiga@xxxxxxxxxxxx>
> Cc: Heiko Stuebner <heiko@xxxxxxxxx>
> Cc: Sean Paul <seanpaul@xxxxxxxxxxxx>
Thank you for revising this!
Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx>
> Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx>
> ---
> 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
