Re: T480s Trackpoint drifts

2018-03-19 Thread Peter Hutterer
On Mon, Mar 19, 2018 at 04:28:20PM +0800, Kai Hendry wrote:
> Hi there,
> 
> I'm an Archlinux user who has a new T480s Thinkpad and unfortunately
> the Trackpoint can (not always or reproducibly) "drift". I hope that
> makes sense!

what exactly is drift in this context? it moves on its own to the edge of
the screen?

> https://bbs.archlinux.org/viewtopic.php?id=235400
> 
> The trouble is I can't seem to disable the Trackpoint pointer itself,
> but still have the buttons work, which I do use out of habit.

not something we support in libinput directly but you can possibly work
around this by setting the sensitivity ridiculously low. 

As for the drifting, is this something on your device only or a general
issue affecting this class of devices? Try playing around with the knobs in 
/sys/class/input/event18/device/device (replace event18 with your event node 
number)

> 
> Generally speaking I find the the track point is a bit too fast (and
> imprecise and hence useless) 


run libinput measure trackpoint-range and go from there, it's likely that
you need a custom range for that device. And the speed setting can be
changed, I'm having a hard time believing that at the lowest speed setting
it's still too fast given that it's almost tar-like at that speed on every
device I've tested.

> and the track pad a bit slow.
> For example if I drag my finger across the Trackpad, it's not enough
> to get from one side of the screen to the other.

did you try changing the touchpad speed settings?

Cheers,
   Peter

> I deleted Windows [1] so I don't really have anything to compare to. I
> do know years ago, my old old Thinkpads, the trackpoint did use to be
> a good UX. Now... not so much!
> 
> https://s.natalian.org/2018-03-19/t480s-list-devices.txt
> 
> Many thanks!
> 
> 
> 
> 
> [1] 
> https://www.youtube.com/watch?v=WFlt7PI0sw8=PLiKgVPlhUNuz9k0xIp7PGUV5mmDdTS1vJ=2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH] xwm: Fix memory leak

2018-03-19 Thread Scott Moreau
Fix memory leak introduced by 6b58ea8c. weston_wm_handle_icon() was
calling xcb_get_property_reply() without freeing the reply.
---
 xwayland/window-manager.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
index c307e19..24e7213 100644
--- a/xwayland/window-manager.c
+++ b/xwayland/window-manager.c
@@ -1387,6 +1387,8 @@ weston_wm_handle_icon(struct weston_wm *wm, struct 
weston_wm_window *window)
CAIRO_FORMAT_ARGB32,
width, height, width * 4);
 
+   free(reply);
+
/* Bail out in case anything wrong happened during surface creation. */
if (cairo_surface_status(new_surface) != CAIRO_STATUS_SUCCESS) {
cairo_surface_destroy(new_surface);
-- 
2.7.4

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[ANNOUNCE] weston 3.0.92

2018-03-19 Thread Derek Foreman
This is the beta for the weston 4.0.0 release.

Chris Wilson (1):
  gl-renderer: Create a high priority context

Daniel Stone (4):
  compositor-drm: Don't need safe view-list traversal
  compositor-drm: Rename region variable
  compositor-drm: Remove no_addfb2 handling
  compositor-wayland: Ignore pointer enter on destroyed surface

Derek Foreman (2):
  libweston-desktop/xdg-shell-v6: Fix crash when surface has buffer
at creation
  configure.ac: bump to version 3.0.92 for the beta release

Dima Ryazanov (2):
  weston: Add a help string for --xwayland
  Fix an uninitialized variable

Emmanuel Gil Peyrot (1):
  autoconf: Remove configure line forgotten in
bb707dc0fe331c9af112a0552b7aa6fde755dd83

Guido Günther (9):
  simple-dmabuf-drm: allow multiple backends
  simpla-dmabuf-drm: Use more weston like coding style
  simple-dmabuf-drm: use vfunc for drm_device_destroy
  .gitignore weston-simple-dmabuf-drm
  Allow simple-dmabuf-drm to pass y_inverted flag
  simple-dmabuf-drm: use opt bitmask instead of is_immediate
  simple-dmabuf-drm: use getopt_long
  simple-dmabuf-drm: Always define ALIGN
  simple-dmabuf-drm: use appropriately sized buffer (freedreno)

Ilia Bozhinov (1):
  compositor: do not free output region twice in
weston_output_set_transform()

Jan Engelhardt (1):
  build: honour libinput header location

Marius Vlad (1):
  libweston/compositor: Place timeline recording after checking if
stamp is valid

Michael Tretter (1):
  configure.ac: fix have_dbus if dbus support is disabled

Pekka Paalanen (4):
  compositor-wayland: handle wl_keyboard.enter(NULL)
  input: never set keyboard focus without wl_resource
  clients: consolidate timer code part 1
  clients: consolidate timer code part 2

git tag: 3.0.92

https://wayland.freedesktop.org/releases/weston-3.0.92.tar.xz
MD5:  bf5fd585a96f24b5d23d4a6507cb831c  weston-3.0.92.tar.xz
SHA1: 22b9815aa40ff001702711de256ddd81e25e4088  weston-3.0.92.tar.xz
SHA256: d62234732fa40afc60fa9c06c369f2774c1712f3aaba7949b60a43602966b35a
 weston-3.0.92.tar.xz
SHA512:
e2e3e961de19df8a2da328676506bc5ea922302e89ab62f7c7b30b0327d4baab92b4069fd3d2e7dbe273ba5fc73c9736eabe5274aa4115ad945695ac73b33881
 weston-3.0.92.tar.xz
PGP:  https://wayland.freedesktop.org/releases/weston-3.0.92.tar.xz.sig




signature.asc
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[ANNOUNCE] wayland 1.14.92

2018-03-19 Thread Derek Foreman
This is the beta for the wayland 1.15 release.

Daniel Stone (2):
  wayland-egl: Pass nm path to check script
  wayland-egl: Make symbol test fail on failure

Derek Foreman (3):
  client: Don't inappropriatly close fds for zombie objects
  walyand-client: Fix trivial build break from previous commit
  configure.ac: bump to version 1.14.92 for the beta release

Emil Velikov (5):
  configure.ac: don't install the static libraries
  wayland-egl: set the correct path to libwayland-egl.so
  wayland-egl: fail the symbol check if lib is missing
  wayland-egl: enhance the symbol test
  wayland-egl: bump the version number to 18.1.0

Pekka Paalanen (1):
  tests: disable coredumps on sanity-test

git tag: 1.14.92

https://wayland.freedesktop.org/releases/wayland-1.14.92.tar.xz
MD5:  6e04afc4076bf275a69cfe05847a9a58  wayland-1.14.92.tar.xz
SHA1: 17cf6eb0f532d92271c40e9ec404886f56eb54a0  wayland-1.14.92.tar.xz
SHA256: 80bda4f714d7f48638234c6e7a1eced117943eacb1c872c201c8f0e47542e7f1
 wayland-1.14.92.tar.xz
SHA512:
d0ba6838652165d65399bd11e7e5abdf13a6cf9cb2c54fe5534ac7d3056922844001b237064224c82bb7dcea2b92111c1c25e798c2d39e5637cc1615c318e618
 wayland-1.14.92.tar.xz
PGP:  https://wayland.freedesktop.org/releases/wayland-1.14.92.tar.xz.sig





signature.asc
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v4 4/4] simple-dmabuf-drm: support etnaviv drm as well

2018-03-19 Thread Derek Foreman

On 2018-03-19 11:45 AM, Guido Günther wrote:

Signed-off-by: Guido Günther 
---
  Makefile.am |  1 +
  clients/simple-dmabuf-drm.c | 77 +
  configure.ac|  5 ++-
  3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 69ca6cba..64a8006c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -629,6 +629,7 @@ nodist_weston_simple_dmabuf_drm_SOURCES =   \
  weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) 
$(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS)
  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
diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 22891639..0bc99a59 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -39,6 +39,7 @@
  #include 
  #include 
  #include 
+#include 
  
  #include 
  
@@ -49,6 +50,9 @@

  #ifdef HAVE_LIBDRM_FREEDRENO
  #include 
  #endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+#include 
+#endif
  #include 
  
  #include 

@@ -108,6 +112,10 @@ struct buffer {
struct fd_device *fd_dev;
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;
@@ -272,6 +280,65 @@ fd_device_destroy(struct buffer *buf)
fd_device_del(buf->fd_dev);
  }
  #endif /* HAVE_LIBDRM_FREEDRENO */
+#ifdef HAVE_LIBDRM_ETNAVIV
+
+static int
+etna_alloc_bo(struct buffer *buf)
+{
+   int flags = DRM_ETNA_GEM_CACHE_WC;
+   int size;
+
+   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   size =  buf->stride * buf->height;
+   buf->etna_dev = etna_device_new(buf->drm_fd);
+   buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
+
+   if (!buf->etna_bo)
+   return 0;
+   return 1;
+}
+
+static void
+etna_free_bo(struct buffer *buf)
+{
+   etna_bo_del(buf->etna_bo);
+}
+
+static int
+etna_bo_export_to_prime(struct buffer *buf)
+{
+   buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
+   if (buf->dmabuf_fd < 0)
+   return 1;
+
+   return 0;
+}
+
+static int
+etna_map_bo(struct buffer *buf)
+{
+   buf->mmap = etna_bo_map(buf->etna_bo);
+
+   if (buf->mmap != NULL)
+   return 1;
+
+   return 0;
+}
+
+static void
+etna_unmap_bo(struct buffer *buf)
+{
+   if (munmap(buf->mmap, buf->stride * buf->height) < 0)
+   fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));


Pekka had said something about cache flushing here in a previous 
comment, I think.  I think this munmap() is enough, but if it isn't I'd 
like to know what's required too.



+   buf->mmap = NULL;
+}
+
+static void
+etna_device_destroy(struct buffer *buf)
+{
+   etna_device_del(buf->etna_dev);
+}
+#endif /* HAVE_LIBDRM_ENTAVIV */
  
  static void

  fill_content(struct buffer *my_buf)
@@ -339,6 +406,16 @@ drm_device_init(struct buffer *buf)
dev->unmap_bo = fd_unmap_bo;
dev->device_destroy = fd_device_destroy;
}
+#endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+   else if (!strcmp(dev->name, "etnaviv")) {
+   dev->alloc_bo = etna_alloc_bo;
+   dev->free_bo = etna_free_bo;
+   dev->export_bo_to_prime = etna_bo_export_to_prime;
+   dev->map_bo = etna_map_bo;
+   dev->unmap_bo = etna_unmap_bo;
+   dev->device_destroy = etna_device_destroy;
+   }
  #endif
else {
fprintf(stderr, "Error: drm device %s unsupported.\n",
diff --git a/configure.ac b/configure.ac
index 90ffc88d..29926388 100644
--- a/configure.ac
+++ b/configure.ac
@@ -393,11 +393,14 @@ if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; 
then
PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
[true])
+  PKG_CHECK_MODULES(LIBDRM_PLATFORM_ETNAVIV, [libdrm_etnaviv],
+  AC_DEFINE([HAVE_LIBDRM_ETNAVIV], [1], [Build etnaviv dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
+  [have_etnaviv=no])


why the have_etnaviv=no here when the rest are all [true]?

With that explained, this is
Reviewed-by: Derek Foreman 

But I'm about to start the release process for the beta, so I think this 
will need to wait a while to land.


  
if test "x$have_simple_dmabuf_drm_client" != "xyes" -o \

  "x$have_simple_dmabuf_libs" = "xno" && \
   test "x$enable_simple_dmabuf_drm_client" = "xyes"; then
-AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or 
libdrm_freedreno not found])
+AC_MSG_ERROR([DRM dmabuf client explicitly 

Re: [PATCH v2] xwm: Update input region on resize

2018-03-19 Thread Scott Moreau
On Mon, Mar 19, 2018 at 1:13 PM, Derek Foreman 
wrote:

> On 2018-03-16 06:42 PM, Scott Moreau wrote:
>
>> Hi Pekka,
>>
>> On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen > > wrote:
>>
>> On Tue, 13 Mar 2018 21:22:04 -0600
>> 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.
>> >
>> > ---
>> >
>> > Changed in v2:
>> >
>> > - Bail in shape function if (window->override_redirect ||
>> !window->frame)
>> > - Call shape function from send_configure()
>> >
>> >  xwayland/window-manager.c | 53 +-
>> -
>> >  1 file changed, 33 insertions(+), 20 deletions(-)
>>
>> Hi,
>>
>> while trying to wrap my head around this, I started feeling dizzy. For
>> real. So I have to keep this short.
>>
>>
>> I think this is what happens when trying to sync two display servers.
>>
>>
>> The first decision we should make is, do we want a quick patch for an
>> immediate issue at hand, or do we want to make things better in the
>> long
>> run. Taking in this patch as is seems to be the former to me, and
>> given
>> the phase of the release cycle can be justified.
>>
>> The following mind flow is for a long term solution, and the comments
>> inlined with the code below are for the quick patch.
>>
>
> FWIW, I'd be open to landing something quick at this point.  The bug it
> fixes seems hugely annoying.  I resize an xterm, and I can't click in the
> new area.
>
> Alternately, I'm wondering if we should consider reverting the patch that
> introduced this bug?  I guess it wasn't tested well enough, and this
> behaviour seems more painful than what it was intended to fix?


I think a revert would be a good place to start. The patch also has
whitespace that does not match surrounding code.

To clarify the bug, start an xwayland window and resize it to a larger
dimension. Mouse input will only work for the size of the window before
resize. The rest of the window does not respond to input.

Thanks,
Scott


>
>
>
>> Taking a step back, the aim to keep the input shape up-to-date
>> whenever
>> something happens.
>>
>> If we have a frame window with decorations, then we call
>> frame_resize_inside() to adjust its size. Would it not be logical to
>> set the input shape in at least all those cases?
>>
>>
>> Yes, maybe there can be a function that calls frame_resize_inside and the
>> shape function to replace calls to frame_resize_inside.
>>
>>
>> Except it looks like we can have decorated O-R windows, and those
>> should be exempt? Oh, I'm told decorated O-R windows don't make sense,
>> but I see some code in weston that would only apply in such case...
>> if (window->override_redirect) { ... if (window->frame)
>> frame_resize_inside(...)
>>
>> All windows that go through map_request handler will get the frame
>> window (window->frame_id) and the frame (window->frame) created. The
>> only windows that don't get this treatment are therefore windows that
>> are O-R at the time of mapping them. It's somewhat complicated by the
>> fact that XWM does not support dynamically changing O-R flag... or
>> maybe it makes it easier.
>>
>> Now, we have configure_request and configure_notify handlers. O-R
>> windows will not hit configure_request handler, and the X server
>> processes XWM's configure commands immediately. This sounds like
>> configure_request handler would be a good place to update the input
>> shape.
>>
>>
>> Yes
>>
>>
>> configure_notify handler gets called for O-R as well, and it happens
>> after the fact, so updating there would be a tiny bit late. Would you
>> agree?
>>
>> I was thinking there might be some change that comes in configure notify
>> that we don't know about until the event happens.
>>
>>
>> That leaves the XWM originated resizes, which boils down to...
>> send_configure(), or actually weston_wm_window_configure()?
>>
>> Yes
>>
>>
>> It looks like configure_request handler is open-coding all of
>> weston_wm_window_configure() but it also adds some bits specific to
>> the
>> configure request.
>>
>> Am I making sense?
>>
>> Yes, and thanks for taking the time to try and help unravel this.
>>
>>
>>  > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
>>  > index c307e19..cd72a59 100644
>>  > --- a/xwayland/window-manager.c
>>  > +++ b/xwayland/window-manager.c
>>  > @@ -659,6 +659,33 @@ weston_wm_window_get_input_rect(struct
>> weston_wm_window *window,
>>  >  }
>>  

Re: [PATCH weston v4 2/4] simple-dmabuf-drm: use large enough buffer (freedreno)

2018-03-19 Thread Derek Foreman

On 2018-03-19 12:23 PM, Eric Engestrom wrote:

On Monday, 2018-03-19 17:45:19 +0100, Guido Günther wrote:

Use stride instead of width for buffer calculation.

Signed-off-by: Guido Günther 
---
  clients/simple-dmabuf-drm.c | 6 --
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index efe3b7f5..a184d7e7 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -222,13 +222,15 @@ fd_alloc_bo(struct buffer *buf)
  {
int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
int size = buf->width * buf->height * buf->bpp / 8;


You forgot to remove the initialisation here ^


And the commit log is weird, as it's been kept from a previous patch.

As both these problems seem trivial, I've fixed them while landing with 
my Reviewed-by.


Thanks,
Derek


-   buf->fd_dev = fd_device_new(buf->drm_fd);
  
+	buf->fd_dev = fd_device_new(buf->drm_fd);

+   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   size = buf->stride * buf->height;
+   buf->fd_dev = fd_device_new(buf->drm_fd);
buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
  
  	if (!buf->fd_bo)

return 0;
-   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
return 1;
  }
  
--

2.16.1

___
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



___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH v2] xwm: Update input region on resize

2018-03-19 Thread Derek Foreman

On 2018-03-16 06:42 PM, Scott Moreau wrote:

Hi Pekka,

On Fri, Mar 16, 2018 at 9:20 AM, Pekka Paalanen > wrote:


On Tue, 13 Mar 2018 21:22:04 -0600
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.
>
> ---
>
> Changed in v2:
>
> - Bail in shape function if (window->override_redirect || !window->frame)
> - Call shape function from send_configure()
>
>  xwayland/window-manager.c | 53 
+--
>  1 file changed, 33 insertions(+), 20 deletions(-)

Hi,

while trying to wrap my head around this, I started feeling dizzy. For
real. So I have to keep this short.


I think this is what happens when trying to sync two display servers.


The first decision we should make is, do we want a quick patch for an
immediate issue at hand, or do we want to make things better in the long
run. Taking in this patch as is seems to be the former to me, and given
the phase of the release cycle can be justified.

The following mind flow is for a long term solution, and the comments
inlined with the code below are for the quick patch.


FWIW, I'd be open to landing something quick at this point.  The bug it 
fixes seems hugely annoying.  I resize an xterm, and I can't click in 
the new area.


Alternately, I'm wondering if we should consider reverting the patch 
that introduced this bug?  I guess it wasn't tested well enough, and 
this behaviour seems more painful than what it was intended to fix?




Taking a step back, the aim to keep the input shape up-to-date whenever
something happens.

If we have a frame window with decorations, then we call
frame_resize_inside() to adjust its size. Would it not be logical to
set the input shape in at least all those cases?


Yes, maybe there can be a function that calls frame_resize_inside and 
the shape function to replace calls to frame_resize_inside.



Except it looks like we can have decorated O-R windows, and those
should be exempt? Oh, I'm told decorated O-R windows don't make sense,
but I see some code in weston that would only apply in such case...
if (window->override_redirect) { ... if (window->frame)
frame_resize_inside(...)

All windows that go through map_request handler will get the frame
window (window->frame_id) and the frame (window->frame) created. The
only windows that don't get this treatment are therefore windows that
are O-R at the time of mapping them. It's somewhat complicated by the
fact that XWM does not support dynamically changing O-R flag... or
maybe it makes it easier.

Now, we have configure_request and configure_notify handlers. O-R
windows will not hit configure_request handler, and the X server
processes XWM's configure commands immediately. This sounds like
configure_request handler would be a good place to update the input
shape.


Yes


configure_notify handler gets called for O-R as well, and it happens
after the fact, so updating there would be a tiny bit late. Would you
agree?

I was thinking there might be some change that comes in configure notify 
that we don't know about until the event happens.



That leaves the XWM originated resizes, which boils down to...
send_configure(), or actually weston_wm_window_configure()?

Yes


It looks like configure_request handler is open-coding all of
weston_wm_window_configure() but it also adds some bits specific to the
configure request.

Am I making sense?

Yes, and thanks for taking the time to try and help unravel this.


 > diff --git a/xwayland/window-manager.c b/xwayland/window-manager.c
 > index c307e19..cd72a59 100644
 > --- a/xwayland/window-manager.c
 > +++ b/xwayland/window-manager.c
 > @@ -659,6 +659,33 @@ 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;
 > +
 > +     if (window->override_redirect || !window->frame)
 > +             return;
 > +
 > +     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. */
 

Re: [PATCH weston v4 1/4] simple-dmabuf-drm: Always define ALIGN

2018-03-19 Thread Derek Foreman

On 2018-03-19 11:45 AM, Guido Günther wrote:

Other backends might want to use it.

Signed-off-by: Guido Günther 


Reviewed-by: Derek Foreman 
(and pushed)

Thanks,
Derek


---
  clients/simple-dmabuf-drm.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 4f26e4a9..efe3b7f5 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -65,6 +65,7 @@ struct buffer;
  #define OPT_Y_INVERTED 1  /* contents has y axis inverted */
  #define OPT_IMMEDIATE  2  /* create wl_buffer immediately */
  
+#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
  
  struct display {

struct wl_display *display;
@@ -215,7 +216,6 @@ intel_device_destroy(struct buffer *my_buf)
  
  #endif /* HAVE_LIBDRM_INTEL */

  #ifdef HAVE_LIBDRM_FREEDRENO
-#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
  
  static int

  fd_alloc_bo(struct buffer *buf)



___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v4 3/4] simple-dmabuf-drm: 0 is a valid fd (freedreno)

2018-03-19 Thread Eric Engestrom
On Monday, 2018-03-19 17:45:20 +0100, Guido Günther wrote:
> Signed-off-by: Guido Günther 

It is indeed :)
Reviewed-by: Eric Engestrom 

> ---
>  clients/simple-dmabuf-drm.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index a184d7e7..22891639 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -244,10 +244,10 @@ static int
>  fd_bo_export_to_prime(struct buffer *buf)
>  {
>   buf->dmabuf_fd = fd_bo_dmabuf(buf->fd_bo);
> - if (buf->dmabuf_fd > 0)
> - return 0;
> + if (buf->dmabuf_fd < 0)
> + return 1;
>  
> - return 1;
> + return 0;

Nit: could be simplified as `return buf->dmabuf_fd < 0;`

>  }
>  
>  static int
> -- 
> 2.16.1
> 
> ___
> 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: [PATCH weston v4 2/4] simple-dmabuf-drm: use large enough buffer (freedreno)

2018-03-19 Thread Eric Engestrom
On Monday, 2018-03-19 17:45:19 +0100, Guido Günther wrote:
> Use stride instead of width for buffer calculation.
> 
> Signed-off-by: Guido Günther 
> ---
>  clients/simple-dmabuf-drm.c | 6 --
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index efe3b7f5..a184d7e7 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -222,13 +222,15 @@ fd_alloc_bo(struct buffer *buf)
>  {
>   int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
>   int size = buf->width * buf->height * buf->bpp / 8;

You forgot to remove the initialisation here ^

> - buf->fd_dev = fd_device_new(buf->drm_fd);
>  
> + buf->fd_dev = fd_device_new(buf->drm_fd);
> + buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
> + size = buf->stride * buf->height;
> + buf->fd_dev = fd_device_new(buf->drm_fd);
>   buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
>  
>   if (!buf->fd_bo)
>   return 0;
> - buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
>   return 1;
>  }
>  
> -- 
> 2.16.1
> 
> ___
> 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: [PATCH wayland] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Derek Foreman

On 2018-03-19 11:20 AM, Eric Engestrom wrote:

On Monday, 2018-03-19 16:10:57 +, Daniel Stone wrote:

Hi,

On 19 March 2018 at 16:08, Eric Engestrom  wrote:

On Monday, 2018-03-19 15:13:14 +, Daniel Stone wrote:

+if ! test -f "$LIB"; then
+ echo "Test binary \"$LIB\" does not exist"
+ exit 99
+fi
+
+if ! test -n "$NM"; then
+ echo "nm environment variable not set"
+ exit 99


99? Were you looking for the "skip this test" 77?


I did mean 99 'some kind of inexplicable internal error happened'
rather than 77 skip, but I have no strong opinion on it and am happy
to change to whatever is suggested.


I guess "don't have the tools to test this, skipping" would be fine, but
I'm not really involved in the wayland project so my opinion isn't the
one that matters the most :P


Additional review is valuable, thanks. :)


"I have no strong feelings one way or the other"


In the absence of strong feelings, I've pushed this as-is.

Along with the recent version of the "pass nm path to check script" patch.

Thanks,
Derek



Cheers,
Daniel

___
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


[PATCH weston v4 4/4] simple-dmabuf-drm: support etnaviv drm as well

2018-03-19 Thread Guido Günther
Signed-off-by: Guido Günther 
---
 Makefile.am |  1 +
 clients/simple-dmabuf-drm.c | 77 +
 configure.ac|  5 ++-
 3 files changed, 82 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 69ca6cba..64a8006c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -629,6 +629,7 @@ nodist_weston_simple_dmabuf_drm_SOURCES =   \
 weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) 
$(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS)
 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
diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 22891639..0bc99a59 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -39,6 +39,7 @@
 #include 
 #include 
 #include 
+#include 
 
 #include 
 
@@ -49,6 +50,9 @@
 #ifdef HAVE_LIBDRM_FREEDRENO
 #include 
 #endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+#include 
+#endif
 #include 
 
 #include 
@@ -108,6 +112,10 @@ struct buffer {
struct fd_device *fd_dev;
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;
@@ -272,6 +280,65 @@ fd_device_destroy(struct buffer *buf)
fd_device_del(buf->fd_dev);
 }
 #endif /* HAVE_LIBDRM_FREEDRENO */
+#ifdef HAVE_LIBDRM_ETNAVIV
+
+static int
+etna_alloc_bo(struct buffer *buf)
+{
+   int flags = DRM_ETNA_GEM_CACHE_WC;
+   int size;
+
+   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   size =  buf->stride * buf->height;
+   buf->etna_dev = etna_device_new(buf->drm_fd);
+   buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
+
+   if (!buf->etna_bo)
+   return 0;
+   return 1;
+}
+
+static void
+etna_free_bo(struct buffer *buf)
+{
+   etna_bo_del(buf->etna_bo);
+}
+
+static int
+etna_bo_export_to_prime(struct buffer *buf)
+{
+   buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
+   if (buf->dmabuf_fd < 0)
+   return 1;
+
+   return 0;
+}
+
+static int
+etna_map_bo(struct buffer *buf)
+{
+   buf->mmap = etna_bo_map(buf->etna_bo);
+
+   if (buf->mmap != NULL)
+   return 1;
+
+   return 0;
+}
+
+static void
+etna_unmap_bo(struct buffer *buf)
+{
+   if (munmap(buf->mmap, buf->stride * buf->height) < 0)
+   fprintf(stderr, "Failed to unmap buffer: %s", strerror(errno));
+   buf->mmap = NULL;
+}
+
+static void
+etna_device_destroy(struct buffer *buf)
+{
+   etna_device_del(buf->etna_dev);
+}
+#endif /* HAVE_LIBDRM_ENTAVIV */
 
 static void
 fill_content(struct buffer *my_buf)
@@ -339,6 +406,16 @@ drm_device_init(struct buffer *buf)
dev->unmap_bo = fd_unmap_bo;
dev->device_destroy = fd_device_destroy;
}
+#endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+   else if (!strcmp(dev->name, "etnaviv")) {
+   dev->alloc_bo = etna_alloc_bo;
+   dev->free_bo = etna_free_bo;
+   dev->export_bo_to_prime = etna_bo_export_to_prime;
+   dev->map_bo = etna_map_bo;
+   dev->unmap_bo = etna_unmap_bo;
+   dev->device_destroy = etna_device_destroy;
+   }
 #endif
else {
fprintf(stderr, "Error: drm device %s unsupported.\n",
diff --git a/configure.ac b/configure.ac
index 90ffc88d..29926388 100644
--- a/configure.ac
+++ b/configure.ac
@@ -393,11 +393,14 @@ if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; 
then
   PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
   AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
   [true])
+  PKG_CHECK_MODULES(LIBDRM_PLATFORM_ETNAVIV, [libdrm_etnaviv],
+  AC_DEFINE([HAVE_LIBDRM_ETNAVIV], [1], [Build etnaviv dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
+  [have_etnaviv=no])
 
   if test "x$have_simple_dmabuf_drm_client" != "xyes" -o \
  "x$have_simple_dmabuf_libs" = "xno" && \
  test "x$enable_simple_dmabuf_drm_client" = "xyes"; then
-AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or 
libdrm_freedreno not found])
+AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but neither 
libdrm_intel, libdrm_freedreno or libdrm_etnaviv found])
   fi
 
   if test "x$have_simple_dmabuf_drm_client" = "xyes" -a 
"x$have_simple_dmabuf_libs" = "xyes"; then
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v4 0/4] simple-dmabuf-drm: Support etnaviv and freedreno cleanups

2018-03-19 Thread Guido Günther
Changes from v3:
- Use ALIGN macro by default
- Don't multiply by bpp twice
- unmap buffer

Former patches 1-3 where already applied. Thanks!

Changes from v2:
- Use stride to calculate buffer size (etnaviv)
- Use stride to calculate buffer size (freedreno)
- Use less generic name for ALIGN macro

Changes from v1:
- Split up changes
- autoconf:
  - don't define unused have_ variables. Use true instead to
prevent abort of PKG_CHECK_MODULES
  - use && instead of -a in test
  - properly check variables
- use vfunc for drm_device_destroy


Guido Günther (4):
  simple-dmabuf-drm: Always define ALIGN
  simple-dmabuf-drm: use large enough buffer (freedreno)
  simple-dmabuf-drm: 0 is a valid fd (freedreno)
  simple-dmabuf-drm: support etnaviv drm as well

 Makefile.am |  1 +
 clients/simple-dmabuf-drm.c | 91 ++---
 configure.ac|  5 ++-
 3 files changed, 90 insertions(+), 7 deletions(-)

-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v4 3/4] simple-dmabuf-drm: 0 is a valid fd (freedreno)

2018-03-19 Thread Guido Günther
Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index a184d7e7..22891639 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -244,10 +244,10 @@ static int
 fd_bo_export_to_prime(struct buffer *buf)
 {
buf->dmabuf_fd = fd_bo_dmabuf(buf->fd_bo);
-   if (buf->dmabuf_fd > 0)
-   return 0;
+   if (buf->dmabuf_fd < 0)
+   return 1;
 
-   return 1;
+   return 0;
 }
 
 static int
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v4 1/4] simple-dmabuf-drm: Always define ALIGN

2018-03-19 Thread Guido Günther
Other backends might want to use it.

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 4f26e4a9..efe3b7f5 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -65,6 +65,7 @@ struct buffer;
 #define OPT_Y_INVERTED 1  /* contents has y axis inverted */
 #define OPT_IMMEDIATE  2  /* create wl_buffer immediately */
 
+#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
 
 struct display {
struct wl_display *display;
@@ -215,7 +216,6 @@ intel_device_destroy(struct buffer *my_buf)
 
 #endif /* HAVE_LIBDRM_INTEL */
 #ifdef HAVE_LIBDRM_FREEDRENO
-#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
 
 static int
 fd_alloc_bo(struct buffer *buf)
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v4 2/4] simple-dmabuf-drm: use large enough buffer (freedreno)

2018-03-19 Thread Guido Günther
Use stride instead of width for buffer calculation.

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index efe3b7f5..a184d7e7 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -222,13 +222,15 @@ fd_alloc_bo(struct buffer *buf)
 {
int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
int size = buf->width * buf->height * buf->bpp / 8;
-   buf->fd_dev = fd_device_new(buf->drm_fd);
 
+   buf->fd_dev = fd_device_new(buf->drm_fd);
+   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   size = buf->stride * buf->height;
+   buf->fd_dev = fd_device_new(buf->drm_fd);
buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
 
if (!buf->fd_bo)
return 0;
-   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
return 1;
 }
 
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Eric Engestrom
On Monday, 2018-03-19 16:10:57 +, Daniel Stone wrote:
> Hi,
> 
> On 19 March 2018 at 16:08, Eric Engestrom  wrote:
> > On Monday, 2018-03-19 15:13:14 +, Daniel Stone wrote:
> >> +if ! test -f "$LIB"; then
> >> + echo "Test binary \"$LIB\" does not exist"
> >> + exit 99
> >> +fi
> >> +
> >> +if ! test -n "$NM"; then
> >> + echo "nm environment variable not set"
> >> + exit 99
> >
> > 99? Were you looking for the "skip this test" 77?
> 
> I did mean 99 'some kind of inexplicable internal error happened'
> rather than 77 skip, but I have no strong opinion on it and am happy
> to change to whatever is suggested.

I guess "don't have the tools to test this, skipping" would be fine, but
I'm not really involved in the wayland project so my opinion isn't the
one that matters the most :P

"I have no strong feelings one way or the other"

> 
> Cheers,
> Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Daniel Stone
Hi,

On 19 March 2018 at 16:08, Eric Engestrom  wrote:
> On Monday, 2018-03-19 15:13:14 +, Daniel Stone wrote:
>> +if ! test -f "$LIB"; then
>> + echo "Test binary \"$LIB\" does not exist"
>> + exit 99
>> +fi
>> +
>> +if ! test -n "$NM"; then
>> + echo "nm environment variable not set"
>> + exit 99
>
> 99? Were you looking for the "skip this test" 77?

I did mean 99 'some kind of inexplicable internal error happened'
rather than 77 skip, but I have no strong opinion on it and am happy
to change to whatever is suggested.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Eric Engestrom
On Monday, 2018-03-19 15:13:14 +, Daniel Stone wrote:
> The previous rewrite of the wayland-egl ABI checker introduced checks
> for removed symbols as well as added symbols, but broke some failure
> conditions. Add an explict return-code variable set in failure paths,
> rather than chaining or conditions.
> 
> If we cannot find the binary or nm, we regard this as an error
> condition, rather than test failure.
> 
> v2: Don't test if we can execute $NM.
> 
> Signed-off-by: Daniel Stone 
> Reported-by: Pekka Paalanen 
> Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
> Cc: Emil Velikov 
> ---
>  egl/wayland-egl-symbols-check | 27 +--
>  1 file changed, 21 insertions(+), 6 deletions(-)
> 
> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
> index c47026b2..70fe1f4c 100755
> --- a/egl/wayland-egl-symbols-check
> +++ b/egl/wayland-egl-symbols-check
> @@ -1,11 +1,17 @@
>  #!/bin/sh
>  set -eu
>  
> +RET=0
>  LIB=${WAYLAND_EGL_LIB}
>  
> -if [ ! -f "$LIB" ]; then
> - echo "The test binary \"$LIB\" does no exist"
> - exit 1
> +if ! test -f "$LIB"; then
> + echo "Test binary \"$LIB\" does not exist"
> + exit 99
> +fi
> +
> +if ! test -n "$NM"; then
> + echo "nm environment variable not set"
> + exit 99

99? Were you looking for the "skip this test" 77?

>  fi
>  
>  AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
> @@ -32,7 +38,11 @@ NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
>  echo $func
>  done)
>  
> -test ! -n "$NEW_ABI" || echo "New ABI detected - If intentional, update the 
> test."; echo "$NEW_ABI"
> +if test -n "$NEW_ABI"; then
> + echo "New ABI detected - If intentional, update the test."
> + echo "$NEW_ABI"
> + RET=1
> +fi
>  
>  REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
>  echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
> @@ -40,5 +50,10 @@ REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
>  echo $func
>  done)
>  
> -test ! -n "$REMOVED_ABI" || echo "ABI break detected - Required symbol(s) no 
> longer exported!"; echo "$REMOVED_ABI"
> -test ! -n "$NEW_ABI" || test ! -n "$REMOVED_ABI"
> +if test -n "$REMOVED_ABI"; then
> + echo "ABI break detected - Required symbol(s) no longer exported!"
> + echo "$REMOVED_ABI"
> + RET=1
> +fi
> +
> +exit $RET
> -- 
> 2.16.2
> 
> ___
> 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: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Emil Velikov
On 19 March 2018 at 13:39, Daniel Stone  wrote:
> Hi Emil,
>
> On 19 March 2018 at 13:27, Emil Velikov  wrote:
>> On 19 March 2018 at 13:20, Daniel Stone  wrote:
>>> Me neither really, but it seemed best for consistency with the rest of
>>> the file which used test rather than [.
>>
>> Sounds fine either way - but the "test ||" -> "if test" changes seems 
>> spurious.
>> If they stand out so much, guess one could have pointed it out?
>
> Not so much spurious as just broken? The final line, as written, will
> only exit if _both_ added and removed are non-empty. If one but not
> the other is non-empty, then the test will exit success[0].
>
Right s/||/&&/ should address that

> I tried to think of a rewrite which would work, but in the end decided
> making it explicit was the least error-prone thing to do, since this
> one managed to slip past review with no-one noticing.
>
Fair point.

Thanks!
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Daniel Stone
The previous rewrite of the wayland-egl ABI checker introduced checks
for removed symbols as well as added symbols, but broke some failure
conditions. Add an explict return-code variable set in failure paths,
rather than chaining or conditions.

If we cannot find the binary or nm, we regard this as an error
condition, rather than test failure.

v2: Don't test if we can execute $NM.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Cc: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index c47026b2..70fe1f4c 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,11 +1,17 @@
 #!/bin/sh
 set -eu
 
+RET=0
 LIB=${WAYLAND_EGL_LIB}
 
-if [ ! -f "$LIB" ]; then
-   echo "The test binary \"$LIB\" does no exist"
-   exit 1
+if ! test -f "$LIB"; then
+   echo "Test binary \"$LIB\" does not exist"
+   exit 99
+fi
+
+if ! test -n "$NM"; then
+   echo "nm environment variable not set"
+   exit 99
 fi
 
 AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
@@ -32,7 +38,11 @@ NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
 echo $func
 done)
 
-test ! -n "$NEW_ABI" || echo "New ABI detected - If intentional, update the 
test."; echo "$NEW_ABI"
+if test -n "$NEW_ABI"; then
+   echo "New ABI detected - If intentional, update the test."
+   echo "$NEW_ABI"
+   RET=1
+fi
 
 REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
 echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
@@ -40,5 +50,10 @@ REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
 echo $func
 done)
 
-test ! -n "$REMOVED_ABI" || echo "ABI break detected - Required symbol(s) no 
longer exported!"; echo "$REMOVED_ABI"
-test ! -n "$NEW_ABI" || test ! -n "$REMOVED_ABI"
+if test -n "$REMOVED_ABI"; then
+   echo "ABI break detected - Required symbol(s) no longer exported!"
+   echo "$REMOVED_ABI"
+   RET=1
+fi
+
+exit $RET
-- 
2.16.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Simon McVittie
On Mon, 19 Mar 2018 at 13:39:30 +, Daniel Stone wrote:
> On 19 March 2018 at 13:27, Emil Velikov  wrote:
> >> I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe
> >> replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it
> >> works if it's non-empty is fine too.
> >>
> > I'd go with non-empty - everything else seems like an overkill.

If you ever want the more full version for something else, I think

command -v "$command" >/dev/null

is the canonical way to ask a POSIX shell "could I invoke $command?".
It exits successfully if $command is a function, an alias, a shell
built-in, the basename of an executable in $PATH, an executable found
by relative or absolute path, an alias or a shell reserved word.

smcv
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston v3 1/3] simple-dmabuf-drm: support etnaviv drm as well

2018-03-19 Thread Pekka Paalanen
On Mon, 19 Mar 2018 13:38:11 +0100
Guido Günther  wrote:

> Signed-off-by: Guido Günther 
> ---
>  Makefile.am |  1 +
>  clients/simple-dmabuf-drm.c | 74 
> +
>  configure.ac|  5 ++-
>  3 files changed, 79 insertions(+), 1 deletion(-)
> 

Hi,

I see freedreno has the same problems as mentioned below.

> diff --git a/Makefile.am b/Makefile.am
> index 69ca6cba..64a8006c 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -629,6 +629,7 @@ nodist_weston_simple_dmabuf_drm_SOURCES = \
>  weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) 
> $(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS)
>  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
> diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
> index 4f26e4a9..174d0f85 100644
> --- a/clients/simple-dmabuf-drm.c
> +++ b/clients/simple-dmabuf-drm.c
> @@ -49,6 +49,9 @@
>  #ifdef HAVE_LIBDRM_FREEDRENO
>  #include 
>  #endif
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +#include 
> +#endif
>  #include 
>  
>  #include 
> @@ -107,6 +110,10 @@ struct buffer {
>   struct fd_device *fd_dev;
>   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;
> @@ -270,6 +277,63 @@ fd_device_destroy(struct buffer *buf)
>   fd_device_del(buf->fd_dev);
>  }
>  #endif /* HAVE_LIBDRM_FREEDRENO */
> +#ifdef HAVE_LIBDRM_ETNAVIV
> +#define ETNA_ALIGN(v, a) ((v + a - 1) & ~(a - 1))

This is the same as freedreno has, you can just move it outside of the
#ifdef block. I think this is really the expected form of an ALIGN
macro. If something else needs to compute something else, it will use a
macro named something else.

> +
> +static int
> +etna_alloc_bo(struct buffer *buf)
> +{
> + int flags = DRM_ETNA_GEM_CACHE_WC;
> + int size;
> +
> + buf->stride = ETNA_ALIGN(buf->width, 32) * buf->bpp / 8;
> + size =  buf->stride * buf->height * buf->bpp / 8;

Oops, multiplying twice by the bytes-per-pixel.

The next patch introduces the same bug to freedreno path.

Stride must be in bytes already, at least that is how fill_content()
and create_dmabuf_buffer() use it, so the multiplication for size by
bytes-per-pixel is too much.

> + buf->etna_dev = etna_device_new(buf->drm_fd);
> + buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
> +
> + if (!buf->etna_bo)
> + return 0;
> + return 1;
> +}
> +
> +static void
> +etna_free_bo(struct buffer *buf)
> +{
> + etna_bo_del(buf->etna_bo);
> +}
> +
> +static int
> +etna_bo_export_to_prime(struct buffer *buf)
> +{
> + buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
> + if (buf->dmabuf_fd > 0)

Zero is a valid file descriptor, it should be allowed.

> + return 0;
> +
> + return 1;
> +}
> +
> +static int
> +etna_map_bo(struct buffer *buf)
> +{
> + buf->mmap = etna_bo_map(buf->etna_bo);
> +
> + if (buf->mmap != NULL)
> + return 1;
> +
> + return 0;
> +}
> +
> +static void
> +etna_unmap_bo(struct buffer *buf)
> +{

Missing implementation. Isn't this missing a memory flush after CPU
write access?

> +}
> +
> +static void
> +etna_device_destroy(struct buffer *buf)
> +{
> + etna_device_del(buf->etna_dev);
> +}
> +#endif /* HAVE_LIBDRM_ENTAVIV */
>  
>  static void
>  fill_content(struct buffer *my_buf)
> @@ -337,6 +401,16 @@ drm_device_init(struct buffer *buf)
>   dev->unmap_bo = fd_unmap_bo;
>   dev->device_destroy = fd_device_destroy;
>   }
> +#endif
> +#ifdef HAVE_LIBDRM_ETNAVIV
> + else if (!strcmp(dev->name, "etnaviv")) {
> + dev->alloc_bo = etna_alloc_bo;
> + dev->free_bo = etna_free_bo;
> + dev->export_bo_to_prime = etna_bo_export_to_prime;
> + dev->map_bo = etna_map_bo;
> + dev->unmap_bo = etna_unmap_bo;
> + dev->device_destroy = etna_device_destroy;
> + }
>  #endif
>   else {
>   fprintf(stderr, "Error: drm device %s unsupported.\n",
> diff --git a/configure.ac b/configure.ac
> index 90ffc88d..29926388 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -393,11 +393,14 @@ if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; 
> then
>PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
>AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
> have_simple_dmabuf_drm_client=yes,
>[true])
> +  PKG_CHECK_MODULES(LIBDRM_PLATFORM_ETNAVIV, [libdrm_etnaviv],
> +  AC_DEFINE([HAVE_LIBDRM_ETNAVIV], [1], [Build etnaviv dmabuf client]) 
> have_simple_dmabuf_drm_client=yes,
> +  [have_etnaviv=no])
>  
>if test 

Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Daniel Stone
Hi Emil,

On 19 March 2018 at 13:27, Emil Velikov  wrote:
> On 19 March 2018 at 13:20, Daniel Stone  wrote:
>> Me neither really, but it seemed best for consistency with the rest of
>> the file which used test rather than [.
>
> Sounds fine either way - but the "test ||" -> "if test" changes seems 
> spurious.
> If they stand out so much, guess one could have pointed it out?

Not so much spurious as just broken? The final line, as written, will
only exit if _both_ added and removed are non-empty. If one but not
the other is non-empty, then the test will exit success[0].

I tried to think of a rewrite which would work, but in the end decided
making it explicit was the least error-prone thing to do, since this
one managed to slip past review with no-one noticing.

 +if ! test -x "$NM"; then
 +   echo "nm binary \"$NM\" does not exist"
 +   exit 99
   fi
>>>
>>> Here, however, you are introducing an -x test, which is not good for all the
>>> people (including packagers) that set NM to e.g. -nm (so not the
>>> full path).
>>
>> I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe
>> replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it
>> works if it's non-empty is fine too.
>>
> I'd go with non-empty - everything else seems like an overkill.

Sure, I'll respin.

Cheers,
Daniel

[0]:
strictly:~% cat foo.sh
#!/bin/sh -e

ADDED_YES="added"
ADDED_NO=""
REMOVED_YES="removed"
REMOVED_NO=""

test ! -n "$ADDED_NO" || test ! -n "$REMOVED_YES"
echo "no-yes passed"

test ! -n "$ADDED_YES" || test ! -n "$REMOVED_NO"
echo "yes-no passed"

test ! -n "$ADDED_NO" || test ! -n "$REMOVED_NO"
echo "both-no passed"

test ! -n "$ADDED_YES" || test ! -n "$REMOVED_YES"
echo "both-yes passed"
strictly:~% ./foo.sh
no-yes passed
yes-no passed
both-no passed
zsh: exit 1 ./foo.sh
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Emil Velikov
On 19 March 2018 at 13:20, Daniel Stone  wrote:
> Hi Quentin,
> Thanks for the quick review!
>
> On 19 March 2018 at 13:06, Quentin Glidic
>  wrote:
>> On 3/19/18 1:31 PM, Daniel Stone wrote:
>>> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>>> index c47026b2..d1a4a6be 100755
>>> --- a/egl/wayland-egl-symbols-check
>>> +++ b/egl/wayland-egl-symbols-check
>>> @@ -1,11 +1,17 @@
>>>   #!/bin/sh
>>>   set -eu
>>>   +RET=0
>>>   LIB=${WAYLAND_EGL_LIB}
>>>   -if [ ! -f "$LIB" ]; then
>>> -   echo "The test binary \"$LIB\" does no exist"
>>> -   exit 1
>>> +if ! test -f "$LIB"; then
>>> +   echo "Test binary \"$LIB\" does not exist"
>>> +   exit 99
>>> +fi
>>
>> Not a big fan of replacing [!-f] with !test-f but it’s a cosmetic issue so
>> not a big deal.
>
> Me neither really, but it seemed best for consistency with the rest of
> the file which used test rather than [.
>
Sounds fine either way - but the "test ||" -> "if test" changes seems spurious.
If they stand out so much, guess one could have pointed it out?

>>> +if ! test -x "$NM"; then
>>> +   echo "nm binary \"$NM\" does not exist"
>>> +   exit 99
>>>   fi
>>
>> Here, however, you are introducing an -x test, which is not good for all the
>> people (including packagers) that set NM to e.g. -nm (so not the
>> full path).
>
> I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe
> replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it
> works if it's non-empty is fine too.
>
I'd go with non-empty - everything else seems like an overkill.

HTH
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Daniel Stone
Hi Quentin,
Thanks for the quick review!

On 19 March 2018 at 13:06, Quentin Glidic
 wrote:
> On 3/19/18 1:31 PM, Daniel Stone wrote:
>> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> index c47026b2..d1a4a6be 100755
>> --- a/egl/wayland-egl-symbols-check
>> +++ b/egl/wayland-egl-symbols-check
>> @@ -1,11 +1,17 @@
>>   #!/bin/sh
>>   set -eu
>>   +RET=0
>>   LIB=${WAYLAND_EGL_LIB}
>>   -if [ ! -f "$LIB" ]; then
>> -   echo "The test binary \"$LIB\" does no exist"
>> -   exit 1
>> +if ! test -f "$LIB"; then
>> +   echo "Test binary \"$LIB\" does not exist"
>> +   exit 99
>> +fi
>
> Not a big fan of replacing [!-f] with !test-f but it’s a cosmetic issue so
> not a big deal.

Me neither really, but it seemed best for consistency with the rest of
the file which used test rather than [.

>> +if ! test -x "$NM"; then
>> +   echo "nm binary \"$NM\" does not exist"
>> +   exit 99
>>   fi
>
> Here, however, you are introducing an -x test, which is not good for all the
> people (including packagers) that set NM to e.g. -nm (so not the
> full path).

I'd not realised AC_PROG_NM didn't set a full path. Brilliant. Maybe
replace test -x "$NM" with $NM -V >/dev/null 2>&1? Or just trusting it
works if it's non-empty is fine too.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Quentin Glidic

On 3/19/18 1:31 PM, Daniel Stone wrote:

The previous rewrite of the wayland-egl ABI checker introduced checks
for removed symbols as well as added symbols, but broke some failure
conditions. Add an explict return-code variable set in failure paths,
rather than chaining or conditions.

If we cannot find the binary or nm, we regard this as an error
condition, rather than test failure.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Cc: Emil Velikov 
---
  egl/wayland-egl-symbols-check | 27 +--
  1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index c47026b2..d1a4a6be 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,11 +1,17 @@
  #!/bin/sh
  set -eu
  
+RET=0

  LIB=${WAYLAND_EGL_LIB}
  
-if [ ! -f "$LIB" ]; then

-   echo "The test binary \"$LIB\" does no exist"
-   exit 1
+if ! test -f "$LIB"; then
+   echo "Test binary \"$LIB\" does not exist"
+   exit 99
+fi


Not a big fan of replacing [!-f] with !test-f but it’s a cosmetic issue 
so not a big deal.




+
+if ! test -x "$NM"; then
+   echo "nm binary \"$NM\" does not exist"
+   exit 99
  fi


Here, however, you are introducing an -x test, which is not good for all 
the people (including packagers) that set NM to e.g. -nm (so not 
the full path).




  AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"


Maybe just checking for -n "$NM" then -n "$AVAIL_FUNCS" would be enough?


Cheers,



@@ -32,7 +38,11 @@ NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
  echo $func
  done)
  
-test ! -n "$NEW_ABI" || echo "New ABI detected - If intentional, update the test."; echo "$NEW_ABI"

+if test -n "$NEW_ABI"; then
+   echo "New ABI detected - If intentional, update the test."
+   echo "$NEW_ABI"
+   RET=1
+fi
  
  REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do

  echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
@@ -40,5 +50,10 @@ REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
  echo $func
  done)
  
-test ! -n "$REMOVED_ABI" || echo "ABI break detected - Required symbol(s) no longer exported!"; echo "$REMOVED_ABI"

-test ! -n "$NEW_ABI" || test ! -n "$REMOVED_ABI"
+if test -n "$REMOVED_ABI"; then
+   echo "ABI break detected - Required symbol(s) no longer exported!"
+   echo "$REMOVED_ABI"
+   RET=1
+fi
+
+exit $RET




--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland 1/2] wayland-egl: Pass nm path to check script

2018-03-19 Thread Quentin Glidic

On 3/19/18 1:31 PM, Daniel Stone wrote:

A previous patch used $NM as an environment variable, but this was only
set as a make variable. Make sure it is passed through from make to the
environment we use to run tests.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
Fixes: 6903e4d53925 ("wayland-egl: use correct `nm` path when cross-compiling")
Cc: Emil Velikov 


Straightforward:
Reviewed-by: Quentin Glidic 

Thanks,


---
  Makefile.am | 1 +
  1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index 6f59c369..514468aa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -198,6 +198,7 @@ AM_TESTS_ENVIRONMENT =  
\
TEST_OUTPUT_DIR='$(top_builddir)/tests/output'  \
WAYLAND_EGL_LIB='$(top_builddir)/.libs/libwayland-egl.so'   \
SED=$(SED)  \
+   NM=$(NM)\
;
  
  TESTS = $(built_test_programs)			\





--

Quentin “Sardem FF7” Glidic
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v3 3/3] simple-dmabuf-drm: name ALIGN FD_ALIGN

2018-03-19 Thread Guido Günther
Other backends might have different alignment requirements and since we
can have several backends we want a less generic name.

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 08e794c8..2779712e 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -222,7 +222,7 @@ intel_device_destroy(struct buffer *my_buf)
 
 #endif /* HAVE_LIBDRM_INTEL */
 #ifdef HAVE_LIBDRM_FREEDRENO
-#define ALIGN(v, a) ((v + a - 1) & ~(a - 1))
+#define FD_ALIGN(v, a) ((v + a - 1) & ~(a - 1))
 
 static int
 fd_alloc_bo(struct buffer *buf)
@@ -230,7 +230,7 @@ fd_alloc_bo(struct buffer *buf)
int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
int size = buf->width * buf->height * buf->bpp / 8;
 
-   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   buf->stride = FD_ALIGN(buf->width, 32) * buf->bpp / 8;
size = buf->stride * buf->height * buf->bpp / 8;
buf->fd_dev = fd_device_new(buf->drm_fd);
buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v3 2/3] simple-dmabuf-drm: use large enough buffer for freedreno

2018-03-19 Thread Guido Günther
Use stride instead of width fur buffer calculation.

Signed-off-by: Guido Günther 
---
 clients/simple-dmabuf-drm.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 174d0f85..08e794c8 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -229,13 +229,14 @@ fd_alloc_bo(struct buffer *buf)
 {
int flags = DRM_FREEDRENO_GEM_CACHE_WCOMBINE;
int size = buf->width * buf->height * buf->bpp / 8;
-   buf->fd_dev = fd_device_new(buf->drm_fd);
 
+   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
+   size = buf->stride * buf->height * buf->bpp / 8;
+   buf->fd_dev = fd_device_new(buf->drm_fd);
buf->fd_bo = fd_bo_new(buf->fd_dev, size, flags);
 
if (!buf->fd_bo)
return 0;
-   buf->stride = ALIGN(buf->width, 32) * buf->bpp / 8;
return 1;
 }
 
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v3 1/3] simple-dmabuf-drm: support etnaviv drm as well

2018-03-19 Thread Guido Günther
Signed-off-by: Guido Günther 
---
 Makefile.am |  1 +
 clients/simple-dmabuf-drm.c | 74 +
 configure.ac|  5 ++-
 3 files changed, 79 insertions(+), 1 deletion(-)

diff --git a/Makefile.am b/Makefile.am
index 69ca6cba..64a8006c 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -629,6 +629,7 @@ nodist_weston_simple_dmabuf_drm_SOURCES =   \
 weston_simple_dmabuf_drm_CFLAGS = $(AM_CFLAGS) 
$(SIMPLE_DMABUF_DRM_CLIENT_CFLAGS)
 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
diff --git a/clients/simple-dmabuf-drm.c b/clients/simple-dmabuf-drm.c
index 4f26e4a9..174d0f85 100644
--- a/clients/simple-dmabuf-drm.c
+++ b/clients/simple-dmabuf-drm.c
@@ -49,6 +49,9 @@
 #ifdef HAVE_LIBDRM_FREEDRENO
 #include 
 #endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+#include 
+#endif
 #include 
 
 #include 
@@ -107,6 +110,10 @@ struct buffer {
struct fd_device *fd_dev;
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;
@@ -270,6 +277,63 @@ fd_device_destroy(struct buffer *buf)
fd_device_del(buf->fd_dev);
 }
 #endif /* HAVE_LIBDRM_FREEDRENO */
+#ifdef HAVE_LIBDRM_ETNAVIV
+#define ETNA_ALIGN(v, a) ((v + a - 1) & ~(a - 1))
+
+static int
+etna_alloc_bo(struct buffer *buf)
+{
+   int flags = DRM_ETNA_GEM_CACHE_WC;
+   int size;
+
+   buf->stride = ETNA_ALIGN(buf->width, 32) * buf->bpp / 8;
+   size =  buf->stride * buf->height * buf->bpp / 8;
+   buf->etna_dev = etna_device_new(buf->drm_fd);
+   buf->etna_bo = etna_bo_new(buf->etna_dev, size, flags);
+
+   if (!buf->etna_bo)
+   return 0;
+   return 1;
+}
+
+static void
+etna_free_bo(struct buffer *buf)
+{
+   etna_bo_del(buf->etna_bo);
+}
+
+static int
+etna_bo_export_to_prime(struct buffer *buf)
+{
+   buf->dmabuf_fd = etna_bo_dmabuf(buf->etna_bo);
+   if (buf->dmabuf_fd > 0)
+   return 0;
+
+   return 1;
+}
+
+static int
+etna_map_bo(struct buffer *buf)
+{
+   buf->mmap = etna_bo_map(buf->etna_bo);
+
+   if (buf->mmap != NULL)
+   return 1;
+
+   return 0;
+}
+
+static void
+etna_unmap_bo(struct buffer *buf)
+{
+}
+
+static void
+etna_device_destroy(struct buffer *buf)
+{
+   etna_device_del(buf->etna_dev);
+}
+#endif /* HAVE_LIBDRM_ENTAVIV */
 
 static void
 fill_content(struct buffer *my_buf)
@@ -337,6 +401,16 @@ drm_device_init(struct buffer *buf)
dev->unmap_bo = fd_unmap_bo;
dev->device_destroy = fd_device_destroy;
}
+#endif
+#ifdef HAVE_LIBDRM_ETNAVIV
+   else if (!strcmp(dev->name, "etnaviv")) {
+   dev->alloc_bo = etna_alloc_bo;
+   dev->free_bo = etna_free_bo;
+   dev->export_bo_to_prime = etna_bo_export_to_prime;
+   dev->map_bo = etna_map_bo;
+   dev->unmap_bo = etna_unmap_bo;
+   dev->device_destroy = etna_device_destroy;
+   }
 #endif
else {
fprintf(stderr, "Error: drm device %s unsupported.\n",
diff --git a/configure.ac b/configure.ac
index 90ffc88d..29926388 100644
--- a/configure.ac
+++ b/configure.ac
@@ -393,11 +393,14 @@ if ! test "x$enable_simple_dmabuf_drm_client" = "xno"; 
then
   PKG_CHECK_MODULES(LIBDRM_PLATFORM_INTEL, [libdrm_intel],
   AC_DEFINE([HAVE_LIBDRM_INTEL], [1], [Build intel dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
   [true])
+  PKG_CHECK_MODULES(LIBDRM_PLATFORM_ETNAVIV, [libdrm_etnaviv],
+  AC_DEFINE([HAVE_LIBDRM_ETNAVIV], [1], [Build etnaviv dmabuf client]) 
have_simple_dmabuf_drm_client=yes,
+  [have_etnaviv=no])
 
   if test "x$have_simple_dmabuf_drm_client" != "xyes" -o \
  "x$have_simple_dmabuf_libs" = "xno" && \
  test "x$enable_simple_dmabuf_drm_client" = "xyes"; then
-AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but libdrm_intel or 
libdrm_freedreno not found])
+AC_MSG_ERROR([DRM dmabuf client explicitly enabled, but neither 
libdrm_intel, libdrm_freedreno or libdrm_etnaviv found])
   fi
 
   if test "x$have_simple_dmabuf_drm_client" = "xyes" -a 
"x$have_simple_dmabuf_libs" = "xyes"; then
-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH weston v3 0/3] simple-dmabuf-drm: Support etnaviv (and other cleanups)

2018-03-19 Thread Guido Günther
Former patches 1-3 where already applied. Thanks!

Changes from v2:
- Use stride to calculate buffer size (etnaviv)
- Use stride to calculate buffer size (freedreno)
- Use less generic name for ALIGN macro

Changes from v1:
- Split up changes
- autoconf:
  - don't define unused have_ variables. Use true instead to
prevent abort of PKG_CHECK_MODULES
  - use && instead of -a in test
  - properly check variables
- use vfunc for drm_device_destroy


Guido Günther (3):
  simple-dmabuf-drm: support etnaviv drm as well
  simple-dmabuf-drm: use large enough buffer for freedreno
  simple-dmabuf-drm: name ALIGN FD_ALIGN

 Makefile.am |  1 +
 clients/simple-dmabuf-drm.c | 81 +++--
 configure.ac|  5 ++-
 3 files changed, 83 insertions(+), 4 deletions(-)

-- 
2.16.1

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH libinput] meson: point users to disabled documentation when dot/doxygen is missing

2018-03-19 Thread Peter Hutterer
Especially dot is hard to find for some users, so provide the solution to
their problems right there in the error message.

And because users are likely to just copy/paste, remove the disable-libwacom
option. Save them from themselves...

Signed-off-by: Peter Hutterer 
---
 doc/building.dox |  5 -
 meson.build  | 11 +--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/doc/building.dox b/doc/building.dox
index 8e302e0d..ef7793b0 100644
--- a/doc/building.dox
+++ b/doc/building.dox
@@ -28,9 +28,12 @@ $> sudo udevadm hwdb --update
 
 Additional options may also be specified. For example:
 @code
-$> meson --prefix=/usr -Ddocumentation=false -Dlibwacom=false builddir/
+$> meson --prefix=/usr -Ddocumentation=false builddir/
 @endcode
 
+We recommend that users disable the documentation, it's not usually required
+for testing and reduces the number of dependencies needed.
+
 The ```prefix``` or other options can be changed later with the
 ```mesonconf``` command. For example:
 @code
diff --git a/meson.build b/meson.build
index 58ef31a4..fcd3d8c4 100644
--- a/meson.build
+++ b/meson.build
@@ -254,7 +254,15 @@ 
meson.add_install_script('src/libinput-restore-selinux-context.sh',
  documentation 
 
 if get_option('documentation')
-   doxygen = find_program('doxygen')
+   doxygen = find_program('doxygen', required : false)
+   if not doxygen.found()
+   error('Program "doxygen" not found or not executable. Try 
building with -Ddocumentation=false')
+   endif
+   dot = find_program('dot', required : false)
+   if not dot.found()
+   error('Program "dot" not found or not executable. Try building 
with -Ddocumentation=false')
+   endif
+
doxygen_version_cmd = run_command(doxygen.path(), '--version')
if doxygen_version_cmd.returncode() != 0
error('Command "doxygen --version" failed.')
@@ -263,7 +271,6 @@ if get_option('documentation')
if doxygen_version.version_compare('< 1.8.3')
error('doxygen needs to be at least version 1.8.3 (have 
@0@)'.format(doxygen_version))
endif
-   dot = find_program('dot')
grep = find_program('grep')
dot_version_cmd = run_command(dot.path(), '-V')
if dot_version_cmd.returncode() != 0
-- 
2.14.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-19 Thread Daniel Stone
Hi,

On 19 March 2018 at 11:43, Emil Velikov  wrote:
> On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
>> On Fri, 16 Mar 2018 16:18:57 +
>> Emil Velikov  wrote:
>>> Right - I was aiming to remove the bonkers envvar and forgot about
>>> this preexisting bug.
>>> Patch (to be applied before the series) coming in a second.
>>
>> In egl/wayland-egl-symbols-check.log I have:
>>
>> ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
>>
> Seems like a 'fun' side-effect of the $NM patch. The series predates
> the patch by a week.
> First suggestion that comes to mind - use $(NM-nm}
>
> Eric any suggestions?

The root cause is that NM is only ever set as a make variable (by
AC_PROG_NM); it's never actually added to the shell-based test
environment.

Thanks for catching this, Pekka - I just noted that the tests
succeeded, and didn't think to check the failure conditions. :\ I've
sent out two patches now, one fixing NM and one fixing the failure to
fail.

Cheers,
Daniel
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 1/2] wayland-egl: Pass nm path to check script

2018-03-19 Thread Daniel Stone
A previous patch used $NM as an environment variable, but this was only
set as a make variable. Make sure it is passed through from make to the
environment we use to run tests.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
Fixes: 6903e4d53925 ("wayland-egl: use correct `nm` path when cross-compiling")
Cc: Emil Velikov 
---
 Makefile.am | 1 +
 1 file changed, 1 insertion(+)

diff --git a/Makefile.am b/Makefile.am
index 6f59c369..514468aa 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -198,6 +198,7 @@ AM_TESTS_ENVIRONMENT =  
\
TEST_OUTPUT_DIR='$(top_builddir)/tests/output'  \
WAYLAND_EGL_LIB='$(top_builddir)/.libs/libwayland-egl.so'   \
SED=$(SED)  \
+   NM=$(NM)\
;
 
 TESTS = $(built_test_programs) \
-- 
2.16.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[PATCH wayland 2/2] wayland-egl: Make symbol test fail on failure

2018-03-19 Thread Daniel Stone
The previous rewrite of the wayland-egl ABI checker introduced checks
for removed symbols as well as added symbols, but broke some failure
conditions. Add an explict return-code variable set in failure paths,
rather than chaining or conditions.

If we cannot find the binary or nm, we regard this as an error
condition, rather than test failure.

Signed-off-by: Daniel Stone 
Reported-by: Pekka Paalanen 
Fixes: 21b1f22eb056 ("wayland-egl: enhance the symbol test")
Cc: Emil Velikov 
---
 egl/wayland-egl-symbols-check | 27 +--
 1 file changed, 21 insertions(+), 6 deletions(-)

diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
index c47026b2..d1a4a6be 100755
--- a/egl/wayland-egl-symbols-check
+++ b/egl/wayland-egl-symbols-check
@@ -1,11 +1,17 @@
 #!/bin/sh
 set -eu
 
+RET=0
 LIB=${WAYLAND_EGL_LIB}
 
-if [ ! -f "$LIB" ]; then
-   echo "The test binary \"$LIB\" does no exist"
-   exit 1
+if ! test -f "$LIB"; then
+   echo "Test binary \"$LIB\" does not exist"
+   exit 99
+fi
+
+if ! test -x "$NM"; then
+   echo "nm binary \"$NM\" does not exist"
+   exit 99
 fi
 
 AVAIL_FUNCS="$($NM -D --format=bsd --defined-only $LIB | awk '{print $3}')"
@@ -32,7 +38,11 @@ NEW_ABI=$(echo "$AVAIL_FUNCS" | while read func; do
 echo $func
 done)
 
-test ! -n "$NEW_ABI" || echo "New ABI detected - If intentional, update the 
test."; echo "$NEW_ABI"
+if test -n "$NEW_ABI"; then
+   echo "New ABI detected - If intentional, update the test."
+   echo "$NEW_ABI"
+   RET=1
+fi
 
 REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
 echo "$AVAIL_FUNCS" | grep -q "^$func$" && continue
@@ -40,5 +50,10 @@ REMOVED_ABI=$(echo "$REQ_FUNCS" | while read func; do
 echo $func
 done)
 
-test ! -n "$REMOVED_ABI" || echo "ABI break detected - Required symbol(s) no 
longer exported!"; echo "$REMOVED_ABI"
-test ! -n "$NEW_ABI" || test ! -n "$REMOVED_ABI"
+if test -n "$REMOVED_ABI"; then
+   echo "ABI break detected - Required symbol(s) no longer exported!"
+   echo "$REMOVED_ABI"
+   RET=1
+fi
+
+exit $RET
-- 
2.16.2

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-19 Thread Eric Engestrom
On Monday, 2018-03-19 12:11:50 +, Eric Engestrom wrote:
> On Monday, 2018-03-19 11:43:08 +, Emil Velikov wrote:
> > On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
> > > On Fri, 16 Mar 2018 16:18:57 +
> > > Emil Velikov  wrote:
> > >
> > >> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> > >> > On Thu, 15 Mar 2018 14:30:27 +
> > >> > Emil Velikov  wrote:
> > >> >
> > >> >> From: Emil Velikov 
> > >> >>
> > >> >> Based on a similar patch (in Mesa) by Eric Engestrom.
> > >> >>
> > >> >> v2: Rebase on top of $NM patch
> > >> >> v3: Rebase
> > >> >>
> > >> >> Reviewed-by: Eric Engestrom  (v1)
> > >> >> Signed-off-by: Emil Velikov 
> > >> >> ---
> > >> >>  egl/wayland-egl-symbols-check | 10 +-
> > >> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> > >> >>
> > >> >> diff --git a/egl/wayland-egl-symbols-check 
> > >> >> b/egl/wayland-egl-symbols-check
> > >> >> index 6ad28f3..8b3d711 100755
> > >> >> --- a/egl/wayland-egl-symbols-check
> > >> >> +++ b/egl/wayland-egl-symbols-check
> > >> >> @@ -1,6 +1,14 @@
> > >> >>  #!/bin/sh
> > >> >> +set -eu
> > >> >>
> > >> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | 
> > >> >> cut -c 3- | while read func; do
> > >> >> +LIB=${WAYLAND_EGL_LIB}
> > >> >> +
> > >> >> +if [ ! -f "$LIB" ]; then
> > >> >> + echo "The test binary \"$LIB\" does no exist"
> > >> >> + exit 1
> > >> >> +fi
> > >> >> +
> > >> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | 
> > >> >> while read func; do
> > >> >>  ( grep -q "^$func$" || echo $func )  < > >> >>  wl_egl_window_resize
> > >> >>  wl_egl_window_create
> > >> >
> > >> > Hi Emil,
> > >> >
> > >> > this patch makes distcheck fail with:
> > >> >
> > >> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
> > >> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
> > >> >
> > >> Right - I was aiming to remove the bonkers envvar and forgot about
> > >> this preexisting bug.
> > >> Patch (to be applied before the series) coming in a second.
> > >
> > > Hi Emil,
> > >
> > > because this set of four patches no longer regresses, and I think they
> > > very much need to be in the release, I have pushed them:
> > >f34af17..5f5945b  master -> master
> > >
> > > However, I did a quick test: I renamed wl_egl_window_resize() into
> > > wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> > > symbol not present, extra symbol. The test suite still passes with
> > > success.
> > >
> > > In egl/wayland-egl-symbols-check.log I have:
> > >
> > > ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
> > >
> > Seems like a 'fun' side-effect of the $NM patch. The series predates
> > the patch by a week.
> > First suggestion that comes to mind - use $(NM-nm}
> 
> yeah that would work, but I'm confused: what's the setup, why is NM
> undefined? Both autotools and meson should be defining NM.

Oh hold on, just as I sent the email I noticed this isn't Mesa, this is
Wayland...

I haven't followed wayland dev for a few months (maybe a year if I'm
honest), I've heard there was a move to meson but I think it hasn't
landed in master?

I guess my question remains though: what's the setup, ie how is the
check script being called? autotools? meson? other?

> 
> > 
> > Eric any suggestions?
> > 
> > Thanks
> > Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-19 Thread Eric Engestrom
On Monday, 2018-03-19 11:43:08 +, Emil Velikov wrote:
> On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
> > On Fri, 16 Mar 2018 16:18:57 +
> > Emil Velikov  wrote:
> >
> >> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> >> > On Thu, 15 Mar 2018 14:30:27 +
> >> > Emil Velikov  wrote:
> >> >
> >> >> From: Emil Velikov 
> >> >>
> >> >> Based on a similar patch (in Mesa) by Eric Engestrom.
> >> >>
> >> >> v2: Rebase on top of $NM patch
> >> >> v3: Rebase
> >> >>
> >> >> Reviewed-by: Eric Engestrom  (v1)
> >> >> Signed-off-by: Emil Velikov 
> >> >> ---
> >> >>  egl/wayland-egl-symbols-check | 10 +-
> >> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >> >>
> >> >> diff --git a/egl/wayland-egl-symbols-check 
> >> >> b/egl/wayland-egl-symbols-check
> >> >> index 6ad28f3..8b3d711 100755
> >> >> --- a/egl/wayland-egl-symbols-check
> >> >> +++ b/egl/wayland-egl-symbols-check
> >> >> @@ -1,6 +1,14 @@
> >> >>  #!/bin/sh
> >> >> +set -eu
> >> >>
> >> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | 
> >> >> cut -c 3- | while read func; do
> >> >> +LIB=${WAYLAND_EGL_LIB}
> >> >> +
> >> >> +if [ ! -f "$LIB" ]; then
> >> >> + echo "The test binary \"$LIB\" does no exist"
> >> >> + exit 1
> >> >> +fi
> >> >> +
> >> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | 
> >> >> while read func; do
> >> >>  ( grep -q "^$func$" || echo $func )  < >> >>  wl_egl_window_resize
> >> >>  wl_egl_window_create
> >> >
> >> > Hi Emil,
> >> >
> >> > this patch makes distcheck fail with:
> >> >
> >> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
> >> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
> >> >
> >> Right - I was aiming to remove the bonkers envvar and forgot about
> >> this preexisting bug.
> >> Patch (to be applied before the series) coming in a second.
> >
> > Hi Emil,
> >
> > because this set of four patches no longer regresses, and I think they
> > very much need to be in the release, I have pushed them:
> >f34af17..5f5945b  master -> master
> >
> > However, I did a quick test: I renamed wl_egl_window_resize() into
> > wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> > symbol not present, extra symbol. The test suite still passes with
> > success.
> >
> > In egl/wayland-egl-symbols-check.log I have:
> >
> > ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
> >
> Seems like a 'fun' side-effect of the $NM patch. The series predates
> the patch by a week.
> First suggestion that comes to mind - use $(NM-nm}

yeah that would work, but I'm confused: what's the setup, why is NM
undefined? Both autotools and meson should be defining NM.

> 
> Eric any suggestions?
> 
> Thanks
> Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-19 Thread Emil Velikov
On 19 March 2018 at 09:56, Pekka Paalanen  wrote:
> On Fri, 16 Mar 2018 16:18:57 +
> Emil Velikov  wrote:
>
>> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
>> > On Thu, 15 Mar 2018 14:30:27 +
>> > Emil Velikov  wrote:
>> >
>> >> From: Emil Velikov 
>> >>
>> >> Based on a similar patch (in Mesa) by Eric Engestrom.
>> >>
>> >> v2: Rebase on top of $NM patch
>> >> v3: Rebase
>> >>
>> >> Reviewed-by: Eric Engestrom  (v1)
>> >> Signed-off-by: Emil Velikov 
>> >> ---
>> >>  egl/wayland-egl-symbols-check | 10 +-
>> >>  1 file changed, 9 insertions(+), 1 deletion(-)
>> >>
>> >> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
>> >> index 6ad28f3..8b3d711 100755
>> >> --- a/egl/wayland-egl-symbols-check
>> >> +++ b/egl/wayland-egl-symbols-check
>> >> @@ -1,6 +1,14 @@
>> >>  #!/bin/sh
>> >> +set -eu
>> >>
>> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut 
>> >> -c 3- | while read func; do
>> >> +LIB=${WAYLAND_EGL_LIB}
>> >> +
>> >> +if [ ! -f "$LIB" ]; then
>> >> + echo "The test binary \"$LIB\" does no exist"
>> >> + exit 1
>> >> +fi
>> >> +
>> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
>> >> read func; do
>> >>  ( grep -q "^$func$" || echo $func )  <> >>  wl_egl_window_resize
>> >>  wl_egl_window_create
>> >
>> > Hi Emil,
>> >
>> > this patch makes distcheck fail with:
>> >
>> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
>> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
>> >
>> Right - I was aiming to remove the bonkers envvar and forgot about
>> this preexisting bug.
>> Patch (to be applied before the series) coming in a second.
>
> Hi Emil,
>
> because this set of four patches no longer regresses, and I think they
> very much need to be in the release, I have pushed them:
>f34af17..5f5945b  master -> master
>
> However, I did a quick test: I renamed wl_egl_window_resize() into
> wl_egl_window_resize_uu() to try and trigger several kinds of errors:
> symbol not present, extra symbol. The test suite still passes with
> success.
>
> In egl/wayland-egl-symbols-check.log I have:
>
> ./egl/wayland-egl-symbols-check: line 11: NM: unbound variable
>
Seems like a 'fun' side-effect of the $NM patch. The series predates
the patch by a week.
First suggestion that comes to mind - use $(NM-nm}

Eric any suggestions?

Thanks
Emil
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC] weston-launch: add option to start compositors other than weston

2018-03-19 Thread Pekka Paalanen
On Mon, 19 Mar 2018 12:33:27 +0200
Ilia Bozhinov  wrote:

> Oops, sorry, I didn't consider that and I should have. Mods should remove
> this patch.

Hi,

if you mean mark it as rejected in Patchwork, I've now done that. If
you wanted to make a login with Patchwork, you could have done that
yourself as well.

> But anyway, does that mean every compositor should provide its own version
> of weston-launch or require systemd-login?

That is the current state of things, yes.

There might be a solution for sharing weston-launch, but no-one has
pushed for it very hard yet.


Thanks,
pq


pgpM9auLmkih2.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC] weston-launch: add option to start compositors other than weston

2018-03-19 Thread Ilia Bozhinov
Oops, sorry, I didn't consider that and I should have. Mods should remove
this patch.

But anyway, does that mean every compositor should provide its own version
of weston-launch or require systemd-login?

On Mon, Mar 19, 2018 at 12:24 PM, Pekka Paalanen 
wrote:

> On Mon, 19 Mar 2018 11:41:46 +0200
> Ilia Bozhinov  wrote:
>
> > This is helpful for other compositors which utilize libweston without
> > systemd-login support.
> >
> > Signed-off-by: Ilia Bozhinov 
> > ---
> >  libweston/weston-launch.c | 35 +--
> >  1 file changed, 21 insertions(+), 14 deletions(-)
> >
> > diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c
> > index 1adcf21a..b8bceea2 100644
> > --- a/libweston/weston-launch.c
> > +++ b/libweston/weston-launch.c
> > @@ -116,6 +116,7 @@ struct weston_launch {
> >   pid_t child;
> >   int verbose;
> >   char *new_user;
> > + char *compositor_cmd;
> >  };
> >
> >  union cmsg_data { unsigned char b[4]; int fd; };
> > @@ -624,7 +625,7 @@ setup_session(struct weston_launch *wl, char
> **child_argv)
> >   child_argv[0] = "/bin/sh";
> >   child_argv[1] = "-l";
> >   child_argv[2] = "-c";
> > - child_argv[3] = BINDIR "/weston \"$@\"";
> > + child_argv[3] = strcat(wl->compositor_cmd ?: BINDIR "/weston",
> "\"$@\"");
> >   child_argv[4] = "weston";
> >   return 5;
> >  }
> > @@ -652,7 +653,7 @@ launch_compositor(struct weston_launch *wl, int
> argc, char *argv[])
> >   if (wl->new_user) {
> >   o = setup_session(wl, child_argv);
> >   } else {
> > - child_argv[0] = BINDIR "/weston";
> > + child_argv[0] = wl->compositor_cmd ?: BINDIR "/weston";
> >   o = 1;
> >   }
> >   for (i = 0; i < argc; ++i)
> > @@ -683,12 +684,14 @@ static void
> >  help(const char *name)
> >  {
> >   fprintf(stderr, "Usage: %s [args...] [-- [weston args..]]\n",
> name);
> > - fprintf(stderr, "  -u, --user  Start session as specified
> username,\n"
> > - "  e.g. -u joe, requires root.\n");
> > - fprintf(stderr, "  -t, --tty   Start session on alternative
> tty,\n"
> > - "  e.g. -t /dev/tty4, requires -u
> option.\n");
> > - fprintf(stderr, "  -v, --verbose   Be verbose\n");
> > - fprintf(stderr, "  -h, --help  Display this help message\n");
> > + fprintf(stderr, "  -u, --user   Start session as specified
> username,\n"
> > + "   e.g. -u joe, requires
> root.\n");
> > + fprintf(stderr, "  -t, --ttyStart session on alternative
> tty,\n"
> > + "   e.g. -t /dev/tty4, requires -u
> option.\n");
> > + fprintf(stderr, "  -c, --compositor Start a compositor other than
> weston,\n"
> > + "   e.g. -c /usr/bin/weston.\n");
> > + fprintf(stderr, "  -v, --verboseBe verbose\n");
> > + fprintf(stderr, "  -h, --help   Display this help message\n");
> >  }
>
> Hi,
>
> I do not think we can do this. weston-launch is a setuid-root program,
> which gives the program it launches special privileges to e.g. open
> input devices. If we do not restrict the possible programs it can
> launch, anyone who can run weston-launch will be able to spy on all
> input devices by using weston-launch to run a spy program.
>
> If we had a trusted list of compositor binaries in trusted system
> paths (a la /etc/shells), then that might work, but I don't trust
> myself enough to say it would be a secure solution.
>
>
> Thanks,
> pq
>
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH weston 0/3] Allow simple-dmabuf-drm to pass y_inverted flag and cleanups

2018-03-19 Thread Guido Günther
On Mon, Mar 19, 2018 at 12:12:36PM +0200, Pekka Paalanen wrote:
> On Fri, 16 Mar 2018 18:56:47 +0100
> Guido Günther  wrote:
> 
> > Changes from v2
> > 
> >  - Explain OPT_* usage
> >  - Switch is_immediate to OPT_*
> >  - use getopt_long
> > 
> > Changes from v1
> > 
> >  - introduce opts as a bitmask of features that can be used to
> >manipulate the displayed image
> > 
> > Guido Günther (3):
> >   Allow simple-dmabuf-drm to pass y_inverted flag
> >   simple-dmabuf-drm: use opt bitmask instead of is_immediate
> >   simple-dmabuf-drm: use getopt_long
> > 
> >  clients/simple-dmabuf-drm.c | 76 
> > -
> >  1 file changed, 47 insertions(+), 29 deletions(-)
> > 
> 
> Hi,
> 
> all three pushed:
>63fcad48..60970ec2  master -> master


> For review, it would have been easier to convert to getopt_long first
> and then add new options, to avoid reviewing code that gets removed
> immediately after.

Thanks a lot for your review!
 -- Guido
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [RFC] weston-launch: add option to start compositors other than weston

2018-03-19 Thread Pekka Paalanen
On Mon, 19 Mar 2018 11:41:46 +0200
Ilia Bozhinov  wrote:

> This is helpful for other compositors which utilize libweston without
> systemd-login support.
> 
> Signed-off-by: Ilia Bozhinov 
> ---
>  libweston/weston-launch.c | 35 +--
>  1 file changed, 21 insertions(+), 14 deletions(-)
> 
> diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c
> index 1adcf21a..b8bceea2 100644
> --- a/libweston/weston-launch.c
> +++ b/libweston/weston-launch.c
> @@ -116,6 +116,7 @@ struct weston_launch {
>   pid_t child;
>   int verbose;
>   char *new_user;
> + char *compositor_cmd;
>  };
>  
>  union cmsg_data { unsigned char b[4]; int fd; };
> @@ -624,7 +625,7 @@ setup_session(struct weston_launch *wl, char **child_argv)
>   child_argv[0] = "/bin/sh";
>   child_argv[1] = "-l";
>   child_argv[2] = "-c";
> - child_argv[3] = BINDIR "/weston \"$@\"";
> + child_argv[3] = strcat(wl->compositor_cmd ?: BINDIR "/weston", 
> "\"$@\"");
>   child_argv[4] = "weston";
>   return 5;
>  }
> @@ -652,7 +653,7 @@ launch_compositor(struct weston_launch *wl, int argc, 
> char *argv[])
>   if (wl->new_user) {
>   o = setup_session(wl, child_argv);
>   } else {
> - child_argv[0] = BINDIR "/weston";
> + child_argv[0] = wl->compositor_cmd ?: BINDIR "/weston";
>   o = 1;
>   }
>   for (i = 0; i < argc; ++i)
> @@ -683,12 +684,14 @@ static void
>  help(const char *name)
>  {
>   fprintf(stderr, "Usage: %s [args...] [-- [weston args..]]\n", name);
> - fprintf(stderr, "  -u, --user  Start session as specified 
> username,\n"
> - "  e.g. -u joe, requires root.\n");
> - fprintf(stderr, "  -t, --tty   Start session on alternative tty,\n"
> - "  e.g. -t /dev/tty4, requires -u 
> option.\n");
> - fprintf(stderr, "  -v, --verbose   Be verbose\n");
> - fprintf(stderr, "  -h, --help  Display this help message\n");
> + fprintf(stderr, "  -u, --user   Start session as specified 
> username,\n"
> + "   e.g. -u joe, requires root.\n");
> + fprintf(stderr, "  -t, --ttyStart session on alternative tty,\n"
> + "   e.g. -t /dev/tty4, requires -u 
> option.\n");
> + fprintf(stderr, "  -c, --compositor Start a compositor other than 
> weston,\n"
> + "   e.g. -c /usr/bin/weston.\n");
> + fprintf(stderr, "  -v, --verboseBe verbose\n");
> + fprintf(stderr, "  -h, --help   Display this help message\n");
>  }

Hi,

I do not think we can do this. weston-launch is a setuid-root program,
which gives the program it launches special privileges to e.g. open
input devices. If we do not restrict the possible programs it can
launch, anyone who can run weston-launch will be able to spy on all
input devices by using weston-launch to run a spy program.

If we had a trusted list of compositor binaries in trusted system
paths (a la /etc/shells), then that might work, but I don't trust
myself enough to say it would be a secure solution.


Thanks,
pq


pgp0LpIOU0qyD.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] Fix an uninitialized variable

2018-03-19 Thread Pekka Paalanen
On Sun, 18 Mar 2018 00:20:29 -0400
Dima Ryazanov  wrote:

> "has_discrete" gets set to true in if/else if, but gets left unset otherwise.
> So let's initialize it to false.
> 
> (This was caught by valgrind.)
> 
> Signed-off-by: Dima Ryazanov 
> ---
>  libweston/compositor-wayland.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/libweston/compositor-wayland.c b/libweston/compositor-wayland.c
> index f51f78dd..111c4c09 100644
> --- a/libweston/compositor-wayland.c
> +++ b/libweston/compositor-wayland.c
> @@ -1707,6 +1707,7 @@ input_handle_axis(void *data, struct wl_pointer 
> *pointer,
>  
>   weston_event.axis = axis;
>   weston_event.value = wl_fixed_to_double(value);
> + weston_event.has_discrete = false;
>  
>   if (axis == WL_POINTER_AXIS_VERTICAL_SCROLL &&
>   input->vert.has_discrete) {

Nice, pushed:
   60970ec2..6b2fb180  master -> master

Thanks,
pq


pgpuceRui3xsK.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 0/3] Allow simple-dmabuf-drm to pass y_inverted flag and cleanups

2018-03-19 Thread Pekka Paalanen
On Fri, 16 Mar 2018 18:56:47 +0100
Guido Günther  wrote:

> Changes from v2
> 
>  - Explain OPT_* usage
>  - Switch is_immediate to OPT_*
>  - use getopt_long
> 
> Changes from v1
> 
>  - introduce opts as a bitmask of features that can be used to
>manipulate the displayed image
> 
> Guido Günther (3):
>   Allow simple-dmabuf-drm to pass y_inverted flag
>   simple-dmabuf-drm: use opt bitmask instead of is_immediate
>   simple-dmabuf-drm: use getopt_long
> 
>  clients/simple-dmabuf-drm.c | 76 
> -
>  1 file changed, 47 insertions(+), 29 deletions(-)
> 

Hi,

all three pushed:
   63fcad48..60970ec2  master -> master

For review, it would have been easier to convert to getopt_long first
and then add new options, to avoid reviewing code that gets removed
immediately after.


Thanks,
pq


pgpPiNB1s7pFP.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland v3 1/3] wayland-egl: fail the symbol check if lib is missing

2018-03-19 Thread Pekka Paalanen
On Fri, 16 Mar 2018 16:18:57 +
Emil Velikov  wrote:

> On 16 March 2018 at 13:52, Pekka Paalanen  wrote:
> > On Thu, 15 Mar 2018 14:30:27 +
> > Emil Velikov  wrote:
> >  
> >> From: Emil Velikov 
> >>
> >> Based on a similar patch (in Mesa) by Eric Engestrom.
> >>
> >> v2: Rebase on top of $NM patch
> >> v3: Rebase
> >>
> >> Reviewed-by: Eric Engestrom  (v1)
> >> Signed-off-by: Emil Velikov 
> >> ---
> >>  egl/wayland-egl-symbols-check | 10 +-
> >>  1 file changed, 9 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/egl/wayland-egl-symbols-check b/egl/wayland-egl-symbols-check
> >> index 6ad28f3..8b3d711 100755
> >> --- a/egl/wayland-egl-symbols-check
> >> +++ b/egl/wayland-egl-symbols-check
> >> @@ -1,6 +1,14 @@
> >>  #!/bin/sh
> >> +set -eu
> >>
> >> -FUNCS=$($NM -D --defined-only ${WAYLAND_EGL_LIB} | grep -o "T .*" | cut 
> >> -c 3- | while read func; do
> >> +LIB=${WAYLAND_EGL_LIB}
> >> +
> >> +if [ ! -f "$LIB" ]; then
> >> + echo "The test binary \"$LIB\" does no exist"
> >> + exit 1
> >> +fi
> >> +
> >> +FUNCS=$($NM -D --defined-only $LIB | grep -o "T .*" | cut -c 3- | while 
> >> read func; do
> >>  ( grep -q "^$func$" || echo $func )  < >>  wl_egl_window_resize
> >>  wl_egl_window_create  
> >
> > Hi Emil,
> >
> > this patch makes distcheck fail with:
> >
> > The test binary "./egl/.libs/libwayland-egl.so" does no exist
> > FAIL egl/wayland-egl-symbols-check (exit status: 1)
> >  
> Right - I was aiming to remove the bonkers envvar and forgot about
> this preexisting bug.
> Patch (to be applied before the series) coming in a second.

Hi Emil,

because this set of four patches no longer regresses, and I think they
very much need to be in the release, I have pushed them:
   f34af17..5f5945b  master -> master

However, I did a quick test: I renamed wl_egl_window_resize() into
wl_egl_window_resize_uu() to try and trigger several kinds of errors:
symbol not present, extra symbol. The test suite still passes with
success.

In egl/wayland-egl-symbols-check.log I have:

./egl/wayland-egl-symbols-check: line 11: NM: unbound variable

ABI break detected - Required symbol(s) no longer exported!
wl_egl_window_resize
wl_egl_window_create
wl_egl_window_destroy
wl_egl_window_get_attached_size
PASS egl/wayland-egl-symbols-check (exit status: 0)

So it seems 'set -u' is not quite effective, and when ABI breaks are
detected, they don't make the test failed.


Thanks,
pq


pgpa5QQ5FYWV5.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland] wayland-egl: set the correct path to libwayland-egl.so

2018-03-19 Thread Pekka Paalanen
On Fri, 16 Mar 2018 16:14:54 +
Emil Velikov  wrote:

> From: Emil Velikov 
> 
> Earlier commit changed to passing the binary name as env. variable
> introducing a typo.
> 
> That went unnoticed, since we do not check if the file is present or
> not.
> 
> Cc: Pukka Paalanen 
> Cc: Daniel Stone 
> Fixes: 85cb5ed64aa ("wayland-egl-symbols-check: pass the DSO name via
> the build system")
> Signed-off-by: Emil Velikov 
> ---
>  Makefile.am | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Makefile.am b/Makefile.am
> index 2731ee7..6f59c36 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -196,7 +196,7 @@ AM_TESTS_ENVIRONMENT =
> \
>   export WAYLAND_SCANNER='$(top_builddir)/wayland-scanner'\
>   TEST_DATA_DIR='$(top_srcdir)/tests/data'\
>   TEST_OUTPUT_DIR='$(top_builddir)/tests/output'  \
> - WAYLAND_EGL_LIB='$(top_builddir)/egl/.libs/libwayland-egl.so'   \
> + WAYLAND_EGL_LIB='$(top_builddir)/.libs/libwayland-egl.so'   \
>   SED=$(SED)  \
>   ;
>  

Pushed:
   f34af17..5f5945b  master -> master


Thanks,
pq


pgp6mrkWFwv4P.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


[RFC] weston-launch: add option to start compositors other than weston

2018-03-19 Thread Ilia Bozhinov
This is helpful for other compositors which utilize libweston without
systemd-login support.

Signed-off-by: Ilia Bozhinov 
---
 libweston/weston-launch.c | 35 +--
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/libweston/weston-launch.c b/libweston/weston-launch.c
index 1adcf21a..b8bceea2 100644
--- a/libweston/weston-launch.c
+++ b/libweston/weston-launch.c
@@ -116,6 +116,7 @@ struct weston_launch {
pid_t child;
int verbose;
char *new_user;
+   char *compositor_cmd;
 };
 
 union cmsg_data { unsigned char b[4]; int fd; };
@@ -624,7 +625,7 @@ setup_session(struct weston_launch *wl, char **child_argv)
child_argv[0] = "/bin/sh";
child_argv[1] = "-l";
child_argv[2] = "-c";
-   child_argv[3] = BINDIR "/weston \"$@\"";
+   child_argv[3] = strcat(wl->compositor_cmd ?: BINDIR "/weston", 
"\"$@\"");
child_argv[4] = "weston";
return 5;
 }
@@ -652,7 +653,7 @@ launch_compositor(struct weston_launch *wl, int argc, char 
*argv[])
if (wl->new_user) {
o = setup_session(wl, child_argv);
} else {
-   child_argv[0] = BINDIR "/weston";
+   child_argv[0] = wl->compositor_cmd ?: BINDIR "/weston";
o = 1;
}
for (i = 0; i < argc; ++i)
@@ -683,12 +684,14 @@ static void
 help(const char *name)
 {
fprintf(stderr, "Usage: %s [args...] [-- [weston args..]]\n", name);
-   fprintf(stderr, "  -u, --user  Start session as specified 
username,\n"
-   "  e.g. -u joe, requires root.\n");
-   fprintf(stderr, "  -t, --tty   Start session on alternative tty,\n"
-   "  e.g. -t /dev/tty4, requires -u 
option.\n");
-   fprintf(stderr, "  -v, --verbose   Be verbose\n");
-   fprintf(stderr, "  -h, --help  Display this help message\n");
+   fprintf(stderr, "  -u, --user   Start session as specified 
username,\n"
+   "   e.g. -u joe, requires root.\n");
+   fprintf(stderr, "  -t, --ttyStart session on alternative tty,\n"
+   "   e.g. -t /dev/tty4, requires -u 
option.\n");
+   fprintf(stderr, "  -c, --compositor Start a compositor other than 
weston,\n"
+   "   e.g. -c /usr/bin/weston.\n");
+   fprintf(stderr, "  -v, --verboseBe verbose\n");
+   fprintf(stderr, "  -h, --help   Display this help message\n");
 }
 
 int
@@ -698,16 +701,17 @@ main(int argc, char *argv[])
int i, c;
char *tty = NULL;
struct option opts[] = {
-   { "user",required_argument, NULL, 'u' },
-   { "tty", required_argument, NULL, 't' },
-   { "verbose", no_argument,   NULL, 'v' },
-   { "help",no_argument,   NULL, 'h' },
-   { 0, 0, NULL,  0  }
+   { "user",   required_argument, NULL, 'u' },
+   { "tty",required_argument, NULL, 't' },
+   { "verbose",no_argument,   NULL, 'v' },
+   { "compositor", required_argument, NULL, 'c' },
+   { "help",   no_argument,   NULL, 'h' },
+   { 0,0, NULL,  0  }
};
 
memset(, 0, sizeof wl);
 
-   while ((c = getopt_long(argc, argv, "u:t:vh", opts, )) != -1) {
+   while ((c = getopt_long(argc, argv, "u:t:c:vh", opts, )) != -1) {
switch (c) {
case 'u':
wl.new_user = optarg;
@@ -720,6 +724,9 @@ main(int argc, char *argv[])
case 'v':
wl.verbose = 1;
break;
+   case 'c':
+   wl.compositor_cmd = optarg;
+   break;
case 'h':
help("weston-launch");
exit(EXIT_FAILURE);
-- 
2.14.3

___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


Re: [PATCH wayland-protcols v3] unstable: add xdg-toplevel-decoration protocol

2018-03-19 Thread Pekka Paalanen
On Sun, 18 Mar 2018 14:59:58 -0400
Simon Ser  wrote:

> On March 16, 2018 1:22 PM, Pekka Paalanen  wrote:
> > > > I'm missing a comment that describes what happens if the xdg_toplevel is
> > > > destroyed. There is some object dependency here that needs to be 
> > > > stated. Do
> > > > we need an event here? Or are we assuming that clients are smart enough 
> > > > to
> > > > keep track of destroy events. Either way - needs to be explicitly 
> > > > stated.
> > > 
> > > What about requiring clients to destroy the zxdg_toplevel_decoration_v1 
> > > before
> > > the xdg_toplevel?  
> > 
> > Hi,
> > 
> > you can, but then you need an error code for clients that get it wrong.
> > 
> > A common convention is to instead say that zxdg_toplevel_decoration_v1
> > becomes "inert" when the associated xdg_toplevel is destroyed. It means
> > the object will not send any events, and all requests except destroy
> > will be ignored. This approach is often used with racy cases where
> > requiring a single order of actions is not possible, and it comes with
> > the disadvantage of needing to define what happens if you pass an inert
> > object of this type as an argument in some other interface, or if the
> > inert object had requests that create more objects.  
> 
> Thanks for the explanation!
> 
> > Both ways are possible. You may want to check how xdg_toplevel deals
> > with xdg_surface destruction and how xdg_surface deals with wl_surface
> > destruction. Maybe consistency would be the best plan?  
> 
> The protocol states that: "An xdg_surface must only be destroyed after its 
> role
> object has been destroyed". But it doesn't define any error type for protocol
> violations. I didn't find anything about the relationship between xdg_surface
> and xdg_surface.
> 
> So I'm not sure what to do to be consistent with xdg-shell. The options are:
> - Become inert
> - Send an error on the decoration object
> - Automatically destroying the decoration when the toplevel is destroyed?
> 
> I'll just go with becoming inert for now. Let me know what you think.

Hi,

I'd just like to point out that the third option to automatically
destroy the object is not a good option. If a client first destroys
xdg_toplevel and that causes the server to internally destroy the
decoration object, then the client will either cause a protocol error
with destroying the decoration object (you have protocol for destroy
request, and the request will refer to an object that does not exist in
the server anymore), or the decoration object needs to have two kinds of
destroy functions: one that sends destroy request (I assume there are
use cases which do want it) and another that does not, plus the client
needs to be careful to use the right one. So to me that seems like the
worst option.

It's always dangerous for a server to automatically destroy a protocol
object, if the object interface has any requests defined. It would need
very careful design and use to avoid protocol error inducing races or
conditions, so I would advice against it.

A destroy request is a request. Where we often have the server
automatically destroy an object are interfaces which essentially act as
a "return value" delivery, and therefore do not have requests defined.
These objects must also never be used as arguments in any requests, or
you fall into the can of worms again.

I forget what the decoration interface spec says, but if you have a
defined consequence from destroying the protocol object, e.g. while the
window is still mapped, you need to consider if that may cause a
user visible glitch. You cannot assume that sequential requests are
handled together, it is possible the compositor can go do something else
between *any* two requests. But because usually compositors don't and
clients flush out requests in bursts, it is very unlikely to see the
glitch problems in normal testing. You would need to use a protocol
debugger that added an explicit flush and delay between every single
request and event.

I hope this helps you decide on a better design.


Thanks,
pq


pgpwTFWXcL8pK.pgp
Description: OpenPGP digital signature
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel


T480s Trackpoint drifts

2018-03-19 Thread Kai Hendry
Hi there,

I'm an Archlinux user who has a new T480s Thinkpad and unfortunately
the Trackpoint can (not always or reproducibly) "drift". I hope that
makes sense!

https://bbs.archlinux.org/viewtopic.php?id=235400

The trouble is I can't seem to disable the Trackpoint pointer itself,
but still have the buttons work, which I do use out of habit.

Generally speaking I find the the track point is a bit too fast (and
imprecise and hence useless) and the track pad a bit slow.
For example if I drag my finger across the Trackpad, it's not enough
to get from one side of the screen to the other.

I deleted Windows [1] so I don't really have anything to compare to. I
do know years ago, my old old Thinkpads, the trackpoint did use to be
a good UX. Now... not so much!

https://s.natalian.org/2018-03-19/t480s-list-devices.txt

Many thanks!




[1] 
https://www.youtube.com/watch?v=WFlt7PI0sw8=PLiKgVPlhUNuz9k0xIp7PGUV5mmDdTS1vJ=2
___
wayland-devel mailing list
wayland-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/wayland-devel