Re: wl_buffer is not released for long time.

2018-03-12 Thread Matt Hoosier
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 Hoosier  wrote:
> 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.

2018-03-12 Thread Matt Hoosier
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


wl_buffer is not released for long time.

2018-03-12 Thread Sichem Zhou
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

2018-03-12 Thread Guido Günther
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"

2018-03-12 Thread Daniel Stone
Hi,

On 12 March 2018 at 11:21, Emil Velikov  wrote:
> 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"

2018-03-12 Thread Emil Velikov
On 9 March 2018 at 11:09, Daniel Stone  wrote:
> 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

2018-03-12 Thread Daniel Stone
Hi Scott,

On 19 February 2018 at 19:18, 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.

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

2018-03-12 Thread Pekka Paalanen
On Tue, 6 Mar 2018 20:13:54 +0100
Emmanuel Gil Peyrot  wrote:

> 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

2018-03-12 Thread Pekka Paalanen
On Fri, 9 Mar 2018 11:32:47 -0600
Derek Foreman  wrote:

> 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