On Tue, 4 Apr 2017 17:54:33 +0100 Daniel Stone <[email protected]> wrote:
> page_flip_pending is only be set when do a pageflip to a newly-rendered > buffer; if the flag is not set, we have landed in the start_repaint_loop > path where the vblank query fails, and thus we must pageflip to the same > buffer. > > This test was not sufficient for what it was supposed to guard: > releasing framebuffers back. When using client-supplied framebuffers, it > is possible to reuse the same buffer multiple times, and we would send a > framebuffer-release event too early. > > However, since we have a properly reference-counted drm_fb now, we can > just drop this test, and rely on the reference counting to prevent > too-early release of client framebuffers. > > page_flip_pending now becomes exactly what the name suggests: a flag > which indicates whether or not we are expecting a pageflip event. Add > asserts here to verify that we never receive a pageflip event we weren't > expecting. > > Signed-off-by: Daniel Stone <[email protected]> > > Differential Revision: https://phabricator.freedesktop.org/D1417 > > Signed-off-by: Daniel Stone <[email protected]> > --- > libweston/compositor-drm.c | 19 +++++++++++-------- > 1 file changed, 11 insertions(+), 8 deletions(-) > > diff --git a/libweston/compositor-drm.c b/libweston/compositor-drm.c > index 21415775..88975007 100644 > --- a/libweston/compositor-drm.c > +++ b/libweston/compositor-drm.c > @@ -883,6 +883,7 @@ drm_output_repaint(struct weston_output *output_base, > output->fb_current = output->fb_pending; > output->fb_pending = NULL; > > + assert(!output->page_flip_pending); > output->page_flip_pending = 1; > > if (output->pageflip_timer) > @@ -1008,6 +1009,9 @@ drm_output_start_repaint_loop(struct weston_output > *output_base) > */ > fb_id = output->fb_current->fb_id; > > + assert(!output->page_flip_pending); > + assert(!output->fb_last); > + > if (drmModePageFlip(backend->drm.fd, output->crtc_id, fb_id, > DRM_MODE_PAGE_FLIP_EVENT, output) < 0) { > weston_log("queueing pageflip failed: %m\n"); > @@ -1018,6 +1022,9 @@ drm_output_start_repaint_loop(struct weston_output > *output_base) > wl_event_source_timer_update(output->pageflip_timer, > backend->pageflip_timeout); > > + output->fb_last = drm_fb_ref(output->fb_current); > + output->page_flip_pending = 1; > + > return; > > finish_frame: > @@ -1081,16 +1088,12 @@ page_flip_handler(int fd, unsigned int frame, > > drm_output_update_msc(output, frame); > > - /* We don't set page_flip_pending on start_repaint_loop, in that case > - * we just want to page flip to the current buffer to get an accurate > - * timestamp */ > - if (output->page_flip_pending) { > - drm_fb_unref(output->fb_last); > - output->fb_last = NULL; > - } > - > + assert(output->page_flip_pending); > output->page_flip_pending = 0; > > + drm_fb_unref(output->fb_last); > + output->fb_last = NULL; > + > if (output->destroy_pending) > drm_output_destroy(&output->base); > else if (output->disable_pending) Reviewed-by: Pekka Paalanen <[email protected]> Btw. I'll get rid of the phab tags and dual-SoB when I push. Unless someone else pushes first. :-) The reason we cannot just remove page_flip_pending completely in favour of the other variables like fb_last is that then we would miss the initial flip from nothing to the very first FB, right? Thanks, pq
pgpVStZTXNILq.pgp
Description: OpenPGP digital signature
_______________________________________________ wayland-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/wayland-devel
