Re: [Mesa-dev] [PATCH v5] egl/dri2: implement platform_surfaceless

2015-06-12 Thread Daniel Stone
Hi,

On 12 June 2015 at 15:43, Emil Velikov  wrote:
> On 11/06/15 17:43, Zach Reizner wrote:
>> +  dri2_dpy->fd = open(card_path, O_RDWR);
> Pretty sure that we'd want some O_CLOEXEC/fcntl() magic but that can be
> done as a separate patch.

We absolutely want O_CLOEXEC; if that could be put in now, that would be great.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 00/13] Avoid double promotion

2015-07-14 Thread Daniel Stone
Hi,

On 14 July 2015 at 00:22, Matt Turner  wrote:
but it's not really
> useful in general because float arguments are always cast to double
> when passed as arguments to varargs functions like printf (why?), and
> it warns about that, generating a lot of noise.

It might shock you to learn that the answer is awful historical
reasons. Or maybe not.

Some types undergo unconditional promotion, similar to pre-prototype
functions, when used in vararg calls: float is promoted to double, and
char/short are promoted to int.

http://c-faq.com/~scs/cgi-bin/faqcat.cgi?sec=ansi#argpromos

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 11/13] mesa: add API dispatch for GL_ARB_get_texture_sub_image

2015-07-14 Thread Daniel Stone
Hi,

On 14 July 2015 at 02:21, Brian Paul  wrote:
> +
> +
> +

Surely texture rather than program?

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] mesa: automake: replace $(RM) with rm -f

2015-07-20 Thread Daniel Stone
Hi,

On 20 July 2015 at 17:40, Emil Velikov  wrote:
> On 18 July 2015 at 23:13, Jonathan Gray  wrote:
>> $(RM) is set to 'rm -f' by GNU make, this is not true of other versions
>> of make and RM is not one of the macros required by POSIX.
>>
> Slightly unfortunate but I think we can live with it. Would like to
> see if Matt/other have objections against this, but as-is

I'd suggest using "RM ?= rm -f", but obviously that doesn't work on
non-GNU make either ...

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC 1/2] gallium: add renderonly driver

2015-10-26 Thread Daniel Stone
Hi Lucas,

On 19 October 2015 at 10:25, Lucas Stach  wrote:
> Am Sonntag, den 18.10.2015, 21:41 +0200 schrieb Christian Gmeiner:
>> 2015-10-16 15:31 GMT+02:00 Thierry Reding :
>> > Gallium driver is rather complicated, but that's due to the fact that
>> > graphics drivers are complex beasts.
>> >
>> > That said, I think for some areas it might be beneficial to have helpers
>> > to reduce the amount of duplication. However I think at this point in
>> > time we haven't had enough real-world exposure for this kind of driver
>> > to know what the requirements are. For that reason I think it is
>> > premature to use a generic midlayer such as this. Yes, I know that the
>> > alternative is roughly 2000 lines of code per driver, but on one hand
>> > that's nothing compared to the amount of code required by a proper GPU
>> > driver and on the other hand this will (ideally) be temporary until we
>> > get a better picture of where things need to go. At which point it may
>> > become more obvious how we can solve the boilerplate problem while at
>> > the same time avoiding the restrictions imposed by the midlayer.
>>
>> For the moment I have no problem to go that way, but I am not sure how long
>> we should wait to collect the needed requirements. Do you know of any other
>> renderonly gpu coming in the near future (with opensource drivers)?

Well, everything on ARM, and any multi-GPU/hybrid/Prime situation on Intel.

>> I am really no expert in this area but the most interesting part for a
>> renderonly
>> GPU midlayer is "how to display the rendering result?".
>> So we need to support different tiling formats and if a intermediate buffer 
>> is
>> needed (for IP combination where GPU or scanout format are not compatible).
>>
>> For etnaviv we need to support all this cases (with the GC2000+ coming the 
>> next
>> months we can render into the scanout buffer directly - same as tegra).

Luckily etnaviv+imx-drm in the current generation seems to be a
glaring special case. Especially with higher resolutions becoming the
norm (1080p being non-negotiable and 4K working its way into even the
smallest mobile systems, e.g. Mali-450 on Amlogic, where the 6xx+
family is around 4 years old), intermediate copies just aren't viable
anymore.

I'd suggest keeping this as a total special case hidden internally,
quite apart from the general attempt to resolve the disjoint
GPU/scanout issue.

>> I know that it is quite hard to design a good api/midlayer/whatever to be 
>> future
>> proof for the next 1-2 years without (bigger) refactoring work. But at
>> some point
>> we need to jump into the cold water, start to design it and make use of it.
>> I am open to any discussion about software architecture and requirements.
>
> The problems I see with this series are not technical/implementation
> issues, but IMHO this whole device fusing is wrong from an architectural
> point of view.

Absolutely agreed.

> This series implements a way to fuse multiple DRM devices into a single
> EGL device, solely on the base that current EGL users expect that a
> single device is able to render and scanout at the same time. This is a
> flawed premise on itself and this series actively tries to keep the
> illusion that this is true, even for SoC and prime systems. While doing
> so it is putting a lot device specific knowledge into the implementation
> of the midlayer/helpers.
>
> IMHO we should not try to keep that illusion, but work out a way for
> client applications to deal with the situation of having multiple
> scanout/render devices in a easy way.
>
> I mean having render/scanout on different devices isn't exactly news, in
> fact we already have to deal with that for the various prime laptops.
> Currently all the complexity of those setups is hidden in the X server,
> but that one will go away and we have to find a way to make it easy for
> the various wayland compositors to deal with that.

So far we agree ...

> My current proposal would be as follows:
>
> 1. Implement etnaviv/nouveau on tegra as EGL devices that are only able
> to render into offscreen surfaces.
>
> 2. Implement an easy way for compositors to discover EGL devices and
> their capabilities. (There are already patches for EGL device and
> related stuff on the ML)

Indeed. The EGL_EXT_device_base series does need a respin for the new
libdrm device-discovery API, but it would still be nice to get review
here. Having this at least allows us the correct primitives (in the
sense of the driver having all required information), in that gbm has
a handle to the KMS/presentation device, and EGL then has a handle to
the EGL/render device.

> 3. Implement a generic KMS EGL device that is only able to prove
> scanout. Have EGL_EXT_stream_consumer_egloutput [1] working on top of
> that.
>
> 4. Implement EGL_KHR_stream_producer_eglsurface for the render only EGL
> devices.

At this point I violently disagree. My view on EGLStreams is roughly
the same as that of STREAMS (or OpenMAX). I can see the

Re: [Mesa-dev] [PATCH v4 7/9] st/va: add headless support, i.e. VA_DISPLAY_DRM

2015-10-30 Thread Daniel Stone
Hi,
I know this isn't your fault, but I really really don't see any reason
why the vl winsys bits should continue to exist. We already have a
winsys/presentation layer in Mesa ...

Cheers,
Daniel

On 29 October 2015 at 17:40, Julien Isorce  wrote:
> This patch allows to use gallium vaapi without requiring
> a X server running for your second graphic card.
>
> Signed-off-by: Julien Isorce 
> ---
>  src/gallium/state_trackers/va/Makefile.am |  9 
>  src/gallium/state_trackers/va/context.c   | 70 
> ---
>  2 files changed, 73 insertions(+), 6 deletions(-)
>
> diff --git a/src/gallium/state_trackers/va/Makefile.am 
> b/src/gallium/state_trackers/va/Makefile.am
> index 2a93a90..348cfe1 100644
> --- a/src/gallium/state_trackers/va/Makefile.am
> +++ b/src/gallium/state_trackers/va/Makefile.am
> @@ -30,6 +30,15 @@ AM_CFLAGS = \
> $(VA_CFLAGS) \
> -DVA_DRIVER_INIT_FUNC="__vaDriverInit_$(VA_MAJOR)_$(VA_MINOR)"
>
> +AM_CFLAGS += \
> +   $(GALLIUM_PIPE_LOADER_DEFINES) \
> +   -DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\"
> +
> +if HAVE_GALLIUM_STATIC_TARGETS
> +AM_CFLAGS += \
> +   -DGALLIUM_STATIC_TARGETS=1
> +endif
> +
>  AM_CPPFLAGS = \
> -I$(top_srcdir)/include
>
> diff --git a/src/gallium/state_trackers/va/context.c 
> b/src/gallium/state_trackers/va/context.c
> index a107cc4..bd533c4 100644
> --- a/src/gallium/state_trackers/va/context.c
> +++ b/src/gallium/state_trackers/va/context.c
> @@ -28,7 +28,8 @@
>
>  #include "pipe/p_screen.h"
>  #include "pipe/p_video_codec.h"
> -
> +#include "pipe-loader/pipe_loader.h"
> +#include "state_tracker/drm_driver.h"
>  #include "util/u_memory.h"
>  #include "util/u_handle_table.h"
>  #include "util/u_video.h"
> @@ -36,6 +37,8 @@
>
>  #include "va_private.h"
>
> +#include 
> +
>  static struct VADriverVTable vtable =
>  {
> &vlVaTerminate,
> @@ -99,6 +102,8 @@ PUBLIC VAStatus
>  VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>  {
> vlVaDriver *drv;
> +   int drm_fd;
> +   struct drm_state *drm_info;
>
> if (!ctx)
>return VA_STATUS_ERROR_INVALID_CONTEXT;
> @@ -107,9 +112,56 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
> if (!drv)
>return VA_STATUS_ERROR_ALLOCATION_FAILED;
>
> -   drv->vscreen = vl_screen_create(ctx->native_dpy, ctx->x11_screen);
> -   if (!drv->vscreen)
> -  goto error_screen;
> +   switch (ctx->display_type) {
> +   case VA_DISPLAY_ANDROID:
> +   case VA_DISPLAY_WAYLAND:
> +  FREE(drv);
> +  return VA_STATUS_ERROR_UNIMPLEMENTED;
> +   case VA_DISPLAY_GLX:
> +   case VA_DISPLAY_X11:
> +  drv->vscreen = vl_screen_create(ctx->native_dpy, ctx->x11_screen);
> +  if (!drv->vscreen)
> + goto error_screen;
> +  break;
> +   case VA_DISPLAY_DRM:
> +   case VA_DISPLAY_DRM_RENDERNODES: {
> +  drm_info = (struct drm_state *) ctx->drm_state;
> +  if (!drm_info) {
> + FREE(drv);
> + return VA_STATUS_ERROR_INVALID_PARAMETER;
> +  }
> +
> +#if GALLIUM_STATIC_TARGETS
> +  drm_fd = drm_info->fd;
> +#else
> +  drm_fd = dup(drm_info->fd);
> +#endif
> +
> +  if (drm_fd < 0) {
> + FREE(drv);
> + return VA_STATUS_ERROR_INVALID_PARAMETER;
> +  }
> +
> +  drv->vscreen = CALLOC_STRUCT(vl_screen);
> +  if (!drv->vscreen)
> + goto error_screen;
> +
> +#if GALLIUM_STATIC_TARGETS
> +  drv->vscreen->pscreen = dd_create_screen(drm_fd);
> +#else
> +  if (pipe_loader_drm_probe_fd(&drv->dev, drm_fd))
> + drv->vscreen->pscreen = pipe_loader_create_screen(drv->dev, 
> PIPE_SEARCH_DIR);
> +#endif
> +
> +  if (!drv->vscreen->pscreen)
> + goto error_pipe;
> +
> +  }
> +  break;
> +   default:
> +  FREE(drv);
> +  return VA_STATUS_ERROR_INVALID_DISPLAY;
> +   }
>
> drv->pipe = drv->vscreen->pscreen->context_create(drv->vscreen->pscreen,
>   drv->vscreen, 0);
> @@ -145,7 +197,10 @@ error_htab:
> drv->pipe->destroy(drv->pipe);
>
>  error_pipe:
> -   vl_screen_destroy(drv->vscreen);
> +   if (ctx->display_type == VA_DISPLAY_GLX || ctx->display_type == 
> VA_DISPLAY_X11)
> +  vl_screen_destroy(drv->vscreen);
> +   else
> +  FREE(drv->vscreen);
>
>  error_screen:
> FREE(drv);
> @@ -282,7 +337,10 @@ vlVaTerminate(VADriverContextP ctx)
> vl_compositor_cleanup_state(&drv->cstate);
> vl_compositor_cleanup(&drv->compositor);
> drv->pipe->destroy(drv->pipe);
> -   vl_screen_destroy(drv->vscreen);
> +   if (ctx->display_type == VA_DISPLAY_GLX || ctx->display_type == 
> VA_DISPLAY_X11)
> +  vl_screen_destroy(drv->vscreen);
> +   else
> +  FREE(drv->vscreen);
> handle_table_destroy(drv->htab);
> FREE(drv);
>
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freede

Re: [Mesa-dev] [PATCH v4 7/9] st/va: add headless support, i.e. VA_DISPLAY_DRM

2015-10-30 Thread Daniel Stone
Hi Julien,

On 30 October 2015 at 11:32, Julien Isorce  wrote:
> Thx for pointing this out. Where are the files related to
> winsys/presentation :) ?

The one you're changing. ;)

src/gbm already has a great deal of infrastructure for handling direct
connections to DRM/KMS devices, buffer allocation, et al.
src/egl/drivers/dri2/platform_x11.c already has the support for
connecting to the X server, displaying buffers inside a window using
DRI2/DRI3, etc etc. src/egl/drivers/dri2/platform_wayland.c implements
everything you need to do the same with Wayland.

It would be really great if it were possible to reuse all that
infrastructure rather than reinventing it inside the VA-API code.

> Is your remark a blocker for landing the patches I submitted ? Maybe we can
> still land them and then if you could guide me what I should change to use
> newer api that would be great.

Right, as I said it's certainly not your fault! I'd be happy to see
this series land if there were an aspiration towards unifying the
codepaths.

At the moment, the winsys support in src/egl is, unsurprisingly,
pretty closely tied to EGL. So I guess it would be a matter of
modifying the existing internal EGL platform API to be able to use it
from vl_*, rather than straight reuse. DItto for gbm if there's
anything useful in there that could use the same codepaths.

Cheers,
Daniel

> Cheers
> Julien
>
>
> On 30 October 2015 at 08:47, Daniel Stone  wrote:
>>
>> Hi,
>> I know this isn't your fault, but I really really don't see any reason
>> why the vl winsys bits should continue to exist. We already have a
>> winsys/presentation layer in Mesa ...
>>
>> Cheers,
>> Daniel
>>
>> On 29 October 2015 at 17:40, Julien Isorce  wrote:
>> > This patch allows to use gallium vaapi without requiring
>> > a X server running for your second graphic card.
>> >
>> > Signed-off-by: Julien Isorce 
>> > ---
>> >  src/gallium/state_trackers/va/Makefile.am |  9 
>> >  src/gallium/state_trackers/va/context.c   | 70
>> > ---
>> >  2 files changed, 73 insertions(+), 6 deletions(-)
>> >
>> > diff --git a/src/gallium/state_trackers/va/Makefile.am
>> > b/src/gallium/state_trackers/va/Makefile.am
>> > index 2a93a90..348cfe1 100644
>> > --- a/src/gallium/state_trackers/va/Makefile.am
>> > +++ b/src/gallium/state_trackers/va/Makefile.am
>> > @@ -30,6 +30,15 @@ AM_CFLAGS = \
>> > $(VA_CFLAGS) \
>> > -DVA_DRIVER_INIT_FUNC="__vaDriverInit_$(VA_MAJOR)_$(VA_MINOR)"
>> >
>> > +AM_CFLAGS += \
>> > +   $(GALLIUM_PIPE_LOADER_DEFINES) \
>> > +   -DPIPE_SEARCH_DIR=\"$(libdir)/gallium-pipe\"
>> > +
>> > +if HAVE_GALLIUM_STATIC_TARGETS
>> > +AM_CFLAGS += \
>> > +   -DGALLIUM_STATIC_TARGETS=1
>> > +endif
>> > +
>> >  AM_CPPFLAGS = \
>> > -I$(top_srcdir)/include
>> >
>> > diff --git a/src/gallium/state_trackers/va/context.c
>> > b/src/gallium/state_trackers/va/context.c
>> > index a107cc4..bd533c4 100644
>> > --- a/src/gallium/state_trackers/va/context.c
>> > +++ b/src/gallium/state_trackers/va/context.c
>> > @@ -28,7 +28,8 @@
>> >
>> >  #include "pipe/p_screen.h"
>> >  #include "pipe/p_video_codec.h"
>> > -
>> > +#include "pipe-loader/pipe_loader.h"
>> > +#include "state_tracker/drm_driver.h"
>> >  #include "util/u_memory.h"
>> >  #include "util/u_handle_table.h"
>> >  #include "util/u_video.h"
>> > @@ -36,6 +37,8 @@
>> >
>> >  #include "va_private.h"
>> >
>> > +#include 
>> > +
>> >  static struct VADriverVTable vtable =
>> >  {
>> > &vlVaTerminate,
>> > @@ -99,6 +102,8 @@ PUBLIC VAStatus
>> >  VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>> >  {
>> > vlVaDriver *drv;
>> > +   int drm_fd;
>> > +   struct drm_state *drm_info;
>> >
>> > if (!ctx)
>> >return VA_STATUS_ERROR_INVALID_CONTEXT;
>> > @@ -107,9 +112,56 @@ VA_DRIVER_INIT_FUNC(VADriverContextP ctx)
>> > if (!drv)
>> >return VA_STATUS_ERROR_ALLOCATION_FAILED;
>> >
>> > -   drv->vscreen = vl_screen_create(ctx->native_dpy, ctx->x11_screen);
>> > -   if (!drv->vscreen)
>> > -  goto error_screen;
>> > +   switch (ctx->display_type) {

[Mesa-dev] [PATCH] egl/wayland: Ignore rects from SwapBuffersWithDamage

2015-11-07 Thread Daniel Stone
eglSwapBuffersWithDamage accepts damage-region rectangles to hint the
compositor that it only needs to redraw certain areas, which was passed
through the wl_surface_damage request, as designed.

Wayland also offers a buffer transformation interface, e.g. to allow
users to render pre-rotated buffers. Unfortunately, there is no way to
query buffer transforms, and the damage region was provided in surface,
rather than buffer, co-ordinate space.

Users could perhaps account for this themselves, but EGL also requires
co-ordinates to be passed in GL/mathematical co-ordinate space, with an
inversion to Wayland's natural/scanout co-orodinate space, so the
transformation is so convoluted that it's unreasonable to expect anyone
to do it.

Pending creation and acceptance of a wl_surface.buffer_damage request,
which will accept co-ordinates in buffer co-ordinate space, pessimise to
always sending full-surface damage.

bce64c6c provides the explanation for why we send maximum-range damage,
rather than the full size of the surface: in the presence of buffer
transformations, full-surface damage may not actually cover the entire
surface.

Signed-off-by: Daniel Stone 
Cc: Jasper St. Pierre 
---
 src/egl/drivers/dri2/platform_wayland.c | 14 ++
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 0d161f6..9e8e6ce 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -703,18 +703,8 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
dri2_surf->dx = 0;
dri2_surf->dy = 0;
 
-   if (n_rects == 0) {
-  wl_surface_damage(dri2_surf->wl_win->surface,
-0, 0, INT32_MAX, INT32_MAX);
-   } else {
-  for (i = 0; i < n_rects; i++) {
- const int *rect = &rects[i * 4];
- wl_surface_damage(dri2_surf->wl_win->surface,
-   rect[0],
-   dri2_surf->base.Height - rect[1] - rect[3],
-   rect[2], rect[3]);
-  }
-   }
+wl_surface_damage(dri2_surf->wl_win->surface,
+  0, 0, INT32_MAX, INT32_MAX);
 
if (dri2_dpy->is_different_gpu) {
   _EGLContext *ctx = _eglGetCurrentContext();
-- 
2.5.0

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl/wayland: Ignore rects from SwapBuffersWithDamage

2015-11-13 Thread Daniel Stone
On 12 November 2015 at 16:47, Jason Ekstrand  wrote:
> On Thu, Nov 12, 2015 at 6:46 AM, Pekka Paalanen  wrote:
>> Reviewed-by: Pekka Paalanen 
>>
>> Perhaps a TODO comment referring to
>> https://bugs.freedesktop.org/show_bug.cgi?id=78190
>> would be nice.
>
> Yes, that would be good.  One day, we need to add a
> wl_surface.buffer_damage request.  I blame Kristian for that not
> happening.  I was working on that almost two years ago when he
> co-opted me to work on drivers. :-P
>
> Reviewed-by: Jason Ekstrand 

Ha. Thanks both for review; pushed now as d1314de.

Emil, this would make a good stable candidate.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Implement GLX_EXT_buffer_age for DRI2

2015-02-18 Thread Daniel Stone
Hi,

On 20 January 2015 at 21:49, Dave Airlie  wrote:
> On 19 January 2015 at 21:00, Chris Wilson  wrote:
>> In order to suport GLX_EXT_buffer_age in DRI2, we need to pass back the
>> last swap buffer count that the back buffer was defined for. For
>> simplicity, we can reuse an existing field in the DRI2GetBuffers reply
>> that is not used by current drivers, the flags. Since we change the
>> interpretation of this flag, we also declare the semantic change with a
>> DRI2 parameter and depend upon the DDX to enable the change
>> responsibility (which is just a matter of reviewing whether the flags
>> field has ever been used for a non-zero value).
>
> This is just missing the why do we need to add this to DRI2 when we
> have DRI3/Present that should be solving it.
>
> Doesn't dri3 already do this?

DRI3/Present still seem to be pretty unstable and break a bunch of
stuff, and buffer_age is definitely a useful optimisation for non-TBDR
platforms, so I don't see why not.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] [RFC] egl: propose simple EGL_MESA_image_dma_buf_export v2.2

2015-03-03 Thread Daniel Stone
Hi,

On Tuesday, March 3, 2015, Dave Airlie  wrote:
>
> +EGLBoolean eglExportDMABUFImageQueryMESA(EGLDisplay dpy,
> +  EGLImageKHR image,
> + int *fourcc,
> + int *num_planes);
>

Shouldn't this contain the modifier(s) as well?

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] [RFC] egl: propose simple EGL_MESA_image_dma_buf_export v2.2

2015-03-03 Thread Daniel Stone
Hi,

On 3 March 2015 at 18:40, Chad Versace  wrote:
> On 03/03/2015 12:13 AM, Daniel Stone wrote:
>> On Tuesday, March 3, 2015, Dave Airlie > <mailto:airl...@gmail.com>> wrote:
>> +EGLBoolean eglExportDMABUFImageQueryMESA(EGLDisplay dpy,
>> +  EGLImageKHR image,
>> + int *fourcc,
>> + int *num_planes);
>>
>>
>> Shouldn't this contain the modifier(s) as well?
>
> What is the motivation for defining two export calls, and exporting
> a subset of information from each call? I don't understand why the
> fourcc values are exported by one call, but the strides exported a
> separate call.

Querying the image gives you what you need in order to know if you can
actually deal with the image, before you go to all the effort of
exporting it.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 2/2] egl: implement EGL_MESA_transparent_alpha for x11 and wayland

2015-03-03 Thread Daniel Stone
Hi,

On 3 March 2015 at 18:56, Jason Ekstrand  wrote:
> On Tue, Mar 3, 2015 at 10:07 AM, Chad Versace 
> wrote:
>> On 02/23/2015 06:32 AM, Jonny Lamb wrote:
>> > +   static const EGLint argb_attrs[] = {
>> > +   EGL_TRANSPARENT_TYPE, EGL_TRANSPARENT_ALPHA_MESA,
>> > +   EGL_NONE
>> > +   };
>> >
>>
>> I tested this patch with X11 and Mesa, and it works as advertised. BUT...
>>
>> Pre-patch, Wayland applications who requested an EGLConfig with alpha
>> received one. The EGLSurface had compositor-transparent alpha, which
>> the application may not have expected. But the alpha configs *were*
>> available.
>>
>> Post-patch, existing Wayland applications that request an EGLConfig with
>> alpha will no longer receive one, because the existing applications
>> do not yet know about EGL_TRANSPARENT_ALPHA_MESA.
>>
>> I believe a correct solution is to continue create EGLConfigs
>> with EGL_ALPHA and without EGL_TRANSPARENT_TYPE, as Mesa does today,
>> but tell the compositor to ignore the alpha channel. The alpha channel
>> will be used for client-side alpha-blending but not for compositing.
>> Jason says that should be possible by telling the compositor that
>> the buffer format is WL_DRM_FORMAT_XRGB. And in addition to the
>> existing
>> EGLConfigs, also create new EGLConfigs with EGL_TRANSPARENT_ALPHA_MESA, as
>> this patch already does.
>
> Yes, XRGB would be the correct *technical* way to handle that, but it
> still makes me wonder... What about Wayland apps that expect to get
> transparency by simply asking for alpha now?  Won't this break them?  I
> guess we have to break one class of apps or the other.

Yeah, it will break them. But then again, we had the same flag day for
X11 in that exact Bugzilla discussion, when X11 apps which requested
ALPHA_SIZE == 8 went from getting ARGB32 drawables which would be
blended by the compositor, to not - a change which was deemed totally
fine to enforce on people because it improved performance and matched
the spec.

Perhaps a better interim solution is to assume for Wayland that
EGL_TRANSPARENT_TYPE == EGL_DONT_CARE means that applications will get
a format determined by ALPHA_SIZE (i.e. size 8 means ARGB32, size 0
means XRGB32), but respect explicit demands for
TRANSPARENT_{NONE,ALPHA}.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] egl dma-buf export extension again

2015-03-04 Thread Daniel Stone
Hi,

On 3 March 2015 at 23:26, Kristian Høgsberg  wrote:
> On Tue, Mar 3, 2015 at 10:43 AM, Chad Versace  wrote:
>> Please use EGLImages as the import/export object. That provides
>> consistency with existing similar EGL APIs. And it allows a clean
>> separation between EGL-aware code and GL-aware code in applications
>> and compositors.
>>
>> For what it's worth, extensions in this area being developed by Google's
>> Chrome OS team also use EGLImages as the transport object.
>
> We can't both implement EGLImages in a compliant way *and* use them
> for cross-process sharing like we do.  The spec language around
> orphaning and siblings is obscure, but the idea is that whenever you
> add a sibling (another client API object, eg a texture or a vg
> renderbuffer) the contents become invalid.  The intended use case is
> that you create all siblings up front and then start using them.  For
> example, to share an image between VG and GL, you create the VG
> buffer, then the EGLImage and then the GL texture.  Implementations
> will defer allocation until all the siblings are set up and then only
> once you start rendering allocate the memory.  This allows the
> implementation to accommodate constraints from all siblings. If you
> add another sibling, the implementation may have to reallocate storage
> to satisfy new constraints.  This clearly isn't compatible with how we
> create and share EGLImages left and right across process boundaries.
> Khronos specifically developed EGLStreams as a cross-process color
> buffer sharing mechanism without pulling in EGLImage as a dependency.
>
> Anyway, the damage is done and we can continue digging ourselves
> deeper into this hole, but we should at least 1) have a good reason
> why the extra EGLImage indirection is necessary (it also rules out GLX
> usage) and 2) acknowledge that we're going to continue to rely on our
> broken EGLImage semantics.

Which is all fair enough, but I think it's worth noting that
SurfaceFlinger, when confronted with the exact same problem space,
decided that the same answer was the best way forward. So we've got
some precedent established there.

Given the various interesting and creative ways in which people have
broken their EGLImage implementations, I'm not really sure that I want
to leave them to chance another, much less standard, spec, which has
less of a precedent around it.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 06/34] gbm: Export a per plane getter for stride

2017-02-05 Thread Daniel Stone
Hi,

On 4 February 2017 at 05:29, Ben Widawsky  wrote:
> On 17-01-31 11:33:39, Jason Ekstrand wrote:
>> On Mon, Jan 23, 2017 at 10:19 PM, Ben Widawsky  wrote:
>>> +   /* Preserve legacy behavior if plane is 0 */
>>> +   if (plane == 0)
>>> +  return _bo->stride;
>>
>> This implies that, for multi-planar images, you have to have the BO stride
>> be the same as the image stride.  Do we want this or do we only want to
>> return bo->stride if we don't have enough DRI stuff (in the if below) to
>> use the image?  I think it's probably ok either way.
>>
>
> The biggest problem is if I change it (see hunk below) is that it
> potentially
> doesn't match the old behavior, and I don't really have a great way to test
> this
> outside of our environment. I went with the path of least risk, however you
> do
> raise a good point.
>
> Daniel, do you have an opinion here?

I think Jason's suggestion is the right thing to do. The main case I
worry about is dumb buffers, covered by !bo->image, which cannot be
multi-planar. If we ever have a single-planar image where the stride
doesn't match the DRIimage stride, that sounds like something to be
fixed anyway. So, let's go with this and fix up anything which breaks,
though I can't immediately see how that would happen.

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


Re: [Mesa-dev] [PATCH 14/34] gbm: Get modifiers from DRI

2017-02-06 Thread Daniel Stone
Hi,

On 6 February 2017 at 19:22, Jason Ekstrand  wrote:
> On Sun, Feb 5, 2017 at 1:15 PM, Ben Widawsky  wrote:
>> Introducing the LINEAR modifier (which happened after v2 of this series) did
>> make things complex because it's possible in some horrific future that a 
>> image
>> doesn't support linear. As a result, you are correct. I think for this case, 
>> the
>> client can handle it pretty easily, and returning INVALID is the right
>> thing to do.
>>
>> Daniel, are you okay with changing this to return DRM_FORMAT_MOD_INVALID?

Hm, it's a little less clean, but sure, works for me.

>> Yeah, this is also a lie but way trickier than the above. Again before this 
>> rev
>> of the series, 0 meant DRM_FORMAT_MOD_NONE, and that was actually legit,
>> however, now it does mean LINEAR. I believe it's safe to assume that all dumb
>> BOs are linear, but it should probably be baked in somewhere better. One 
>> option
>> would be to create a proper DRIimage for a dumb BO, but I think the best bet 
>> is
>> to just replace 0 with DRM_FORMAT_MOD_LINEAR.
>
> That sounds fairly reasonable to me.  I guess someone could create a BO with
> GBM and then call the kernel ioctl to set the tiling mode to X-tiled and
> then ask what it has.  However, short of calling into the driver and having
> it query the kernel, I don't see a good way to get around that.  I think I'd
> be ok with just returning LINEAR and saying "don't do that".  Daniel?

That's impressively contrived, which is a polite way of saying deeply
stupid; wouldn't that break Mesa anyway? I'm happy to ban that.

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


Re: [Mesa-dev] [RFC] EGL extension for additional DRM_BUFFER_FORMATs

2017-02-07 Thread Daniel Stone
Hi Nicolai,

On 7 February 2017 at 09:07, Nicolai Hähnle  wrote:
> On 06.02.2017 20:57, Dave Airlie wrote:
>> I just saw this, EGL_MESA_drm_image is really not a good place to start,
>>
>> we have two specs instead,
>> EGL_EXT_image_dma_buf_import.
>> EGL_MESA_image_dma_buf_export
>>
>> does one of those not cover this?
>
> Sometimes you just have to push things to flush out the comments :)
>
> Glamor uses EGL_MESA_drm_image here:
> https://cgit.freedesktop.org/xorg/xserver/tree/glamor/glamor_egl.c?id=f4a41155479e68bf55740c1dfffafc78e4c02087#n112
>
> This code path is used for allocating the main framebuffer, i.e. without
> this extension 16-bit or 30-bit (10bpc) modes do not work.
>
> We do have to actually _create_ an image of the correct format with the
> scanout flag somewhere. Importing and exporting alone doesn't solve that
> problem.

Given that exporting doesn't seem to be used at all in Glamor, and it
only uses GEM name -> EGLImage importing, this could in fact just be
replaced with EGL_EXT_image_dma_buf_import. That also has the
advantage of EGL_EXT_image_dma_buf_import_modifiers support, so you
can both import buffers with modifiers, as well as query the
implementation for which formats/modifiers it supports.

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


[Mesa-dev] [PATCH] egl/wayland: Don't use DRM format codes for SHM

2017-02-13 Thread Daniel Stone
The wl_drm interface (akin to X11's DRI2) uses the standard set of DRM
FourCC format codes. wl_shm copies this, except for ARGB/XRGB,
which use their own definitions.

Make sure we only use wl_shm format codes when we're working with
wl_shm. Otherwise, using swrast with 32bpp formats would fail with an
error.

Signed-off-by: Daniel Stone 
Cc: Emil Velikov 
Cc: mesa-sta...@lists.freedesktop.org
Fixes: cb5e799448 ("egl/wayland: unify dri2_wl_create_surface implementations")
---
 src/egl/drivers/dri2/platform_wayland.c | 21 ++---
 1 file changed, 18 insertions(+), 3 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index 37360c7..0bc4e87 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -1402,15 +1402,30 @@ os_create_anonymous_file(off_t size)
 
 static EGLBoolean
 dri2_wl_swrast_allocate_buffer(struct dri2_egl_display *dri2_dpy,
-   int format, int w, int h,
+   int drm_format, int w, int h,
void **data, int *size,
struct wl_buffer **buffer)
 {
struct wl_shm_pool *pool;
+   uint32_t shm_format;
int fd, stride, size_map;
void *data_map;
 
-   stride = dri2_wl_swrast_get_stride_for_format(format, w);
+   switch (drm_format) {
+   case WL_DRM_FORMAT_RGB565:
+  shm_format = WL_SHM_FORMAT_RGB565;
+  break;
+   case WL_DRM_FORMAT_ARGB:
+  shm_format = WL_SHM_FORMAT_ARGB;
+  break;
+   case WL_DRM_FORMAT_XRGB:
+  shm_format = WL_SHM_FORMAT_XRGB;
+  break;
+   default:
+  return EGL_FALSE;
+   }
+
+   stride = dri2_wl_swrast_get_stride_for_format(drm_format, w);
size_map = h * stride;
 
/* Create a sharable buffer */
@@ -1426,7 +1441,7 @@ dri2_wl_swrast_allocate_buffer(struct dri2_egl_display 
*dri2_dpy,
 
/* Share it in a wl_buffer */
pool = wl_shm_create_pool(dri2_dpy->wl_shm, fd, size_map);
-   *buffer = wl_shm_pool_create_buffer(pool, 0, w, h, stride, format);
+   *buffer = wl_shm_pool_create_buffer(pool, 0, w, h, stride, shm_format);
wl_shm_pool_destroy(pool);
close(fd);
 
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] egl/wayland: Don't use DRM format codes for SHM

2017-02-13 Thread Daniel Stone
Hey,

On 13 February 2017 at 17:27, Emil Velikov  wrote:
> On 13 February 2017 at 14:06, Daniel Stone  wrote:
>>  static EGLBoolean
>>  dri2_wl_swrast_allocate_buffer(struct dri2_egl_display *dri2_dpy,
>> -   int format, int w, int h,
>> +   int drm_format, int w, int h,
>> void **data, int *size,
>> struct wl_buffer **buffer)
>>  {
>> struct wl_shm_pool *pool;
>> +   uint32_t shm_format;
>> int fd, stride, size_map;
>> void *data_map;
>>
>> -   stride = dri2_wl_swrast_get_stride_for_format(format, w);
>> +   switch (drm_format) {
>> +   case WL_DRM_FORMAT_RGB565:
>> +  shm_format = WL_SHM_FORMAT_RGB565;
>> +  break;
>> +   case WL_DRM_FORMAT_ARGB:
>> +  shm_format = WL_SHM_FORMAT_ARGB;
>> +  break;
>> +   case WL_DRM_FORMAT_XRGB:
>> +  shm_format = WL_SHM_FORMAT_XRGB;
>> +  break;
>> +   default:
>> +  return EGL_FALSE;
>> +   }
>
> Wouldn't it be better to have this in dri2_wl_create_window_surface() ?
> As-is we're passing the DRM formats on the wire, which is quite likely
> to cause issues elsewhere.

There were a few other codepaths touching the format that seemed like
they might be affected. This is the only one which actually hits the
wire (AFAICT - which others are you thinking of ... ?), so that seemed
like it was the easiest.

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


Re: [Mesa-dev] [Mesa-stable] [PATCH] egl/wayland: Don't use DRM format codes for SHM

2017-02-13 Thread Daniel Stone
Hi,

On 13 February 2017 at 17:49, Emil Velikov  wrote:
> On 13 February 2017 at 17:34, Daniel Stone  wrote:
>> On 13 February 2017 at 17:27, Emil Velikov  wrote:
>>> Wouldn't it be better to have this in dri2_wl_create_window_surface() ?
>>> As-is we're passing the DRM formats on the wire, which is quite likely
>>> to cause issues elsewhere.
>>
>> There were a few other codepaths touching the format that seemed like
>> they might be affected. This is the only one which actually hits the
>> wire (AFAICT - which others are you thinking of ... ?), so that seemed
>> like it was the easiest.
>>
> From a quick look shm_handle_format() comes to mind. Keep in mind that
> I'm not that big of an expert on the Wayland code so I might have
> missed something ;-)

Oh, that's fine. shm_handle_format (and drm_handle_format, for
non-swrast) gets called to handle events coming in from the server
side. So we will receive WL_SHM_FORMAT_* (not WL_DRM_FORMAT_*) from
the wire, and use that to set enum values which are constant between
SHM and DRM. So I don't see the problem with that.

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


Re: [Mesa-dev] [PATCH v2 30/30] egl/dri2: set WL_bind_wayland_display in a consistent way

2016-09-18 Thread Daniel Stone
On 16 September 2016 at 18:02, Emil Velikov  wrote:
> Introduce a helper and use it throughout the platform code. This allows
> us to reduce the amount of ifdef(s) and (potentially) use
> kms_swrast_dri.so for !drm platforms (namely wayland and x11).

Reviewed-by: Daniel Stone 
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2] egl/wayland: Avoid race conditions when on non-main thread

2016-10-16 Thread Daniel Stone
Hi Jonas,

On 24 June 2016 at 03:46, Jonas Ådahl  wrote:
> @@ -74,9 +74,8 @@ roundtrip(struct dri2_egl_display *dri2_dpy)
> struct wl_callback *callback;
> int done = 0, ret = 0;
>
> -   callback = wl_display_sync(dri2_dpy->wl_dpy);
> +   callback = wl_display_sync(dri2_dpy->wl_dpy_wrapper);
> wl_callback_add_listener(callback, &sync_listener, &done);
> -   wl_proxy_set_queue((struct wl_proxy *) callback, dri2_dpy->wl_queue);
> while (ret != -1 && !done)
>ret = wl_display_dispatch_queue(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);

If we're bumping the libwayland-client dep anyway, might as well just
make this use wl_display_roundtrip_queue.

> @@ -1235,6 +1237,10 @@ dri2_initialize_wayland_drm(_EGLDriver *drv, 
> _EGLDisplay *disp)
> wl_drm_destroy(dri2_dpy->wl_drm);
>   cleanup_registry:
> wl_registry_destroy(dri2_dpy->wl_registry);
> +   wl_proxy_wrapper_destroy(dri2_dpy->wl_dpy_wrapper);
> + cleanup_dpy_wrapper:
> +   if (dri2_dpy->wl_dpy != disp->PlatformDisplay)
> +  wl_display_disconnect(dri2_dpy->wl_dpy);
> wl_event_queue_destroy(dri2_dpy->wl_queue);
>   cleanup_dpy:
> free(dri2_dpy);

Adding wl_display_disconnect should be a separate change, and in any
case needs to come after wl_event_queue_destroy to avoid a
use-after-free.

With those fixed, assuming we're fine with an increased hard dep:
Reviewed-by: Daniel Stone 

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


Re: [Mesa-dev] [PATCH] Remove GL_GLEXT_PROTOTYPES guards from non-ext headers.

2016-09-12 Thread Daniel Stone
Hi,

On 12 September 2016 at 13:37, Emil Velikov  wrote:
> A earlier sync with the Khronos headers added _extension_ prototype
> guards to all the GLES2/3/31/32 core entry points. Effectively breaking
> all the applications that aim to be portable and do not set the define.
>
> The issue has been reported to Khronos (internal bugzilla #14206) and is
> being worked on. Until updated/fixed headers are released locally fix
> the issue.
>
> The following report is when building weston.
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=97773

Weston is the only report now, but unless the world has switched to
requiring get_all_proc_addresses, it'll break a hell of a lot more.

Reviewed-by: Daniel Stone 

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


Re: [Mesa-dev] [PATCH mesa] wsi/wayland: fix error path

2016-10-20 Thread Daniel Stone
Hi Eric,

On 20 October 2016 at 00:09, Eric Engestrom  wrote:
> I'm not sure this is the right fix. The other thing I'm starting to lean
> toward is to just remove these last two instruction (ie. everything after
> `return VK_SUCCESS`).
> Also, this is untested. It compiles, but that's it.

Indeed, that would be the right fix. Since Wayland is fully
asynchronous, create_prime_buffer will either succeed, or generate a
protocol error much further down the line.

That being said, the use of wl_display_roundtrip is really broken in
the presence of multiple threads: much like Jonas's fix for
wayland-egl, all the Wayland objects created by the WSI _must_ use a
separate event queue.

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


[Mesa-dev] [PATCH] Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"

2016-10-24 Thread Daniel Stone
This reverts commit 25cc889004aad6d1cab9edd76db898658e347b97, though
since the code has changed, it was applied manually.

The intent of moving blocking from SwapBuffers to get_back_bo, was to
avoid unnecessary triple-buffering by ensuring that the compositor had
fully processed the previous frame before we started rendering. This
means that the only time we would have to resort to triple-buffering
would be when the buffer is directly scanned out, thus saving an extra
buffer for composition anyway.

The 'repaint window' changes introduced in Weston since then, however,
have narrowed the window of time between the frame event being sent and
the repaint loop needing to conclude, to 7ms by default, in order to
reduce latency. This means however that blocking in get_back_bo gives a
maximum of 7ms for the entire GL submission to begin and complete.

Not only this, but if a client is using buffer_age to avoid full
repaints, the buffer-age request will stall in get_back_bo until the
frame callback completes, meaning that the client cannot even calculate
the repaint area before the 7ms window.

The combination of the two meant that WebKit-GTK+ was failing to
achieve full framerate on a Minnowboard, due to spending a great deal of
its time attempting to query the age of the next buffer before redraw.

Revert to the previous behaviour of allowing rendering to begin but
delaying SwapBuffers, unless and until we can find a more gentle
behaviour.

Signed-off-by: Daniel Stone 
Cc: Neil Roberts 
Cc: Kristian Høgsberg 
---
 src/egl/drivers/dri2/platform_wayland.c | 30 ++
 1 file changed, 14 insertions(+), 16 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_wayland.c 
b/src/egl/drivers/dri2/platform_wayland.c
index ccab192..ba74d33 100644
--- a/src/egl/drivers/dri2/platform_wayland.c
+++ b/src/egl/drivers/dri2/platform_wayland.c
@@ -324,14 +324,8 @@ get_back_bo(struct dri2_egl_surface *dri2_surf)
   return -1;
}
 
-   /* We always want to throttle to some event (either a frame callback or
-* a sync request) after the commit so that we can be sure the
-* compositor has had a chance to handle it and send us a release event
-* before we look for a free buffer */
-   while (dri2_surf->throttle_callback != NULL)
-  if (wl_display_dispatch_queue(dri2_dpy->wl_dpy,
-dri2_dpy->wl_queue) == -1)
- return -1;
+   /* There might be a buffer release already queued that wasn't processed */
+   wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
 
if (dri2_surf->back == NULL) {
   for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
@@ -705,6 +699,11 @@ dri2_wl_swap_buffers_with_damage(_EGLDriver *drv,
struct dri2_egl_surface *dri2_surf = dri2_egl_surface(draw);
int i;
 
+   while (dri2_surf->throttle_callback != NULL)
+  if (wl_display_dispatch_queue(dri2_dpy->wl_dpy,
+dri2_dpy->wl_queue) == -1)
+ return -1;
+
for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++)
   if (dri2_surf->color_buffers[i].age > 0)
  dri2_surf->color_buffers[i].age++;
@@ -1464,14 +1463,8 @@ swrast_update_buffers(struct dri2_egl_surface *dri2_surf)
 
/* find back buffer */
 
-   /* We always want to throttle to some event (either a frame callback or
-* a sync request) after the commit so that we can be sure the
-* compositor has had a chance to handle it and send us a release event
-* before we look for a free buffer */
-   while (dri2_surf->throttle_callback != NULL)
-  if (wl_display_dispatch_queue(dri2_dpy->wl_dpy,
-dri2_dpy->wl_queue) == -1)
- return -1;
+   /* There might be a buffer release already queued that wasn't processed */
+   wl_display_dispatch_queue_pending(dri2_dpy->wl_dpy, dri2_dpy->wl_queue);
 
/* try get free buffer already created */
for (i = 0; i < ARRAY_SIZE(dri2_surf->color_buffers); i++) {
@@ -1552,6 +1545,11 @@ dri2_wl_swrast_commit_backbuffer(struct dri2_egl_surface 
*dri2_surf)
 {
struct dri2_egl_display *dri2_dpy = 
dri2_egl_display(dri2_surf->base.Resource.Display);
 
+   while (dri2_surf->throttle_callback != NULL)
+  if (wl_display_dispatch_queue(dri2_dpy->wl_dpy,
+dri2_dpy->wl_queue) == -1)
+ return -1;
+
if (dri2_surf->base.SwapInterval > 0) {
   dri2_surf->throttle_callback =
  wl_surface_frame(dri2_surf->wl_win->surface);
-- 
2.9.3

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"

2016-10-31 Thread Daniel Stone
On 24 October 2016 at 20:42, Daniel Stone  wrote:
> This reverts commit 25cc889004aad6d1cab9edd76db898658e347b97, though
> since the code has changed, it was applied manually.
>
> The intent of moving blocking from SwapBuffers to get_back_bo, was to
> avoid unnecessary triple-buffering by ensuring that the compositor had
> fully processed the previous frame before we started rendering. This
> means that the only time we would have to resort to triple-buffering
> would be when the buffer is directly scanned out, thus saving an extra
> buffer for composition anyway.
>
> The 'repaint window' changes introduced in Weston since then, however,
> have narrowed the window of time between the frame event being sent and
> the repaint loop needing to conclude, to 7ms by default, in order to
> reduce latency. This means however that blocking in get_back_bo gives a
> maximum of 7ms for the entire GL submission to begin and complete.
>
> Not only this, but if a client is using buffer_age to avoid full
> repaints, the buffer-age request will stall in get_back_bo until the
> frame callback completes, meaning that the client cannot even calculate
> the repaint area before the 7ms window.
>
> The combination of the two meant that WebKit-GTK+ was failing to
> achieve full framerate on a Minnowboard, due to spending a great deal of
> its time attempting to query the age of the next buffer before redraw.
>
> Revert to the previous behaviour of allowing rendering to begin but
> delaying SwapBuffers, unless and until we can find a more gentle
> behaviour.

Ping - adding a couple more CCs.

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


Re: [Mesa-dev] [PATCH] Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"

2016-11-10 Thread Daniel Stone
Hi,

On 10 November 2016 at 06:08, Jonas Ådahl  wrote:
> On Mon, Oct 24, 2016 at 08:42:59PM +0100, Daniel Stone wrote:
>> The intent of moving blocking from SwapBuffers to get_back_bo, was to
>> avoid unnecessary triple-buffering by ensuring that the compositor had
>> fully processed the previous frame before we started rendering. This
>> means that the only time we would have to resort to triple-buffering
>> would be when the buffer is directly scanned out, thus saving an extra
>> buffer for composition anyway.
>>
>> The 'repaint window' changes introduced in Weston since then, however,
>> have narrowed the window of time between the frame event being sent and
>> the repaint loop needing to conclude, to 7ms by default, in order to
>> reduce latency. This means however that blocking in get_back_bo gives a
>> maximum of 7ms for the entire GL submission to begin and complete.
>>
>> Not only this, but if a client is using buffer_age to avoid full
>> repaints, the buffer-age request will stall in get_back_bo until the
>> frame callback completes, meaning that the client cannot even calculate
>> the repaint area before the 7ms window.
>>
>> The combination of the two meant that WebKit-GTK+ was failing to
>> achieve full framerate on a Minnowboard, due to spending a great deal of
>> its time attempting to query the age of the next buffer before redraw.
>>
>> Revert to the previous behaviour of allowing rendering to begin but
>> delaying SwapBuffers, unless and until we can find a more gentle
>> behaviour.
>>
>> Signed-off-by: Daniel Stone 
>> Cc: Neil Roberts 
>> Cc: Kristian Høgsberg 
>
> The explanations sounds sane to me, and even though it's a revert, I
> looked at the patch and it seems correct, so this is
>
> Reviewed-by: Jonas Ådahl 
>
> I suspect we can't avoid tripple buffering if the client asks for
> information about the next buffer early enough, since mesa won't have a
> clue what the future will be regarding any buffer being released or not
> before the frame callback.

Thanks both for the review/testing - pushed now.

> Or maybe we could possibly fake the buffer age, pretending the buffer is
> a fresh one if the client queries "too early", but then eventually
> re-use the buffer had it been released before the frame callback. We'd
> at least avoid the extra buffer being allocated, while instead only
> wasting resources painting the whole buffer which we would do anyway if
> we allocated the third buffer.

That relies on the compositor releasing buffers in a strictly FIFO
manner though, which is ... dangerous.

If you're interested in buffer age though, here's another one:
https://lists.freedesktop.org/archives/mesa-dev/2016-November/134787.html

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


Re: [Mesa-dev] [PATCH] gbm/drm: Pick the oldest available buffer in get_back_bo

2016-11-10 Thread Daniel Stone
Hi,  
  

  
On Nov 10 2016, at 2:09 pm, Pekka Paalanen  wrote:  

> On Wed, 9 Nov 2016 14:57:22 -0600  
Derek Foreman  wrote:

>

> > We find the oldest backbuffer we can, on the grounds that clients are  
> only going to keep a fixed history queue, so this gives them the  
> greatest chance of being able to use that queue via making sure  
> the age is ~always less than the depth of the swapchain

>

> right, that is one side of the story.  

>

> On the other side, damage accumulates frame by frame. When you always  
pick the oldest free buffer, you also maximize the accumulated damage  
and need to repaint more than if you picked the youngest free buffer.

>

> Furthermore, if you normally can do with cycling just two buffers, and  
a hickup causes you to allocate up to four buffers and afterwards you  
come back to the normal steady flow, you are now always cycling through  
all the four buffers and repainting damage for age=4 instead of age=2.

  
This would be a separate problem; the queue depth should really not be any
longer than need be. If there is a case where it ends up longer (say when a
surface switches from direct scanout to GPU composition), then Mesa should be
realising this and trimming the queue. Other stacks do this.  
  

> If one was always picking the youngest free buffer, we could even have  
heuristics to destroy buffers that have not been used for N swaps to  
release some memory.

  
That is not strictly dependent on picking the youngest buffer; it could/should
be done regardless.  
  

> There is a trade-off between repainting a little more than necessary in  
the normal case to mitigate the impact on hickups and making the normal  
case optimized while hickups cause a heavy hit (full repaint plus  
possibly even allocating a new buffer). However, in a proper compositor  
I fail to see how such hickups would even occur - so you wouldn't need  
to mitigate hickups but also using the oldest would not be any worse  
since you never allocate extra buffers.

  
Repainting more than necessary will only occur when the damage area is
changing literally frame-by-frame. This is kind of an odd case, since it means
that your eyes will have to be retargeting every 16ms.  
  

> It would be nice to see more rationale in the commit message about why  
picking the oldest is better than picking the youngest buffer. Age  
greater than the length of the swapchain is only a trigger for full  
repaint, just like a freshly allocated buffer is, except you skip the  
cost of allocating.

>

> My counter-proposal is probably worse, but I'd like to see an  
explanation because it's not obvious.

  

Picking the youngest means that, as you say, predictability decreases in the
case where one buffer gets stuck for a very long time. This arguably shouldn't
happen generally, but provably does right now until Mesa gains the smarts to
trim the buffer queue. I can see how this would be provoked on a CPU-bound
system where we are doing direct scanout; by picking the youngest, we react to
this situation by generating more CPU load.

  

Picking the oldest means that the queue remains balanced, which does imply a
complexity increase in the case that the surface damage area changes from
frame to frame. I do not believe that this is a common case, and even if it
were, the downside of this is not as bad as the downside of picking the
youngest.

  

Cheers,

Daniel

___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/3] egl/wayland: use the destroy_window_callback for swrast

2016-11-11 Thread Daniel Stone
Hi,

On 11 November 2016 at 16:44, Emil Velikov  wrote:
> As described in commit 690ead4a135 ("egl/wayland-egl: Fix for segfault
> in dri2_wl_destroy_surface.") if we attempt to destroy a EGL surface
> attached to already destroyed Wayland window we'll get a segfault.

s/destroy/resize/ ... ? The patch itself looks fine, but I don't
really see how the description relates.

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


Re: [Mesa-dev] [PATCH 3/3] egl/wayland: unify dri2_wl_create_surface implementations

2016-11-11 Thread Daniel Stone
Hi,

On 11 November 2016 at 16:45, Emil Velikov  wrote:
> @@ -174,14 +172,24 @@ dri2_wl_create_surface(_EGLDriver *drv, _EGLDisplay 
> *disp,
> config = dri2_get_dri_config(dri2_conf, EGL_WINDOW_BIT,
>  dri2_surf->base.GLColorspace);
>
> -   dri2_surf->dri_drawable =
> -  (*dri2_dpy->dri2->createNewDrawable)(dri2_dpy->dri_screen, config,
> -   dri2_surf);
> +   if (dri2_dpy->dri2) {
> +  dri2_surf->wl_win->resize_callback = resize_callback;
> +
> +  createNewDrawable = dri2_dpy->dri2->createNewDrawable;
> +   } else {
> +  createNewDrawable = dri2_dpy->swrast->createNewDrawable;
> +   }

Hm, we've just lost the place we set resize_callback for swrast now.
:\ This should be called for both paths, no? And presumably also only
set after calling createNewDrawable, so we the following doesn't
explode:
win = wl_egl_window_create(...);
surf = eglCreateSurface(..., win); /* fails */
wl_egl_window_resize(win, ...); /* unexpectedly calls into
resize_callback */

No wait, after reading that, resize_callback is essentially a no-op:
the flush extension doesn't exist for swrast, so resize_callback will
explode if ever called on swrast. Does this mean that 1/3 is generally
wrong, and needs to set destroy_window_callback rather than
resize_callback instead?

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


Re: [Mesa-dev] [PATCH] egl/wayland: fix return value in dri2_wl_swrast_commit_backbuffer

2016-11-11 Thread Daniel Stone
Hi,

On 11 November 2016 at 16:27, Emil Velikov  wrote:
> The function returns "void" rather than int. We could rework that, yet
> again there will be no benefit since all the callers have no use of it.
>
> Fixes: 9ca6711faa0 ("Revert "wayland: Block for the frame callback in
> get_back_bo not dri2_swap_buffers"")
> Cc: Daniel Stone 
> Signed-off-by: Emil Velikov 

Not sure how I missed this; apologies. Pushed now with my R-b.

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


Re: [Mesa-dev] [PATCH v2] egl_dri2: add support for using modifier attributes in eglCreateImageKHR

2016-11-18 Thread Daniel Stone
Hi,

On 18 November 2016 at 14:50, Emil Velikov  wrote:
> On 16 November 2016 at 09:28, Varad Gautam  wrote:
>> +   if (nonzero_modifier_found && dri2_dpy->image->createImageFromDmaBufs2) {
>> +  dri_image =
>> + dri2_dpy->image->createImageFromDmaBufs2(dri2_dpy->dri_screen,
>> +attrs.Width, attrs.Height, attrs.DMABufFourCC.Value,
>> +fds, num_fds, pitches, offsets, modifiers,
>> +attrs.DMABufYuvColorSpaceHint.Value,
>> +attrs.DMABufSampleRangeHint.Value,
>> +attrs.DMABufChromaHorizontalSiting.Value,
>> +attrs.DMABufChromaVerticalSiting.Value,
>> +&error,
>> +NULL);
>> +   } else {
>> +  if (nonzero_modifier_found) {
>> + _eglError(EGL_BAD_MATCH, "unsupported dma_buf format modifier");
>> + return EGL_NO_IMAGE_KHR;
>> +  }
>> +
> Using something like the following might be better?
>
> if (nonzero_modifier_found) {
>if (!dri2_dpy->image->createImageFromDmaBufs2)
>  # assert should never reach here, since the extension should be
> advertised only if the API is available.
>use new API
> else
>use old API

Actually, present-and-zero modifier has a very well-defined meaning:
it _forces_ linear interpretation of the buffer, whereas a non-present
modifier may cause a kernel query (e.g. i915_gem_get_tiling) to
discover a hidden tiling mode. So, if present, the modifier should be
passed.

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


Re: [Mesa-dev] [PATCH v2] egl_dri2: add support for using modifier attributes in eglCreateImageKHR

2016-11-18 Thread Daniel Stone
Hi Emil,

On 18 November 2016 at 15:24, Emil Velikov  wrote:
> On 18 November 2016 at 15:17, Daniel Stone  wrote:
>> Actually, present-and-zero modifier has a very well-defined meaning:
>> it _forces_ linear interpretation of the buffer, whereas a non-present
>> modifier may cause a kernel query (e.g. i915_gem_get_tiling) to
>> discover a hidden tiling mode. So, if present, the modifier should be
>> passed.
>>
> You are suggesting that we should track "has_modifier" (as opposed to
> nonzero_modifier_found) and pass it to DmaBuf2 regardless of the
> contents, right ?

Yep, exactly that.

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


Re: [Mesa-dev] Stable release process

2016-11-19 Thread Daniel Stone
Hey Marek,

On 18 November 2016 at 19:09, Marek Olšák  wrote:
> On Fri, Nov 18, 2016 at 7:45 PM, Kai Wasserbäch
>  wrote:
>> wouldn't a tool like Phabricator be much better for reviewing and reliably
>> tracking whether a patch has landed or not? Especially if you use it in
>> combination with Arcanist? While I'm certainly not a core developer, I find
>> patchwork clunky. Sometimes it doesn't pick up R-bs or doesn't recognise 
>> series,
>> which makes seeing the actual state of a patch a bit tricky from time to 
>> time.
>>
>> In addition you would get things like automatically closure of bugs, nice
>> referencing features and lots of other nice features. And AFAIK 
>> freedesktop.org
>> already has a Phabricator instance, which could be used.
>
> OK, off topic we go.
>
> I have some experience with Phabricator and Arcanist from LLVM and
> it's not very good.
>
> Phabricator (or Arcanist) doesn't support patch series. You can only
> submit one patch, or a range of commits as one patch (which is pretty
> bad - why would anyone on Earth want to do that). It also doesn't
> support downloading patches in the mbox format (only plain diffs).
> Based on that, I don't recommend it.

Off-topic indeed. Arcanist is crippled, as you say, and I don't like
its UI. But git-phab does exist to let you attach patch series, e.g.
https://phabricator.freedesktop.org/T7573 contains a load.

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


Re: [Mesa-dev] [PATCH] Revert "wayland: Block for the frame callback in get_back_bo not dri2_swap_buffers"

2016-11-21 Thread Daniel Stone
Hi Jonas,

On 21 November 2016 at 05:56, Jonas Ådahl  wrote:
> On Thu, Nov 10, 2016 at 10:38:51AM +0000, Daniel Stone wrote:
>> On 10 November 2016 at 06:08, Jonas Ådahl  wrote:
>> > On Mon, Oct 24, 2016 at 08:42:59PM +0100, Daniel Stone wrote:
>> That relies on the compositor releasing buffers in a strictly FIFO
>> manner though, which is ... dangerous.
>
> Is FIFO an issue here? Lets assume it looks like this:
>
> [...]
>
> In other terms, if we don't have a buffer released at the point we
> query, we say its a new buffer, making the application assume it needs
> to redraw everything. When we then actually need the buffer for doing
> things, we see if there is any buffer to reuse. What buffer was actually
> released doesn't matter, since the application will redraw everything
> anyway, and if we actually did reuse a buffer, the previously returned
> buffer age was a lie, but a harmless one, and if we eventually actually
> did allocate a new one, it wouldn't even be a lie.

Sorry, I see what you mean now - thanks for the snippet!

Yes, I think that's certainly doable; we could add a parameter to
get_back_bo controlling its wait behaviour. Specifically, when called
from query_buffer_age we would return a released buffer (if any), do
one roundtrip and check again if we didn't have one yet, and if the
roundtrip didn't return any buffers, then we could return an age of -1
to allow the client to continue on. When called from
get_buffers_with_format, it would spin or allocate a new buffer. I
suppose there is a degenerate case there, where you could end up
always failing the age check but not allocating new buffers, but I'm
far too Monday'ed to sketch out what that would be.

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


Re: [Mesa-dev] [PATCH 00/27] Renderbuffer Decompression (and GBM modifiers)

2016-12-02 Thread Daniel Stone
Hey Ben,
Sorry I didn't get to testing this before now; have been tied up with
all manner of stuff.

On 1 December 2016 at 22:09, Ben Widawsky  wrote:
> The overall strategy is that the buffer/surface is created with a list of
> modifiers. The list of modifiers the hardware is capable of using will come 
> from
> a new kernel API that is aware of the hardware and general constraints. A 
> client
> will request the list of modifiers and pass it directly back in during buffer
> creation (potentially the client can prune the list, but as of now there is no
> reason to.) This new API is being developed by Kristian. I did not get far
> enough to play with that.
>
> For EGL, a similar mechanism would exist whereby when importing a buffer into
> EGL, one would provide a modifier and probably a pointer to the auxiliary data
> upon import. (Import therefore might require multiple dma-buf fds), but for 
> i965
> and Intel, this wouldn't be necessary.

Right, we have EGL_EXT_image_dma_buf_import_modifiers; Varad has a
series on the list already for this which just needs some reviews
(ahem).

> Here is a brief description of the series:
> 1-6 Adds support in GBM for per plane functions where necessary. This is
> required because the kernel expects the auxiliary buffer to be passed along 
> as a
> plane. It has its own offset, and stride, and the client shouldn't need to
> calculate those.

This is missing gbm_bo_get_handle_for_plane(); as you say, a lot of
other hardware tends to use separate buffers rather than
adjacent/offset. So adding that would be nice. Having
gbm_bo_get_plane_count() is really nice though, since it allows us to
have a completely agnostic client (i.e. I don't have to have a map
inside Weston with every exotic format/modifier combination).

> 7-9 Adds support in GBM to understand modifiers. When creating a buffer or
> surface, the client is expected to pass in a list of modifiers that the driver
> will optimally choose from. As a result of this, the GBM APIs need to support
> modifiers.

This bit seems good, and like a reasonable fit for the draft of
GETPLANE2 which is kicking around.

> 10-12 Support Y-tiled modifier. Y-tiling was already a modifier exposed by the
> kernel. With the previous patches in place, it's easy to support this too.

And it works! \o/

> 13-26 Plumbing to support sending CCS buffers to display. Leveraging much of 
> the
> existing code for MCS buffers, these patches creating an MCS for the scanout
> buffer. The trickery here is that a single BO contains both the main surface 
> and
> the auxiliary data. Previously, auxiliary data always lived in its own BO.
>
> 27 Support CCS-modifier. Finally, the code can parse the CCS fb modifier(s) 
> and
> realize the bandwidth savings that come with it.

I've not rebuilt my kernel to test the new CCS bits, so I haven't tested this.

> This was tested using kmscube
> (https://github.com/bwidawsk/kmscube/tree/modifiers). The kmscube 
> implementation
> is missing support for GET_PLANE2 - which is currently being worked on by
> Kristian.

There's also a Weston branch here:
https://git.collabora.com/cgit/user/daniels/weston.git/log/?h=wip/2016-11/gbm-planes-modifiers

This works with Y-tiling for me, but with the same need for
GET_PLANE2; also the branch as-is will provoke a segfault inside
gbm_dri_bo_get_modifier(), which ends up calling intel_query_image()
with image == NULL, when using cursor images. To get it to succeed,
you need to shove an early 'return -1' inside
drm_output_init_cursor_egl() so we fall back to software (well OK, GL)
cursors.

The branch is broken with multihead, but that's the branch it's based
on being broken/WIP, not a result of these patches.

> Upstream plan:
> 1. All of the patches up through 26 should be mergeable today after review.
> 2. After 1-12 land, client support of Y-tiling should be achievable. 
> Modesetting
> driver can probably be updated as can things like Weston. Clients assuming a 
> new
> enough kernel should be able to blindly set the y tiled modifier.
> 3. Once kernel and libdrm support for CCS modifiers, patch 27 can land, 
> however
> CCS isn't yet usable, it is only available as a prototype.
> 4. Kristian's GET_PLANE2 interface needs to be solidified and land.
> 5. Clients will utilize #3 and #4 to use CCS.
> 6. Protocol work, EGL, Wayland, DRIX - etc

Wayland has modifier support already; there are patches out for review
for Weston to support this via the EGL extension above, as well as
inside KMS (part of the atomic branch).

> When Kristian's interface is ready, kmscube can be modified to make use of it.

And I'll modify Weston to use it as well.

Thanks for this, and sorry for the tardy review.

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


Re: [Mesa-dev] [PATCH 09/27] gbm: Introduce modifiers into surface/bo creation

2016-12-02 Thread Daniel Stone
Hi Ben,

On 1 December 2016 at 22:09, Ben Widawsky  wrote:
> @@ -996,13 +997,22 @@ gbm_dri_bo_create(struct gbm_device *gbm,
> dri_use |= __DRI_IMAGE_USE_SHARE;
>
> bo->image =
> -  dri->image->createImage(dri->screen,
> -  width, height,
> -  dri_format, dri_use,
> -  bo);
> +  dri->image->createImageWithModifiers(dri->screen,
> +   width, height,
> +   dri_format, dri_use,
> +   modifiers, count,
> +   bo);
> if (bo->image == NULL)
>goto failed;
>
> +   bo->base.base.modifiers = calloc(count, sizeof(*modifiers));
> +   if (!bo->base.base.modifiers) {

if (count && ...)

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


Re: [Mesa-dev] [PATCH 11/27] gbm: Get modifiers from DRI

2016-12-02 Thread Daniel Stone
Hi Ben,

On 1 December 2016 at 22:09, Ben Widawsky  wrote:
> @@ -678,6 +679,28 @@ gbm_dri_bo_get_offset(struct gbm_bo *_bo, int plane)
> return (uint32_t)offset;
>  }
>
> +static uint64_t
> +gbm_dri_bo_get_modifier(struct gbm_bo *_bo)
> +{
> +   struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
> +   struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
> +
> +   if (!dri->image || dri->image->base.version < 14) {
> +  errno = ENOSYS;
> +  return 0;
> +   }

Sticking this here prevents my cursor crash:
+   /* Dumb buffers have no modifiers */
+   if (!bo->image)
+  return 0;

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


Re: [Mesa-dev] [PATCH 02/27] gbm: Fix width height getters return type (trivial)

2016-12-02 Thread Daniel Stone
Hi,

On 2 December 2016 at 17:56, Eric Engestrom  wrote:
> On Thursday, 2016-12-01 14:09:43 -0800, Ben Widawsky wrote:
>> --- a/src/gbm/main/gbm.h
>> +++ b/src/gbm/main/gbm.h
>> @@ -294,10 +294,10 @@ gbm_bo_map(struct gbm_bo *bo,
>>  void
>>  gbm_bo_unmap(struct gbm_bo *bo, void *map_data);
>>
>> -uint32_t
>> +unsigned int
>>  gbm_bo_get_width(struct gbm_bo *bo);
>>
>> -uint32_t
>> +unsigned int
>>  gbm_bo_get_height(struct gbm_bo *bo);
>
> I'm not sure I understand this change. Why would you want to remove the
> information of the type size? If the point is to increase it on 64-bit
> machines, I'd go with an explicit `uint64_t` instead.

I have to admit I didn't catch this one. It doesn't help on 64-bit
since unsigned int is still 32-bit there, and in any case it's library
ABI, so if it doesn't change anything then it doesn't help, and if it
does then it's an ABI break, so NAK from me.

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


Re: [Mesa-dev] [PATCH 02/27] gbm: Fix width height getters return type (trivial)

2016-12-02 Thread Daniel Stone
Hi Ben,

On 2 December 2016 at 18:17, Ben Widawsky  wrote:
> On 16-12-02 18:07:22, Daniel Stone wrote:
>> On 2 December 2016 at 17:56, Eric Engestrom  
>> wrote:
>> I have to admit I didn't catch this one. It doesn't help on 64-bit
>> since unsigned int is still 32-bit there, and in any case it's library
>> ABI, so if it doesn't change anything then it doesn't help, and if it
>> does then it's an ABI break, so NAK from me.
>
> It was like the patch says, meant to match the definition of the
> implementation.
> The exported symbol is defined as unsigned int. It had nothing to do with
> 64-bit.
>
> GBM_EXPORT unsigned int
> gbm_bo_get_height(struct gbm_bo *bo)
>
> I'd say they should match, and both can be uint32_t. I don't care much
> either
> way.

Right. Given that we have multiple implementations of libgbm in the
wild, I'd be much more comfortable saying that the existing gbm.h is
canonical, and changing the Mesa implementation to match.

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


Re: [Mesa-dev] [PATCH mesa] add EGL_TEXTURE_EXTERNAL_WL to WL_bind_wayland_display spec

2016-12-05 Thread Daniel Stone
Hi,

On 10 June 2014 at 15:46, Rob Clark  wrote:
> On Mon, Jun 9, 2014 at 5:53 AM, Pekka Paalanen  wrote:
>> On Thu, 16 Aug 2012 17:28:19 -0500 Rob Clark  wrote:
>>> From: Rob Clark 
>>
>> it looks like this patch never made it into Mesa. Also the
>> implementation apparently didn't make it into Mesa, as git pick-axe
>> does not find any mention of EGL_TEXTURE_EXTERNAL_WL.
>>
>> Still, the Weston patch was merged on Aug 31st, 2012.
>>
>> Oops. :-)
>
> heh, well I guess if weston is already using it, perhaps we should
> think about merging the mesa patch ;-)

Yeah, the fact there are three different email addresses for you in
this thread probably says that it's been long enough. ;) Pushed now.

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


Re: [Mesa-dev] [PATCH 1/3] gallium: add renderonly library

2016-12-09 Thread Daniel Stone
Hi Alexandre,

On 9 December 2016 at 13:20, Alexandre Courbot  wrote:
> On 12/08/2016 04:16 PM, Alexandre Courbot wrote:
>> First, setting the tiling works indeed just fine if we are using an
>> ioctl for this. However my impression was that the preferred way of
>> doing it was through FB modifiers, and we started moving Tegra to this
>> scheme. Problem: the FB modifier is passed through a call to
>> drmModeAddFB2WithModifiers(), which is called by the client program, not
>> Mesa - which in this case leaves the program with the burden of figuring
>> out what the modifier should be. So with FB modifiers the problem is
>> still here.
>>
>> Another issue I have seen is that GLX does not seem to work with this.
>> X/modesetting starts just fine, and GLamor also seems to initialize.
>> However glxinfo freezes on a xshmfence_await() call, and all GLX
>> programs fail as follow:
>
> Solved that issue by forcing is_different_gpu to true in
> loader_dri3_drawable_init() (pretty hackish, looking for a better way).
>
> Also I had another issue with Wayland where EGL windows would be
> displayed all black. I traced this to the fact that Wayland was trying
> to share the buffer by calling the old FLINK ioctl on the rendernode
> device, which is forbidden. Opening card1 instead of renderD128 did the
> trick as a workaround, but I am surprised as I thought Wayland was using
> DRI3 exclusively? I am not very familiar with neither Mesa nor Wayland
> though, so my assumption may very well be incorrect.

Wayland doesn't use DRI-anything; Mesa has its own interface for
Wayland. I'm really surprised that you're seeing this behaviour
though: if you search for WL_DRM_CAPABILITY_PRIME (i.e. send dmabufs
rather than flink names) in src/egl/drivers/dri2/platform_wayland.c,
you'll see that a) we always use it when available, and b) we refuse
to initialise when the device is a rendernode and we don't have PRIME.
So I'm not sure how this could ever happen ...

> Anyway, with this patch and the corresponding Tegra support, I have a
> working solution that can run unmodified Mesa applications using KMS,
> EGL/Wayland and GLX backends on TK1 and TX1 platforms. Neat!

Cool! I assume this will work on Tegra124 more generally then - do you
have a branch somewhere?

> Considering that we have been ressorting to hacking all the KMS
> applications of interest to connect the render and display nodes
> together with the right tiling settings for the last two years, I regard
> this patch as a huge improvement for mobile graphics and would like to
> strongly support it.
>
> My only remaining concern is that this scheme cannot support the case
> where the tiling format is specified using FB modifiers, since this
> requires drmModeAddFB2WithModifiers() to be called from the application.
> So for Tegra we have to resort to a staging, not enabled by default
> SET_TILING ioctl. Not ideal, but recompiling your kernel with an
> additional config option is much less a hassle than patching every KMS
> app under the sun.
>
> So while thoughts about how this last issue can be addressed are
> welcome, I think this little lib can improve the life of many SoC users.

Check out Ben Widawsky's 'Renderbuffer Decompression (and GBM
modifiers)' patchset. With this, as well as krh's pending GETPLANE2
ioctl that will allow us to get a list of acceptable modifiers for
display from KMS, we can trivially implement this in clients without
the need for a backchannel ioctl:
https://git.collabora.com/cgit/user/daniels/weston.git/commit/?h=wip/2016-11/gbm-planes-modifiers

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


Re: [Mesa-dev] Proposal of date-based Mesa versioning for 2017

2016-12-12 Thread Daniel Stone
Hi,

On 12 December 2016 at 15:28, Emil Velikov  wrote:
> As mentioned by others - having the second number represent the month
> would be better, afaict.
> Namely: YY.MM.PP. Thus 17.02.01 provides direct and clear feedback that
>  - 2017 release, from the second month (Feb).
>  - first bugfix release.

Not being funny, but does this mean that 17.02 bugfix releases would
have to all be done in February, or could .mm.xx with xx > 0, mean
that the release was not done in that month, but just the branching
was?

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


Re: [Mesa-dev] [PATCH] anv: add function to get prime buffer from memory+image

2016-07-20 Thread Daniel Stone
Hi Jonathan,

On 19 July 2016 at 20:47, Jonathan  wrote:
> +typedef VkResult (VKAPI_PTR *PFN_vkGetDmaBufINTEL)(VkDevice device, 
> VkDeviceMemory mem, VkImage image, int *fd, uint32_t *pitch);

Some things you should consider adding to this:
  - multi-plane support for multi-buffer formats (multiple fds,
multiple pitches, per-plane offset parameter)
  - an out parameter for format, using the DRM FourCC format codes
  - out parameters for a DRM modifier per-plane, to account for tiling
etc (and no longer calling anv_gem_set_tiling)

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


Re: [Mesa-dev] [PATCH] anv: add function to get prime buffer from memory+image

2016-07-20 Thread Daniel Stone
On 20 July 2016 at 13:47, Daniel Stone  wrote:
> On 19 July 2016 at 20:47, Jonathan  wrote:
>> +typedef VkResult (VKAPI_PTR *PFN_vkGetDmaBufINTEL)(VkDevice device, 
>> VkDeviceMemory mem, VkImage image, int *fd, uint32_t *pitch);
>
> Some things you should consider adding to this:
>   - multi-plane support for multi-buffer formats (multiple fds,
> multiple pitches, per-plane offset parameter)
>   - an out parameter for format, using the DRM FourCC format codes
>   - out parameters for a DRM modifier per-plane, to account for tiling
> etc (and no longer calling anv_gem_set_tiling)

Oops, hit send too early. Being able to export a VkFence to a kernel
fence fd, as a companion, would also be incredibly helpful.

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


Re: [Mesa-dev] [PATCH 8/8] egldevice: implement eglQueryDisplayAttribEXT

2016-07-21 Thread Daniel Stone
Hi,

On 21 July 2016 at 15:11, Emil Velikov  wrote:
> On 21 July 2016 at 14:57, Adam Jackson  wrote:
>>> +   device_name = drv->QueryDeviceName(disp);
>>
>> This is /dev/dri/renderD128...
>>
>>> +   mtx_lock(_eglGlobal.Mutex);
>>> +
>>> +   assert(info->got_devices);
>>> +
>>> +   for (dev = info->devices; dev; dev = dev->Next) {
>>> +  const char *devname = udev_device_get_property_value(
>>> + dev->Info, "DEVNAME");
>>
>> And this is /dev/dri/card0, so querying the display will always fail.
>>
>> Obviously I can paper over this when there's only one device in the
>> list, but the whole reason I want this is to make multi-GPU work
>> better. Any ideas on a better approach here?
>>
> I'd suggest opting for the drmDevice libdrm API. It can provide a list
> of devices with all the nodes and other misc info. Thus we could use
> the render/card/other node as any point as needed.

Indeed.

I don't believe Jonny is working on this anymore, and I'm pretty
preoccupied, so it would be great if someone could pick this one up.

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


Re: [Mesa-dev] Move wayland-drm protocol from Mesa to wayland-core

2013-11-28 Thread Daniel Stone
Hi,

On 28 November 2013 10:04, Pekka Paalanen  wrote:
> On Thu, 28 Nov 2013 10:24:33 +0100
> Benjamin Gaignard  wrote:
>> From my point of view wl_drm isn't link to Mesa, it is only about
>> exchange buffers by using a file descriptor and, for example, doesn't
>> rely on EGL.
>>
>> I understand that other graphic stacks could have defined their own
>> way to for zero-copy (and so other protocols).
>> I don't aim to make gstreamer wayland sink works for all of them but
>> at least with wl_drm protocol which is quite generic.
>>
>> Since dmabuf has been adopted in kernel we have the opportunity to
>> rationalize also some code in userland and for that we need a common
>> wayland protocol.
>> Move wl_drm into wayland-core make it more easily accessible for all
>> software even for those who don't use Mesa.
>
> if you aim particularly for dma-buf, then I would propose making a new
> Wayland protocol extension mesa_dma_buf or gbm_dma_buf or whatever name
> is appropriate, and drop all the unneeded parts compared to wl_drm.

Given that dmabuf doesn't specify format codes or anything like that,
you can't make it properly generic.  My original thought was to make a
generic wl_dmabuf interface, but given the format code issue, it
seemed prudent to leave it as a part of wl_drm for now.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-14 Thread Daniel Stone
Hi,

On Fri, 11 Jan 2019 at 17:05, Jason Ekstrand  wrote:
>  5. There's no way with gitlab for Reviewed-by tags to get automatically 
> applied as part of the merging process.  This makes merging a bit more manual 
> than it needs to be but is really no worse than it was before.

I'm still on the side of not seeing the value in them. Most of the
time when I go to pursue someone who reviewed a commit, I'll go to see
what came up in review anyway. Maybe someone had the same comment
which was found to be not applicable or otherwise explained away.
Reviewed-by and Acked-by are also pretty lossy anyway, and freeform
text descriptors in a comment can much better capture the intent (e.g.
'I'm strongly OK with the driver changes and weakly OK with the core
changes as it's not really my area of expertise').

In other projects, we looked for ways to apply the tags and ended up
concluding that they didn't bring enough value to make it worthwhile.
I don't know if that holds for Mesa, but it would be better to start
with an actual problem statement - what value does R-b bring and how?
- then look at ways to solve that problem, rather than just very
directly finding a way to insert that literal text string into every
commit message.

FWIW, if you go to
https://gitlab.freedesktop.org/mesa/mesa/commit/SHA1 then you get a
hyperlink from the web UI which points you to the MR. The API to do
this is pretty straightforward and amenable to piping through jq:
https://docs.gitlab.com/ce/api/commits.html#list-merge-requests-associated-with-a-commit

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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-15 Thread Daniel Stone
Hi,

On Tue, 15 Jan 2019 at 12:21, Rob Clark  wrote:
> On Tue, Jan 15, 2019 at 1:02 AM Tapani Pälli  wrote:
> > On 1/14/19 2:36 PM, Daniel Stone wrote:
> > > On Fri, 11 Jan 2019 at 17:05, Jason Ekstrand  wrote:
> > > In other projects, we looked for ways to apply the tags and ended up
> > > concluding that they didn't bring enough value to make it worthwhile.
> > > I don't know if that holds for Mesa, but it would be better to start
> > > with an actual problem statement - what value does R-b bring and how?
> > > - then look at ways to solve that problem, rather than just very
> > > directly finding a way to insert that literal text string into every
> > > commit message.
> >
> > IMO it brings some 'shared responsibility' for correctness of the patch

Oh, no doubt - we certainly haven't abandoned thorough review! So far
we haven't seen that compromised by not having a name in the commit
message.

> > and quickly accessible information on who were looking at the change. So
> > ideally later when filing bug against commit/series there would be more
> > people than just the committer that should take a look at the possible
> > regressions. At least in my experience people filing bugs tend to often
> > also CC the reviewer.

Yeah, that's really helpful. So maybe a useful flow - assuming we
eventually switch to GitLab issues - would be the ability to associate
an issue with a commit, which could then automatically drag in people
who commented on the MR which landed that commit, as well as (at
least) the reporter of the issue(s) fixed by that MR. That would need
some kind of clever - probably at least semi-manual - filtering to
make sure it wasn't just spamming the world, but it's at least a
starting point.

> +1 .. and also it is nice to see things like Reported-by/Reviewed-by
> without having to go search somewhere else (ie. outside of git/tig)

My question would again be what value that brings you. Do you just
like seeing the name there, or do you go poke the people on IRC, or
follow up via email, or ... ? Again I personally go look through the
original review to see what came up during that first, but everyone's
different, so I'm just trying to understand what you actually do with
that information, so we can figure out if there's a better way to do
things for everyone rather than just blindly imitating what came
before.

> (ofc it would be pretty awesome incentive to switch to gitlab issues
> if gitlab could automate adding Reported-by tags for MR's associated
> with an issue.. but I guess checkbox to add Reviewed-by tag would
> already make my day)

I saw this the other day, which might be more incentive:
https://csoriano.pages.gitlab.gnome.org/csoriano-blog/post/2019-01-07-issue-handling-automation/

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


Re: [Mesa-dev] [PATCH] egl/wayland: break double/tripple buffering feedback loops

2019-01-15 Thread Daniel Stone
Hi,

On Tue, 18 Dec 2018 at 17:59, Lucas Stach  wrote:
> Am Dienstag, den 18.12.2018, 17:43 + schrieb Emil Velikov:
> > > On Tue, 18 Dec 2018 at 11:16, Lucas Stach  wrote:
> > >   if (dri2_surf->back == NULL)
> > >  dri2_surf->back = &dri2_surf->color_buffers[i];
> > > - else if (dri2_surf->back->dri_image == NULL)
> > > + else if (dri2_surf->back->dri_image == NULL && 
> > > dri2_surf->color_buffers[i].dri_image)
> > >  dri2_surf->back = &dri2_surf->color_buffers[i];
> > > + age = dri2_surf->back->age;
> > >}
> > >
> >
> > AFAICT this is the wayland equivalent of
> > 4f1d27a406478d405eac6f9894ccc46a80034adb
> > Where the exact same logic/commit message applies.
>
> No it isn't. It's exactly what it says in the commit log. It's keeping
> the tripple buffer around for a bit, even if we don't strictly need it
> when the client is currently doing double buffering.

Right - the crucial part in Derek's GBM commit was removing the
'break' and adding the extra conditional on age.

Derek's patch stabilises the age of buffers handed back to the user,
by always returning the oldest available buffer. That slightly
pessimises rendering if there is a 'free' buffer in the queue: if four
buffers are allocated, then we will always return a buffer from three
flips ago, maybe meaning more rendering work. It means that, barring
the client holding on to one buffer for unexpectedly long, the age of
the oldest buffer in the queue will never be greater than the queue
depth.

This patch instead relies on unbalanced ages, where older buffers in
the queue are allowed to age far beyond the queue depth if not used
during normal rendering.

> When things are on the edge between double buffering being enough and
> sometimes a third buffer being needed to avoid stalling we would
> otherwise bounce rapidly between allocating and disposing the third
> buffer.
>
> The DRM platform has no such optimization and just keeps the third
> buffer around forever. This patch keeps the optimization in the Wayland
> platform, but adds a bit of hysteresis before disposing the buffer when
> going from tripple to double buffering to see if things are settling on
> double buffering.

Ideally we'd have globally optimal behaviour for both platforms, but
that doesn't really seem doable for now. I think this is a good
balance though. There will only be one GBM user at a time, so having
that allocate excessive buffers doesn't seem too bad, and the penalty
for doing so is your entire system stuttering as the compositor
becomes blocked. Given the general stability of compositors, if they
need a larger queue depth at some point, they are likely to need it
again in the near future.

Conversely, there may be a great many Wayland clients, and these
clients may bounce between overlay and GPU composition. Given that, it
seems reasonable to opportunistically free up buffers, to make sure we
have enough memory available across the system.

> > The age check here seems strange - both number used and it's relation
> > to double/triple buffering.
> > Have you considered tracking/checking how many buffers we have?
>
> A hysteresis value of 18 is just something that worked well in
> practice. It didn't appear to defer the buffer destruction for too long
>  while keeping the feedback loop well under control.

Yeah, having this #defined with a comment above it would be nice.

With that, this patch is:
Reviewed-by: Daniel Stone 

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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-15 Thread Daniel Stone
Hey,

On Tue, 15 Jan 2019 at 20:22, Rob Clark  wrote:
> On Tue, Jan 15, 2019 at 7:40 AM Daniel Stone  wrote:
> > My question would again be what value that brings you. Do you just
> > like seeing the name there, or do you go poke the people on IRC, or
> > follow up via email, or ... ? Again I personally go look through the
> > original review to see what came up during that first, but everyone's
> > different, so I'm just trying to understand what you actually do with
> > that information, so we can figure out if there's a better way to do
> > things for everyone rather than just blindly imitating what came
> > before.
>
> If I am curious or have some questions about why some code is the way
> it is I frequently use tig-blame, which makes it easy to step into the
> commit that made the change and see the commit msg and r-b tags..  I
> guess the most important part if I need to ping someone on IRC w/
> questions is the author, but it seems like having the other tags handy
> without context-switching to browser/gitlab is useful.
>
> I guess I don't as frequently dig into the history of the original
> patchset and it's review comments.. mostly because that isn't as easy
> with the email based review process.  Making this easier would defn be
> a win.  But in cases where I don't have to leave the comfort of tig,
> it would be nice not to have to start doing so..
>
> This is not an argument for sticking to email based process, just
> defence of what I think would be a useful feature for gitlab to gain
> ;-)

Thanks, that helps. How about this? It technically even fits in one
line, though you might wish it didn't.

~/mesa/mesa master ← → * % export
GITLAB_TOKEN=secret-api-token-you-get-from-web-UI
~/mesa/mesa master ← → * % export
GITLAB_COMMIT=f967273fb442de8281f8248e8c8bff5b13ab89e4
~/mesa/mesa master ← → * % curl --silent --header "PRIVATE-TOKEN:
$GITLAB_TOKEN" 
https://gitlab.freedesktop.org/api/v4/projects/mesa%2Fmesa/merge_requests/$(curl
--silent --header "PRIVATE-TOKEN: $GITLAB_TOKEN"
https://gitlab.freedesktop.org/api/v4/projects/mesa%2Fmesa/repository/commits/${GITLAB_COMMIT}/merge_requests
| jq -r '.[] | .iid')/participants | jq -r '.[] | { username:
.username, realname: .name }'
{
  "username": "sroland",
  "realname": "Roland Scheidegger"
}
{
  "username": "kwg",
  "realname": "Kenneth Graunke"
}
{
  "username": "mareko",
  "realname": "Marek Olšák"
}
{
  "username": "tpalli",
  "realname": "Tapani Pälli"
}

> (Also, I suppose preserving those artifacts of "the old process" is
> probably useful for folks who run git statistics, although personally
> that does not effect me.)

[mumbles something about GDPR]

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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-16 Thread Daniel Stone
Hi,

On Tue, 15 Jan 2019 at 23:47, Matt Turner  wrote:
> On Mon, Jan 14, 2019 at 4:36 AM Daniel Stone  wrote:
> > On Fri, 11 Jan 2019 at 17:05, Jason Ekstrand  wrote:
> > >  5. There's no way with gitlab for Reviewed-by tags to get automatically 
> > > applied as part of the merging process.  This makes merging a bit more 
> > > manual than it needs to be but is really no worse than it was before.
> >
> > I'm still on the side of not seeing the value in them.
>
> Reviewed-by tags are useful for measuring the quantity of patch review
> people do (which is useful in a corporate environment...). It's often
> a thankless task that's valued much lower than first order
> contributions, so having a way to at least quantify patch reviews
> shows that people are spending their time to help others contribute.
>
> The number of R-b tags is not a 100% accurate picture of the
> situation, but it gives at least a good overview of who is doing the
> tedious work of patch review. For instance, in 2018 the top reviewers
> are
>
> 620 Bas Nieuwenhuizen 
> 530 Marek Olšák 
> 505 Jason Ekstrand 
> 452 Kenneth Graunke 
>
> If my name were in there, it would definitely be something I put on my
> yearly review.

Obviously shell scripting will not do in the advanced enterprise
environment, so we're into the realm of Python. Nevertheless, I've
attached a script which will let you prove your individual worth to
the company, as well as producing a leaderboard for team morale
purposes:

~/tmp % ./kwg-gitlab-rb.py
kwg participated in 10 merged MRs this year (61 total):
MR  105: gallium: Add a PIPE_CAP_QUERY_PIPELINE_STATISTICS_SINGLE
capability and query type
MR   96: st/mesa: Optionally override RGB/RGBX dst alpha blend factors
MR   95: st/nir: Lower TES gl_PatchVerticesIn to a constant if
linked with a TCS.
MR   92: nir: Allow a non-existent sampler deref in
nir_lower_samplers_as_deref
MR   62: i965: fix VF cache issue workaround
MR   39: i965: Enable software support for
ARB_gpu_shader_fp64/ARB_gpu_shader_int64
MR   26: Revert "nir/lower_indirect: Bail early if modes == 0"
MR   12: Add preemption support to i965 on gen9+
MR9: Misc optimizations
MR4: mesa/st: Expose compute shaders when NIR support is advertised.

Top 10 MR participants:
 29 jekstrand
 16 anholt
 14 bnieuwenhuizen
 10 jljusten
 10 kwg
 10 idr
  9 dbaker
  9 eric
  8 llandwerlin
  6 cmarcelo

Cheers,
Daniel
#!/usr/bin/python3

from collections import Counter
import datetime
from itertools import chain
import gitlab
import os
import sys

MY_USERNAME="kwg"
LEADERBOARD_LEN=10

# Look back over the past year
end_of_year = datetime.datetime.now()
start_of_year = end_of_year - datetime.timedelta(days=365)

# ... or calendar years are available
#start_of_year = datetime.datetime(year=2017, month=12, day=31)
#end_of_year = datetime.datetime(year=2019, month=1, day=1)

gl = gitlab.Gitlab("https://gitlab.freedesktop.org";, os.environ["GITLAB_TOKEN"], api_version=4)
project = gl.projects.get(id="mesa/mesa")

merged_mrs = project.mergerequests.list(state="merged", updated_after=start_of_year, updated_before=end_of_year, all=True, as_list=False)
my_mrs = list(filter(lambda mr: MY_USERNAME in [p["username"] for p in mr.participants()], merged_mrs))
print("{user} participated in {me} merged MRs this year ({all} total):".format(user=MY_USERNAME, me=len(my_mrs), all=len(list(merged_mrs
for mr in my_mrs:
print("MR {:4d}: {}".format(mr.iid, mr.title))

print()
print("Top {} MR participants:".format(LEADERBOARD_LEN))

leaderboard = Counter(chain.from_iterable(map(lambda mr: [p["username"] for p in mr.participants()], merged_mrs)))
for hero, count in leaderboard.most_common(LEADERBOARD_LEN):
print("{:3d} {}".format(count, hero))
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-16 Thread Daniel Stone
Hi,

On Wed, 16 Jan 2019 at 13:01, Lionel Landwerlin
 wrote:
> - It seems we only get notifications when adding to an MR, I could like to 
> subscribe to particular tags

If you go to https://gitlab.freedesktop.org/mesa/mesa/labels/ then you
can subscribe to things per-label. That applies to both issues and
MRs, but might help.

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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-17 Thread Daniel Stone
Hi,

On Thu, 17 Jan 2019 at 07:38, Erik Faye-Lund
 wrote:
> 1. New MRs should probably get their cover-letter automatically sent to
> the mailing list for incrased visibility.
>
> [...]
>
> I don't think any of these issues are show-stoppers from moving
> entirely to MRs, though. Perhaps issue #1 here should be fixed first,
> dunno...
>
> Issue #1 is of course "just" a matter of writing some script that gets
> triggered by a hook. It's just work that needs to be done; any of us
> that complain about it could take up the work, I suppose.
>
> The only question is where to host such a service... perhaps it's just
> a gitlab CI step? That way we'd get some "automatic" cloud computing
> power to do the work, but it doesn't really seem like a perfect fit;
> we'd need to be able to distinguis between CI steps that are triggered
> due to new

Unfinished sentence?

Anyway, Jason did actually write that hook, and it's something I'm
happy to host on existing fd.o machines. I just haven't got to doing
it, since I ended up taking my sabbatical a lot more seriously than I
expected, and now I'm back to work I've got a bit of a backlog. But we
can definitely do it, and pretty soon.

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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-17 Thread Daniel Stone
Hi,

On Thu, 17 Jan 2019 at 16:35, Jason Ekstrand  wrote:
> On January 17, 2019 08:58:03 Erik Faye-Lund  
> wrote:
> > Whoops! I meant to say something like "we'd need to be able to
> > distinguis between CI steps that are triggered due to new MRs versus
> > updated MRs, or pushes to existing branches".
> >
> >> Anyway, Jason did actually write that hook, and it's something I'm
> >> happy to host on existing fd.o machines. I just haven't got to doing
> >> it, since I ended up taking my sabbatical a lot more seriously than I
> >> expected, and now I'm back to work I've got a bit of a backlog. But
> >> we
> >> can definitely do it, and pretty soon.
> >
> > Cool, then I won't worry about it, and just assume it'll appear
> > magically soon :)
>
> My script was a total hack. It's probably massively insecure and doesn't
> include any code to provide a diffstat which has been requested by several
> people. Someone taking it a bit more seriously would probably be good
> before we deploy anything.

With the caveat that I can no longer see the script because it's been
expired out of the pastebin (why not make a GitLab repo or at least
upload it to a snippet?) ...

I had the same assumption when you posted it, but came to the
conclusion it was actually OK, or at least would be with very minimal
work. We can configure Apache and GitLab pretty easily so it can only
be triggered with a secret token which is buried in the repo config
and/or accessible only to admins. It calls back into GitLab to get the
changes, so there's no danger of it sending completely arbitrary
content even if someone does figure out how to trigger it when they
shouldn't. It also has GitLab project -> email destination hardcoded
in the script, so there's no danger of it being used to spam arbitrary
addresses either.

Even without that, given that people currently only need to sign up to
Bugzilla (without a captcha) in order to send email with arbitrary
content to mesa-dev@, 'less spam-prone than the status quo' is an
embarrassingly low bar anyway.

Whoever wants to see this happen should ping Jason to get the script
and his suggested changes, get it in a GitLab repo, then file an issue
on https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/new/
and I'll get it deployed.

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


Re: [Mesa-dev] [PATCH] docs: add note about sending merge-requests from forks

2019-01-23 Thread Daniel Stone
Hi,

On Wed, 23 Jan 2019 at 10:09, Eric Engestrom  wrote:
> On Wednesday, 2019-01-23 10:36:15 +0100, Erik Faye-Lund wrote:
> > Sending MRs from the main Mesa repository increase clutter in the
> > repository, and decrease visibility of project-wide branches. So it's
> > better if MRs are sent from forks instead.
> >
> > Let's add a note about this, in case its not obvious to everyone.
>
> Yes please, we already have way too many dead branches in the main repo
> without adding that kind of things to it.
>
> One other thing is that (last time I checked) the scripts propagating
> the repo to github et al. don't propagate branch deletions, which means
> the clutter never gets cleaned there.

They're propagated from gitlab.fd.o to git.fd.o, as the hook is run
within regular post-receive, but you're right that pushing from
git.fd.o to GitHub doesn't notice old branches, as it just pushes
everything present in the git.fd.o repo to GitHub, and doesn't notice
any branches no longer present.

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


Re: [Mesa-dev] Thoughts after hitting 100 merge requests?

2019-01-27 Thread Daniel Stone
Hi,

On Fri, 25 Jan 2019 at 23:24, Rob Clark  wrote:
> (Hmm, I guess I should take a look at what sort of API gitlab offers,
> but that will probably have to wait until after the branchpoint.. tbh
> I'd actually be pretty happy w/ a gitlab equiv of 'git pw as -s' for
> merging things from cmdline.)

Add this to your Git remote specification:
fetch = +refs/merge-requests/*:refs/heads/mr/*

And then the actual branch submitted for an MR will be present as mr/100/head.

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


Re: [Mesa-dev] Fwd: Review for last remaining Mesa wayland-drm depth 30 bits?

2019-01-29 Thread Daniel Stone
Hi,

On Tue, 29 Jan 2019 at 16:05, Adam Jackson  wrote:
> On Tue, 2019-01-29 at 14:45 +0100, Mario Kleiner wrote:
> > could i get some review of the last two missing patches of mine for
> > depth 30 support in Mesa's egl/wayland wl-drm backend? They are over
> > six months old now, well-tested at time of original submission:
> >
> > https://patchwork.freedesktop.org/project/mesa/list/?submitter=14956
> >
> > Would be good to get this into Mesa 19 before new color formats are
> > added. Should be useful for new formats as well.
>
> 4 and 5 are:
>
> Reviewed-by: Adam Jackson 

And they are also:
Reviewed-by: Daniel Stone 

Thanks for chasing this up!

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


Re: [Mesa-dev] [PATCH 13/13] wayland: Add buffer handling and visuals for fp16 formats

2019-01-30 Thread Daniel Stone
Hi Kevin,

On Mon, 28 Jan 2019 at 18:43, Kevin Strasser  wrote:
> Set loader caps indicating that wayland can handle both rgba ordering and
> fp16 formats.
>
> NOTE: This requries libwayland to provide definitions for
> WL_SHM_FORMAT_XBGR16161616F and WL_SHM_FORMAT_ABGR16161616F

To be honest, we wouldn't be very likely to add those until at least
Pixman got support for FP16 formats. (Can you also upload those
through, e.g., TexImage2D when using GLES?)

>  static const __DRIimageLoaderExtension image_loader_extension = {
> diff --git a/src/egl/wayland/wayland-drm/wayland-drm.c 
> b/src/egl/wayland/wayland-drm/wayland-drm.c
> index 3c6696d..9dd7fd3 100644
> --- a/src/egl/wayland/wayland-drm/wayland-drm.c
> +++ b/src/egl/wayland/wayland-drm/wayland-drm.c
> @@ -117,6 +117,8 @@ drm_create_buffer(struct wl_client *client, struct 
> wl_resource *resource,
>  case WL_DRM_FORMAT_XRGB:
>  case WL_DRM_FORMAT_YUYV:
>  case WL_DRM_FORMAT_RGB565:
> +case WL_DRM_FORMAT_ABGR16161616F:
> +case WL_DRM_FORMAT_XBGR16161616F:
>  break;
>  default:
>  wl_resource_post_error(resource,
> @@ -220,6 +222,10 @@ bind_drm(struct wl_client *client, void *data, uint32_t 
> version, uint32_t id)
>WL_DRM_FORMAT_XRGB);
>  wl_resource_post_event(resource, WL_DRM_FORMAT,
> WL_DRM_FORMAT_RGB565);
> +wl_resource_post_event(resource, WL_DRM_FORMAT,
> +   WL_DRM_FORMAT_ABGR16161616F);
> +wl_resource_post_event(resource, WL_DRM_FORMAT,
> +   WL_DRM_FORMAT_XBGR16161616F);

These should only be advertised if the underlying driver actually
supports texturing from FP16.

Regardless, we try to avoid adding ~anything new to wl_drm. It would
be much better to have DRM FourCC definitions for these formats and
use them via the linux_dmabuf extension if available.

If the driver advertises DRM_FORMAT_*16161616F formats through the EGL
dmabuf format query extension, then the server will automatically
advertise them to clients and import will just work on the server side
without having to touch src/egl/wayland/wayland-drm/, which we'd
mostly prefer to be preserved in amber.

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


Re: [Mesa-dev] [PATCH] freedreno: Fix meson build.

2019-02-07 Thread Daniel Stone
On Thu, 7 Feb 2019 at 14:59, Eric Engestrom  wrote:
> On Wednesday, 2019-02-06 18:36:09 +, Vinson Lee wrote:
> > ../src/gallium/drivers/freedreno/freedreno_resource.c: In function 
> > ‘fd_resource_create_with_modifiers’:
> > ../src/gallium/drivers/freedreno/freedreno_resource.c:884:30: error: 
> > ‘DRM_FORMAT_MOD_QCOM_COMPRESSED’ undeclared (first use in this function)
> >allow_ubwc = find_modifier(DRM_FORMAT_MOD_QCOM_COMPRESSED, modifiers, 
> > count);
>
> That's a weird error... I would've expected the `#include `
> to fail with a "No such file". If you copied the wrong part of the error
> message, can you update that?

Presumably it just finds an outdated copy of drm_fourcc.h, which
doesn't have the Qualcomm modifier, from the system include path.

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


Re: [Mesa-dev] [PATCH 1/3] egl/android: Delete set_damage_region from egl dri vtbl

2019-02-08 Thread Daniel Stone
Hi,

On Fri, 20 Jul 2018 at 19:32, Eric Anholt  wrote:
> Harish Krupo  writes:
> > Thank you, understood it. I should have read the spec better :(.
> > Also, generalizing Android/deqp's usage seems to be wrong. Android's
> > deqp passed previously even when the driver wasn't restricting the
> > rendering to only the damaged regions.
> > Should I update these in the comments section of the extension?
>
> Yes, please.  If we're building an API, we should be documenting what it
> does.

I've resurrected this series in the form of a GitLab MR:
https://gitlab.freedesktop.org/wayland/weston/merge_requests/106

Looking at the original implementation, I think it was misguided: the
Intel driver hook replaced the scissor region in the context with the
extents of the damage region. Unless I've completely misread the spec,
the damage region should be additive to the scissor region, rather
than replacing it: it should also only apply to winsys buffers and not
user FBOs. Given that, I changed the DRI interface to target a
drawable rather than a context, as well as cleaned it up, renamed it,
added substantial documentation, and a Gallium + VC4 implementation.

The MR is tagged WIP because I still have a lot of outstanding
questions on how the implementation should look.

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


[Mesa-dev] Fwd: PSA: Mailman changes, From addresses no longer accurate

2019-02-12 Thread Daniel Stone
Hi all,
Unfortunately, freedesktop.org's job of sending mail to a huge number
of people whilst pretending to be other people, has just got even
harder than it was.

fd.o can no longer (at least for the moment) send mail with the From
addresses of DMARC/DKIM/SPF-protected sender domains. When we try to
do so, large providers reject the mail, despite DMARC records
explicitly specifying that the mail should be accepted. Worse than
that, not only does the immediate delivery attempt fail, but it
punishes our sender reputation enough that _other_ mail delivery
fails: after we hit a DMARC-related failure, large providers often
refuse delivery attempts for mail from non-DMARC-protected domains.

As a result, if the sender domain has a DMARC record, we rewrite the
From address to be the list's address, preserving the original author
in Reply-To.

I'm chasing this up through a few channels, but in the meantime,
please be aware that the From address on mails may no longer be
accurate. If you are attempting to apply patches with git-am, please
check that the commit author is not 'Person Name via listname-devel
'.

If you are replying privately to a list mail, please be _very_ careful
that you are replying to the original sender (in Reply-To) and not the
list itself.

Cheers,
Daniel

-- Forwarded message -----
From: Daniel Stone 
Date: Mon, 11 Feb 2019 at 23:38
Subject: PSA: Mailman changes, From addresses no longer accurate
To: , 


Hi all,
We have hit another step change in aggressive anti-spam techniques
from major mail providers. Over the past few days, we saw a huge spike
in the number of mails we were failing to deliver to GMail and
outlook.com in particular.

It looks like it is now no longer acceptable for us to break
DMARC/DKIM/SPF. These are DNS-based extensions to SMTP, which allow
domains to publish policies as to who should be allowed to send email
on their behalf. SPF provides source filtering, so e.g.
freedesktop.org could specify that no-one should accept mail with a
From: *@freedesktop.org unless it came from gabe.freedesktop.org.
Mailman completely breaks this: if I specified a filter only allowing
Google to send mail for @fooishbar.org, then anyone enforcing SPF
would reject receipt of this mail, as it would arrive from fd.o with
my From address.

DKIM allows domains to publish a public key in DNS, inserting a header
into mails sent from their domain cryptographically signing the value
of named headers. Mailman breaks this too: changing the Sender header
(such that bounces get processed by Mailman, rather than sending a
deluge of out-of-office and mailbox-over-quota messages to whoever
posts to the list) can break most DKIM signatures. Mailman adding the
unsubscribe footer also breaks this; we could make it not add the
footer, but in order to do so we'd have to convince ourselves that we
were not decreasing our GDPR compliance.

DMARC ties the two together, allowing domains to specify whether or
not DKIM/SPF should be mandatory, and if they fail, what action should
be taken. Despite some domains specifying a fail action of 'none'
(receiving MTA to send an advisory report to a named email address,
but still allow the email), it seems that popular services still
interpret 'none' as 'reject'.

As a result, Google in particular is dropping some number of our mails
on the floor. This does _not_ just apply to mails which fail
DMARC/DKIM/SPF: every mail we send that fails these filters (which is
a lot - e.g. Intel and NVIDIA both have SPF) decreases our sender
reputation with GMail and causes it to reject legitimate mails.

I've reached out to Google through a couple of channels to see what we
can do to increase our delivery rate to them. In the meantime, if your
mail is hosted by Google, or Outlook, and you think you're missing
mails - you probably are.

Mailman has also now been reconfigured such that if it spots a
potential DMARC violation, it rewrites the From address to be
@lists.freedesktop.org, keeping the original author in Reply-To. It
also strips DKIM headers. This seems to be about the best we can do,
unless and until the major mail service providers offer us some
alternate way to send mail. If you are replying privately to someone,
you should check very carefully that you are replying to them and not
to the list.

Unfortunately we don't have a good answer in the long run. The latest
published RFC at https://tools.ietf.org/html/rfc6377 suggests that
there are no good solutions. If anyone is blessed with an abundance of
time and familiar with the current email landscape, I would love to
talk to you and get your help to work on this. Unfortunately we don't
have the manpower to actually properly monitor email; it can often
take a collapse in successful-delivery rates for us to notice.

Ultimately, I suspect the best solution is to move most of our
discussions to dedicated fora like GitLab issues, or some

[Mesa-dev] Mesa CI is too slow

2019-02-18 Thread Daniel Stone
Hi all,
A few people have noted that Mesa's GitLab CI is just too slow, and
not usable in day-to-day development, which is a massive shame.

I looked into it a bit this morning, and also discussed it with Emil,
though nothing in this is speaking for him.

Taking one of the last runs as representative (nothing in it looks
like an outlier to me, and 7min to build RadeonSI seems entirely
reasonable):
https://gitlab.freedesktop.org/mesa/mesa/pipelines/19692/builds

This run executed 24 jobs, which is beyond the limit of our CI
parallelism. As documented on
https://www.freedesktop.org/wiki/Infrastructure/ we have 14 concurrent
job slots (each with roughly 4 vCPUs). Those 24 jobs cumulatively took
177 minutes of execution time, taking 120 minutes for the end-to-end
pipeline.

177 minutes of runtime is too long for the runners we have now: if it
perfectly occupies all our runners it will take over 12 minutes, which
means that even if no-one else was using the runners, they could
execute 5 Mesa builds per hour at full occupancy. Unfortunately,
VirGL, Wayland/Weston, libinput, X.Org, IGT, GStreamer,
NetworkManager/ModemManager, Bolt, Poppler, etc, would all probably
have something to say about that.

When the runners aren't occupied and there's less contention for jobs,
it looks quite good:
https://gitlab.freedesktop.org/anholt/mesa/pipelines/19621/builds

This run 'only' took 20.5 minutes to execute, but then again, 3
pipelines per hour isn't that great either.

Two hours of end-to-end pipeline time is also obviously far too long.
Amongst other things, it practically precludes pre-merge CI: by the
time your build has finished, someone will have pushed to the tree, so
you need to start again. Even if we serialised it through a bot, that
would limit us to pushing 12 changesets per day, which seems too low.

I'm currently talking to two different hosts to try to get more
sponsored time for CI runners. Those are both on hold this week due to
travel / personal circumstances, but I'll hopefully find out more next
week. Eric E filed an issue
(https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/120) to
enable ccache cache but I don't see myself having the time to do it
before next month.

In the meantime, it would be great to see how we could reduce the
number of jobs Mesa runs for each pipeline. Given we're already
exceeding the limits of parallelism, having so many independent jobs
isn't reducing the end-to-end pipeline time, but instead just
duplicating effort required to fetch and check out sources, cache (in
the future), start the container, run meson or ./configure, and build
any common files.

I'm taking it as a given that at least three separate builds are
required: autotools, Meson, and SCons. Fair enough.

It's been suggested to me that SWR should remain separate, as it takes
longer to build than the other drivers, and getting fast feedback is
important, which is fair enough.

Suggestion #1: merge scons-swr into scons-llvm. scons-nollvm will
already provide fast feedback on if we've broken the SCons build, and
the rest is pretty uninteresting, so merging scons-swr into scons-llvm
might help cut down on duplication.

Suggestion #2: merge the misc Gallium jobs together. Building
gallium-radeonsi and gallium-st-other are both relatively quick. We
could merge these into gallium-drivers-other for a very small increase
in overall runtime for that job, and save ourselves probably about 10%
of the overall build time here.

Suggestion #3: don't build so much LLVM in autotools. The Meson
clover-llvm builds take half the time the autotools builds do. Perhaps
we should only build one LLVM variant within autotools (to test the
autotools LLVM selection still works), and then build all the rest
only in Meson. That would be good for another 15-20% reduction in
overall pipeline run time.

Suggestion #4 (if necessary): build SWR less frequently. Can we
perhaps demote SWR to an 'only:' job which will only rebuild SWR if
SWR itself or Gallium have changed? This would save a good chunk of
runtime - again close to 10%.

Doing the above would reduce the run time fairly substantially, for
what I can tell is no loss in functional coverage, and bring the
parallelism to a mere 1.5x oversubscription of the whole
organisation's available job slots, from the current 2x.

Any thoughts?

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

Re: [Mesa-dev] Mesa CI is too slow

2019-02-18 Thread Daniel Stone
Hi,

On Mon, 18 Feb 2019 at 18:58, Eric Engestrom  wrote:
> On Monday, 2019-02-18 17:31:41 +0000, Daniel Stone wrote:
> > Two hours of end-to-end pipeline time is also obviously far too long.
> > Amongst other things, it practically precludes pre-merge CI: by the
> > time your build has finished, someone will have pushed to the tree, so
> > you need to start again. Even if we serialised it through a bot, that
> > would limit us to pushing 12 changesets per day, which seems too low.
> >
> > I'm currently talking to two different hosts to try to get more
> > sponsored time for CI runners. Those are both on hold this week due to
> > travel / personal circumstances, but I'll hopefully find out more next
> > week. Eric E filed an issue
> > (https://gitlab.freedesktop.org/freedesktop/freedesktop/issues/120) to
> > enable ccache cache but I don't see myself having the time to do it
> > before next month.
>
> Just to chime in to this point, I also have an MR to enable ccache per
> runner, which with our static runners setup is not much worse than the
> shared cache:
> https://gitlab.freedesktop.org/mesa/mesa/merge_requests/240
>
> From my cursory testing, this should already cut the compilations by
> 80-90% :)

That's great! Is there any reason not to merge it?

> > Doing the above would reduce the run time fairly substantially, for
> > what I can tell is no loss in functional coverage, and bring the
> > parallelism to a mere 1.5x oversubscription of the whole
> > organisation's available job slots, from the current 2x.
> >
> > Any thoughts?
>
> Your suggestions all sound good, although I can't speak for #1 and #2.
>
> #3 sounds good, I guess we can keep meson builds with the "oldest supported
> llvm" and the "current llvm version", and only the "oldest supported"
> for autotools?

We could have Meson building all the LLVM versions autotools does for
not much overhead at all. At the moment though Meson builds 3 and
autotools builds 6, which isn't bring us increased code coverage.

> You've suggested reducing the amount that's built (ccache,
> dropping/merging jobs) and making it more parallel (fewer jobs), but
> there's another avenue to look at: run the CI less often.
>
> In my opinion, the CI should run on every single commit. Since that's
> not realistic, we need to decide what's essential.
> From most to least important:
>
> - master: everything that hits master needs to be build- and smoke-tested
>
> - stable branches: we obviously don't want to break stable branches
>
> - merge requests: the reason I wrote the CI was to automatically test MRs
>
> - personal work on forks: it would be really useful to test things
>   before sending out an MR, especially with the less-used build systems
>   that we often forget to update, but this should be opt-in, not opt-out
>   as it is right now.
>
> Ideally, this means we add this to the .gitlab.yml:
>   only:
> - master
> - merge_requests
> - ci/*
>
> Until this morning, I thought `merge_requests` was an Enterprise Edition
> only feature, which I why I didn't put it in, but it appears I was wrong,
> see:
> https://docs.gitlab.com/ce/ci/merge_request_pipelines/
> (Thanks Caio for reading through the docs more carefully than I did! :)
>
> I'll send an MR in a bit with the above. This will mean that master and
> MRs get automatic CI, and pushes on forks don't (except the fork's
> master), but one can push a `ci/*` branch to their own fork to run the
> CI on it.
>
> I think this should massively drop the use of the CI, but mostly remove
> unwanted uses :)

It depends on the definition of 'unwanted', of course ... I personally
like the idea of having a very early canary in the coalmine, and
building it into peoples' workflows as quickly as possible. If a more
sensible job split could reduce compilation time by 30-40%, and using
ccache could drop the compilation overhead by a huge amount as well,
that sounds like more than enough to not need to stop CI on personal
forks. Why don't we pursue those avenues first, rather than
restricting the audience?

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

Re: [Mesa-dev] Bugzilla (Was: [Bug 93686] Performance improvement : Please consider hardware ɢᴘᴜ rendering in llvmpipe)

2016-01-18 Thread Daniel Stone
Hi,

On 18 January 2016 at 12:48, Jose Fonseca  wrote:
> There's something weird going on with bugzilla and maybe ytrezq's browser.
> mesa-dev received a lot of CC add/remove notification emails from bugzilla,
> but if you look at https://bugs.freedesktop.org/show_bug.cgi?id=93686 web
> page there's only one user CCed (the reporter), and no records of CC users
> list being changed.

Click the history link: https://bugs.freedesktop.org/show_activity.cgi?id=93686

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Mesa for OpenVMS

2016-01-19 Thread Daniel Stone
Hi,

On 19 January 2016 at 02:14, Timothy Arceri
 wrote:
> On Mon, 2016-01-18 at 16:47 +0100, Jouk Jansen wrote:
>> Can someone insert these patches in the git-repository.
>> I cannot do it myself, because the git-client on my OpenVMS is very
>> -very
>> limited and does not allow this.
>
> Why not make the changes on another system and send the patches for
> review from that system. I doubt anyone here is going to do the work
> for you.

Not only that, but the FTP link won't open in Chrome ('command not
supported'), and using a command line client to get /openvms fails,
because you need 'cd OPENVMS.DIR' instead. Once you've got over that,
and figured out how to put a semicolon in FTP command names, you then
get a zip archive which expands just containing a collection of files
which form a partial source tree.

The normal patch contribution guidelines are:
  - make only one change (or set of related changes, e.g. 'remove
unsupported #pragma once in this file', 'add #ifdef __VMS__ to these
files', etc) per patch
  - build up a set of patches (using 'patch')
  - send these patches individually to the mailing list for review

A ZIP file on an OpenVMS FTP server you can't access with regular
clients, containing 70 files (some of which are automatically
generated from other source, e.g. glsl_lexer.cpp) and no indication of
the actual differences from git, is really very far outside these
guidelines. I'm sorry to hear OpenVMS doesn't have a functional git
client, but perhaps you could consider using an operating system which
allows you to generate patches in the way that any normal open source
project would expect.

(As an aside, I found 'VERSION=Mesa V3.4' worth a chuckle.)

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl/wayland: Set __DRI_IMAGE_USE_SCANOUT for shared buffers

2016-01-27 Thread Daniel Stone
Hi,

On 27 January 2016 at 09:34, Michel Dänzer  wrote:
> The compositor may have the hardware scan out directly from the buffers
> sent by the client, so we must make sure the buffers we create are
> suitable for scanout.

If the compositor wants to scan out directly, it will import via GBM,
which is in a position to reject the import if the buffer is not
suitable for scanout. So there's something missing here, either in the
GBM implementation to set magic flags when importing, or failure to
communicate tiling mode correctly, or whatever.

So for now, I would NAK this and fix the underlying problem, before
forcing every client buffer to be scanout-capable, which can cause
performance issues of its own. This also introduces a
resource-contention issue (e.g. if the display controller can only
scan out from physically-contiguous memory); you'll cause all client
buffer allocations to fail unless it can allocate a potentially
contended resource.

Please fix the radeonsi/Gallium implementations of GBM and/or wl_drm
to deal with tiling/compression correctly instead.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl/wayland: Set __DRI_IMAGE_USE_SCANOUT for shared buffers

2016-01-27 Thread Daniel Stone
Hi,

On 27 January 2016 at 14:16, Axel Davy  wrote:
> On 27/01/2016 13:43, Daniel Stone wrote:
>> On 27 January 2016 at 09:34, Michel Dänzer  wrote:
>>> The compositor may have the hardware scan out directly from the buffers
>>> sent by the client, so we must make sure the buffers we create are
>>> suitable for scanout.
>>
>> If the compositor wants to scan out directly, it will import via GBM,
>> which is in a position to reject the import if the buffer is not
>> suitable for scanout. So there's something missing here, either in the
>> GBM implementation to set magic flags when importing, or failure to
>> communicate tiling mode correctly, or whatever.
>>
>> So for now, I would NAK this and fix the underlying problem, before
>> forcing every client buffer to be scanout-capable, which can cause
>> performance issues of its own. This also introduces a
>> resource-contention issue (e.g. if the display controller can only
>> scan out from physically-contiguous memory); you'll cause all client
>> buffer allocations to fail unless it can allocate a potentially
>> contended resource.
>>
>> Please fix the radeonsi/Gallium implementations of GBM and/or wl_drm
>> to deal with tiling/compression correctly instead.
>
> Scanout buffers are already used for DRI2 and DRI3 by default (for Mesa
> drivers),
> and it looks like a sane default behaviour given there is no current
> way to know if the compositor would like to use the buffer as scanout
> or not.

Sure, but the DRI protocols are rather fixed, whereas wl_drm is
completely extensible to better describe the formats. I feel like this
is mainly being used as a proxy to paper over a few issues, including
negotiating tiling formats between different GPUs, which we can now
fully describe via FB modifiers.

The only places I can see this actually happening by default, are in
the GBM drivers (obviously correct), and DRI3 when using a different
GPU to render (less obviously correct, but does paper over the lack of
common-format negotiation, as above). I can't see it happening by
default for DRI2, unless I'm missing something ... ?

> As long as the app has no way to know what the compositor wants (there
> was talks I remember on irc on ways to do that), I think enforcing scanout
> as default is sane.

Again, negotiating scanout is something we can handle internally
through wl_drm without having to have a flag-day API change. If you
assume that scanout negates tiling, then every browser you launch is
going to be rendered to a non-tiled format. This has a non-trivial
impact on a lot of architectures, and is only really mostly kind of
fine on desktop architectures where you have the power to waste
anyway.

> The other thing you mention, ie failing to import non-scanout buffers when
> scanout is requested is, I believe,
> unrelated to this patch, and would require a bit more work. In particular
> this is handled by a function validateUsage,
> which doesn't seem to be fully implemented on gallium. I suggest to file a
> bug on bugzillla.

Well, I could, but if this fix goes in, I assume nothing will ever
happen for Gallium drivers, so not sure there's much point, aside from
having a URL to point to the next time this comes up.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl/wayland: Set __DRI_IMAGE_USE_SCANOUT for shared buffers

2016-01-28 Thread Daniel Stone
On 28 January 2016 at 03:21, Michel Dänzer  wrote:
> On 27.01.2016 23:54, Daniel Stone wrote:
>> On 27 January 2016 at 14:16, Axel Davy  wrote:
>>> On 27/01/2016 13:43, Daniel Stone wrote:
>>>> If the compositor wants to scan out directly, it will import via
>>>> GBM, which is in a position to reject the import if the buffer is
>>>> not suitable for scanout. So there's something missing here,
>>>> either in the GBM implementation to set magic flags when
>>>> importing, or failure to communicate tiling mode correctly, or
>>>> whatever.
>>>>
>>>> So for now, I would NAK this
>
> And you can NAK this because... ? :)

Well, why not just skip review and bang whatever you feel like into
egl/wayland then. Seems like that's the prevailing feeling anyway, or
at least the impression I get trying to solve problems correctly in
Mesa rather than bodging around everything.

>>> As long as the app has no way to know what the compositor wants
>>> (there was talks I remember on irc on ways to do that), I think
>>> enforcing scanout as default is sane.
>>
>> Again, negotiating scanout is something we can handle internally
>> through wl_drm without having to have a flag-day API change. If you
>> assume that scanout negates tiling, then every browser you launch is
>> going to be rendered to a non-tiled format. This has a non-trivial
>> impact on a lot of architectures, and is only really mostly kind of
>> fine on desktop architectures where you have the power to waste
>> anyway.
>
> You're saying there are architectures where tiling is essential for
> energy consumption, but the rendering and display hardware don't have
> any common tiling formats? The mind boggles.
>
> FWIW, on our hardware there's no such issue, scanout just requires
> special tiling modes.

No, 'special tiling modes' is what I meant. It is not at all uncommon
to have hardware which requires special tiling modes (for variants of
'special tiling modes' which include 'no tiling') which force a less
efficient render pass, or even a separate resolve pass, for scanout.
It follows that pessimising _all_ client rendering to account for this
special case is a bad idea. If there was no impact, then every buffer
would just be allocated as scanout, no? Else why is it a separate
flag?

By power, I mean performance (where memory bandwidth is approximately
infinite, and one resolve pass has zero impact on your overall
performance) as much as energy consumption.

>>> The other thing you mention, ie failing to import non-scanout
>>> buffers when scanout is requested is, I believe, unrelated to this
>>> patch, and would require a bit more work. In particular this is
>>> handled by a function validateUsage, which doesn't seem to be fully
>>> implemented on gallium. I suggest to file a bug on bugzillla.
>>
>> Well, I could, but if this fix goes in, I assume nothing will ever
>> happen for Gallium drivers, so not sure there's much point, aside
>> from having a URL to point to the next time this comes up.
>
> intel_validate_usage doesn't handle scanout either, but AFAIR at least
> older generations of Intel GPUs don't support all tiling modes for
> scanout either. So I don't see how that could work in the scheme you
> describe other than by luck.
>
> Assuming there was any validateUsage hook which would properly catch
> buffers which can't be scanned out, can you point us to other code which
> would allow the Wayland compositor to make sure clients allocate buffers
> which can be scanned out?

The code doesn't exist now, but essentially gbm_bo_import would be
able to track a wl_buffer back to the source, and send a wl_drm event
back to Mesa (remember that this is private protocol, so can be
changed at will) informing it that it should allocate with more
optimal flags. Not a great deal of work.

Anyway, it all comes down to what you want to optimise for; with
pretty much every web browser using GL-accelerated composition, that's
a _lot_ of performance/energy you're wasting by allocating every
client surface as scanout. The tradeoff is of course that you can
potentially use hardware overlays to display the content, but without
something like HWComposer you're probably negative on power using them
anyway.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] egl/wayland: Set __DRI_IMAGE_USE_SCANOUT for shared buffers

2016-01-29 Thread Daniel Stone
On 29 January 2016 at 03:44, Michel Dänzer  wrote:
> It still sounds like significant work (particularly for somebody like me
> who isn't very familiar with Wayland details yet). It should be done by
> somebody who cares about the difference you're describing. I think it's
> unreasonable to expect myself or Axel to do it, especially since you
> said on IRC:
>
>  08:35 #dri-devel: < daniels> mannerov: it makes the flag totally
>meaningless - s/SCANOUT/NOT_A_FBO/ and i'll ack it
>
> Which makes little sense to me — even if the SCANOUT flags aren't used
> optimally yet, their meaning is quite clear.

It's nothing to do with Wayland really.

The core problem is that:
a) buffer allocation is incompletely described over the wire (AFAICT,
tiling is enabled for that buffer, but the combination of RGB format +
0 modifier implies a purely linear format),
which prevents an exacerbating problem from being properly fixed:
b) the Gallium gbm implementation does not reject tiled buffers when
importing to a display which cannot show tiled buffers
and you are proposing a hack:
c) pessimise all winsys allocations to the lowest common denominator:
avoiding tiling by mandating they be scanout-compatible

It works I guess, but until a and b are fixed, performance is going to
be suboptimal - again, if there was no performance impact to these
allocations, then they would always just happen by default.

Merging the hack makes it impossible for non-Gallium drivers to have a
and b correct (I'm not sure off the top of my head whether they are
correct or whether we just get lucky on allocations), because as soon
as those are fixed, we can't remove the scanout flag without breaking
Gallium until it also fixes those two. And it seems like that's
exceedingly unlikely to happen, so all of Mesa gets reduced
performance.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC PATCH] GBM: Add map/unmap functions

2016-03-31 Thread Daniel Stone
Hi,

On 31 March 2016 at 04:21, Rob Herring  wrote:
> diff --git a/include/GL/internal/dri_interface.h 
> b/include/GL/internal/dri_interface.h
> index 6bbd3fa..b059112 100644
> --- a/include/GL/internal/dri_interface.h
> +++ b/include/GL/internal/dri_interface.h
> @@ -1101,6 +1101,9 @@ struct __DRIdri2ExtensionRec {
>  #define __DRI_IMAGE_USE_CURSOR 0x0004 /* Depricated */
>  #define __DRI_IMAGE_USE_LINEAR 0x0008
>
> +#define __DRI_IMAGE_USE_READ0x1
> +#define __DRI_IMAGE_USE_WRITE   0x2

Nitpick: the other USE flags are passed at allocation time. Is this
something you're intending to plumb through into allocation as well?
If not, should probably move these flags to another namespace.

> diff --git a/src/gallium/state_trackers/dri/dri2.c 
> b/src/gallium/state_trackers/dri/dri2.c
> index 29aaa96..b12fc50 100644
> --- a/src/gallium/state_trackers/dri/dri2.c
> +++ b/src/gallium/state_trackers/dri/dri2.c
> @@ -1217,6 +1217,42 @@ dri2_blit_image(__DRIcontext *context, __DRIimage 
> *dst, __DRIimage *src,
> }
>  }
>
> +static void *
> +dri2_lock_image(__DRIcontext *context, __DRIimage *image,
> +int x0, int y0, int width, int height,
> +unsigned int usage)
> +{
> +   struct dri_context *ctx = dri_context(context);
> +   struct pipe_context *pipe = ctx->st->pipe;
> +   enum pipe_transfer_usage pipe_usage = PIPE_TRANSFER_READ;
> +
> +   if (!image)
> +  return NULL;
> +
> +   if (usage & __DRI_IMAGE_USE_WRITE)
> + pipe_usage |= PIPE_TRANSFER_WRITE;
> +
> +   assert(!image->pipe_private);
> +
> +   /*
> +   * ignore x, y, w and h so that returned addr points at the
> +   * start of the buffer
> +   */
> +   return pipe_transfer_map(pipe, image->texture,
> +0, 0, pipe_usage, x0, y0, width, height,
> +(struct pipe_transfer **)&image->pipe_private);
> +}
> +
> +static void
> +dri2_unlock_image(__DRIcontext *context, __DRIimage *image)
> +{
> +   struct dri_context *ctx = dri_context(context);
> +   struct pipe_context *pipe = ctx->st->pipe;
> +
> +   pipe_transfer_unmap(pipe, (struct pipe_transfer *)image->pipe_private);
> +   image->pipe_private = NULL;
> +}

The pipe_private dance suggests to me that either you need to pass
more data to the lock/unlock handlers, or that you need to explicitly
disallow multiple concurrent mappings. Allowing multiple active
mappings seems like it could be desirable, especially for systems
which need to upload on unmap, who would then be able to aggregate the
uploads.

> diff --git a/src/gbm/backends/dri/gbm_dri.c b/src/gbm/backends/dri/gbm_dri.c
> index c34e39c..77b2b10 100644
> --- a/src/gbm/backends/dri/gbm_dri.c
> +++ b/src/gbm/backends/dri/gbm_dri.c
> @@ -918,6 +918,34 @@ failed:
> return NULL;
>  }
>
> +static void *
> +_gbm_dri_bo_map(struct gbm_bo *_bo,
> +  uint32_t x, uint32_t y,
> +  uint32_t width, uint32_t height,
> +  uint32_t usage)
> +{
> +   struct gbm_dri_device *dri = gbm_dri_device(_bo->gbm);
> +   struct gbm_dri_bo *bo = gbm_dri_bo(_bo);
> +
> +   if (!dri->context)
> +  dri->context = dri->dri2->createNewContext(dri->screen, NULL, NULL, 
> NULL);

I'm a bit wary about creating a new context - especially as this ends
up creating a full OpenGL context - but I also don't really know what
would end up being better than this.

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


Re: [Mesa-dev] [PATCH] Don't manually close fds for queryImage. drm uses CLOEXEC

2016-04-14 Thread Daniel Stone
Hi,

On 14 April 2016 at 13:19, Chokshi, Mitul  wrote:
> If create_wl_buffer() closes the file descriptor returned by
> dri2_dpy->image->queryImage then OS hands out the same file
> descriptor number to other process which then experiences error
> when fd is closed due to CLOEXEC.

I don't really understand the intent of this change.

The __DRI_IMAGE_ATTRIB_FD query creates a new file descriptor for the
DRIImage, which is then passed to wl_drm_create_prime_buffer.
wl_drm_create_prime_buffer internally, as part of the Wayland client
library wrapper, duplicates the fd for its own internal use and passes
it to the compositor.

After this point, there is no reason to keep the fd around. If the fd
is not closed, it will be leaked, and the buffer can never be freed as
the reference count will never be zero, thanks to the leaked fd.

None of this has anything to do with CLOEXEC.

What is the exact problem you see, and how do you reproduce it?

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


Re: [Mesa-dev] Why is www.freedesktop.org/wiki/AccountRequests/ so "complicated"?

2016-04-18 Thread Daniel Stone
Hi,

On 17 April 2016 at 16:58, ⚛ <0xe2.0x9a.0...@gmail.com> wrote:
> Secondly, the content of the wiki page feels overly bureaucratic to me. Not
> in absolute sense, but in relative sense: when compared to services such as
> github.com, the freedesktop.org wiki page feels like a nice example of
> excessive bureaucracy. Why does the wiki page use so many words to describe
> how to make an account request (in year 2016)?
>
> I wouldn't care about the content of the wiki page at all if there existed
> an alternative web page that would make the process of creating an account
> request simpler. "Simpler" web page in my opinion means a web form with just
> a few fields, a single button and several sentences of text. Many account
> registration web pages around the world look like that, because it is less
> time consuming and therefore more efficient/productive.
>
> What is the logic/rationale explaining the content of
> https://www.freedesktop.org/wiki/AccountRequests/? I am unable to find an
> explanation for it.

The simple explanation is that our available admin time, and keeping
the infrastructure running, stable, and performant is already
surprisingly time-consuming. The coda to that is that GitHub is open
to everyone as a general-purpose hosting service, where you can put
whatever you want there. On the other hand, fd.o accounts are only
given out to active contributors to a relatively small number of
projects (infinitesmally so, compared to GitHub's 35 million
repositories), and so optimising the account-creation process is not a
particularly high priority compared to other infrastructure work.

That being said, making it easier to create and administer accounts is
on our to-do list, and the infrastructure work to let us do it is
about half-complete.

Not that it seems like it applies to you anyway, since we do require
real names for accounts.

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


Re: [Mesa-dev] [RFC 1/5] wayland-drm: add a few lovely notes about future work.

2016-04-25 Thread Daniel Stone
Hi,

On 21 April 2016 at 13:24, Emil Velikov  wrote:
> diff --git a/src/egl/wayland/wayland-drm/wayland-drm.h 
> b/src/egl/wayland/wayland-drm/wayland-drm.h
> index 7892d56..33d7fd4 100644
> --- a/src/egl/wayland/wayland-drm/wayland-drm.h
> +++ b/src/egl/wayland/wayland-drm/wayland-drm.h
> @@ -3,6 +3,10 @@
>
>  #include 
>
> +// XXX: These formats should go. One should use the auto-generated ones.
> +// XXX: Ensure that no-one but wayland-egl and/or wayland EGL platform codes
> +// includes this. Doing so give us an ABI, which we don't check but perhaps 
> we should.
> +
>  #ifndef WL_DRM_FORMAT_ENUM
>  #define WL_DRM_FORMAT_ENUM
>  enum wl_drm_format {

Indeed, this is already provided by wayland-drm-*-protocol.h, and only
used in places which may include those files.

Reviewed-by: Daniel Stone 

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


Re: [Mesa-dev] [RFC] add MAINTAINERS and get_maintainer.pl script

2016-04-25 Thread Daniel Stone
Hi,

On 20 April 2016 at 00:32, Rob Clark  wrote:
> On Tue, Apr 19, 2016 at 7:04 PM, Matt Turner  wrote:
>> Let's let people add themselves to the file if they want. No point in
>> trying to populate it up front.
>
> yeah, I expect people to add themselves, and for the MAINTAINERS file
> to evolve over time..  if people like the idea I'll send a non-rfc
> version of the patch which whatever entries people ask me to add
> themselves for over the next week or so.. mostly just to avoid
> starting off with a completely empty file.  But wasn't planning to
> wait for it to be completely populated to start with.

If you want a bit more to add:

WAYLAND EGL SUPPORT
R: Daniel Stone 
F: src/egl/wayland/*
F: src/egl/drivers/dri2/platform_wayland.c

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


Re: [Mesa-dev] [PATCH 2/4] DRI: Add DRIimage map and unmap functions

2016-04-25 Thread Daniel Stone
Hi,

On 22 April 2016 at 19:12, Eric Anholt  wrote:
> I think this needs a longer comment to explain what the interface does:
>
> "Returns a map of the specified region of a __DRIimage for the specified
> usage.
>
> flags must always include __DRI_IMAGE_TRANSFER_READ and may include
> __DRI_IMAGE_TRANSFER_WRITE if the mapping is written[*].  If
> __DRI_IMAGE_TRANSFER_WRITE is not included, behavior when writing the
> mapping is undefined.

Hrm. Though the Gallium implementation currently forces READ, it seems
like drivers requiring transfer would benefit from READ being
optional, if it elides a copy of data which will just be overwritten
anyway. How about:
'flags may include __DRI_IMAGE_TRANSFER_READ, which will populate the
mapping with the current buffer content. If __DRI_IMAGE_TRANSFER_READ
is not included in the flags, the buffer content at map time is
undefined. Users wanting to modify the mapping must include
__DRI_IMAGE_TRANSFER_WRITE; if __DRI_IMAGE_TRANSFER_WRITE is not
included, behaviour when writing the mapping is undefined.'

Also, you're missing the footnote ... ?

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


Re: [Mesa-dev] [PATCH 4/4] gbm: Add map/unmap functions

2016-04-25 Thread Daniel Stone
Hi,

On 22 April 2016 at 19:21, Eric Anholt  wrote:
> I don't think we want to always make a spare context just in case
> someone uses the map API.  Contexts can be pretty expensive to set up,
> in time (for piglit tests on gbm) and memory (for X.Org).
>
> It's too bad I don't think we have a way to get the existing
> __DRIcontext from EGL to pass as an arg to map/unmap here, which could
> be nice for glamor.

You don't always have one: think of the Plymouth case in particular,
or anything else using Pixman, which could benefit from this I
suppose.

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


Re: [Mesa-dev] [PATCH 4/4] gbm: Add map/unmap functions

2016-04-25 Thread Daniel Stone
Hi,

On 23 April 2016 at 03:08, Rob Herring  wrote:
> On Fri, Apr 22, 2016 at 6:32 PM, Emil Velikov  
> wrote:
>> Can we take a look at the GBM gralloc as well. One thing that worries
>> me is that (most likely) you are requesting/creating a bo without
>> GBM_BO_USE_WRITE whist using MAP + CPU write UNMAP. If you do set the
>> USE_WRITE flag, you're getting a dumb buffer, which I'm not sure how
>> well is going to work.
>
> I'm not using GBM_BO_USE_WRITE and that is not a condition for mapping
> given that flag is tied to cursors (according to comments) and gives
> dumb buffers. Also of note, if gralloc flags are set for r/w often,
> then I request a linear buffer. Here's the gralloc side:

Right, I wouldn't take GBM_BO_USE_WRITE to have much of an effect on
mappings, as it pessimises allocation like you say.

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


Re: [Mesa-dev] [PATCH] wayland/egl: Assigning window size to window surface size property

2016-04-25 Thread Daniel Stone
Hi Adrian,

On 25 April 2016 at 14:23, Adrian Pielech  wrote:
> Till now window surface size on wayland was -1.
> It's better to assign windows size to proper surface property.

Hm. This will be done from update_buffers(), so will only be 'invalid'
before the first rendering has been done. I guess this is for querying
EGL_WIDTH/EGL_HEIGHT?

Note that this is symmetrical with wl_egl_window_resize() currently;
calling resize does not result in these values being updated until
updating the surface buffers (from get_back_bo, triggered after the
first rendering call after a swap). So we have the following behaviour
for eglQuerySurface:
  - querying dimensions after surface creation: invalid currently,
valid with this patch
  - querying dimensions after resize: stale/old size
  - querying dimensions after rendering: correct values

Do you have a real-world usecase which depends on this?

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


Re: [Mesa-dev] [PATCH 4/4] gbm: Add map/unmap functions

2016-04-25 Thread Daniel Stone
Hi,

On 25 April 2016 at 15:25, Emil Velikov  wrote:
> On 25 April 2016 at 13:46, Daniel Stone  wrote:
>> On 23 April 2016 at 03:08, Rob Herring  wrote:
>>> I'm not using GBM_BO_USE_WRITE and that is not a condition for mapping
>>> given that flag is tied to cursors (according to comments) and gives
>>> dumb buffers. Also of note, if gralloc flags are set for r/w often,
>>> then I request a linear buffer. Here's the gralloc side:
>>
>> Right, I wouldn't take GBM_BO_USE_WRITE to have much of an effect on
>> mappings, as it pessimises allocation like you say.
>>
> Ftr, I'm not objecting as to how things are done. Just saying that
> things should be blindly obvious as one reads the documentation alone.
> I'm assuming that most people involved are "tainted" (the know to a
> level how things are implemented) thus things are clearer for them.

And on that, we agree. :)

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


Re: [Mesa-dev] [PATCH 7/7] i965/sync: Implement DRI2_Fence extension

2015-05-04 Thread Daniel Stone
Hi,

On 1 May 2015 at 21:02, Chad Versace  wrote:
> +static bool
> +brw_fence_has_completed(struct brw_fence *fence)
> +{
> +   if (fence->signalled)
> +  return true;
> +
> +   if (fence->batch_bo && !drm_intel_bo_busy(fence->batch_bo)) {
> +  drm_intel_bo_unreference(fence->batch_bo);
> +  fence->batch_bo = NULL;

Should this branch be setting fence->signalled = true as well, to
match client_wait?

The rest looks good to me, going on what I remember of having looked
at this about 5 years ago, so:
Reviewed-by: Daniel Stone 

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 0/9] Some egl/wayland patches

2015-05-04 Thread Daniel Stone
Hi,

On 2 May 2015 at 11:15, Axel Davy  wrote:
> Since gallium egl state tracker was dropped,
> there was no way to use swrast with wayland,
> since it was not implemented for egl_dri2.
> https://bugs.freedesktop.org/show_bug.cgi?id=86701
>
> This patch serie aimed at implementing swrast support
> for the wayland egl_dri2 backend.
> But I also went at fixing a few other related things.
>
> I also took the opportunity to make new versions of my
> patches from last year to implement render-nodes and
> DRI_PRIME support. The only major difference is that
> it doesn't have fallback when blitImage is unimplemented
> by driver. It is implemented by gallium drivers only.
> It just means for now that you can use a radeon or nouveau
> card with DRI_PRIME, but not an intel card. So all normal
> use cases are supported.
>
> The swrast __DRIswrastLoaderExtension API seems to have been
> designed with X11 in mind. As a result, it doesn't fit perfectly
> wayland: a copy could be avoided by upgrading this API.
> There doesn't seem to be interest in doing this work for a small
> gain for something that's not as efficient as hw rendering anyway.

Patches 1-7 are Reviewed-by: Daniel Stone .
Patch 8 looks OK, except that it seems like the anonymous-file helper
could be made shared somewhere; if so, brilliant, then if not, R-b
anyway. I'm not really competent to review patch 9 if I'm honest.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 7/9] egl/wayland: assume EGL_WINDOW_BIT

2015-05-06 Thread Daniel Stone
Hi,

On 6 May 2015 at 07:25, Pekka Paalanen  wrote:
> On Wed, 6 May 2015 11:00:13 +1000
> Dave Airlie  wrote:
>> On 2 May 2015 at 20:15, Axel Davy  wrote:
>> > Only EGL_WINDOW_BIT is supported. Remove tests related.
>>
>> Is this there no plans to support pixmap/pbuffer/ or any of the other bits?
>>
>> Seems like a step in the wrong direction if we really should be supporting
>> other things than WINDOW_BIT in the future.
>
> EGL Wayland by definition does not have pixmaps:
> https://www.khronos.org/registry/egl/extensions/EXT/EGL_EXT_platform_wayland.txt
>
> Pbuffers OTOH I suppose are possible.

Possible, yes. I _think_ PVR supports them, but pretty sure Mali
doesn't. I'm entirely comfortable with not doing so either.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 04/15] egl: import headers from Khronos EGL registry

2015-05-15 Thread Daniel Stone
Hi,

On 14 May 2015 at 23:33, Emil Velikov  wrote:
> Hi Marek,
> On 12/05/15 22:54, Marek Olšák wrote:
>> -#elif defined(__WINSCW__) || defined(__SYMBIAN32__)  /* Symbian */
>> +#elif defined(__APPLE__) || defined(__WINSCW__) || defined(__SYMBIAN32__)  
>> /* Symbian */
>>
>>  typedef int   EGLNativeDisplayType;
>>  typedef void *EGLNativeWindowType;
>>  typedef void *EGLNativePixmapType;
>>
>> -#elif defined(WL_EGL_PLATFORM)
>> +#elif defined(__ANDROID__) || defined(ANDROID)
>>
>> -typedef struct wl_display *EGLNativeDisplayType;
>> -typedef struct wl_egl_pixmap  *EGLNativePixmapType;
>> -typedef struct wl_egl_window  *EGLNativeWindowType;
>> +#include 
>>
>> -#elif defined(__GBM__)
>> -
>> -typedef struct gbm_device  *EGLNativeDisplayType;
>> -typedef struct gbm_bo  *EGLNativePixmapType;
>> -typedef void   *EGLNativeWindowType;
>> -
>> -#elif defined(ANDROID) /* Android */
>> -
>> -struct ANativeWindow;
>>  struct egl_native_pixmap_t;
>>
>> -typedef struct ANativeWindow*EGLNativeWindowType;
>> -typedef struct egl_native_pixmap_t  *EGLNativePixmapType;
>> -typedef void*EGLNativeDisplayType;
>> +typedef struct ANativeWindow*   EGLNativeWindowType;
>> +typedef struct egl_native_pixmap_t* EGLNativePixmapType;
>> +typedef void*   EGLNativeDisplayType;
>>
>>  #elif defined(__unix__)
>>
>> -#if defined(MESA_EGL_NO_X11_HEADERS)
>> -
>> -typedef void*EGLNativeDisplayType;
>> -typedef khronos_uintptr_t EGLNativePixmapType;
>> -typedef khronos_uintptr_t EGLNativeWindowType;
>> -
>> -#else
>> -
>>  /* X11 (tentative)  */
>>  #include 
>>  #include 
>> @@ -122,18 +103,8 @@ typedef Display *EGLNativeDisplayType;
>>  typedef Pixmap   EGLNativePixmapType;
>>  typedef Window   EGLNativeWindowType;
>>
>> -#endif /* MESA_EGL_NO_X11_HEADERS */
>> -
>> -#elif __HAIKU__
>> -#include 
>> -typedef void *EGLNativeDisplayType;
>> -typedef khronos_uintptr_t EGLNativePixmapType;
>> -typedef khronos_uintptr_t EGLNativeWindowType;
>> -
> Upon closer look, one could get away with the above changes, although
> there may be something more subtle to it.
>
> Kristian, Chad,
>
> Would you have any suggestions for/against nuking the Wayland/GBM/other
> typedefs ? With an extra eye on the Haiku changes, what would it take to
> get the current eglplatform.h (or equivalent) accepted with Khronos ?

No objection from me; I don't know of anyone using Wayland/GBM.
Android might (will) be harder to get away with though. Luckily for
us, platform_base should hopefully prevent people from ever trying
silly Native{Display,Window}Type hacks on Wayland/GBM.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] Releasing a surfaceless EGL context doesn't release underlying DRI context.

2014-10-24 Thread Daniel Stone
Hi,

On 24 October 2014 11:03, Kalyan Kondapally <
kondapallykalyancontrib...@gmail.com> wrote:

> driUnbindContext() checks for valid drawables before calling the driver
> unbind function. In case of Surfaceless contexts, the drawables are always
> Null and we end up not releasing the underlying DRI context. Moving the
> call to the driver function before the drawable validity checks fixes
> things.
>

Yep, that looks good to me; seems like you've found the only possible case
that would trigger this breakage. Calling DestroyContext will always unbind
if it's current in that thread (see the end _mesa_free_context_data); it's
only when you follow that exact pattern of:
  - create surfaceless context
  - make ctx current
  - make something else current
  - {another thread} destroy ctx
  - make ctx current again
that you hit the crash.

The only effect of calling UnbindContext() is that _mesa_make_current(NULL,
NULL, NULL) gets called, which is what we want.

Could you please document some part of that in the commit message for
future archaeology?

Reviewed-by: Daniel Stone 

Cheers,
Daniel


> Signed-off-by: Alexandros Frantzis 
> Signed-off-by: Kalyan Kondapally 
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=74563
> ---
>  src/mesa/drivers/dri/common/dri_util.c | 8 ++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/drivers/dri/common/dri_util.c
> b/src/mesa/drivers/dri/common/dri_util.c
> index 6c78928..02499f2 100644
> --- a/src/mesa/drivers/dri/common/dri_util.c
> +++ b/src/mesa/drivers/dri/common/dri_util.c
> @@ -569,6 +569,12 @@ static int driUnbindContext(__DRIcontext *pcp)
>  if (pcp == NULL)
> return GL_FALSE;
>
> +/*
> +** Call driUnbindContext before checking for valid drawables
> +** to handle surfaceless contexts properly.
> +*/
> +pcp->driScreenPriv->driver->UnbindContext(pcp);
> +
>  pdp = pcp->driDrawablePriv;
>  prp = pcp->driReadablePriv;
>
> @@ -576,8 +582,6 @@ static int driUnbindContext(__DRIcontext *pcp)
>  if (!pdp && !prp)
> return GL_TRUE;
>
> -pcp->driScreenPriv->driver->UnbindContext(pcp);
> -
>  assert(pdp);
>  if (pdp->refcount == 0) {
> /* ERROR!!! */
> --
> 1.9.1
>
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] Releasing a surfaceless EGL context doesn't release underlying DRI context.

2014-10-24 Thread Daniel Stone
On 24 October 2014 11:18, Daniel Stone  wrote:

> Yep, that looks good to me; seems like you've found the only possible case
> that would trigger this breakage. Calling DestroyContext will always unbind
> if it's current in that thread (see the end _mesa_free_context_data); it's
> only when you follow that exact pattern of:
>   - create surfaceless context
>   - make ctx current
>   - make something else current
>   - {another thread} destroy ctx
>   - make ctx current again
>

Obviously the last line should read 'make another context current'.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH V2] mesa: add SSE optimisation for glDrawElements

2014-10-24 Thread Daniel Stone
Hi,

On 24 October 2014 18:51, Emil Velikov  wrote:

> Sigh... why can't everyone be like Gentoo - set compiler flags and
> rebuild for your machine/cpu :P
>
> Apart from the Makefile.sources change spotted by Matt, can you make use
> of USE_SSE41 ? Take a look at commit b3121bfd413 for the whys and hows :)
>

Maybe slightly overkill, but have you considered using ORC (replacement for
liboil) instead of hand-writing ASM? GStreamer already uses it pretty
extensively.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 3/3] glx: Allow to create any OpenGL ES version.

2014-11-12 Thread Daniel Stone
Hi,

On 12 November 2014 12:37,  wrote:

> @@ -544,9 +544,22 @@ dri2_convert_glx_attribs(unsigned num_attribs, const
> uint32_t *attribs,
>case GLX_CONTEXT_COMPATIBILITY_PROFILE_BIT_ARB:
>  *api = __DRI_API_OPENGL;
>  break;
> -  case GLX_CONTEXT_ES2_PROFILE_BIT_EXT:
> -*api = __DRI_API_GLES2;
> -break;
> +  case GLX_CONTEXT_ES_PROFILE_BIT_EXT:
> + switch (*major_ver) {
> + case 3:
> +*api = __DRI_API_GLES3;
> +break;
> + case 2:
> +*api = __DRI_API_GLES2;
> +break;
> + case 1:
> +*api = __DRI_API_GLES;
> +break;
> + default:
> +*error = __DRI_CTX_ERROR_BAD_API;
> +return false;
> + }
> + break;
>default:
>  *error = __DRI_CTX_ERROR_BAD_API;
>  return false;
> @@ -577,19 +590,6 @@ dri2_convert_glx_attribs(unsigned num_attribs, const
> uint32_t *attribs,
>return false;
> }
>
> -   /* The GLX_EXT_create_context_es2_profile spec says:
> -*
> -* "... If the version requested is 2.0, and the
> -* GLX_CONTEXT_ES2_PROFILE_BIT_EXT bit is set in the
> -* GLX_CONTEXT_PROFILE_MASK_ARB attribute (see below), then the
> context
> -* returned will implement OpenGL ES 2.0. This is the only way in
> which
> -* an implementation may request an OpenGL ES 2.0 context."
> -*/
> -   if (*api == __DRI_API_GLES2 && (*major_ver != 2 || *minor_ver != 0)) {
>

It looks like you're missing minor_ver checking here? For instance, 2.99
isn't a valid GLES version.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Move mesa version scheme to a more common style ?

2014-11-14 Thread Daniel Stone
Hi,

On 14 November 2014 15:07, Erik Faye-Lund  wrote:

> On Fri, Nov 14, 2014 at 3:39 PM, Emil Velikov 
> wrote:
> > Are there any objections if I move to the above format starting with
> > mesa 10.4-rc1 ? I would appreciate any feedback over the next 2-3 days,
> > and based on it I'll tag the first RC.
>
> Shouldn't it be the other way around? IMO we should have strong
> arguments for *changing* it, rather than keep going as-is... Any
> change can break something, so only changes that have clear benefits
> should be done, no?
>
> AFAICT, the current scheme conveys more relevant, obvious information
> than the proposed one, namely that it's a release *candidate* for
> v10.4.1. If no blocking issues are found, it'll become the *actual*
> release...


You can encode that in the versions too. The other advantage is especially
when checking for dependencies, that you have a linear version comparison.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [RFC] tegra: Initial support

2014-11-28 Thread Daniel Stone
Hi,

On 28 November 2014 at 09:17, Thierry Reding 
wrote:

> On Fri, Nov 28, 2014 at 12:32:43AM -0500, Ilia Mirkin wrote:> Also, can
> you explain why it's advantageous for the setup to appear as
> > though it is a single device? i.e. what's wrong with just using
> > nouveau as a render node or whatever? [The answer may be simple, but
> > I'm very unfamiliar with a lot of that stuff, and perhaps others are
> > in a similar predicament.]
>
> There are two reasons. For one, pretty much every software out there
> that runs on the "bare metal" (i.e. GBM) uses the same initialization
> sequence and it doesn't involve opening two DRM devices. So in order to
> support Tegra and other SoCs with a similar architecture, each of these
> applications would need to be patched. Now typically a lot of the
> applications would run under X or Wayland, so the number of applications
> that need patching is somewhat reduced. However, it would still mean
> that every Wayland compositor would need to be patched in order to
> support this, and each of them would use a mostly identical copy of that
> code.
>

More specifically, the gbm/EGL API works thusly:
  drm_fd = drmOpen("display_controller");
  gbm_dev = gbm_create_device(drm_fd);
  egl_dpy = eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_EXT, gbm_dev, NULL);

So in the last stage, you have to magically infer a relationship between
your display controller's device and your GPU's. On most mobile devices you
can just work this out (there's only one GPU), but it also breaks in the
multi-GPU case.

The best idea I've come up with to fix this in the long term, is to use
EGL_EXT_device_base to enumerate the GPUs and then feed the device ID you
get from that as a new attrib to eglGetPlatformDisplayEXT. It's either
that, or a new gbm_device_create_multi(display_controller_fd, gpu_fd)
entrypoint.

Of course, the first complication in all of this is that Weston doesn't
support platform_base at all anyway ...

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH ] i965/aa: fixing anti-aliasing bug for thinnest width lines - GEN7

2015-03-17 Thread Daniel Stone
Hi,

On 17 March 2015 at 16:37,   wrote:
> --- a/src/mesa/drivers/dri/i965/gen7_sf_state.c
> +++ b/src/mesa/drivers/dri/i965/gen7_sf_state.c
> @@ -198,9 +198,15 @@ upload_sf_state(struct brw_context *brw)
>float line_width =
>   roundf(CLAMP(ctx->Line.Width, 0.0, ctx->Const.MaxLineWidth));
>uint32_t line_width_u3_7 = U_FIXED(line_width, 7);
> -  /* TODO: line width of 0 is not allowed when MSAA enabled */
> -  if (line_width_u3_7 == 0)
> - line_width_u3_7 = 1;
> +
> +  if (!(multisampled_fbo && ctx->Multisample.Enabled)) {
> + if (ctx->Line.SmoothFlag && ctx->Line.Width <= 1 )
> +line_width_u3_7 = 0;
> +  } else {
> +  if (line_width_u3_7 = 0)
> + line_width_u3_7 = 1;

You almost certainly meant 'if (line_width_u3 == 0)', rather than an
assignment - surprised the compiler didn't throw a warning here.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] egl streams, trying to gain some knowledge

2015-04-02 Thread Daniel Stone
Hi,
My two cents, which largely parallels Jason's ...

On 2 April 2015 at 08:35, Dave Airlie  wrote:
> So nvidia have indicated they would like to use an EGLstreams based
> solution to enable wayland on their binary driver stack at some point.

No real surprise, I guess. NVIDIA have long pushed EGLStreams, but we
already _have_ an EGL API for Wayland (wl_display + wl_surface +
wl_egl_surface), and we're not going to change it.

> I'm just trying to gauge how people in mesa/wayland feel about this as
> a solution, is it a solution looking for a problem, when you have
> EGLstreams everything looks like a nail type situation etc

No thanks.

I can see the attraction of the idea, and I can see why you'd want to
pick a display API to extend to avoid the pain of interfacing clients,
display servers, media content, etc, but EGL is really just not that
API.

For the 3D client <-> server usecase, we already have native APIs for
those, and I don't really see that it adds anything over
eglSwapBuffers in that case, aside from perhaps helping

For the display server <-> display hardware usecase, firstly we
already have KMS, and secondly, the only thing Streams would buy is is
the ability to transparently pipe client content into a hardware
display pipeline (think overlay). But we've spent so long getting to
atomic and only just made it: why on earth would we throw that away
now? How is it even supposed to fit into an atomic display
configuration pipeline? Even if we decide to throw atomicity away,
what happens when we need to do mutually-dependent reconfiguration -
how do I know when an EGLOutput has actually been released by its
stream? Does it block until the relevant kernel/hw reconfiguration has
been completed (thus destroying atomicity), or does it return
immediately and just make you guess/poll at its status (which, tbh,
isn't really any better)?

For the media client <-> anything usecase, EGLStreams is useless as it
offers nothing in the way of timing other than a fixed latency, which
isn't good enough. What happens when I have to switch between doing a
blit pass and flipping directly to an overlay? What happens when I
move from my local laptop panel output to a HDMI output, which has an
intermediate sink which introduces 100ms of latency (something
queryable through CEA)? Saying 'the client should insert it at the
exact appropriate point' isn't good enough for media.

The biggest problem though, is that EGL just totally abrogates event
handling. For things like fence handling (and even buffer
release/handover), all it offers is query and wait-until-complete
APIs, rather than any kind of events or signalling. This is pretty
horrendous for both power usage and stable timing, and that alone
rules it out entirely from my point of view.

I've seen quite a few issues with the presentation side of things, but
I've never found myself wishing for more, rather than less, EGL. I
think gbm offers a good (if imperfect) model, and would like to see
more platforms allowing the user to control buffer allocation and
presentation.

> Also if anyone has any idea if any other EGL vendors are heading down
> this road, or if this is a one-company extension, ratified to KHR
> because nobody objected.

I haven't seen it ever implemented, no. Maybe comparing it with OpenWF
is unfair, but maybe not. Either way, it's not going to happen for
Wayland, or at least if it does, it's not going to displace the
standard existing stable API/ABI.

Cheers,
Dan
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH 1/2] egl/dri2: implement platform_null (v2).

2015-04-06 Thread Daniel Stone
On 4 April 2015 at 08:46, Jordan Justen  wrote:
> On 2015-04-03 19:18:35, Stéphane Marchesin wrote:
>> > Perhaps EGL_MESA_platform_surfaceless and platform_surfaceless.c?
>>
>> That's a very good name. As it happens, it also matches Chrome's naming.
>
> Chad made the point that this probably isn't going to come up too
> often in code, so maybe a short name isn't required.
>
> But, at least for waffle utils it might be nice to have something
> shorter for the command line.
>
> The syllable onsets of surfaceless give sfl. Thoughts? Backronyms? :)

dummy? headless? void? faux? nodisp?

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] st_TexSubImage: unaligned memcpy performance

2015-04-08 Thread Daniel Stone
Hi,

On 8 April 2015 at 10:57, Vasilis Liaskovitis  wrote:
> I have an issue where st_TexSubImage causes very high CPU load in
> __memcpy_sse2_unaligned (Mesa 10.1.3, Xorg 1.15.1, radeon driver, HD 7870).
>
> Any obvious causes / tips for this? e.g. align textures or use different
> format/type? I 've tried using GL_BGRA/GL_UNSIGNED_BYTE and
> GL_BGRA/GL_UNSIGNED_INT_8_8_8_8_REV
>
> __memcpy_sse2_unaligned () at
> ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:85
> 85../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S: No such file or
> directory.
> (gdb) bt
> #0  __memcpy_sse2_unaligned () at
> ../sysdeps/x86_64/multiarch/memcpy-sse2-unaligned.S:85
> #1  0x7fffb572f154 in memcpy (__len=7680, __src=,
> __dest=0x7fff5835f800) at /usr/include/x86_64-linux-gnu/bits/string3.h:51
> #2  st_TexSubImage (ctx=0x1b91420, dims=, texImage=0x1f81710,
> xoffset=0, yoffset=0, zoffset=0, width=1920, height=1080, depth=1,
> format=32993, type=5121, pixels=0xdacf90, unpack=0x1bad590)
> at ../../../../src/mesa/state_tracker/st_cb_texture.c:752

Your source (0xdacf90) is only aligned to a 16-byte boundary, not 32.
This will cause issues particularly on ARM, where natural alignment is
required (i.e. 32-byte load/stores must be on 32-byte boundaries). By
contrast, the destination is already aligned to a 128-byte boundary.
So fixing the caller, rather than Mesa, should take care of the
problem.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


[Mesa-dev] Valve games for Mesa/DRI developers

2015-04-09 Thread Daniel Stone
Hi,
At Collabora (my lovely dayjob), we've been working with Valve on
SteamOS. Valve are keen to give back to the community, and we've been
discussing ways they can help do that, including providing free access
to Valve games on Steam to Debian developers last year.

We're happy to say that this has been extended to Mesa developers as
well, to say thanks for all the great work. If you have 25 commits or
more (an arbitrary number) to Mesa[0] in the past five years, please
drop me an email (with 'Steam' in the subject) with your freedesktop
username and Steam username. We can then get you access to all past
and future Valve-produced games available on Steam[1].

Thanks for all the great work, and enjoy.

Cheers,
Daniel

[0]: Or DRI-type stuff in the kernel too.
[1]: Currently this looks like
https://store.steampowered.com/search/?snr=1_4_4__12&term=#category1=998&publisher=Valve&sort_order=ASC&page=1
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] Valve games for Mesa/DRI developers

2015-04-13 Thread Daniel Stone
Hi,

On 13 April 2015 at 15:06, Thierry Reding  wrote:
> I'm not much of a gamer myself, but I imagine that these games would be
> useful, real-life tests and/or entertaining benchmarks. Given that I
> work mostly on ARM systems, do you know if there are any plans on making
> these games available on ARM? I know some of Valve's games have been
> ported to ARM for Android, but perhaps there isn't enough of an audience
> to make it beneficial to get them to run on regular Linux on ARM?

I don't actually know myself, but even if I did, it wouldn't really be
for me to pre-empt Valve saying so. I agree it would be nice though!

Cheers,
Daniel

> Thierry
>
> On Thu, Apr 09, 2015 at 06:10:42PM +0100, Daniel Stone wrote:
>> Hi,
>> At Collabora (my lovely dayjob), we've been working with Valve on
>> SteamOS. Valve are keen to give back to the community, and we've been
>> discussing ways they can help do that, including providing free access
>> to Valve games on Steam to Debian developers last year.
>>
>> We're happy to say that this has been extended to Mesa developers as
>> well, to say thanks for all the great work. If you have 25 commits or
>> more (an arbitrary number) to Mesa[0] in the past five years, please
>> drop me an email (with 'Steam' in the subject) with your freedesktop
>> username and Steam username. We can then get you access to all past
>> and future Valve-produced games available on Steam[1].
>>
>> Thanks for all the great work, and enjoy.
>>
>> Cheers,
>> Daniel
>>
>> [0]: Or DRI-type stuff in the kernel too.
>> [1]: Currently this looks like
>> https://store.steampowered.com/search/?snr=1_4_4__12&term=#category1=998&publisher=Valve&sort_order=ASC&page=1
>> ___
>> dri-devel mailing list
>> dri-de...@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/dri-devel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [Mesa-stable] [PATCH] i965: Add XRGB8888 format to intel_screen_make_configs

2015-04-16 Thread Daniel Stone
Hi,

On 9 April 2015 at 17:20, Kristian Høgsberg  wrote:
> On Wed, Apr 8, 2015 at 11:37 AM, Emil Velikov  
> wrote:
>> Hi all,
>>
>> Can we get a pair of eyes on this patch please ?
>
> Reviewed-by: Kristian Høgsberg 
>
> Thanks for the reminder.

Mind you, this does break 75b7e1df13, which explicitly removes them in
order to 'fix' (scare quotes in original) a conformance test. That
commit also made our lives harder with
https://bugs.freedesktop.org/show_bug.cgi?id=67676 for which the
suggested fix was a separate EGL_XXX_transparent_alpha extension, but
between the two of these, it does seem like a more nuanced fix might
be in order.

Not being able to choose between XRGB and ARGB formats in the GBM
backend does actually impair our ability to hoist ARGB content to
planes, so at the very least, we'd need to duplicate ARGB
driver_configs in the EGLConfig list: one for an ARGB native visual
ID, and one for XRGB.

Any thoughts?

Cheers,
Daniel

> Kristian
>
>>
>> Boyan
>> For the future can you please include the CC mesa-stable line in the
>> commit message. It will make things a bit more obvious as I'm pursing
>> through the list :-)
>>
>> Thanks
>> Emil
>>
>> On 25 March 2015 at 11:36, Boyan Ding  wrote:
>>> Some application, such as drm backend of weston, uses XRGB config as
>>> default. i965 doesn't provide this format, but before commit 65c8965d,
>>> the drm platform of EGL takes ARGB as XRGB. Now that commit
>>> 65c8965d makes EGL recognize format correctly so weston won't start
>>> because it can't find XRGB. Add XRGB format to i965 just as
>>> other drivers do.
>>>
>>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=89689
>>> Signed-off-by: Boyan Ding 
>>> ---
>>>  src/mesa/drivers/dri/i965/intel_screen.c | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/src/mesa/drivers/dri/i965/intel_screen.c 
>>> b/src/mesa/drivers/dri/i965/intel_screen.c
>>> index 3640b67..2b82c33 100644
>>> --- a/src/mesa/drivers/dri/i965/intel_screen.c
>>> +++ b/src/mesa/drivers/dri/i965/intel_screen.c
>>> @@ -1126,7 +1126,8 @@ intel_screen_make_configs(__DRIscreen *dri_screen)
>>>  {
>>> static const mesa_format formats[] = {
>>>MESA_FORMAT_B5G6R5_UNORM,
>>> -  MESA_FORMAT_B8G8R8A8_UNORM
>>> +  MESA_FORMAT_B8G8R8A8_UNORM,
>>> +  MESA_FORMAT_B8G8R8X8_UNORM
>>> };
>>>
>>> /* GLX_SWAP_COPY_OML is not supported due to page flipping. */
>>> --
>>> 2.3.3
>>>
>>> ___
>>> mesa-stable mailing list
>>> mesa-sta...@lists.freedesktop.org
>>> http://lists.freedesktop.org/mailman/listinfo/mesa-stable
>> ___
>> mesa-dev mailing list
>> mesa-dev@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
> ___
> mesa-dev mailing list
> mesa-dev@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH] glx/dri3: Use a separate xcb connection for Present events

2015-04-19 Thread Daniel Stone
Hi,

On Saturday, April 18, 2015, Axel Davy  wrote:
>
> There's a strange issue with xcb_get_geometry: If I use the new xcb
> connection
> it fails and the error is a drawable error. If I use the Xlib xcb
> connection (what
> I did in this patch) it works. If anyone knows why, please tell. We use
> xcb_get_geometry
> with gallium nine with a different xcb connection than Xlib and it works
> without
>

Not all that surprising I guess. If a client creates a window and then
immediately creates a surface with it, nothing guarantees that the window
creation request on the other display actually ever got processed.

You'd need to pepper this with XSync on the display the client passed in to
recreate the synchronisation that was previously implicit in having both
client and Mesa requests ordered within the same connection.

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] New stable-branch 10.5 candidate pushed

2015-04-23 Thread Daniel Stone
Hi,

On 23 April 2015 at 14:12, Emil Velikov  wrote:
> Boyan Ding (2):
>   i965: Add XRGB format to intel_screen_make_configs
>   i915: Add XRGB format to intel_screen_make_configs

This fixes https://bugs.freedesktop.org/show_bug.cgi?id=89689 but
re-breaks ES3 MSAA by reverting
http://cgit.freedesktop.org/mesa/mesa/commit/?id=75b7e1df13. What a
mess ...

Ian - did you see the TRANSPARENT_ALPHA thread?

Cheers,
Daniel
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/mesa-dev


Re: [Mesa-dev] [PATCH v2] egl/wayland: Try to use wl_surface.damage_buffer for SwapBuffersWithDamage

2016-02-16 Thread Daniel Stone
Hi,

On 16 February 2016 at 16:34, Derek Foreman  wrote:
> +try_damage_buffer(struct dri2_egl_surface *dri2_surf,
> +  const EGLint *rects,
> +  EGLint n_rects)
> +{
> +/* The WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION macro and
> + * wl_proxy_get_version() were both introduced in wayland 1.10.
> + * Instead of bumping our wayland dependency we just make this
> + * function conditional on the required 1.10 features, falling
> + * back to old (correct but suboptimal) behaviour for older
> + * wayland.
> + */
> +#ifdef WL_SURFACE_DAMAGE_BUFFER_SINCE_VERSION

It still bumps the runtime requirement, i.e. once built against >=1.10
it can only ever be run against >= 1.10. Maybe dlsym is overkill, but
OTOH maybe not ...

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


  1   2   3   4   5   6   7   8   9   >