On Wed, Oct 20, 2021 at 04:17:28PM -0700, Brian Norris wrote:
> Prior to commit 6c836d965bad ("drm/rockchip: Use the helpers for PSR"),
> "PSR disable" used non-blocking analogix_dp_send_psr_spd(). The refactor
> accidentally (?) set blocking=true.
IIRC this wasn't accidental.
The reason it became synchronous was:
- 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
The actual latency gains from doing this synchronously are minimal since the
panel will display new content as soon as it can regardless of whether the
kernel is blocking. There is likely a perceptual difference, but that's only
because kernel is lying to userspace and skipping frames without consent.
Going back to the first line, it's entirely possible my memory is failing
and this was accidental!
Sean
>
> This can cause upwards of 60-100ms of unneeded latency when exiting
> self-refresh, which can cause very noticeable lag when, say, moving a
> cursor.
>
> Presumbaly it's OK to let the display finish exiting refresh in parallel
> with clocking out the next video frames, so we shouldn't hold up the
> atomic_enable() step. This also brings behavior in line with the
> downstream ("mainline-derived") variant of the driver currently deployed
> to Chrome OS Rockchip systems.
>
> Tested on a Samsung Chromebook Plus (i.e., Rockchip RK3399 Gru Kevin).
>
> 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>
> ---
> CC list is partially constructed from the commit message of the Fixed
> commit
>
> drivers/gpu/drm/bridge/analogix/analogix_dp_core.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> index b7d2e4449cfa..fbe6eb9df310 100644
> --- a/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> +++ b/drivers/gpu/drm/bridge/analogix/analogix_dp_core.c
> @@ -1055,7 +1055,7 @@ static int analogix_dp_disable_psr(struct analogix_dp_device *dp)
> psr_vsc.db[0] = 0;
> psr_vsc.db[1] = 0;
>
> - return analogix_dp_send_psr_spd(dp, &psr_vsc, true);
> + return analogix_dp_send_psr_spd(dp, &psr_vsc, false);
> }
>
> /*
> --
> 2.33.0.1079.g6e70778dc9-goog
>
--
Sean Paul, Software Engineer, Google / Chromium OS
