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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux