HI Brian, On Wed, 3 Nov 2021 at 21:52, Brian Norris <briannorris@xxxxxxxxxxxx> 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> > Signed-off-by: Brian Norris <briannorris@xxxxxxxxxxxx> > Reviewed-by: Sean Paul <seanpaul@xxxxxxxxxxxx> > --- > CC list is partially constructed from the commit message of the Fixed > commit > > Changes in v3: > - Fix the exiting/entering comment (a reviewer noticed off-mailing-list > that I got it backwards) > - Add Reviewed-by > > 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..6a4f20fccf84 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.34.0.rc0.344.g81b53c2807-goog > Applied to drm-misc-next