All of lore.kernel.org
 help / color / mirror / Atom feed
From: Daniel Vetter <daniel@ffwll.ch>
To: "Kasireddy, Vivek" <vivek.kasireddy@intel.com>
Cc: "Daniel Vetter" <daniel@ffwll.ch>,
	"Michel Dänzer" <michel@daenzer.net>,
	"dri-devel@lists.freedesktop.org"
	<dri-devel@lists.freedesktop.org>,
	"Gerd Hoffmann" <kraxel@redhat.com>,
	"Pekka Paalanen" <ppaalanen@gmail.com>,
	"Simon Ser" <contact@emersion.fr>,
	"Zhang, Tina" <tina.zhang@intel.com>,
	"Kim, Dongwon" <dongwon.kim@intel.com>,
	"Singh, Satyeshwar" <satyeshwar.singh@intel.com>
Subject: Re: [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability
Date: Thu, 5 Aug 2021 14:08:17 +0200	[thread overview]
Message-ID: <YQvUsYgKxPM/2KZm@phenom.ffwll.local> (raw)
In-Reply-To: <8722db11a6d245259c15ca2262a40d07@intel.com>

On Thu, Aug 05, 2021 at 04:15:27AM +0000, Kasireddy, Vivek wrote:
> Hi Daniel,
> 
> > > >>> The solution:
> > > >>> - To ensure full framerate, the Guest compositor has to start it's repaint cycle
> > (including
> > > >>> the 9 ms wait) when the Host compositor sends the frame callback event to its
> > clients.
> > > >>> In order for this to happen, the dma-fence that the Guest KMS waits on -- before
> > sending
> > > >>> pageflip completion -- cannot be tied to a wl_buffer.release event. This means that,
> > the
> > > >>> Guest compositor has to be forced to use a new buffer for its next repaint cycle
> > when it
> > > >>> gets a pageflip completion.
> > > >>
> > > >> Is that really the only solution?
> > > > [Kasireddy, Vivek] There are a few others I mentioned here:
> > > > https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_986572
> > > > But I think none of them are as compelling as this one.
> > > >
> > > >>
> > > >> If we fix the event timestamps so that both guest and host use the same
> > > >> timestamp, but then the guest starts 5ms (or something like that) earlier,
> > > >> then things should work too? I.e.
> > > >> - host compositor starts at (previous_frametime + 9ms)
> > > >> - guest compositor starts at (previous_frametime + 4ms)
> > > >>
> > > >> Ofc this only works if the frametimes we hand out to both match _exactly_
> > > >> and are as high-precision as the ones on the host side. Which for many gpu
> > > >> drivers at least is the case, and all the ones you care about for sure :-)
> > > >>
> > > >> But if the frametimes the guest receives are the no_vblank fake ones, then
> > > >> they'll be all over the place and this carefully tuned low-latency redraw
> > > >> loop falls apart. Aside fromm the fact that without tuning the guests to
> > > >> be earlier than the hosts, you're guaranteed to miss every frame (except
> > > >> when the timing wobbliness in the guest is big enough by chance to make
> > > >> the deadline on the oddball frame).
> > > > [Kasireddy, Vivek] The Guest and Host use different event timestamps as we don't
> > > > share these between the Guest and the Host. It does not seem to be causing any other
> > > > problems so far but we did try the experiment you mentioned (i.e., adjusting the
> > delays)
> > > > and it works. However, this patch series is meant to fix the issue without having to
> > tweak
> > > > anything (delays) because we can't do this for every compositor out there.
> > >
> > > Maybe there could be a mechanism which allows the compositor in the guest to
> > automatically adjust its repaint cycle as needed.
> > >
> > > This might even be possible without requiring changes in each compositor, by adjusting
> > the vertical blank periods in the guest to be aligned with the host compositor repaint
> > cycles. Not sure about that though.
> > >
> > > Even if not, both this series or making it possible to queue multiple flips require
> > corresponding changes in each compositor as well to have any effect.
> > 
> > Yeah from all the discussions and tests done it sounds even with a
> > deeper queue we have big coordination issues between the guest and
> > host compositor (like the example that the guest is now rendering at
> > 90fps instead of 60fps like the host).
> [Kasireddy, Vivek] Oh, I think you are referring to my reply to Gerd. That 90 FPS vs 
> 60 FPS problem is a completely different issue that is associated with Qemu GTK UI
> backend. With the GTK backend -- and also with SDL backend -- we Blit the Guest
> scanout FB onto one of the backbuffers managed by EGL. 
> 
> I am trying to add a new Qemu Wayland UI backend so that we can eliminate that Blit
> and thereby have a truly zero-copy solution. And, this is there I am running into the 
> halved frame-rate issue -- the current problem.

Yes, that's what I referenced. But I disagree that it's a different
problem. The underlying problem in both cases is that the guest and host
compositor free-wheel instead of rendering in sync. It's just that
depending upon how exactly the flip completion event on the gues side
plays out you either get guest rendering that's faster than the host-side
60fps, or guest rendering that's much slower than the host-side 60fps.

The fundamental problem in both cases is that they don't run in lockstep.
If you fix that, through fixing the timestamp and even reporting most
likely, you should be able to fix both bugs.

> > Hence my gut feeling reaction that first we need to get these two
> > compositors aligned in their timings, which propobably needs
> > consistent vblank periods/timestamps across them (plus/minux
> > guest/host clocksource fun ofc). Without this any of the next steps
> > will simply not work because there's too much jitter by the time the
> > guest compositor gets the flip completion events.
> [Kasireddy, Vivek] Timings are not a problem and do not significantly
> affect the repaint cycles from what I have seen so far.
> 
> > 
> > Once we have solid events I think we should look into statically
> > tuning guest/host compositor deadlines (like you've suggested in a
> > bunch of places) to consisently make that deadline and hit 60 fps.
> > With that we can then look into tuning this automatically and what to
> > do when e.g. switching between copying and zero-copy on the host side
> > (which might be needed in some cases) and how to handle all that.
> [Kasireddy, Vivek] As I confirm here: https://gitlab.freedesktop.org/wayland/weston/-/issues/514#note_984065
> tweaking the deadlines works (i.e., we get 60 FPS) as we expect. However,
> I feel that this zero-copy solution I am trying to create should be independent
> of compositors' deadlines, delays or other scheduling parameters.

That's not how compositors work nowadays. Your problem is that you don't
have the guest/host compositor in sync. zero-copy only changes the timing,
so it changes things from "rendering way too many frames" to "rendering
way too few frames".

We need to fix the timing/sync issue here first, not paper over it with
hacks.

Only, and I really mean only, when that shows that it's simply impossible
to hit 60fps with zero-copy and the guest/host fully aligned should we
look into making the overall pipeline deeper.

> > Only when that all shows that we just can't hit 60fps consistently and
> > really need 3 buffers in flight should we look at deeper kms queues.
> > And then we really need to implement them properly and not with a
> > mismatch between drm_event an out-fence signalling. These quick hacks
> > are good for experiments, but there's a pile of other things we need
> > to do first. At least that's how I understand the problem here right
> > now.
> [Kasireddy, Vivek] Experiments done so far indicate that we can hit 59 FPS consistently
> -- in a zero-copy way independent of compositors' delays/deadlines -- with this
> patch series + the Weston MR I linked in the cover letter. The main reason why this
> works is because we relax the assumption that when the Guest compositor gets a
> pageflip completion event that it could reuse the old FB it submitted in the previous
> atomic flip and instead force it to use a new one. And, we send the pageflip completion
> event to the Guest when the Host compositor sends a frame callback event. Lastly,
> we use the (deferred) out_fence as just a mechanism to tell the Guest compositor when
> it can release references on old FBs so that they can be reused again.
> 
> With that being said, the only question is how can we accomplish the above in an upstream
> acceptable way without regressing anything particularly on bare-metal. Its not clear if just
> increasing the queue depth would work or not but I think the Guest compositor has to be told
> when it can start its repaint cycle and when it can assume the old FB is no longer in use.
> On bare-metal -- and also with VKMS as of today -- a pageflip completion indicates both.
> In other words, Vblank event is the same as Flip done, which makes sense on bare-metal.
> But if we were to have two events at-least for VKMS: vblank to indicate to Guest to start
> repaint and flip_done to indicate to drop references on old FBs, I think this problem can
> be solved even without increasing the queue depth. Can this be acceptable?

That's just another flavour of your "increase queue depth without
increasing the atomic queue depth" approach. I still think the underlying
fundamental issue is a timing confusion, and the fact that adjusting the
timings fixes things too kinda proves that. So we need to fix that in a
clean way, not by shuffling things around semi-randomly until the specific
config we tests works.

Iow I think we need a solution here which both slows down the 90fps to
60fps for the blit case, and the 30fps speed up to 60fps for the zerocopy
case. Because the host might need to switch transparently between blt and
zerocopy for various reasons.
-Daniel

> Thanks,
> Vivek
> > 
> > Cheers, Daniel
> > 
> > >
> > >
> > > --
> > > Earthling Michel Dänzer               |               https://redhat.com
> > > Libre software enthusiast             |             Mesa and X developer
> > 
> > 
> > 
> > --
> > Daniel Vetter
> > Software Engineer, Intel Corporation
> > http://blog.ffwll.ch

-- 
Daniel Vetter
Software Engineer, Intel Corporation
http://blog.ffwll.ch

  reply	other threads:[~2021-08-05 12:08 UTC|newest]

Thread overview: 38+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29  8:16 [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability Vivek Kasireddy
2021-07-29  8:16 ` [RFC v1 1/4] drm: Add a capability flag to support deferred out_fence signalling Vivek Kasireddy
2021-07-29  8:16 ` [RFC v1 2/4] virtio-gpu uapi: Add VIRTIO_GPU_F_OUT_FENCE feature Vivek Kasireddy
2021-07-29  9:50   ` Gerd Hoffmann
2021-07-29 18:53     ` Kasireddy, Vivek
2021-07-29  8:16 ` [RFC v1 3/4] drm/virtio: Add VIRTIO_GPU_CMD_RESOURCE_OUT_FENCE cmd Vivek Kasireddy
2021-07-29  8:16 ` [RFC v1 4/4] drm/virtio: Probe and implement VIRTIO_GPU_F_OUT_FENCE feature Vivek Kasireddy
2021-07-29  9:52   ` Gerd Hoffmann
2021-07-29 18:55     ` Kasireddy, Vivek
2021-07-30 10:25 ` [RFC v1 0/4] drm: Add support for DRM_CAP_DEFERRED_OUT_FENCE capability Daniel Vetter
2021-07-30 12:50   ` Michel Dänzer
2021-08-02  7:59     ` Daniel Vetter
2021-08-02  8:49       ` Michel Dänzer
2021-08-02  9:06         ` Daniel Vetter
2021-08-02  9:19           ` Michel Dänzer
2021-07-30 13:38   ` Gerd Hoffmann
2021-08-02  9:09     ` Daniel Vetter
2021-08-02 12:50       ` Gerd Hoffmann
2021-08-02 14:35         ` Daniel Vetter
2021-08-03  6:18         ` Kasireddy, Vivek
2021-08-03  7:51           ` Gerd Hoffmann
2021-08-04  7:27             ` Kasireddy, Vivek
2021-08-02  4:48   ` Zhang, Tina
2021-08-02  6:51   ` Kasireddy, Vivek
2021-08-02  8:14     ` Daniel Vetter
2021-08-03  6:11       ` Kasireddy, Vivek
2021-08-03  7:33         ` Michel Dänzer
2021-08-04  7:25           ` Kasireddy, Vivek
2021-08-04 12:11           ` Daniel Vetter
2021-08-05  4:15             ` Kasireddy, Vivek
2021-08-05 12:08               ` Daniel Vetter [this message]
2021-08-06  7:27                 ` Kasireddy, Vivek
2021-08-09 14:15                   ` Daniel Vetter
2021-08-10  8:21                     ` Kasireddy, Vivek
2021-08-10  8:30                       ` Daniel Vetter
2021-08-10 10:57                         ` Michel Dänzer
2021-08-11  7:25                           ` Kasireddy, Vivek
2021-08-11  7:20                         ` Kasireddy, Vivek

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=YQvUsYgKxPM/2KZm@phenom.ffwll.local \
    --to=daniel@ffwll.ch \
    --cc=contact@emersion.fr \
    --cc=dongwon.kim@intel.com \
    --cc=dri-devel@lists.freedesktop.org \
    --cc=kraxel@redhat.com \
    --cc=michel@daenzer.net \
    --cc=ppaalanen@gmail.com \
    --cc=satyeshwar.singh@intel.com \
    --cc=tina.zhang@intel.com \
    --cc=vivek.kasireddy@intel.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.