Re: wl_buffer is not released for long time.
Here's the discussion and patch that addressed the issue I was mentioning: https://lists.freedesktop.org/archives/wayland-devel/2017-September/035191.html On Mon, Mar 12, 2018 at 3:51 PM, Matt Hoosierwrote: > Hi, > > Unless you're using an unreleased version of Weston, I think you're > probably running into a bug that we fixed a few months ago in which > wl_buffer::release() events were prone to sit undispatched in the > server's outgoing queue until some other event happened to need > transmitted. > > -Matt > > On Mon, Mar 12, 2018 at 1:23 PM, Sichem Zhou wrote: >> Hi all, >> >> Dear wayland devs, I have a question regarding to double `wl_buffer` >> management. I don't seem to have wl_buffer released untill some other events >> triggered (for example, the inputs). My current environment is under >> `X11-backend` and a libweston based compositor. >> >> My pipeline follows: (frame, attach, damage, commit) -> buffer switch -> >> wait until one buffer available -> redraw. The `done` event is handled >> differently since it only signals if ready to draw. >> >> Through my experiments, I found out at I wouldn't get either of the >> `release` of `frame done` event if I wasn't using the compositor (moving >> cursor or typing). In this case I guess there is something wrong with my >> pipeline but I couldn't figure out which. I wish to know if there is obvious >> mistakes in my pipeline or the problem lies in the compositor. >> >> Very appreciate any answers. >> >> SZ >> >> >> >> ___ >> wayland-devel mailing list >> wayland-devel@lists.freedesktop.org >> https://lists.freedesktop.org/mailman/listinfo/wayland-devel >> ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: wl_buffer is not released for long time.
Hi, Unless you're using an unreleased version of Weston, I think you're probably running into a bug that we fixed a few months ago in which wl_buffer::release() events were prone to sit undispatched in the server's outgoing queue until some other event happened to need transmitted. -Matt On Mon, Mar 12, 2018 at 1:23 PM, Sichem Zhouwrote: > Hi all, > > Dear wayland devs, I have a question regarding to double `wl_buffer` > management. I don't seem to have wl_buffer released untill some other events > triggered (for example, the inputs). My current environment is under > `X11-backend` and a libweston based compositor. > > My pipeline follows: (frame, attach, damage, commit) -> buffer switch -> > wait until one buffer available -> redraw. The `done` event is handled > differently since it only signals if ready to draw. > > Through my experiments, I found out at I wouldn't get either of the > `release` of `frame done` event if I wasn't using the compositor (moving > cursor or typing). In this case I guess there is something wrong with my > pipeline but I couldn't figure out which. I wish to know if there is obvious > mistakes in my pipeline or the problem lies in the compositor. > > Very appreciate any answers. > > SZ > > > > ___ > wayland-devel mailing list > wayland-devel@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/wayland-devel > ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
wl_buffer is not released for long time.
Hi all, Dear wayland devs, I have a question regarding to double `wl_buffer` management. I don't seem to have wl_buffer released untill some other events triggered (for example, the inputs). My current environment is under `X11-backend` and a libweston based compositor. My pipeline follows: (frame, attach, damage, commit) -> buffer switch -> wait until one buffer available -> redraw. The `done` event is handled differently since it only signals if ready to draw. Through my experiments, I found out at I wouldn't get either of the `release` of `frame done` event if I wasn't using the compositor (moving cursor or typing). In this case I guess there is something wrong with my pipeline but I couldn't figure out which. I wish to know if there is obvious mistakes in my pipeline or the problem lies in the compositor. Very appreciate any answers. SZ ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
[weston PATCH] simple-dmabuf-drm: support etnaviv drm as well
Since freedreno and etnaviv can live in parallel allow to build in different DRM backends. --- Makefile.am | 6 ++- clients/simple-dmabuf-drm.c | 122 +++- configure.ac| 29 +++ 3 files changed, 121 insertions(+), 36 deletions(-) diff --git a/Makefile.am b/Makefile.am index e028a2a1..19319fa2 100644 --- a/Makefile.am +++ b/Makefile.am @@ -624,7 +624,11 @@ nodist_weston_simple_dmabuf_drm_SOURCES = \ protocol/linux-dmabuf-unstable-v1-protocol.c \ protocol/linux-dmabuf-unstable-v1-client-protocol.h weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) $(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS) -weston_simple_dmabuf_drm_LDADD = $(SIMPLE_DMABUF_DRM_CLIENT_LIBS) $(LIBDRM_PLATFORM_LIBS) libshared.la +weston_simple_dmabuf_drm_LDADD = $(SIMPLE_DMABUF_DRM_CLIENT_LIBS) \ + $(LIBDRM_PLATFORM_FREEDRENO_LIBS) \ + $(LIBDRM_PLATFORM_ETNAVIV_LIBS) \ + $(LIBDRM_PLATFORM_INTEL_LIBS) \ + libshared.la endif if BUILD_SIMPLE_DMABUF_V4L_CLIENT diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c index 1073930f..8b7acd39 100644 --- a/clients/simple-dmabuf-drm.c +++ b/clients/simple-dmabuf-drm.c @@ -44,9 +44,13 @@ #ifdef HAVE_LIBDRM_INTEL #include #include -#elif HAVE_LIBDRM_FREEDRENO +#endif +#ifdef HAVE_LIBDRM_FREEDRENO #include #endif +#ifdef HAVE_LIBDRM_ETNAVIV +#include +#endif #include #include @@ -93,11 +97,16 @@ struct buffer { #ifdef HAVE_LIBDRM_INTEL drm_intel_bufmgr *bufmgr; - drm_intel_bo *bo; -#elif HAVE_LIBDRM_FREEDRENO + drm_intel_bo *intel_bo; +#endif /* HAVE_LIBDRM_INTEL */ +#if HAVE_LIBDRM_FREEDRENO struct fd_device *fd_dev; - struct fd_bo *bo; + struct fd_bo *fd_bo; #endif /* HAVE_LIBDRM_FREEDRENO */ +#if HAVE_LIBDRM_ETNAVIV + struct etna_device *etna_dev; + struct etna_bo *etna_bo; +#endif /* HAVE_LIBDRM_ETNAVIV */ uint32_t gem_handle; int dmabuf_fd; @@ -152,15 +161,15 @@ intel_alloc_bo(struct buffer *my_buf) assert(my_buf->bufmgr); - my_buf->bo = drm_intel_bo_alloc_tiled(my_buf->bufmgr, "test", - my_buf->width, my_buf->height, - (my_buf->bpp / 8), , - _buf->stride, 0); + my_buf->intel_bo = drm_intel_bo_alloc_tiled(my_buf->bufmgr, "test", + my_buf->width, my_buf->height, + (my_buf->bpp / 8), , + _buf->stride, 0); printf("buffer allocated w %d, h %d, stride %lu, size %lu\n", - my_buf->width, my_buf->height, my_buf->stride, my_buf->bo->size); + my_buf->width, my_buf->height, my_buf->stride, my_buf->intel_bo->size); - if (!my_buf->bo) + if (!my_buf->intel_bo) return 0; if (tiling != I915_TILING_NONE) @@ -172,16 +181,16 @@ intel_alloc_bo(struct buffer *my_buf) static void intel_free_bo(struct buffer *my_buf) { - drm_intel_bo_unreference(my_buf->bo); + drm_intel_bo_unreference(my_buf->intel_bo); } static int intel_map_bo(struct buffer *my_buf) { - if (drm_intel_gem_bo_map_gtt(my_buf->bo) != 0) + if (drm_intel_gem_bo_map_gtt(my_buf->intel_bo) != 0) return 0; - my_buf->mmap = my_buf->bo->virtual; + my_buf->mmap = my_buf->intel_bo->virtual; return 1; } @@ -189,15 +198,16 @@ intel_map_bo(struct buffer *my_buf) static int intel_bo_export_to_prime(struct buffer *buffer) { - return drm_intel_bo_gem_export_to_prime(buffer->bo, >dmabuf_fd); + return drm_intel_bo_gem_export_to_prime(buffer->intel_bo, >dmabuf_fd); } static void intel_unmap_bo(struct buffer *my_buf) { - drm_intel_gem_bo_unmap_gtt(my_buf->bo); + drm_intel_gem_bo_unmap_gtt(my_buf->intel_bo); } -#elif HAVE_LIBDRM_FREEDRENO +#endif /* HAVE_LIBDRM_INTEL */ +#ifdef HAVE_LIBDRM_FREEDRENO #define ALIGN(v, a) ((v + a - 1) & ~(a - 1)) static @@ -207,9 +217,9 @@ int fd_alloc_bo(struct buffer *buf) int size = buf->width * buf->height * buf->bpp / 8; buf->fd_dev = fd_device_new(buf->drm_fd); - buf->bo = fd_bo_new(buf->fd_dev, size, flags); + buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags); - if (!buf->bo) + if (!buf->fd_bo) return 0; buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8; return 1; @@ -218,13 +228,13 @@ int fd_alloc_bo(struct buffer *buf) static void fd_free_bo(struct buffer *buf) { - fd_bo_del(buf->bo); + fd_bo_del(buf->fd_bo); } static int fd_bo_export_to_prime(struct buffer *buf) { - buf->dmabuf_fd = fd_bo_dmabuf(buf->bo); + buf->dmabuf_fd = fd_bo_dmabuf(buf->fd_bo); if (buf->dmabuf_fd > 0) return 0; @@
Re: [PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"
Hi, On 12 March 2018 at 11:21, Emil Velikovwrote: > On 9 March 2018 at 11:09, Daniel Stone wrote: >> On 9 March 2018 at 10:59, Emil Velikov wrote: >>> - above all, the internal path is a 'dummy' fallback. anyone can >>> provide the binary name as an argument >>> $ .../wayland-egl-symbols-check .../libwayland-egl.so >>> - since we have a fallback - a plain .../wayland-egl-symbols-check >>> will work most of the time >> >> That makes sense, running it from the build root. Is that just because >> 'make check' is slow, or? (sanity-test is really slow.) >> > Short back story: I was playing with OBS and getting the build > artefacts (the contents of a failing test) was a pain. > Admittedly, there may be another way to handle that, although in general: > > Passing the file as argument makes debugging a lot quicker/easier. Sure, that makes sense. I have a suggestion though: why don't we try argv[1] first, then fall back to $WAYLAND_EGL_LIB, and error out if neither are set? That way we don't hardcode internal autotools implementation details into our own scripts (remove fallback definition), allow autotools tests to run easily (environment variable), and allow people to use it manually for debugging (command-line argument). For the latter, I think being explicit is better: it means you can't accidentally forget to set an environment variable and end up testing the wrong thing. >>> - handling env. variables (as opposed to arguments) is a pain with meson >> >> Hm, not really. You just add an 'env' argument when declaring the test, e.g.: >> egl_sym_check = find_program('wayland-egl-symbols-check') >> test_egl_syms = test('egl-symbols', egl_sym_check, env: [ >> 'WAYLAND_EGL_LIB=@0@'.format(lib_wayland_egl) ]) >> > Once you add NM and potentially others, it does get tiny bit messier. No more complicated than it needs to be: egl_sym_check_env = [ 'WAYLAND_EGL_LIB=@0@'.format(lib_wayland_egl), 'NM=@0@'.format(nm_prog), ... ] test('egl-symbols', egl_sym_check, env: egl_sym_check_env) Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH wayland v2 1/4] Revert "wayland-egl-symbols-check: pass the DSO name via the build system"
On 9 March 2018 at 11:09, Daniel Stonewrote: > Hi Emil, > > On 9 March 2018 at 10:59, Emil Velikov wrote: >> On 9 March 2018 at 09:47, Daniel Stone wrote: >>> Patches 2-4 look fine and I'm happy to merge them with my review, but >>> could you please explain some more about this patch? I very much like >>> keeping details of the build system (specifically its internal build >>> paths) in the build system itself and not in the script. I was >>> assuming something in 2-4 needed this revert to be applied, but >>> couldn't see anything. Is there something I'm missing? >> >> There is one word to describe it - compromise: >> >> - above all, the internal path is a 'dummy' fallback. anyone can >> provide the binary name as an argument >> $ .../wayland-egl-symbols-check .../libwayland-egl.so >> - since we have a fallback - a plain .../wayland-egl-symbols-check >> will work most of the time > > That makes sense, running it from the build root. Is that just because > 'make check' is slow, or? (sanity-test is really slow.) > Short back story: I was playing with OBS and getting the build artefacts (the contents of a failing test) was a pain. Admittedly, there may be another way to handle that, although in general: Passing the file as argument makes debugging a lot quicker/easier. >> - handling env. variables (as opposed to arguments) is a pain with meson > > Hm, not really. You just add an 'env' argument when declaring the test, e.g.: > egl_sym_check = find_program('wayland-egl-symbols-check') > test_egl_syms = test('egl-symbols', egl_sym_check, env: [ > 'WAYLAND_EGL_LIB=@0@'.format(lib_wayland_egl) ]) > Once you add NM and potentially others, it does get tiny bit messier. >> - handling arguments (as opposed to env. variable) is a pain with >> current testing scheme > > Yeah, that doesn't work. > >> - the original code is shorter >> >> Hope you find at least some of those reasonable. > > It's fair enough. I'm just trying to find out the balance of these: > for instance, if it's no problem to add environment variables with > Meson, do you still want to push it for reason #1, or? > Yes, #1 is perhaps the biggest reason behind the patch. Thanks Emil ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwm: Update input region on resize
Hi Scott, On 19 February 2018 at 19:18, Scott Moreauwrote: > Commit 332d1892 introduced a bug because the window was > shaped only when the frame was created, leaving the input > region unchanged regardless if the window was resized. > This patch updates the input region shape on resize, > fixing the problem. Thanks for this! I've had a look - well, as much of a look into XWM as I can stomach on a Monday morning - and I think it's mostly right, but could be improved a little further. In the override-redirect case, we call frame_resize_inside in the branch closed just above where you added the weston_wm_window_shape() call. That is the first point where we know the window has a different size, so would be the right place to call it. On the other hand, for OR we probably just shouldn't be trying to manage the input mask at all ... ? For non-OR windows, send_configure() is where we send the server a ConfigureRequest (which it will process immediately as we hold the substructure redirect), so we should chase that immediately with the input-region update. That resolves a potential race where we have incoherent input/output size. As Pekka says, there's something fishy about decorate/frame confusion I think. Anyone who put clear and concise comments above decorate/frame explaining what they actually do and when they will actually be set would have my eternal gratitude, because I have to go re-learn what they are every time. :\ Cheers, Daniel ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH] xwm: Update input region on resize
On Tue, 6 Mar 2018 20:13:54 +0100 Emmanuel Gil Peyrotwrote: > On Mon, Feb 19, 2018 at 12:18:51PM -0700, Scott Moreau wrote: > > Commit 332d1892 introduced a bug because the window was > > shaped only when the frame was created, leaving the input > > region unchanged regardless if the window was resized. > > This patch updates the input region shape on resize, > > fixing the problem. > > This fixes the issue I had with Firefox, thanks! > > Reviewed-by: Emmanuel Gil Peyrot > Tested-by: Emmanuel Gil Peyrot > > > --- > > xwayland/window-manager.c | 46 > > +++--- > > 1 file changed, 27 insertions(+), 19 deletions(-) > > > > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c > > index c307e19..5588166 100644 > > --- a/xwayland/window-manager.c > > +++ b/xwayland/window-manager.c > > @@ -659,6 +659,30 @@ weston_wm_window_get_input_rect(struct > > weston_wm_window *window, > > } > > > > static void > > +weston_wm_window_shape(struct weston_wm_window *window) > > +{ > > + struct weston_wm *wm = window->wm; > > + xcb_rectangle_t rect; > > + int x, y, width, height; > > + > > + weston_wm_window_get_input_rect(window, , , , ); > > + > > + rect.x = x; > > + rect.y = y; > > + rect.width = width; > > + rect.height = height; > > + > > + /* The window frame was created with position and size which include > > +* an offset for margins and shadow. Set the input region to ignore > > +* shadow. */ > > + xcb_shape_rectangles(wm->conn, > > +XCB_SHAPE_SO_SET, > > +XCB_SHAPE_SK_INPUT, > > +0, window->frame_id, > > +0, 0, 1, ); > > +} > > + > > +static void > > weston_wm_window_send_configure_notify(struct weston_wm_window *window) > > { > > xcb_configure_notify_event_t configure_notify; > > @@ -789,6 +813,8 @@ weston_wm_handle_configure_notify(struct weston_wm *wm, > > xcb_generic_event_t *eve > > xwayland_api->set_xwayland(window->shsurf, > >window->x, window->y); > > } > > + > > + weston_wm_window_shape(window); > > } Hi, something is wrong: Program received signal SIGSEGV, Segmentation fault. 0x7fffedcebc89 in frame_refresh_geometry (frame=0x0) at /home/pq/git/weston/shared/frame.c:535 535 struct theme *t = frame->theme; (gdb) bt #0 0x7fffedcebc89 in frame_refresh_geometry (frame=0x0) at /home/pq/git/weston/shared/frame.c:535 #1 0x7fffedcec01f in frame_input_rect (frame=0x0, x=0x7fffd548, y=0x7fffd54c, width=0x7fffd550, height=0x7fffd554) at /home/pq/git/weston/shared/frame.c:630 #2 0x7fffedcdddb8 in weston_wm_window_get_input_rect (window=0x55e2e280, x=0x7fffd548, y=0x7fffd54c, width=0x7fffd550, height=0x7fffd554) at /home/pq/git/weston/xwayland/window-manager.c:657 #3 0x7fffedcdde00 in weston_wm_window_shape (window=0x55e2e280) at /home/pq/git/weston/xwayland/window-manager.c:668 #4 0x7fffedcde403 in weston_wm_handle_configure_notify (wm=0x55da3230, event=0x55e2e420) at /home/pq/git/weston/xwayland/window-manager.c:817 #5 0x7fffedce1441 in weston_wm_handle_event (fd=35, mask=1, data=0x55da3230) at /home/pq/git/weston/xwayland/window-manager.c:2265 #6 0x7798fd42 in wl_event_loop_dispatch (loop=0x5576a250, timeout=timeout@entry=-1) at src/event-loop.c:641 #7 0x7798e4fa in wl_display_run (display=0x5576a170) at src/wayland-server.c:1260 #8 0xc3b7 in main (argc=1, argv=0x7fffdbb8) at /home/pq/git/weston/compositor/main.c:1867 I triggered this crash with kcachegrind, by opening a drop-down item on the UI a couple of times. This seems to be a regression caused by this patch. (Unrelatedly, the kcachegrind menus and drop-downs open at consistent but wrong positions, but that is not caused by this patch.) I would guess that something is breaking assumptions between window->decorate and window->frame, since it crashes on NULL frame. FWIW, this is my short XWM manual check list: - xterm, and menus - geany, and menus, tooltips - printf 'eka\ntoka\nkol\nnel\nvii\nkuusix\n' | dmenu -l 4 - gimp, and toolboxes, menus, tooltips - kcachegrind, and menus, tooltips Thanks, pq pgpetcfX1zTtx.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel
Re: [PATCH weston 1/2] clients: consolidate timer code part 1
On Fri, 9 Mar 2018 11:32:47 -0600 Derek Foremanwrote: > On 2018-03-09 05:44 AM, Pekka Paalanen wrote: > > From: Pekka Paalanen > > > > There are multiple copies for the timerfd handling code, and I need a > > timer in one more app. Consolidate all the timerfd code into window.c to > > reduce the duplication. Many of the copies were also flawed against the > > race mentioned in toytimer_fire(). > > > > This patch handles clickdot and window.c's tooltip timer and cursor > > timer. > > > > Signed-off-by: Pekka Paalanen > > Just out of curiosity, how'd you decide to break it down this way? Mind flow. First I thought of doing just one patch, but it started to look a bit big, the window.c parts particularly. No rationale behind the split, for once. :-) > Both patches look good to me. I hate that reset/fire race and wasted a > bit of time understanding it the first time I hit it. I'm really happy > to see it handled consistently in one place. > > Reviewed-by: Derek Foreman > For both. Thank you. > I think they look safe to land now, as it's mostly harmless refactor, > and the rest is actual bug fix, as abort()ing on the read fail was wrong. > > I'll let you make the call on landing it. :) Thank you to Daniel as well for the comments, pushed: 3f839374..64a26bc1 master -> master Thanks, pq pgpIW2C8bneEb.pgp Description: OpenPGP digital signature ___ wayland-devel mailing list wayland-devel@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/wayland-devel