All of lore.kernel.org
 help / color / mirror / Atom feed
From: Brian Norris <briannorris@chromium.org>
To: Sean Paul <sean@poorly.run>
Cc: Neil Armstrong <narmstrong@baylibre.com>,
	Robert Foss <robert.foss@linaro.org>,
	Jonas Karlman <jonas@kwiboo.se>,
	Jernej Skrabec <jernej.skrabec@gmail.com>,
	dri-devel@lists.freedesktop.org,
	Laurent Pinchart <Laurent.pinchart@ideasonboard.com>,
	stable@vger.kernel.org, Zain Wang <wzz@rock-chips.com>,
	Tomasz Figa <tfiga@chromium.org>,
	Heiko Stuebner <heiko@sntech.de>,
	Sean Paul <seanpaul@chromium.org>
Subject: Re: [PATCH] drm/bridge: analogix_dp: Make PSR-disable non-blocking
Date: Fri, 29 Oct 2021 16:10:04 -0700	[thread overview]
Message-ID: <YXx/TJ6CAXHfTdrQ@google.com> (raw)
In-Reply-To: <CA+ASDXNNPHfAVuN_Q7UJR6GLaepHghtovDUKyMKrVM_UboiM2A@mail.gmail.com>

On Wed, Oct 20, 2021 at 06:23:35PM -0700, Brian Norris wrote:
> On Wed, Oct 20, 2021 at 5:40 PM Sean Paul <sean@poorly.run> wrote:
> > 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.
> 
> Hmm, you might well be right about some of the first points (I'm still
> learning the DRM framework), but I'm a bit skeptical that the
> perceptual difference is "only" because we're cheating in some way.
> I'm not doing science here, and it's certainly not a blinded test, but
> I'm nearly certain this patch cuts out approx 50-80% of the cursor lag
> I see without this patch (relative to the current Chrome OS kernel). I
> don't see how cheating would produce a smoother cursor movement --
> we'd still be dropping frames, and the movement would appear jumpy
> somewhere along the way.

Aha, so I think I found {a,the} reason for some disagreement here:
looking at the eDP PSR spec, I see that while the current implementation
is looking for psr_state==DP_PSR_SINK_INACTIVE to signal PSR-exit
completion, the spec shows an intermediate state
(DP_PSR_SINK_ACTIVE_RESYNC == 4), where among other things, "the Sink
device must display the incoming active frames from the Source device
with no visible glitches and/or artifacts."

And it happens that we move to DP_PSR_SINK_ACTIVE_RESYNC somewhat
quickly (on the order of 20-40ms), while the move to
DP_PSR_SINK_INACTIVE is a good chunk longer (approx 60ms more). So
pre-commit-6c836d965bad might have been cheating a little (we'd claim
we're "done" about 20-40ms too early), but post-commit-6c836d965bad
we're waiting about 60ms too long.

I'll send v2 to make this block for DP_PSR_SINK_ACTIVE_RESYNC ||
DP_PSR_SINK_INACTIVE, which gets much or all of the same latency win,
and I'll try to document the reasons, etc., better.

I'll probably also include a patch to drop the 'blocking' parameter,
since it's unused, and gives the wrong idea about this state machine.

> In any case, I'm absolutely certain that mainline Linux performs much
> much worse with PSR than the current CrOS kernel, but there are some
> other potential reasons for that, such as the lack of an
> input-notifier [1].
...
> [1] This got locked up in "controversy":
> https://patchwork.kernel.org/project/linux-arm-kernel/patch/20180405095000.9756-25-enric.balletbo@collabora.com/

While I'm here: I also played with this a bit, and I still haven't
gotten all the details right, but I don't believe this alone will get
the latency wins we'd like. We still need something like the above.

Brian

  reply	other threads:[~2021-10-29 23:10 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-20 23:17 [PATCH] drm/bridge: analogix_dp: Make PSR-disable non-blocking Brian Norris
2021-10-21  0:40 ` Sean Paul
2021-10-21  1:23   ` Brian Norris
2021-10-29 23:10     ` Brian Norris [this message]
2021-10-21  1:41   ` Brian Norris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=YXx/TJ6CAXHfTdrQ@google.com \
    --to=briannorris@chromium.org \
    --cc=Laurent.pinchart@ideasonboard.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=heiko@sntech.de \
    --cc=jernej.skrabec@gmail.com \
    --cc=jonas@kwiboo.se \
    --cc=narmstrong@baylibre.com \
    --cc=robert.foss@linaro.org \
    --cc=sean@poorly.run \
    --cc=seanpaul@chromium.org \
    --cc=stable@vger.kernel.org \
    --cc=tfiga@chromium.org \
    --cc=wzz@rock-chips.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.