Re: [Mesa-dev] Mesa Querying Devices Filter

2021-11-12 Thread Emil Velikov
Hi Stephen,

I don't have any time to spend on Mesa these days, so a bit of a late answer.

On Wed, 27 Oct 2021 at 08:46, Stephen Crowell
 wrote:
>
> To whom it may concern,
>
> I am attempting to use Mesa with EGL for a project and I have run into an 
> issue when initializing a device. My setup uses a surfaceless platform with 
> swrast as the gallium driver.
>
> When I query for devices to initialize (using eglQueryDevices), the first 
> device listed is the driver for my GPU (nouveau) and the other devices are 
> the devices I want to initialize (swrast, swr, etc). My project currently 
> loops through all possible devices and initializes each one. Once one is 
> initialized properly, the program continues.
>
Note that you cannot distinguish between classic swrast, gallium
swrast (with either of softpipe, llvmpipe and swr) at the EGL device
level. You can switch between the gallium swrast drivers via -
GALLIUM_DRIVER=swr.

> I was wondering if there is a way to filter out devices (like drivers for 
> GPU). This would make the output of my program more readable and also save 
> time by not initializing devices that will not work.
>
With EGL devices there's the concept of device extensions - the
software one will have "EGL_MESA_device_software",  DRM ones have
"EGL_EXT_device_drm" - you get the idea.

> Also as a side note, why does Mesa query for my GPU drivers in the first 
> place? The only driver I specify to use is swrast and I am not sure why my 
> GPU driver is being found.
>
That's how the spec was designed - gives you up-to the requested
amount of EGL devices. Traditionally only GPU-backed ones, with the
extension mentioned above - you can get software one as well.

One important note: do _not_ use LIBGL_ALWAYS_SOFTWARE=1 when trying
to use EGL devices. It may or may not work and is not something we
want to support if we can avoid it.

HTH
-Emil


Re: [Mesa-dev] [ANNOUNCE] mesa 20.3.4

2021-02-23 Thread Emil Velikov
Hi Dylan,

On Sat, 30 Jan 2021 at 04:12, Dylan Baker  wrote:
>
> Hi list,
>
> Better late than never, mesa 20.3.4 is now available for your
> consumption. Sorry for the brief announcment, I'm a little short on
> time.
>
The website lists 20.3.3 as the latest version. Did you forget to push
those or is something broken on my end?

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


Re: [Mesa-dev] [PATCH] vulkan: Add VK_GOOGLE_display_timing extension (x11+display, anv+radv) [v8]

2020-08-26 Thread Emil Velikov
Hey Keith,

Most of the cool kids prefer gitlab MR, can you open one going forward?
That aside, here is some long overdue input on the patch itself.

My biggest concern that we expose the extension, even when it's not implemented.
The rest are rather minor - more documentation/comments, style fixes
and nitpicks.

On Thu, 30 Jan 2020 at 04:48, Keith Packard  wrote:

> v5:
> Squash core bits and anv/radv wrappers into a single patch
>
We have more Vulkan drivers than anv/radv these days, with a few
others in the works.

For posterity sake, it might be worth keeping core + driver enablement
separate patches.
As an added bonus, this way the respective teams can test, review,
merge or even revert (please no) completely independently from each
other.


> --- a/src/amd/vulkan/radv_extensions.py
> +++ b/src/amd/vulkan/radv_extensions.py
> @@ -166,6 +166,7 @@ EXTENSIONS = [
>  Extension('VK_AMD_shader_trinary_minmax', 1, True),
>  Extension('VK_GOOGLE_decorate_string',1, True),
>  Extension('VK_GOOGLE_hlsl_functionality1',1, True),
> +Extension('VK_GOOGLE_display_timing', 1, True),
Nit: sort?


> --- a/src/amd/vulkan/radv_wsi.c
> +++ b/src/amd/vulkan/radv_wsi.c
> @@ -316,3 +316,36 @@ VkResult radv_GetPhysicalDevicePresentRectanglesKHR(
>  surface,
>  pRectCount, pRects);
>  }
> +
> +/* VK_GOOGLE_display_timing */
> +VkResult
> +radv_GetRefreshCycleDurationGOOGLE(
> +   VkDevice _device,
> +   VkSwapchainKHR swapchain,
> +   VkRefreshCycleDurationGOOGLE *pDisplayTimingProperties)
Nit: inconsistent indentation (follow GetPastPresentationTiming example below).


> --- a/src/intel/vulkan/anv_extensions.py
> +++ b/src/intel/vulkan/anv_extensions.py
> @@ -170,6 +170,7 @@ EXTENSIONS = [
>  Extension('VK_ANDROID_external_memory_android_hardware_buffer', 3, 
> 'ANDROID'),
>  Extension('VK_ANDROID_native_buffer', 7, 'ANDROID'),
>  Extension('VK_GOOGLE_decorate_string',1, True),
> +Extension('VK_GOOGLE_display_timing', 1, True),
The dummy true will always advertise the extension. The extension
depends on VK_KHR_swapchain which uses DRV_HAS_SURFACE guard, which is
a good start.
Although there is no Wayland implementation, so how does the extension
work there? We really should not advertise it in said case.

Same applies for radv as well, obviously.


> --- a/src/vulkan/wsi/wsi_common.c
> +++ b/src/vulkan/wsi/wsi_common.c

> @@ -54,6 +55,7 @@ wsi_device_init(struct wsi_device *wsi,
> WSI_GET_CB(GetPhysicalDeviceProperties2);
> WSI_GET_CB(GetPhysicalDeviceMemoryProperties);
> WSI_GET_CB(GetPhysicalDeviceQueueFamilyProperties);
> +   WSI_GET_CB(GetPhysicalDeviceProperties);
Nit: sort

> @@ -94,11 +104,15 @@ wsi_device_init(struct wsi_device *wsi,
> WSI_GET_CB(GetImageMemoryRequirements);
> WSI_GET_CB(GetImageSubresourceLayout);
> WSI_GET_CB(GetMemoryFdKHR);
> +   WSI_GET_CB(GetPhysicalDeviceProperties);
> WSI_GET_CB(GetPhysicalDeviceFormatProperties);
> WSI_GET_CB(GetPhysicalDeviceFormatProperties2KHR);
Nit: sort - DeviceProperties should be here.


> @@ -210,6 +224,8 @@ wsi_swapchain_init(const struct wsi_device *wsi,
> chain->device = device;
> chain->alloc = *pAllocator;
> chain->use_prime_blit = false;
> +   chain->timing_insert = 0;
> +   chain->timing_count = 0;
>
Not needed we memset(0) at the top of the function.


> +static VkResult
> +wsi_image_init_timestamp(const struct wsi_swapchain *chain,
> + struct wsi_image *image)
> +{
> +   const struct wsi_device *wsi = chain->wsi;
> +   VkResult result;
> +   /* Set up command buffer to get timestamp info */
> +
> +   result = wsi->CreateQueryPool(
> +  chain->device,
> +  &(const VkQueryPoolCreateInfo) {
> + .sType = VK_STRUCTURE_TYPE_QUERY_POOL_CREATE_INFO,
> +.queryType = VK_QUERY_TYPE_TIMESTAMP,
> +.queryCount = 1,
> +},
> +  NULL,
> +  >query_pool);
> +
Current code prefers using local variables. Bonus - it will ease
fixing the indentation. Comments apply for the whole patch.

> +static struct wsi_timing *
> +wsi_next_timing(struct wsi_swapchain *chain, int image_index)
Unused image_index. Left over from earlier revision, or incomplete code?

> +{
> +   uint32_t j = chain->timing_insert;
> +   ++chain->timing_insert;
Please use post increments through the patch.

> +   if (chain->timing_insert >= WSI_TIMING_HISTORY)
> +  chain->timing_insert = 0;
> +   if (chain->timing_count < WSI_TIMING_HISTORY)
> +  ++chain->timing_count;
> +   struct wsi_timing *timing = >timing[j];
> +   memset(timing, '\0', sizeof (*timing));
The memset here is rather misleading. Since the caller already
(re)sets some of the fields one might as well just expand that.


> +void
> +wsi_present_complete(struct 

Re: [Mesa-dev] [PATCH v2 1/7] nv50/ir: add nv50_ir_prog_info_out

2020-03-04 Thread Emil Velikov
Hi Mark,

On Fri, 21 Feb 2020 at 12:20, Mark Menzynski  wrote:

> -   ret = nv50_ir_generate_code(info);
> +   /* these fields might be overwritten by the compiler */
> +   info_out.bin.smemSize = prog->cp.smem_size;
> +   info_out.io.genUserClip = prog->vp.num_ucps;
> +
I suspect that these two should be not be the out "version" of the
variables, but more like in the final patch.
Especially since nv50_ir_generate_code indiscriminately overrides info_out.

While I haven't looked at the code too closely, if does seem like this
commit causes an intermittent regression... Or perhaps we're lucky and
things just work ;-)

Either way, huge thanks for the update. Doubt I'll have the chance to
do a proper review, despite that the performance numbers look great.

> +   ret = nv50_ir_generate_code(info, _out);
> if (ret) {
>NOUVEAU_ERR("shader translation failed: %i\n", ret);
>goto out;
> }
>

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


Re: [Mesa-dev] [PATCH 8/8] nvc0: Add shader disk caching

2020-02-18 Thread Emil Velikov
Hi Mark,

Above all - welcome to nouveau. Glad to see fresh blood joining.

On Mon, 17 Feb 2020 at 17:41, Mark Menzynski  wrote:
>
> Adds shader disk caching for nvc0 to reduce the need to every time compile
> shaders. Shaders are saved into disk_shader_cache from nvc0_screen structure.
>
> It serializes the input nv50_ir_prog_info to compute the hash key and
> also to do a byte compare between the original nv50_ir_prog_info and the one
> saved in the cache. If keys match and also the byte compare returns they
> are equal, shaders are same, and the compiled nv50_ir_prog_info_out from the
> cache can be used instead of compiling input info.
>
> Seems to be significantly improving loading times. Piglit tests seem
> to be OK.
>
Really happy to see this happening. Do you mind adding some rough
numbers in the commit message.

Out of curiosity: Is there any specific reason why nv50 isn't also wired up?
Is it a case of didn't test, or there's something more subtle?

> Signed-off-by: Mark Menzynski 
> ---
>  .../drivers/nouveau/nvc0/nvc0_context.h   |  1 +
>  .../drivers/nouveau/nvc0/nvc0_program.c   | 49 ---
>  .../drivers/nouveau/nvc0/nvc0_shader_state.c  |  3 +-
>  src/gallium/drivers/nouveau/nvc0/nvc0_state.c |  2 +
>  4 files changed, 46 insertions(+), 9 deletions(-)
>
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> index 8a2a8f2797e..4b83d1afeb4 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_context.h
> @@ -321,6 +321,7 @@ extern struct draw_stage *nvc0_draw_render_stage(struct 
> nvc0_context *);
>
>  /* nvc0_program.c */
>  bool nvc0_program_translate(struct nvc0_program *, uint16_t chipset,
> +struct disk_cache *,
>  struct pipe_debug_callback *);
>  bool nvc0_program_upload(struct nvc0_context *, struct nvc0_program *);
>  void nvc0_program_destroy(struct nvc0_context *, struct nvc0_program *);
> diff --git a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c 
> b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> index 1a5073292e8..06b6f7b4db5 100644
> --- a/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> +++ b/src/gallium/drivers/nouveau/nvc0/nvc0_program.c
> @@ -24,6 +24,7 @@
>
>  #include "compiler/nir/nir.h"
>  #include "tgsi/tgsi_ureg.h"
> +#include "util/blob.h"
>
>  #include "nvc0/nvc0_context.h"
>
> @@ -568,11 +569,19 @@ nvc0_program_dump(struct nvc0_program *prog)
>
>  bool
>  nvc0_program_translate(struct nvc0_program *prog, uint16_t chipset,
> +   struct disk_cache *disk_shader_cache,
> struct pipe_debug_callback *debug)
>  {
> +   struct blob blob;
> struct nv50_ir_prog_info *info;
> struct nv50_ir_prog_info_out info_out = {};
> -   int ret;
> +
> +   void *cached_data = NULL;
> +   size_t cached_size;
> +   bool shader_found = false;
> +
> +   int ret = 0;
> +   cache_key key;
>
> info = CALLOC_STRUCT(nv50_ir_prog_info);
> if (!info)
> @@ -631,14 +640,38 @@ nvc0_program_translate(struct nvc0_program *prog, 
> uint16_t chipset,
> info->assignSlots = nvc0_program_assign_varying_slots;
>
> /* these fields might be overwritten by the compiler */
> -   info_out.bin.smemSize = prog->cp.smem_size;
> -   info_out.io.genUserClip = prog->vp.num_ucps;
> -
> -   ret = nv50_ir_generate_code(info, _out);
> -   if (ret) {
> -  NOUVEAU_ERR("shader translation failed: %i\n", ret);
> -  goto out;
> +   info->bin.smemSize = prog->cp.smem_size;
> +   info->io.genUserClip = prog->vp.num_ucps;
> +
> +   blob_init();
> +   nv50_ir_prog_info_serialize(, info);
> +
> +   if (disk_shader_cache) {

Will this work correctly, if the cache is explicitly disabled?

It's been years since I looked at this code so I wonder if the two
original "info_out... = prog->..." shouldn't stay as-is.
At the same time it will make sense to move the "info->... =
prog->..." + serialise() within the if (disk_shader_cache) hunk.

blob_init() seems safe, so it can stay as-is.

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


Re: [Mesa-dev] [PATCH] egl: Mention if swrast is being forced

2019-11-01 Thread Emil Velikov
On Fri, 1 Nov 2019 at 14:32, Chris Wilson  wrote:
>
> Quoting Eric Engestrom (2019-10-31 14:06:40)
> > On Thursday, 2019-10-31 07:35:04 +, Chris Wilson wrote:
> > > The system can be disabling HW acceleration unbeknowst to the user,
> > > leading to a long debug session trying to work out which component is
> > > failing. A quick mention that it is the environment override would be
> > > very useful.
> > > ---
> > >  src/egl/main/egldriver.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/src/egl/main/egldriver.c b/src/egl/main/egldriver.c
> > > index 0d8919aa0e1..132b12ab4cb 100644
> > > --- a/src/egl/main/egldriver.c
> > > +++ b/src/egl/main/egldriver.c
> > > @@ -92,6 +92,8 @@ _eglMatchDriver(_EGLDisplay *disp)
> > > /* set options */
> > > disp->Options.ForceSoftware =
> > >env_var_as_boolean("LIBGL_ALWAYS_SOFTWARE", false);
> > > +   if (disp->Options.ForceSoftware)
> > > +  _eglLog(_EGL_DEBUG, "Found 'LIBGL_ALWAYS_SOFTWARE' set, forcing 
> > > swrast");
> >
> > Good idea!
> > Reviewed-by: Eric Engestrom 
> >
> > I might even suggest going one step further and make that an _EGL_WARNING,
> > so that users are always informed of this by default, without having to
> > set EGL_LOG_LEVEL.
> >
> > I think most users don't want to disable their hardware, so the annoyance
> > if this warning showing up for users who want it should be completely
> > offset by the usefulness of this information for those who don't.
>
> Is there a consensus to go with an always visible _EGL_WARNING instead?

There's been users in the past that explicitly set LIBGL_ALWAYS_SOFTWARE.
In general if you're debugging and EGL_LOG_LEVEL is not set to debug
you're already lost.

That said, personally I'm fine with either DEBUG or WARNING.

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

Re: [Mesa-dev] [PATCH] egl: Mention if swrast is being forced

2019-10-31 Thread Emil Velikov
On Thu, 31 Oct 2019 at 07:35, Chris Wilson  wrote:
>
> The system can be disabling HW acceleration unbeknowst to the user,
Nit: s/unbeknownst/unknown/
unbeknowst isn't that common across non-native speakers, plus there's
a typo in it ;-)

> leading to a long debug session trying to work out which component is
> failing. A quick mention that it is the environment override would be
> very useful.

Reviewed-by: Emil Velikov 

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

[Mesa-dev] [PATCH] glx: set the custom logger across all DRI impls.

2019-09-13 Thread Emil Velikov
From: Emil Velikov 

Earlier commit refactored common code into the loader, yet did not set
the custom logger (one that honours LIBGL_DEBUG).

Thus LIBGL_DEBUG=verbose was working only with DRI3.

Fixes: d971a4230d5 ("loader: Factor out the common driver opening logic from 
each loader.")
Cc: Eric Anholt 
Cc: Eric Engestrom 
Signed-off-by: Emil Velikov 
---
 src/glx/dri2_glx.c  | 2 ++
 src/glx/dri3_glx.c  | 4 ++--
 src/glx/dri_glx.c   | 2 ++
 src/glx/drisw_glx.c | 2 ++
 4 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/glx/dri2_glx.c b/src/glx/dri2_glx.c
index d6a543c8da0..3105e01983b 100644
--- a/src/glx/dri2_glx.c
+++ b/src/glx/dri2_glx.c
@@ -1425,6 +1425,8 @@ dri2CreateDisplay(Display * dpy)
struct dri2_display *pdp;
int eventBase, errorBase, i;
 
+   loader_set_logger(dri_message);
+
if (!DRI2QueryExtension(dpy, , ))
   return NULL;
 
diff --git a/src/glx/dri3_glx.c b/src/glx/dri3_glx.c
index a5cf0e3bf32..b700871fefc 100644
--- a/src/glx/dri3_glx.c
+++ b/src/glx/dri3_glx.c
@@ -1061,6 +1061,8 @@ dri3_create_display(Display * dpy)
xcb_generic_error_t  *error;
const xcb_query_extension_reply_t*extension;
 
+   loader_set_logger(dri_message);
+
xcb_prefetch_extension_data(c, _dri3_id);
xcb_prefetch_extension_data(c, _present_id);
 
@@ -1105,8 +1107,6 @@ dri3_create_display(Display * dpy)
pdp->base.destroyDisplay = dri3_destroy_display;
pdp->base.createScreen = dri3_create_screen;
 
-   loader_set_logger(dri_message);
-
pdp->loader_extensions = loader_extensions;
 
return >base;
diff --git a/src/glx/dri_glx.c b/src/glx/dri_glx.c
index 6e9412d3fb1..ae7b5a736d2 100644
--- a/src/glx/dri_glx.c
+++ b/src/glx/dri_glx.c
@@ -987,6 +987,8 @@ driCreateDisplay(Display * dpy)
int eventBase, errorBase;
int major, minor, patch;
 
+   loader_set_logger(dri_message);
+
if (!XF86DRIQueryExtension(dpy, , )) {
   return NULL;
}
diff --git a/src/glx/drisw_glx.c b/src/glx/drisw_glx.c
index f38dbbca2df..2efee2aa841 100644
--- a/src/glx/drisw_glx.c
+++ b/src/glx/drisw_glx.c
@@ -973,6 +973,8 @@ driswCreateDisplay(Display * dpy)
 {
struct drisw_display *pdpyp;
 
+   loader_set_logger(dri_message);
+
pdpyp = malloc(sizeof *pdpyp);
if (pdpyp == NULL)
   return NULL;
-- 
2.23.0

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

[Mesa-dev] [ANNOUNCE] mesa 19.2.0-rc1

2019-08-20 Thread Emil Velikov
The first release candidate for Mesa 19.2.0 is now available.

The plan is to have one release candidate every Tuesday, until the
anticipated final release on 10th September 2019.

The expectation is that the 19.1 branch will remain alive with bi-weekly
releases until the 19.2.1 release.

In the path to 19.2.0 release, there is a tracker bug for the
regressions found since 19.1:
https://bugs.freedesktop.org/show_bug.cgi?id=111444

Here are the people which helped shape the current release.

 8  Adam Jackson
 1  Ahmad Fatoum
35  Alejandro Piñeiro
 1  Alex Villacís Lasso
26  Alexandros Frantzis
 1  Alistair Strachan
   638  Alyssa Rosenzweig
 1  Amit Pundir
12  Andreas Baierl
 1  Andreas Bergmeier
 3  Andres Gomez
 1  Andres Rodriguez
 1  Andrii Kryvytskyi
 5  Andrii Simiklit
14  Antia Puentes
 7  Anuj Phogat
 5  Arcady Goldmints-Orlov
 1  Arfrever Frehtes Taifersar Arahesis
 1  Arnaud Patard
 1  Axel Davy
   114  Bas Nieuwenhuizen
33  Boris Brezillon
19  Boyuan Zhang
 9  Brian Paul
95  Caio Marcelo de Oliveira Filho
 3  Charmaine Lee
 1  Chenglei Ren
53  Chia-I Wu
11  Chih-Wei Huang
 1  Chris Forbes
 3  Chris Wilson
 1  Christian GMEINER
28  Christian Gmeiner
69  Connor Abbott
24  Daniel Schürmann
 2  Daniel Stone
 7  Danylo Piliaiev
41  Dave Airlie
 1  David Riley
 2  Deepak Rawat
 5  Dongwon Kim
24  Dylan Baker
 7  Eduardo Lima Mitev
13  Emil Velikov
90  Eric Anholt
   169  Eric Engestrom
26  Erico Nunes
89  Erik Faye-Lund
 5  Francisco Jerez
38  Gert Wollny
 8  Greg V
20  Guido Günther
 8  Gurchetan Singh
 1  Haihao Xiang
 2  Harish Krupo
 1  Heinrich Fink
 7  Hyunjun Ko
57  Iago Toral Quiroga
83  Ian Romanick
12  Ilia Mirkin
 1  Illia Iorin
 1  James Clarke
 1  James Zhu
 1  Jan Vesely
20  Jan Zielinski
   198  Jason Ekstrand
 1  Jeremy Newton
 5  John Stultz
 3  Jon Turney
 1  Jonas Ådahl
56  Jonathan Marek
14  Jordan Justen
 1  Jorge Natz
 2  Jory Pratt
 4  Jose Maria Casanova Crespo
 1  José Fonseca
19  Juan A. Suarez Romero
 1  Julien Isorce
 1  Józef Kucia
 1  Kai Wasserbäch
 9  Karol Herbst
   167  Kenneth Graunke
 2  Kevin Strasser
 4  Khaled Emara
10  Kristian Høgsberg
 2  Krzysztof Raszkowski
 8  Leo Liu
 1  Lepton Wu
54  Lionel Landwerlin
 2  Lucas Stach
   274  Marek Olšák
 5  Marek Vasut
62  Mark Janes
 1  Mark Menzynski
 1  Mateusz Krzak
 9  Mathias Fröhlich
 9  Matt Turner
10  Mauro Rossi
13  Michel Dänzer
 1  Michel Zou
 2  Mika Kuoppala
10  Mike Blumenkrantz
 1  Mike Lothian
14  Nanley Chery
 3  Nataraj Deshpande
 1  Neha Bhende
14  Neil Roberts
   133  Nicolai Hähnle
 1  Nicolas Dufresne
 2  Patrick Lerda
 9  Paulo Zanoni
 1  Philipp Zabel
64  Pierre-Eric Pelloux-Prayer
 1  Pratik Vishwakarma
 7  Qiang Yu
18  Rafael Antognolli
15  Rhys Perry
 1  Richard Thier
99  Rob Clark
 2  Rob Herring
 5  Rohan Garg
 5  Roland Scheidegger
 4  Roman Stratiienko
 1  Rui Salvaterra
 1  Ruslan Kabatsayev
28  Sagar Ghuge
 3  Samuel Iglesias Gonsálvez
   282  Samuel Pitoiset
 6  Sergii Romantsov
 2  Sonny Jiang
 9  Tapani Pälli
11  Thomas Hellstrom
42  Timothy Arceri
 2  Timur Kristóf
58  Tomeu Vizoso
 1  Uros Bizjak
23  Vasily Khoruzhick
 1  Ville Syrjälä
 5  Vinson Lee
 1  Xiong, James
11  Yevhenii Kolesnikov
 1  renchenglei


git tag: mesa-19.2.0-rc1

https://mesa.freedesktop.org/archive/mesa-19.2.0-rc1.tar.xz
MD5:  24834b9b9eda02522b6f5ea28cee226b mesa-19.2.0-rc1.tar.xz
SHA1: da226b53126de99b302baca0a64e4d6c8a63b555 mesa-19.2.0-rc1.tar.xz
SHA256: 71baa3bb608cfc818f48d55405d230ca863927e7751a21d88deaeddf17d3b2ec 
mesa-19.2.0-rc1.tar.xz
SHA512: 
babafb6ec668834e041a62e966452fe315e4f5afef8e1f7d35e038993c73b32ea6ad9929399edae4686f44cd11704fd9a8801821befc1d1767bc0662d4c39fac
 mesa-19.2.0-rc1.tar.xz
PGP:  https://mesa.freedesktop.org/archive/mesa-19.2.0-rc1.tar.xz.sig

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

Re: [Mesa-dev] Mesa 19.2.0 release plan

2019-08-19 Thread Emil Velikov
On Sunday, 18 August 2019, Matt Turner  wrote:

> On Thu, Aug 8, 2019 at 2:56 AM Emil Velikov 
> wrote:
> >
> > On Wed, 7 Aug 2019 at 21:43, Mark Janes  wrote:
> > >
> > > Eric Engestrom  writes:
> > >
> > > > On 2019-07-31 at 09:38, Emil Velikov 
> wrote:
> > > >> Hi all,
> > > >>
> > > >> Here is the tentative release plan for 19.2.0.
> > > >>
> > > >> As many of you are well aware, it's time to the next branch point.
> > > >> The calendar is already updated, so these are the tentative dates:
> > > >>
> > > >>  Aug 06 2019 - Feature freeze/Release candidate 1
> > > >>  Aug 13 2019 - Release candidate 2
> > > >>  Aug 20 2019 - Release candidate 3
> > > >>  Aug 27 2019 - Release candidate 4/final release
> > > >>
> > > >> This gives us around 1 week until the branch point.
> > > >>
> > > >> Note: In the spirit of keeping things clearer and more transparent,
> we
> > > >> will be keeping track of any features planned for the release in
> > > >> Bugzilla [1].
> > > >>
> > > >> Do add a separate "Depends on" for each work you have planned.
> > > >> Alternatively you can reply to this email and I'll add them for you.
> > > >>
> > > >> [1] https://bugs.freedesktop.org/show_bug.cgi?id=111265
> > > >
> > > > Thanks!
> > > >
> > > > As per previous discussions (I don't remember where, sorry) as well
> as
> > > > internal discussions, I think we should add all currently open
> > > > regressions since 19.1 as blockers for this release.
> > >
> > > My understanding is that the "feature tracker" blocks the creation of
> > > the release branchpoint.  A separate "release tracker" blocks the
> > > release of 19.2.0.  Unfixed regressions go on the "release tracker",
> not
> > > the "feature tracker".  We backport bug fixes to release branches, but
> > > we don't backport features.
> > >
> > Yes that is correct. We are interested in features for the next few days.
> > Afterwords we'll focus on bugfixes.
>
> The last bug in the feature tracker was closed on the 14th. Can we make
> RC1 now?
>

Hi Matt,

Sure thing. I'm about to board the plane in a few minutes, I'll roll RC1
first thing tomorrow morning.

Thanks
Emil
P.S. Pardon the html formatting
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] Mesa 19.2.0 release plan

2019-08-08 Thread Emil Velikov
On Wed, 7 Aug 2019 at 21:43, Mark Janes  wrote:
>
> Eric Engestrom  writes:
>
> > On 2019-07-31 at 09:38, Emil Velikov  wrote:
> >> Hi all,
> >>
> >> Here is the tentative release plan for 19.2.0.
> >>
> >> As many of you are well aware, it's time to the next branch point.
> >> The calendar is already updated, so these are the tentative dates:
> >>
> >>  Aug 06 2019 - Feature freeze/Release candidate 1
> >>  Aug 13 2019 - Release candidate 2
> >>  Aug 20 2019 - Release candidate 3
> >>  Aug 27 2019 - Release candidate 4/final release
> >>
> >> This gives us around 1 week until the branch point.
> >>
> >> Note: In the spirit of keeping things clearer and more transparent, we
> >> will be keeping track of any features planned for the release in
> >> Bugzilla [1].
> >>
> >> Do add a separate "Depends on" for each work you have planned.
> >> Alternatively you can reply to this email and I'll add them for you.
> >>
> >> [1] https://bugs.freedesktop.org/show_bug.cgi?id=111265
> >
> > Thanks!
> >
> > As per previous discussions (I don't remember where, sorry) as well as
> > internal discussions, I think we should add all currently open
> > regressions since 19.1 as blockers for this release.
>
> My understanding is that the "feature tracker" blocks the creation of
> the release branchpoint.  A separate "release tracker" blocks the
> release of 19.2.0.  Unfixed regressions go on the "release tracker", not
> the "feature tracker".  We backport bug fixes to release branches, but
> we don't backport features.
>
Yes that is correct. We are interested in features for the next few days.
Afterwords we'll focus on bugfixes.

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

Re: [Mesa-dev] Mesa 19.2.0 release plan

2019-08-07 Thread Emil Velikov
Hi all,

On Wed, 31 Jul 2019 at 09:37, Emil Velikov  wrote:
>
> Hi all,
>
> Here is the tentative release plan for 19.2.0.
>
> As many of you are well aware, it's time to the next branch point.
> The calendar is already updated, so these are the tentative dates:
>
>  Aug 06 2019 - Feature freeze/Release candidate 1
>  Aug 13 2019 - Release candidate 2
>  Aug 20 2019 - Release candidate 3
>  Aug 27 2019 - Release candidate 4/final release
>
With multiple teams finalising the final features for their drivers,
I've decided to push the branch point by one week.

I would like to remind teams that they are welcome to merge
functionality early and keep it disabled by default.
This way they can address outstanding issues and enable, during the
stabilisation period.

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

Re: [Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common

2019-08-01 Thread Emil Velikov
On Thu, 1 Aug 2019 at 16:03, Emil Velikov  wrote:
>
> On Thu, 1 Aug 2019 at 14:26, Michel Dänzer  wrote:
> > On 2019-08-01 2:32 p.m., Emil Velikov wrote:
>
> > > Sure I'll do that in a moment.
> >
> > Why don't you just follow my suggestion above?
> >
> That's what I was planning to do :-)
>
Done and dusted. Thanks for the help and extensive how-to, I've
followed it literally.

-Emil

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

Re: [Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common

2019-08-01 Thread Emil Velikov
On Thu, 1 Aug 2019 at 14:26, Michel Dänzer  wrote:
> On 2019-08-01 2:32 p.m., Emil Velikov wrote:

> > Sure I'll do that in a moment.
>
> Why don't you just follow my suggestion above?
>
That's what I was planning to do :-)

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

Re: [Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common

2019-08-01 Thread Emil Velikov
On Thu, 1 Aug 2019 at 09:35, Michel Dänzer  wrote:
>
> On 2019-07-31 11:47 p.m., Eric Engestrom wrote:
> > On Wednesday, 2019-07-31 16:07:45 +0200, Michel Dänzer wrote:
> >> On 2019-07-31 3:26 p.m., Emil Velikov wrote:
> >>> On Wed, 31 Jul 2019 at 14:16, Michel Dänzer  wrote:
> >>>>
> >>>> On 2019-07-31 3:04 p.m., Emil Velikov wrote:
> >>>>> From: Emil Velikov 
> >>>>>
> >>>>> Currently we use the python package to manage repositories. At the same
> >>>>> time we also do that by hand - since it's a trivial echo to a file.
> >>>>>
> >>>>> Stay consistent, remove the package and manage things manually.
> >>>>>
> >>>>> Cc: Eric Engestrom 
> >>>>> Signed-off-by: Emil Velikov 
> >>>>> ---
> >>>>>  .gitlab-ci/debian-install.sh | 11 +--
> >>>>>  1 file changed, 5 insertions(+), 6 deletions(-)
> >>>>>
> >>>>> diff --git a/.gitlab-ci/debian-install.sh b/.gitlab-ci/debian-install.sh
> >>>>> index 578074ddb87..719d7830018 100644
> >>>>> --- a/.gitlab-ci/debian-install.sh
> >>>>> +++ b/.gitlab-ci/debian-install.sh
> >>>>> @@ -16,12 +16,11 @@ apt-get install -y \
> >>>>>curl \
> >>>>>wget \
> >>>>>unzip \
> >>>>> -  gnupg \
> >>>>> -  software-properties-common
> >>>>> +  gnupg
> >>>>>
> >>>>>  curl -fsSL https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
> >>>>> -add-apt-repository "deb https://apt.llvm.org/stretch/ 
> >>>>> llvm-toolchain-stretch-7 main"
> >>>>> -add-apt-repository "deb https://apt.llvm.org/stretch/ 
> >>>>> llvm-toolchain-stretch-8 main"
> >>>>> +echo "deb [trusted=yes] https://apt.llvm.org/stretch/ 
> >>>>> llvm-toolchain-stretch-7 main" >/etc/apt/sources.list.d/llvm7.list
> >>>>> +echo "deb [trusted=yes] https://apt.llvm.org/stretch/ 
> >>>>> llvm-toolchain-stretch-8 main" >/etc/apt/sources.list.d/llvm8.list
> >>>>>
> >>>>>  sed -i -e 's/http:\/\/deb/https:\/\/deb/g' /etc/apt/sources.list
> >>>>>  echo 'deb https://deb.debian.org/debian stretch-backports main' 
> >>>>> >/etc/apt/sources.list.d/backports.list
> >>>>> @@ -46,8 +45,8 @@ apt-get install -y -t stretch-backports \
> >>>>>clang-8
> >>>>>
> >>>>>  # Install remaining packages from Debian buster to get newer versions
> >>>>> -add-apt-repository "deb https://deb.debian.org/debian/ buster main"
> >>>>> -add-apt-repository "deb https://deb.debian.org/debian/ buster-updates 
> >>>>> main"
> >>>>> +echo "deb https://deb.debian.org/debian/ buster main" 
> >>>>> >/etc/apt/sources.list.d/buster.list
> >>>>> +echo "deb https://deb.debian.org/debian/ buster-updates main" 
> >>>>> >/etc/apt/sources.list.d/buster-updates.list
> >>>>>  apt-get update
> >>>>>  apt-get install -y \
> >>>>>bzip2 \
> >>>>>
> >>>>
> >>>> This should be merged as part of an MR which requires the docker image
> >>>> to be re-generated for another reason, and thus bumps DEBIAN_TAG.
> >>>>
> >>> Since this is a non-functional change, I've explicitly omitted bumping
> >>> the DEBIAN_TAG.
> >>> Seemingly I forgot to mention it in the commit message though, oopsie.
> >>>
> >>> Since the image will contain practically the same artefacts, is it
> >>> worth carving out 30 minutes (or so) from the runners?
> >>
> >> No, I agree that would be wasteful for this change alone.
> >>
> >> However, merging this change without bumping the tag isn't good either,
> >> because then any issues with it would only be discovered the next time
> >> it does get bumped. Hence my request above.
> >
> > I agree with Michel here, it's better to waste a re-gen now and notice
> > any issue right away.
>
> That's not exactly what I'm saying though. :)
>
> If you don't want to merge this together with other changes which bump
> the tag, how about:
>
> * Create an MR with an additional commit which bumps the tag
> * Wait for the CI pipeline to come back green
> * Force-push to the source branch without the additional commit, and
>   merge the MR
> * Remove the ephemeral docker image from
> https://gitlab.freedesktop.org/evelikov/mesa/container_registry
>
> The CI pipeline information including the log of the job which generated
> the ephemeral docker image will remain accessible.
>
if I understand you correctly, the goal is to have this explicitly tested.
Since it's a no-op, one might as well keep the test (and resulting
extra image) downstream aka in my repo.

Sure I'll do that in a moment.

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

Re: [Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common

2019-07-31 Thread Emil Velikov
On Wed, 31 Jul 2019 at 14:16, Michel Dänzer  wrote:
>
> On 2019-07-31 3:04 p.m., Emil Velikov wrote:
> > From: Emil Velikov 
> >
> > Currently we use the python package to manage repositories. At the same
> > time we also do that by hand - since it's a trivial echo to a file.
> >
> > Stay consistent, remove the package and manage things manually.
> >
> > Cc: Eric Engestrom 
> > Signed-off-by: Emil Velikov 
> > ---
> >  .gitlab-ci/debian-install.sh | 11 +--
> >  1 file changed, 5 insertions(+), 6 deletions(-)
> >
> > diff --git a/.gitlab-ci/debian-install.sh b/.gitlab-ci/debian-install.sh
> > index 578074ddb87..719d7830018 100644
> > --- a/.gitlab-ci/debian-install.sh
> > +++ b/.gitlab-ci/debian-install.sh
> > @@ -16,12 +16,11 @@ apt-get install -y \
> >curl \
> >wget \
> >unzip \
> > -  gnupg \
> > -  software-properties-common
> > +  gnupg
> >
> >  curl -fsSL https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
> > -add-apt-repository "deb https://apt.llvm.org/stretch/ 
> > llvm-toolchain-stretch-7 main"
> > -add-apt-repository "deb https://apt.llvm.org/stretch/ 
> > llvm-toolchain-stretch-8 main"
> > +echo "deb [trusted=yes] https://apt.llvm.org/stretch/ 
> > llvm-toolchain-stretch-7 main" >/etc/apt/sources.list.d/llvm7.list
> > +echo "deb [trusted=yes] https://apt.llvm.org/stretch/ 
> > llvm-toolchain-stretch-8 main" >/etc/apt/sources.list.d/llvm8.list
> >
> >  sed -i -e 's/http:\/\/deb/https:\/\/deb/g' /etc/apt/sources.list
> >  echo 'deb https://deb.debian.org/debian stretch-backports main' 
> > >/etc/apt/sources.list.d/backports.list
> > @@ -46,8 +45,8 @@ apt-get install -y -t stretch-backports \
> >clang-8
> >
> >  # Install remaining packages from Debian buster to get newer versions
> > -add-apt-repository "deb https://deb.debian.org/debian/ buster main"
> > -add-apt-repository "deb https://deb.debian.org/debian/ buster-updates main"
> > +echo "deb https://deb.debian.org/debian/ buster main" 
> > >/etc/apt/sources.list.d/buster.list
> > +echo "deb https://deb.debian.org/debian/ buster-updates main" 
> > >/etc/apt/sources.list.d/buster-updates.list
> >  apt-get update
> >  apt-get install -y \
> >bzip2 \
> >
>
> This should be merged as part of an MR which requires the docker image
> to be re-generated for another reason, and thus bumps DEBIAN_TAG.
>
Since this is a non-functional change, I've explicitly omitted bumping
the DEBIAN_TAG.
Seemingly I forgot to mention it in the commit message though, oopsie.

Since the image will contain practically the same artefacts, is it
worth carving out 30 minutes (or so) from the runners?

> Reviewed-by: Michel Dänzer 
>

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

Re: [Mesa-dev] [PATCH] egl/drm: ensure the backing gbm is set before using it

2019-07-31 Thread Emil Velikov
On Fri, 5 Jul 2019 at 22:58, Eric Engestrom  wrote:
>
> On Friday, 2019-07-05 11:21:41 +0100, Emil Velikov wrote:
> > From: Emil Velikov 
> >
> > Currently, if we error out before gbm_dri is set (say due to a different
> > name of the backing GBM implementation, or otherwise) the tear down will
> > trigger a NULL ptr deref and crash out.
> >
> > Move the gbm_dri initialization as early as possible. To be on the extra
> > safe side add a NULL check in the teardown.
> >
> > Reported-by: Christian Gmeiner 
> > Cc: Christian Gmeiner 
> > Cc: Eric Engestrom 
> > Cc: mesa-sta...@lists.freedesktop.org
> > Signed-off-by: Emil Velikov 
> > ---
> > Supersedes https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1182
> > ---
> >  src/egl/drivers/dri2/platform_drm.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/egl/drivers/dri2/platform_drm.c 
> > b/src/egl/drivers/dri2/platform_drm.c
> > index a811834114d..3e2124ad39c 100644
> > --- a/src/egl/drivers/dri2/platform_drm.c
> > +++ b/src/egl/drivers/dri2/platform_drm.c
> > @@ -704,6 +704,7 @@ dri2_initialize_drm(_EGLDisplay *disp)
> >   goto cleanup;
> >}
> > }
> > +   dri2_dpy->gbm_dri = gbm_dri_device(gbm);
> >
> > if (strcmp(gbm_device_get_backend_name(gbm), "drm") != 0) {
> >err = "DRI2: gbm device using incorrect/incompatible backend";
> > @@ -718,7 +719,6 @@ dri2_initialize_drm(_EGLDisplay *disp)
> >
> > disp->Device = dev;
> >
> > -   dri2_dpy->gbm_dri = gbm_dri_device(gbm);
> > dri2_dpy->driver_name = strdup(dri2_dpy->gbm_dri->driver_name);
> >
> > dri2_dpy->dri_screen = dri2_dpy->gbm_dri->screen;
> > @@ -777,6 +777,6 @@ cleanup:
> >  void
> >  dri2_teardown_drm(struct dri2_egl_display *dri2_dpy)
> >  {
> > -   if (dri2_dpy->own_device)
> > +   if (dri2_dpy->own_device && dri2_dpy->gbm_dri)
>
> This would only be reachable by eglTerminate()ing an uninitialised
> display, which is explicitly invalid anyway.
>
> I think you can drop this check.
>
> The gbm_dri_device(gbm) move above is:
> Reviewed-by: Eric Engestrom 
>
Dropped the check in dri2_teardown_drm() and pushed to master.

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

[Mesa-dev] [PATCH] gitlab-ci: remove software-properties-common

2019-07-31 Thread Emil Velikov
From: Emil Velikov 

Currently we use the python package to manage repositories. At the same
time we also do that by hand - since it's a trivial echo to a file.

Stay consistent, remove the package and manage things manually.

Cc: Eric Engestrom 
Signed-off-by: Emil Velikov 
---
 .gitlab-ci/debian-install.sh | 11 +--
 1 file changed, 5 insertions(+), 6 deletions(-)

diff --git a/.gitlab-ci/debian-install.sh b/.gitlab-ci/debian-install.sh
index 578074ddb87..719d7830018 100644
--- a/.gitlab-ci/debian-install.sh
+++ b/.gitlab-ci/debian-install.sh
@@ -16,12 +16,11 @@ apt-get install -y \
   curl \
   wget \
   unzip \
-  gnupg \
-  software-properties-common
+  gnupg
 
 curl -fsSL https://apt.llvm.org/llvm-snapshot.gpg.key | apt-key add -
-add-apt-repository "deb https://apt.llvm.org/stretch/ llvm-toolchain-stretch-7 
main"
-add-apt-repository "deb https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 
main"
+echo "deb [trusted=yes] https://apt.llvm.org/stretch/ llvm-toolchain-stretch-7 
main" >/etc/apt/sources.list.d/llvm7.list
+echo "deb [trusted=yes] https://apt.llvm.org/stretch/ llvm-toolchain-stretch-8 
main" >/etc/apt/sources.list.d/llvm8.list
 
 sed -i -e 's/http:\/\/deb/https:\/\/deb/g' /etc/apt/sources.list
 echo 'deb https://deb.debian.org/debian stretch-backports main' 
>/etc/apt/sources.list.d/backports.list
@@ -46,8 +45,8 @@ apt-get install -y -t stretch-backports \
   clang-8
 
 # Install remaining packages from Debian buster to get newer versions
-add-apt-repository "deb https://deb.debian.org/debian/ buster main"
-add-apt-repository "deb https://deb.debian.org/debian/ buster-updates main"
+echo "deb https://deb.debian.org/debian/ buster main" 
>/etc/apt/sources.list.d/buster.list
+echo "deb https://deb.debian.org/debian/ buster-updates main" 
>/etc/apt/sources.list.d/buster-updates.list
 apt-get update
 apt-get install -y \
   bzip2 \
-- 
2.21.0

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

[Mesa-dev] Mesa 19.2.0 release plan

2019-07-31 Thread Emil Velikov
Hi all,

Here is the tentative release plan for 19.2.0.

As many of you are well aware, it's time to the next branch point.
The calendar is already updated, so these are the tentative dates:

 Aug 06 2019 - Feature freeze/Release candidate 1
 Aug 13 2019 - Release candidate 2
 Aug 20 2019 - Release candidate 3
 Aug 27 2019 - Release candidate 4/final release

This gives us around 1 week until the branch point.

Note: In the spirit of keeping things clearer and more transparent, we
will be keeping track of any features planned for the release in
Bugzilla [1].

Do add a separate "Depends on" for each work you have planned.
Alternatively you can reply to this email and I'll add them for you.

[1] https://bugs.freedesktop.org/show_bug.cgi?id=111265

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

Re: [Mesa-dev] [PATCH] mesa: Set minimum possible GLSL version

2019-07-05 Thread Emil Velikov
On Fri, 5 Jul 2019 at 05:59, Ian Romanick  wrote:
>
> From: Ian Romanick 
>
> Set the absolute minimum possible GLSL version.  API_OPENGL_CORE can
> mean an OpenGL 3.0 forward-compatible context, so that implies a minimum
> possible version of 1.30.  Otherwise, the minimum possible version 1.20.
> Since Mesa unconditionally advertises GL_ARB_shading_language_100 and
> GL_ARB_shader_objects, every driver has GLSL 1.20... even if they don't
> advertise any extensions to enable any shader stages (e.g.,
> GL_ARB_vertex_shader).
>
> Converts about 2,500 piglit tests from crash to skip on NV18.
Thanks Ian.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=109524
Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110955
Cc: mesa-sta...@lists.freedesktop.org
Reviewed-by: Emil Velikov 

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

[Mesa-dev] [PATCH] egl/drm: ensure the backing gbm is set before using it

2019-07-05 Thread Emil Velikov
From: Emil Velikov 

Currently, if we error out before gbm_dri is set (say due to a different
name of the backing GBM implementation, or otherwise) the tear down will
trigger a NULL ptr deref and crash out.

Move the gbm_dri initialization as early as possible. To be on the extra
safe side add a NULL check in the teardown.

Reported-by: Christian Gmeiner 
Cc: Christian Gmeiner 
Cc: Eric Engestrom 
Cc: mesa-sta...@lists.freedesktop.org
Signed-off-by: Emil Velikov 
---
Supersedes https://gitlab.freedesktop.org/mesa/mesa/merge_requests/1182
---
 src/egl/drivers/dri2/platform_drm.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index a811834114d..3e2124ad39c 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -704,6 +704,7 @@ dri2_initialize_drm(_EGLDisplay *disp)
  goto cleanup;
   }
}
+   dri2_dpy->gbm_dri = gbm_dri_device(gbm);
 
if (strcmp(gbm_device_get_backend_name(gbm), "drm") != 0) {
   err = "DRI2: gbm device using incorrect/incompatible backend";
@@ -718,7 +719,6 @@ dri2_initialize_drm(_EGLDisplay *disp)
 
disp->Device = dev;
 
-   dri2_dpy->gbm_dri = gbm_dri_device(gbm);
dri2_dpy->driver_name = strdup(dri2_dpy->gbm_dri->driver_name);
 
dri2_dpy->dri_screen = dri2_dpy->gbm_dri->screen;
@@ -777,6 +777,6 @@ cleanup:
 void
 dri2_teardown_drm(struct dri2_egl_display *dri2_dpy)
 {
-   if (dri2_dpy->own_device)
+   if (dri2_dpy->own_device && dri2_dpy->gbm_dri)
   gbm_device_destroy(_dpy->gbm_dri->base);
 }
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH kmscube] Search for a suitable config

2019-07-04 Thread Emil Velikov
On Wed, 3 Jul 2019 at 08:16, Drew DeVault  wrote:
>
> Instead of assuming the first will be suitable. kmscube fails to start
> for me without this change.

Yes please. Picking the first one is rarely the correct thing to do.
Especially when we have platform specific attributes which are exempt
from the sorting rules.

> ---
>  common.c | 52 +---
>  common.h |  1 +
>  2 files changed, 46 insertions(+), 7 deletions(-)
>
> diff --git a/common.c b/common.c
> index f9bd280..45c074d 100644
> --- a/common.c
> +++ b/common.c
> @@ -24,11 +24,47 @@
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>  #include 
>
>  #include "common.h"
>
> +static bool egl_get_config(EGLDisplay disp, EGLint *attribs,
Nit: "const EGLint *attribs" and keep the const notation in the parent function.

> +   EGLConfig *out, EGLint visual_id) {
Nit: { on the next line

> +   EGLint count = 0, matched = 0, ret;
> +
> +   ret = eglGetConfigs(disp, NULL, 0, );
> +   if (ret == EGL_FALSE || count == 0) {
> +   printf("eglGetConfigs returned no configs\n");
> +   return false;
> +   }
> +
> +   EGLConfig configs[count];
> +
VLAs are convenient, although they lead to messy binary, are not
supported on some platforms etc.
How about:

EGLConfig configs[128];
assert(count...)

> +   ret = eglChooseConfig(disp, attribs, configs, count, );
> +   if (ret == EGL_FALSE) {
> +   printf("eglChooseConfig failed\n");
> +   return false;
> +   }
> +
> +   for (int i = 0; i < matched; ++i) {
> +   EGLint visual;
> +   if (!eglGetConfigAttrib(disp, configs[i],
> +   EGL_NATIVE_VISUAL_ID, )) {
> +   continue;
> +   }
> +
> +   if (!visual_id || visual == visual_id) {
> +   *out = configs[i];
> +   return true;
> +   }
> +   }
> +
> +   printf("no valid egl config found\n");
> +   return false;
> +}
> +
>  struct gbm * init_gbm(int drm_fd, int w, int h)
>  {
>  struct gbm *gbm = calloc(1, sizeof (struct gbm));
> @@ -59,7 +95,7 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
> EGL_NONE
> };
>
> -   static const EGLint config_attribs[] = {
> +   static EGLint config_attribs[] = {
Nit: Keep the const.

> EGL_SURFACE_TYPE, EGL_WINDOW_BIT,
> EGL_RED_SIZE, 1,
> EGL_GREEN_SIZE, 1,
> @@ -81,9 +117,10 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
> get_proc(eglDestroySyncKHR);
> get_proc(eglWaitSyncKHR);
> get_proc(eglDupNativeFenceFDANDROID);
> +   get_proc(eglCreatePlatformWindowSurfaceEXT);
Move this and associated changes to a separate commit, preserving the
fallback to eglCreateWindowSurface().

>
> if (egl->eglGetPlatformDisplayEXT) {
> -   egl->display = 
> egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_KHR,
> +   egl->display = 
> egl->eglGetPlatformDisplayEXT(EGL_PLATFORM_GBM_MESA,
Out of curiosity: Both constants are numerically the same. Why the change?

> gbm->dev, NULL);
> } else {
> egl->display = eglGetDisplay((void *)gbm->dev);
> @@ -106,8 +143,9 @@ int init_egl(struct egl *egl, const struct gbm *gbm)
> return -1;
> }
>
> -   if (!eglChooseConfig(egl->display, config_attribs, >config, 1, 
> ) || n != 1) {
> -   printf("failed to choose config: %d\n", n);
> +   if (!egl_get_config(egl->display, config_attribs,
> +   >config, GBM_FORMAT_XRGB)) {
Let's use gbm.format instead of open-coding it.

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

Re: [Mesa-dev] Getting write permissions on the mesa repo to push panfrost stuff

2019-07-04 Thread Emil Velikov
On Thu, 4 Jul 2019 at 13:23, Boris Brezillon
 wrote:
>
> Hello,
>
> Alyssa recently proposed that I push my own panfrost-related
> submissions once they received proper review and are considered
> ready to merged (meaning that received enough A-b/R-b tags).
>
> In order to do that, I'd need to obtain write permissions on the git
> repo. Note that I already have an account on fd.o (my nick is
> bbrezillon). I guess Alyssa and/or Tomew will ack this request soon. Let
> me know if you need anything else from me.
>
You have 29 patches and are always welcome to feedback. With my mesa hat on:
Acked-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH 0/8] Android makefiles clean up

2019-07-02 Thread Emil Velikov
On Tue, 25 Jun 2019 at 11:08, Chih-Wei Huang  wrote:
>
> There are several issues in the current Android makefiles. Though they
> may not affect the functionalities, it's ugly and unprofessional.
>
> * Add unnecessary LOCAL_EXPORT_C_INCLUDE_DIRS
> * Not export necessary include paths
> * Add dummy libraries to LOCAL_WHOLE_STATIC_LIBRARIES
> * Use unnecessary dummy library to generate headers
> * Add include paths which could be imported automatically
>
> This is a series of patches to clean up these issues.
>
>
> Chih-Wei Huang (8):
>   android: anv: remove unused LOCAL_EXPORT_C_INCLUDE_DIRS
>   android: radv: remove unused LOCAL_EXPORT_C_INCLUDE_DIRS
>   android: anv: fix improper use of LOCAL_WHOLE_STATIC_LIBRARIES
>   android: radv: fix improper use of LOCAL_WHOLE_STATIC_LIBRARIES
>   android: vulkan/util: fix export path
>   android: anv: eliminate libmesa_anv_entrypoints
>   android: anv: import include path of libmesa_nir
>   android: radv: import include paths from used libraries
>
From a quick look the series looks great. For the lot:
Acked-by: Emil Velikov 

Mauro, feel free to merge this if you haven't already.
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] egl: Make EGL_EXT_device_drm optional.

2019-06-28 Thread Emil Velikov
On 2019/06/28, mathias.froehl...@gmx.net wrote:
> From: Mathias Fröhlich 
> 
> Hi,
> 
> Ilia mentioned that there are drm rendernode only drivers out there.
> To support an egl device on those platforms, make the EGL_EXT_device_drm
> device extension optional.
> 
Currently DRM core mandates that primary node is available, even for GPU
only devices.

A while back, we had a chat with kernel people why we do so. IIRC the
conclustion was that existing userspace will just work, since it already
must handle cases when you unplug your only monitor.

I think Ilia was having a pretty reasonable assumption, that's why I did
before digging deeper, as opposed to saying there _are_ such cases.

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

[Mesa-dev] [PATCH] ac: change ac_query_gpu_info() signatures

2019-06-21 Thread Emil Velikov
From: Emil Velikov 

Currently libdrm_amdgpu provides a typedef of the various handles. While
the goal was to make those opaque, it effectively became part of the API

To the best of my knowledge there are two ways to have opaque handles:
 - "typedef void *foo;" - rather messy IMHO
 - "stuct foo;" and use "struct foo *" through the API

In our case amdgpU_device_handle is used only internally, plus
respective code is not used or applicable for r300 and r600. Hence  we
copied the typedef.

Seemingly this will be a problem since a libdrm_amdgpu wants to change
the API, while not updating the code(?).

Either way, we can safely s/amdgpU_device_handle/void */ and carry on.

Cc: Michel Dänzer 
Signed-off-by: Emil Velikov 
---
 src/amd/common/ac_gpu_info.c | 3 ++-
 src/amd/common/ac_gpu_info.h | 4 +---
 2 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/src/amd/common/ac_gpu_info.c b/src/amd/common/ac_gpu_info.c
index db7f9e47ce1..8bd47cc26b3 100644
--- a/src/amd/common/ac_gpu_info.c
+++ b/src/amd/common/ac_gpu_info.c
@@ -92,7 +92,7 @@ static bool has_syncobj(int fd)
return value ? true : false;
 }
 
-bool ac_query_gpu_info(int fd, amdgpu_device_handle dev,
+bool ac_query_gpu_info(int fd, void *dev_p,
   struct radeon_info *info,
   struct amdgpu_gpu_info *amdinfo)
 {
@@ -104,6 +104,7 @@ bool ac_query_gpu_info(int fd, amdgpu_device_handle dev,
struct amdgpu_gds_resource_info gds = {};
uint32_t vce_version = 0, vce_feature = 0, uvd_version = 0, uvd_feature 
= 0;
int r, i, j;
+   amdgpu_device_handle dev = dev_p;
drmDevicePtr devinfo;
 
/* Get PCI info. */
diff --git a/src/amd/common/ac_gpu_info.h b/src/amd/common/ac_gpu_info.h
index 11fb77eee87..ba4940af142 100644
--- a/src/amd/common/ac_gpu_info.h
+++ b/src/amd/common/ac_gpu_info.h
@@ -35,8 +35,6 @@
 extern "C" {
 #endif
 
-/* Prior to C11 the following may trigger a typedef redeclaration warning */
-typedef struct amdgpu_device *amdgpu_device_handle;
 struct amdgpu_gpu_info;
 
 struct radeon_info {
@@ -147,7 +145,7 @@ struct radeon_info {
uint32_tcik_macrotile_mode_array[16];
 };
 
-bool ac_query_gpu_info(int fd, amdgpu_device_handle dev,
+bool ac_query_gpu_info(int fd, void *dev_p,
   struct radeon_info *info,
   struct amdgpu_gpu_info *amdinfo);
 
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH] egl: Don't add hardware device if there is no render node v2.

2019-06-18 Thread Emil Velikov
On Mon, 17 Jun 2019 at 19:16, Ilia Mirkin  wrote:
>
> On Mon, Jun 17, 2019 at 6:37 AM  wrote:
> >
> > From: Mathias Fröhlich 
> >
> >
> > Emil,
> >
> > that one probably matches your original intent then.
> >
> > please review
> > Thanks
> >
> > Mathias
> >
> >
> > Do not offer a hardware drm backed egl device if no render node
> > is available. The current implementation will fail on this
> > egl device. On top it issues a warning that is actually missleading.
> > There are finally more error paths that can fail on the way to a
> > hardware backed egl device. Fixing all of them would kind of require
> > opening the drm device and see if there is a usable driver associated
> > with the device. The taken approach avoids a full probe and fixes at
> > least this kind of problem on kvm virtualization hosts I observe here.
> >
> > Signed-off-by: Mathias Fröhlich 
> > ---
> >  src/egl/main/egldevice.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/egl/main/egldevice.c b/src/egl/main/egldevice.c
> > index 76b8960fa5b..99d8a6c1886 100644
> > --- a/src/egl/main/egldevice.c
> > +++ b/src/egl/main/egldevice.c
> > @@ -108,9 +108,9 @@ static int
> >  _eglAddDRMDevice(drmDevicePtr device, _EGLDevice **out_dev)
> >  {
> > _EGLDevice *dev;
> > +   const int wanted_nodes = 1 << DRM_NODE_RENDER | 1 << DRM_NODE_PRIMARY;
>
> Why check for primary? Why require both at this point? I guess the
> original intent was to require render *or* primary... but some devices
> will not have a primary node if they don't have any outputs...
>
My original intent was to require both, although I got it wrong.

Why we require both - simple the spec more or less mandates a card/primary node.
Since there an be other software running hence getting authenticated
will be a pita and primary nodes are not accessible* render node seems
like the only robust solution.

HTH
Emil
* There's a bit more to it:
Users not in the video group and w/o systemd - opening the primary
node will fail with EPERM.
Platforms running systemd - logind relaxes the primary node
permission, only when a local user has logged in.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 1/2] swrast: Avoid creating front buffers with __DRI_SWRAST_LOADER < 3.

2019-06-17 Thread Emil Velikov
On 2019/06/06, mathias.froehl...@gmx.net wrote:
> From: Mathias Fröhlich 
> 
> Align classic swrast with galliums software renderer with respect
> to front buffer creation.
> In case of front buffers swrast uses the __DRI_SWRAST_LOADER extensions
> getImage/putImage callbacks to keep the front buffer in sync between
> the X server and the mesa software rendering. Gallium sofware rasterizers
> only allocates front buffers if the __DRI_SWRAST_LOADER extension version
> is >= 3. By that the getImage/putImage NULL pointers of the recent
> EGL_platform_device, which announces __DRI_SWRAST_LOADER version 1, are
> never called and thus this combination works fine.
> The change makes classic swrast work together with EGL_platform_device
> by using the same __DRI_SWRAST_LOADER version check than egl.
> For reference see drisw_allocate_textures in
> src/gallium/state_trackers/dri/drisw.c.
> 
> Signed-off-by: Mathias Fröhlich 
> ---
>  src/mesa/drivers/dri/swrast/swrast.c | 5 -
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/src/mesa/drivers/dri/swrast/swrast.c 
> b/src/mesa/drivers/dri/swrast/swrast.c
> index 8e8d6bd628e..832a45f1d88 100644
> --- a/src/mesa/drivers/dri/swrast/swrast.c
> +++ b/src/mesa/drivers/dri/swrast/swrast.c
> @@ -572,7 +572,10 @@ dri_create_buffer(__DRIscreen * sPriv,
>  _mesa_initialize_window_framebuffer(fb, visual);
> 
>  /* add front renderbuffer */
> -frontrb = swrast_new_renderbuffer(visual, dPriv, GL_TRUE);
> +if (sPriv->swrast_loader->base.version >= 3)
> +   frontrb = swrast_new_renderbuffer(visual, dPriv, GL_TRUE);
> +else
> +   frontrb = swrast_new_renderbuffer(visual, dPriv, GL_FALSE);
>  _mesa_attach_and_own_rb(fb, BUFFER_FRONT_LEFT, >Base.Base);
> 
Doubt I'll be able to look at the classic swrast, anytime soon. Sorry :-\

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

Re: [Mesa-dev] [PATCH] egl: Don't add hardware device if there is no render node v2.

2019-06-17 Thread Emil Velikov
On 2019/06/17, mathias.froehl...@gmx.net wrote:
> From: Mathias Fröhlich 
> 
> 
> Emil,
> 
> that one probably matches your original intent then.
> 
> please review
> Thanks
> 
> Mathias
> 
> 
> Do not offer a hardware drm backed egl device if no render node
> is available. The current implementation will fail on this
> egl device. On top it issues a warning that is actually missleading.
> There are finally more error paths that can fail on the way to a
> hardware backed egl device. Fixing all of them would kind of require
> opening the drm device and see if there is a usable driver associated
> with the device. The taken approach avoids a full probe and fixes at
> least this kind of problem on kvm virtualization hosts I observe here.
> 
Thanks

Fixes: dbb4457d985 ("egl: add EGL_EXT_device_drm support")
Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH] intel_stub: Wrap fcntl64

2019-06-07 Thread Emil Velikov
On Fri, 7 Jun 2019 at 00:30, Ian Romanick  wrote:
>
> From: Ian Romanick 
>
> This makes the wrapper work on glibc 2.29 on Fedora 30.
> ---
AFAICT this patch is for shader-db and looks spot on.
Reviewed-by: Emil Velikov 

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

[Mesa-dev] [PATCH] docs: fixup 19.0.5 <> 19.0.6 confusion

2019-06-07 Thread Emil Velikov
From: Emil Velikov 

The title of the release notes says 19.0.5 while the rest of the file
(correctly) says 19.0.6

Cc: Dylan Baker 
Fixes: fe79d75ccf9 ("docs: Add relnotes for 19.0.6")
Signed-off-by: Emil Velikov 
---
Aside: Dylan seems like the relnotes were not chery-pick -x hence the
reference to the original commit is missing.

In other words - you'll have to pick this manually for 19.0 since the
nomination script will not detect this.
---
 docs/relnotes/19.0.6.html | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/relnotes/19.0.6.html b/docs/relnotes/19.0.6.html
index 33e217330e6..281b15fbc1d 100644
--- a/docs/relnotes/19.0.6.html
+++ b/docs/relnotes/19.0.6.html
@@ -14,7 +14,7 @@
 
 
 
-Mesa 19.0.5 Release Notes / May 21, 2019
+Mesa 19.0.6 Release Notes / May 21, 2019
 
 
 Mesa 19.0.6 is a bug fix release which fixes bugs found since the 19.0.5 
release.
-- 
2.20.1

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

[Mesa-dev] [PATCH v2 3/3] mapi: correctly handle the full offset table

2019-06-07 Thread Emil Velikov
From: Emil Velikov 

Earlier commit converted ES1 and ES2 to a new, much simpler, dispatch
generator. At the same time, GL/glapi and the driver side are still
using the old code.

There is a hidden ABI between GL*.so and glapi.so, former referencing
entry-points by offset in the _glapi_table. Hence earlier commit added
the full table of entry-points, alongside a marker for other cases like
indirect GL(X) and driver-size remapping.

Yet the patches did not handle things fully, thus it was possible to
get different interpretations of the dispatch table after the marker.

This commit fixes that adding an indicative error message to catch
future bugs.

While here correct the marker (MAX_OFFSETS) comment.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110302
Fixes: cf317bf0937 ("mapi: add all _glapi_table entrypoints tostatic_data.py")
Signed-off-by: Emil Velikov 
Reviewed-by: Dave Airlie 
---
No changes since v1.
---
 src/mapi/glapi/gen/gl_XML.py  | 8 ++--
 src/mapi/glapi/gen/static_data.py | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py
index 478f82ca314..2854a9a5688 100644
--- a/src/mapi/glapi/gen/gl_XML.py
+++ b/src/mapi/glapi/gen/gl_XML.py
@@ -49,7 +49,7 @@ def parse_GL_API( file_name, factory = None ):
 # that are not part of the ABI.
 
 for func in api.functionIterateByCategory():
-if func.assign_offset:
+if func.assign_offset and func.offset < 0:
 func.offset = api.next_offset;
 api.next_offset += 1
 
@@ -683,8 +683,12 @@ class gl_function( gl_item ):
 
 if name in static_data.offsets and static_data.offsets[name] <= 
static_data.MAX_OFFSETS:
 self.offset = static_data.offsets[name]
+elif name in static_data.offsets and static_data.offsets[name] > 
static_data.MAX_OFFSETS:
+self.offset = static_data.offsets[name]
+self.assign_offset = True
 else:
-self.offset = -1
+if self.exec_flavor != "skip":
+raise RuntimeError("Entry-point %s is missing offset in 
static_data.py. Add one at the bottom of the list." % (name))
 self.assign_offset = self.exec_flavor != "skip" or name in 
static_data.unused_functions
 
 if not self.name:
diff --git a/src/mapi/glapi/gen/static_data.py 
b/src/mapi/glapi/gen/static_data.py
index 7c27d89647d..5351bc16de5 100644
--- a/src/mapi/glapi/gen/static_data.py
+++ b/src/mapi/glapi/gen/static_data.py
@@ -29,7 +29,7 @@ MAX_OFFSETS = 407
 """Table of functions that have ABI-mandated offsets in the dispatch table.
 
 The first MAX_OFFSETS entries are required by indirect GLX. The rest are
-required to preserve the glapi <> drivers ABI. This is to be addressed shortly.
+required to preserve the glapi <> GL/GLES ABI. This is to be addressed shortly.
 
 This list will never change."""
 offsets = {
-- 
2.20.1

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

[Mesa-dev] [PATCH v2 2/3] mapi: add static_date offset to EXT_dsa

2019-06-07 Thread Emil Velikov
From: Emil Velikov 

As elaborated in the next patch, there is some hidden ABI that
effectively require most entrypoints to be listed in the file.

Cc: Marek Olšák 
Fixes: d2906293c43 ("mesa: EXT_dsa add selectorless matrix stackfunctions")
Signed-off-by: Emil Velikov 
---
New patch in v2. v1 was against older master, which lacked the above
commit.
---
 src/mapi/glapi/gen/static_data.py | 19 +++
 1 file changed, 19 insertions(+)

diff --git a/src/mapi/glapi/gen/static_data.py 
b/src/mapi/glapi/gen/static_data.py
index dc1926327da..7c27d89647d 100644
--- a/src/mapi/glapi/gen/static_data.py
+++ b/src/mapi/glapi/gen/static_data.py
@@ -1454,6 +1454,25 @@ offsets = {
 "BlendBarrier": 1418,
 "PrimitiveBoundingBox": 1419,
 "MaxShaderCompilerThreadsKHR": 1420,
+"MatrixLoadfEXT": 1421,
+"MatrixLoaddEXT": 1422,
+"MatrixMultfEXT": 1423,
+"MatrixMultdEXT": 1424,
+"MatrixLoadIdentityEXT": 1425,
+"MatrixRotatefEXT": 1426,
+"MatrixRotatedEXT": 1427,
+"MatrixScalefEXT": 1428,
+"MatrixScaledEXT": 1429,
+"MatrixTranslatefEXT": 1430,
+"MatrixTranslatedEXT": 1431,
+"MatrixOrthoEXT": 1432,
+"MatrixFrustumEXT": 1433,
+"MatrixPushEXT": 1434,
+"MatrixPopEXT": 1435,
+"MatrixLoadTransposefEXT": 1436,
+"MatrixLoadTransposedEXT": 1437,
+"MatrixMultTransposefEXT": 1438,
+"MatrixMultTransposedEXT": 1439,
 }
 
 functions = [
-- 
2.20.1

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

[Mesa-dev] [PATCH v2 1/3] mapi: add static_date offset to MaxShaderCompilerThreadsKHR

2019-06-07 Thread Emil Velikov
From: Emil Velikov 

As elaborated in the next patch, there is some hidden ABI that
effectively require most entrypoints to be listed in the file.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110302
Cc: Marek Olšák 
Fixes: c5c38e831ee ("mesa: implement ARB/KHR_parallel_shader_compile")
Signed-off-by: Emil Velikov 
Reviewed-by: Dave Airlie 
---
No changes since v1.
---
 src/mapi/glapi/gen/static_data.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mapi/glapi/gen/static_data.py 
b/src/mapi/glapi/gen/static_data.py
index bc49324348f..dc1926327da 100644
--- a/src/mapi/glapi/gen/static_data.py
+++ b/src/mapi/glapi/gen/static_data.py
@@ -1453,6 +1453,7 @@ offsets = {
 "TexParameterxv": 1417,
 "BlendBarrier": 1418,
 "PrimitiveBoundingBox": 1419,
+"MaxShaderCompilerThreadsKHR": 1420,
 }
 
 functions = [
-- 
2.20.1

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

Re: [Mesa-dev] [PATCH 2/2] egl: Don't add hardware device if there is no render node.

2019-06-07 Thread Emil Velikov
On Thu, 6 Jun 2019 at 12:10,  wrote:
>
> From: Mathias Fröhlich 
>
> Do not offer a hardware drm backed egl device if no render node
> is available.
As far as I can see current implementation does _not_ add the DRM
device if its missing render node (and a primary one).
Looking at the change below, it's effectively making the primary node optional.

Hence the comment does not alight with the code - old and new. Can you
elaborate?

I have not thought exactly how primary node-less DRM will work out
esp. since the EGL_EXT_device_drm extension explicitly mentions one.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 6/8] egl/dri: flesh out and use dri2_create_drawable()

2019-06-05 Thread Emil Velikov
On Mon, 3 Jun 2019 at 23:52, Marek Olšák  wrote:
>
> Would you please review this fixed version:
> https://cgit.freedesktop.org/~mareko/mesa/commit/?h=master=40e4702ef815410f74130f349e2b40cc0524e422
>
> It trivially solves the GBM crash by checking that gbm_surf != NULL before 
> using it.
>
Makes sense. Thanks for following up Marek.

Fwiw the patch is
Reviewed-by: Emil Velikov 

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

[Mesa-dev] [PATCH 1/2] mapi: add static_date offset to MaxShaderCompilerThreadsKHR

2019-06-05 Thread Emil Velikov
From: Emil Velikov 

As elaborated in the next patch, there is some hidden ABI that
effectively require most entrypoints to be listed in the file.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110302
Cc: Marek Olšák 
Fixes: c5c38e831ee ("mesa: implement ARB/KHR_parallel_shader_compile")
Signed-off-by: Emil Velikov 
---
 src/mapi/glapi/gen/static_data.py | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/mapi/glapi/gen/static_data.py 
b/src/mapi/glapi/gen/static_data.py
index bc49324348f..dc1926327da 100644
--- a/src/mapi/glapi/gen/static_data.py
+++ b/src/mapi/glapi/gen/static_data.py
@@ -1453,6 +1453,7 @@ offsets = {
 "TexParameterxv": 1417,
 "BlendBarrier": 1418,
 "PrimitiveBoundingBox": 1419,
+"MaxShaderCompilerThreadsKHR": 1420,
 }
 
 functions = [
-- 
2.20.1

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

[Mesa-dev] [PATCH 2/2] mapi: correctly handle the full offset table

2019-06-05 Thread Emil Velikov
From: Emil Velikov 

Earlier commit converted ES1 and ES2 to a new, much simpler, dispatch
generator. At the same time, GL/glapi and the driver side are still
using the old code.

There is a hidden ABI between GL*.so and glapi.so, former referencing
entry-points by offset in the _glapi_table. Hence earlier commit added
the full table of entry-points, alongside a marker for other cases like
indirect GL(X) and driver-size remapping.

Yet the patches did not handle things fully, thus it was possible to
get different interpretations of the dispatch table after the marker.

This commit fixes that adding an indicative error message to catch
future bugs.

While here correct the marker (MAX_OFFSETS) comment.

Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110302
Fixes: cf317bf0937 ("mapi: add all _glapi_table entrypoints tostatic_data.py")
Signed-off-by: Emil Velikov 
---
 src/mapi/glapi/gen/gl_XML.py  | 8 ++--
 src/mapi/glapi/gen/static_data.py | 2 +-
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/src/mapi/glapi/gen/gl_XML.py b/src/mapi/glapi/gen/gl_XML.py
index 478f82ca314..2854a9a5688 100644
--- a/src/mapi/glapi/gen/gl_XML.py
+++ b/src/mapi/glapi/gen/gl_XML.py
@@ -49,7 +49,7 @@ def parse_GL_API( file_name, factory = None ):
 # that are not part of the ABI.
 
 for func in api.functionIterateByCategory():
-if func.assign_offset:
+if func.assign_offset and func.offset < 0:
 func.offset = api.next_offset;
 api.next_offset += 1
 
@@ -683,8 +683,12 @@ class gl_function( gl_item ):
 
 if name in static_data.offsets and static_data.offsets[name] <= 
static_data.MAX_OFFSETS:
 self.offset = static_data.offsets[name]
+elif name in static_data.offsets and static_data.offsets[name] > 
static_data.MAX_OFFSETS:
+self.offset = static_data.offsets[name]
+self.assign_offset = True
 else:
-self.offset = -1
+if self.exec_flavor != "skip":
+raise RuntimeError("Entry-point %s is missing offset in 
static_data.py. Add one at the bottom of the list." % (name))
 self.assign_offset = self.exec_flavor != "skip" or name in 
static_data.unused_functions
 
 if not self.name:
diff --git a/src/mapi/glapi/gen/static_data.py 
b/src/mapi/glapi/gen/static_data.py
index dc1926327da..5044e0f78cf 100644
--- a/src/mapi/glapi/gen/static_data.py
+++ b/src/mapi/glapi/gen/static_data.py
@@ -29,7 +29,7 @@ MAX_OFFSETS = 407
 """Table of functions that have ABI-mandated offsets in the dispatch table.
 
 The first MAX_OFFSETS entries are required by indirect GLX. The rest are
-required to preserve the glapi <> drivers ABI. This is to be addressed shortly.
+required to preserve the glapi <> GL/GLES ABI. This is to be addressed shortly.
 
 This list will never change."""
 offsets = {
-- 
2.20.1

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

[Mesa-dev] [PATCH] svga: remove open-coded vmw_ioctl_surface_req()

2019-06-03 Thread Emil Velikov
From: Emil Velikov 

Cc: Brian Paul 
Cc: Jose Fonseca 
Cc: Roland Scheidegger 
Signed-off-by: Emil Velikov 
---
 src/gallium/winsys/svga/drm/vmw_screen_dri.c | 29 
 1 file changed, 6 insertions(+), 23 deletions(-)

diff --git a/src/gallium/winsys/svga/drm/vmw_screen_dri.c 
b/src/gallium/winsys/svga/drm/vmw_screen_dri.c
index a85ee18e45b..264c61a5f69 100644
--- a/src/gallium/winsys/svga/drm/vmw_screen_dri.c
+++ b/src/gallium/winsys/svga/drm/vmw_screen_dri.c
@@ -221,7 +221,7 @@ vmw_drm_surface_from_handle(struct svga_winsys_screen *sws,
 union drm_vmw_surface_reference_arg arg;
 struct drm_vmw_surface_arg *req = 
 struct drm_vmw_surface_create_req *rep = 
-uint32_t handle = 0;
+boolean needs_unref = FALSE;
 struct drm_vmw_size size;
 SVGA3dSize base_size;
 int ret;
@@ -233,28 +233,11 @@ vmw_drm_surface_from_handle(struct svga_winsys_screen 
*sws,
return NULL;
 }
 
-switch (whandle->type) {
-case WINSYS_HANDLE_TYPE_SHARED:
-case WINSYS_HANDLE_TYPE_KMS:
-   handle = whandle->handle;
-   break;
-case WINSYS_HANDLE_TYPE_FD:
-   ret = drmPrimeFDToHandle(vws->ioctl.drm_fd, whandle->handle,
-);
-   if (ret) {
- vmw_error("Failed to get handle from prime fd %d.\n",
-   (int) whandle->handle);
- return NULL;
-   }
-   break;
-default:
-   vmw_error("Attempt to import unsupported handle type %d.\n",
- whandle->type);
-   return NULL;
-}
-
 memset(, 0, sizeof(arg));
-req->sid = handle;
+ret = vmw_ioctl_surface_req(vws, whandle, req, _unref);
+if (ret)
+return NULL;
+
 rep->size_addr = (unsigned long)
 
 ret = drmCommandWriteRead(vws->ioctl.drm_fd, DRM_VMW_REF_SURFACE,
@@ -263,7 +246,7 @@ vmw_drm_surface_from_handle(struct svga_winsys_screen *sws,
 /*
  * Need to close the handle we got from prime.
  */
-if (whandle->type == WINSYS_HANDLE_TYPE_FD)
+if (needs_unref)
vmw_ioctl_surface_destroy(vws, handle);
 
 if (ret) {
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH] mesa: Prevent classic swrast crash on a surfaceless context v2.

2019-05-27 Thread Emil Velikov
On 2019/05/27, mathias.froehl...@gmx.net wrote:
> From: Mathias Fröhlich 
> 
> Hi Emil,
> 
> thanks for that hint to look at _mesa_get_incomplete_framebuffer.
> That one seems definitely more appropriate!
> 
> Though, I miss a bit the idea how I can create either a sensible
> helper function for that task or how I can create something above
> in the call stack to the MakeCurrent call that already catches
> this case. Since that incomplete framebuffer is a mesa side thing I
> cannot easily pull that above the __DriverAPIRec::MakeCurrent call.
> But really putting those hand full lines of code into a helper does
> as well not gain much. So I implemented that for the swrast case
> directly.
> 
> 
For the patch:
Reviewed-by: Emil Velikov 

Here is a quick and dirty example. Feel free to follow-up if you like
the idea.

Note: it also updates/considers the fact that only one of the two
drawables can be NULL. Technically a bugfix - not sure if many tests
flex that code-path.

diff --git a/src/mesa/drivers/dri/i915/intel_context.c 
b/src/mesa/drivers/dri/i915/intel_context.c
index aa3175816cf..035abe13ff8 100644
--- a/src/mesa/drivers/dri/i915/intel_context.c
+++ b/src/mesa/drivers/dri/i915/intel_context.c
@@ -617,45 +617,62 @@ intelUnbindContext(__DRIcontext * driContextPriv)
return true;
 }
 
+/* XXX: make me public - aka move to src/mesa/ or sr/mesa/main even */
+/* when TRUE is returned read/writeFb are set. on FALSE we should call 
make_current(null...) */
+bool
+make_current_setup(__DRIcontext * driContextPriv,
+ __DRIdrawable * driDrawPriv,
+ __DRIdrawable * driReadPriv,
+struct **gl_framebuffer readFb,
+struct **gl_framebuffer writeFb)
+{
+   if (!driContextPriv)
+  return false;
+
+   struct *gl_framebuffer incomplete = _mesa_get_incomplete_framebuffer();
+
+   if (driReadPriv == NULL) {
+  *readFb = incomplete;
+   } else {
+   *readFb = driReadPriv->driverPrivate;
+   driContextPriv->dri2.read_stamp = driReadPriv->dri2.stamp - 1;
+}
+
+   if (driDrawPriv == NULL) {
+   *writeFb = incomplete;
+   } else {
+   *writeFb = driDrawPriv->driverPrivate;
+   driContextPriv->dri2.draw_stamp = driDrawPriv->dri2.stamp - 1;
+   }
+
+   return true;
+}
+
 GLboolean
 intelMakeCurrent(__DRIcontext * driContextPriv,
  __DRIdrawable * driDrawPriv,
  __DRIdrawable * driReadPriv)
 {
struct intel_context *intel;
+   struct gl_framebuffer *fb, *readFb;
 
-   if (driContextPriv)
-  intel = (struct intel_context *) driContextPriv->driverPrivate;
-   else
-  intel = NULL;
-
-   if (driContextPriv) {
-  struct gl_context *ctx = >ctx;
-  struct gl_framebuffer *fb, *readFb;
-  
-  if (driDrawPriv == NULL && driReadPriv == NULL) {
-fb = _mesa_get_incomplete_framebuffer();
-readFb = _mesa_get_incomplete_framebuffer();
-  } else {
-fb = driDrawPriv->driverPrivate;
-readFb = driReadPriv->driverPrivate;
-driContextPriv->dri2.draw_stamp = driDrawPriv->dri2.stamp - 1;
-driContextPriv->dri2.read_stamp = driReadPriv->dri2.stamp - 1;
-  }
-
-  intel_prepare_render(intel);
-  _mesa_make_current(ctx, fb, readFb);
-
-  /* We do this in intel_prepare_render() too, but intel->ctx.DrawBuffer
-   * is NULL at that point.  We can't call _mesa_makecurrent()
-   * first, since we need the buffer size for the initial
-   * viewport.  So just call intel_draw_buffer() again here. */
-  intel_draw_buffer(ctx);
-   }
-   else {
+   if (!make_current_setup(driContextPriv, driDrawPriv, driReadPriv, , 
)) {
   _mesa_make_current(NULL, NULL, NULL);
+  return true;
}
 
+   intel = (struct intel_context *) driContextPriv->driverPrivate;
+   struct gl_context *ctx = >ctx;
+
+   intel_prepare_render(intel);
+   _mesa_make_current(ctx, fb, readFb);
+
+   /* We do this in intel_prepare_render() too, but intel->ctx.DrawBuffer
+* is NULL at that point.  We can't call _mesa_makecurrent()
+* first, since we need the buffer size for the initial
+* viewport.  So just call intel_draw_buffer() again here. */
+   intel_draw_buffer(ctx);
+
return true;
 }
 

> So, to mention, I sent that change including our egl device code
> with some unrelated egl device fixes through intels CI and did not
> get regressions.
>
Thanks, having a look. Too many fires going at the same time :-\
 
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 7/8] egl: keep the software device at the end of the list

2019-05-16 Thread Emil Velikov
From: Emil Velikov 

By default, the user is likely to pick the first device so it should
not be the least performant (aka software) one.

v2: Drop odd comment (Marek)

Suggested-by: Marek Olšák 
Reviewed-by: Mathias Fröhlich  (v1)
Reviewed-by: Marek Olšák  (v1)
Signed-off-by: Emil Velikov 
---
 src/egl/main/egldevice.c | 14 +-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/egl/main/egldevice.c b/src/egl/main/egldevice.c
index c5c9a21273a..82af1f47fed 100644
--- a/src/egl/main/egldevice.c
+++ b/src/egl/main/egldevice.c
@@ -293,13 +293,25 @@ _eglQueryDevicesEXT(EGLint max_devices,
   goto out;
}
 
+   /* Push the first device (the software one) to the end of the list.
+* Sending it to the user only if they've requested the full list.
+*
+* By default, the user is likely to pick the first device so having the
+* software (aka least performant) one is not a good idea.
+*/
*num_devices = MIN2(num_devs, max_devices);
 
-   for (i = 0, dev = devs; i < *num_devices; i++) {
+   for (i = 0, dev = devs->Next; dev && i < max_devices; i++) {
   devices[i] = dev;
   dev = dev->Next;
}
 
+   /* User requested the full device list, add the sofware device. */
+   if (max_devices >= num_devs) {
+  assert(_eglDeviceSupports(devs, _EGL_DEVICE_SOFTWARE));
+  devices[num_devs - 1] = devs;
+   }
+
 out:
mtx_unlock(_eglGlobal.Mutex);
 
-- 
2.21.0

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

[Mesa-dev] [PATCH 8/8] egl: add EGL_platform_device support

2019-05-16 Thread Emil Velikov
This new 'platform' is added by default with no guards.

It is effectively a copy of the surfaceless one, with updated function
names and brand new probe function.

Due to the reuse, some of the ifdef HAVE_SURFACELESS_PLATFORM guards
have been dropped.

A worthy mention are the changes in _egFindDisplay, since the original
and dup'd fd are required, we make use of the plat_opt argument.

Note that no hacks for eglGetDisplay are added - the API works only with
the eglGetPlatformDisplay* API.

v2:
 - s/_eglCompareDeviceDisplay/_eglSameDeviceDisplay/ (Eric)
 - let ^^ return bool (Eric)
 - fixup meson build, move files() further up (Eric)
 - copy from plat. surfaceless w/o the visual cleanups
 - close and free when destroying the dpy
 - sprinkle a few _eglDeviceSupports
 - split fd handling into separate function
 - use directly the render node if no FD is given (Mathias)

v3:
 - s/dpy/disp/g
 - drop swap_buffers* callbacks
 - drop loader_set_logger()
 - drop local define
 - re-introduce _eglGetDRMDeviceRenderNode()
 - EGL_WARN on ForceSoftware with HW device - continue using the HW device
 - bail out for "EGL_MESA_device_software" until it's fixed
 - wire-up the Android build

v4:
 - use new style _eglFindDisplay()
 - split hw vs sw code paths
 - don't close the internal fd (already handled in FiniDisplay())
 - make swrast work (bit hacky bit will do for now)
 - Android for real, drop autotools
 - Correct HW + LIBGL_ALWAYS_SOFTWARE check
 - use the dri2_create_drawable() helper

v5:
 - enhance comment around fd checks (Mathias)
 - rebase for dri2_init_surface() changes

Cc: Mathias Fröhlich 
Acked-by: Marek Olšák  (v4)
Signed-off-by: Emil Velikov 
---
 src/egl/Android.mk |   1 +
 src/egl/drivers/dri2/egl_dri2.c|   3 +
 src/egl/drivers/dri2/egl_dri2.h|  13 +-
 src/egl/drivers/dri2/platform_device.c | 435 +
 src/egl/main/eglapi.c  |  13 +-
 src/egl/main/egldevice.c   |  16 +
 src/egl/main/egldevice.h   |   3 +
 src/egl/main/egldisplay.c  |  67 
 src/egl/main/egldisplay.h  |   7 +-
 src/egl/main/eglglobals.c  |   1 +
 src/egl/meson.build|   1 +
 11 files changed, 549 insertions(+), 11 deletions(-)
 create mode 100644 src/egl/drivers/dri2/platform_device.c

diff --git a/src/egl/Android.mk b/src/egl/Android.mk
index a9319f56ae7..01c33298ee7 100644
--- a/src/egl/Android.mk
+++ b/src/egl/Android.mk
@@ -36,6 +36,7 @@ include $(CLEAR_VARS)
 LOCAL_SRC_FILES := \
$(LIBEGL_C_FILES) \
$(dri2_backend_core_FILES) \
+   drivers/dri2/platform_device.c \
drivers/dri2/platform_android.c
 
 LOCAL_CFLAGS := \
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 17a0176b646..7427f199e8a 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -874,6 +874,9 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
case _EGL_PLATFORM_SURFACELESS:
   ret = dri2_initialize_surfaceless(drv, disp);
   break;
+   case _EGL_PLATFORM_DEVICE:
+  ret = dri2_initialize_device(drv, disp);
+  break;
case _EGL_PLATFORM_X11:
   ret = dri2_initialize_x11(drv, disp);
   break;
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index bcea3ab..e122f31a539 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -331,10 +331,10 @@ struct dri2_egl_surface
} color_buffers[3], *back;
 #endif
 
-#if defined(HAVE_SURFACELESS_PLATFORM)
-  __DRIimage   *front;
-  unsigned int visual;
-#endif
+   /* surfaceless and device */
+   __DRIimage   *front;
+   unsigned int visual;
+
int out_fence_fd;
EGLBoolean enable_out_fence;
 };
@@ -492,6 +492,11 @@ dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay 
*disp)
 }
 #endif
 
+EGLBoolean
+dri2_initialize_device(_EGLDriver *drv, _EGLDisplay *disp);
+static inline void
+dri2_teardown_device(struct dri2_egl_display *dri2_dpy) { /* noop */ }
+
 void
 dri2_flush_drawable_for_swapbuffers(_EGLDisplay *disp, _EGLSurface *draw);
 
diff --git a/src/egl/drivers/dri2/platform_device.c 
b/src/egl/drivers/dri2/platform_device.c
new file mode 100644
index 000..81725ae7de9
--- /dev/null
+++ b/src/egl/drivers/dri2/platform_device.c
@@ -0,0 +1,435 @@
+/*
+ * Mesa 3-D graphics library
+ *
+ * Copyright 2018 Collabora
+ *
+ * Based on platform_surfaceless, which has:
+ *
+ * Copyright (c) 2014 The Chromium OS Authors.
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to pe

[Mesa-dev] [PATCH 6/8] egl/dri: flesh out and use dri2_create_drawable()

2019-05-16 Thread Emil Velikov
From: Emil Velikov 

Wrap the loader->createNewDrawable() dance into a helper and use it
throughout the codebase.

This addresses a cases like surfaceless (SL) on swrast (SL on kms_swrast
is fine) where we'd attempt using the wrong driver and crash out.

v2: fixup quirky GBM (Mathias)

Cc: mesa-sta...@lists.freedesktop.org
Cc: Mathias Fröhlich 
Reviewed-by: Mathias Fröhlich  (v1)
Reviewed-by: Marek Olšák  (v1)
Signed-off-by: Emil Velikov 
---
 src/egl/drivers/dri2/egl_dri2.c | 39 +
 src/egl/drivers/dri2/egl_dri2.h |  5 +++
 src/egl/drivers/dri2/platform_android.c | 12 +--
 src/egl/drivers/dri2/platform_drm.c | 17 +
 src/egl/drivers/dri2/platform_surfaceless.c |  7 +---
 src/egl/drivers/dri2/platform_wayland.c | 14 +---
 src/egl/drivers/dri2/platform_x11.c | 15 +---
 7 files changed, 49 insertions(+), 60 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 09d6315b19e..17a0176b646 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1425,6 +1425,45 @@ dri2_surf_update_fence_fd(_EGLContext *ctx,
dri2_surface_set_out_fence_fd(surf, fence_fd);
 }
 
+static inline _EGLDisplay *
+dri2_display_to_egl_display(struct dri2_egl_display *dri2_dpy)
+{
+   const size_t offset = offsetof(_EGLDisplay, DriverData);
+   return (_EGLDisplay *)(((void*) dri2_dpy) - offset);
+}
+
+EGLBoolean
+dri2_create_drawable(struct dri2_egl_display *dri2_dpy,
+ const __DRIconfig *config,
+ struct dri2_egl_surface *dri2_surf)
+{
+   __DRIcreateNewDrawableFunc createNewDrawable;
+   _EGLDisplay *disp = dri2_display_to_egl_display(dri2_dpy);
+   void *loaderPrivate = dri2_surf;
+
+   if (dri2_dpy->image_driver)
+  createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
+   else if (dri2_dpy->dri2)
+  createNewDrawable = dri2_dpy->dri2->createNewDrawable;
+   else if (dri2_dpy->swrast)
+  createNewDrawable = dri2_dpy->swrast->createNewDrawable;
+   else
+  return _eglError(EGL_BAD_ALLOC, "no createNewDrawable");
+
+   /* As always gbm is a bit special.. */
+#ifdef HAVE_DRM_PLATFORM
+   if (disp->Platform == _EGL_PLATFORM_DRM)
+  loaderPrivate = dri2_surf->gbm_surf;
+#endif
+
+   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen,
+  config, loaderPrivate);
+   if (dri2_surf->dri_drawable == NULL)
+  return _eglError(EGL_BAD_ALLOC, "createNewDrawable");
+
+   return EGL_TRUE;
+}
+
 /**
  * Called via eglMakeCurrent(), drv->API.MakeCurrent().
  */
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index 4918517b572..bcea3ab 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -541,6 +541,11 @@ dri2_init_surface(_EGLSurface *surf, _EGLDisplay *disp, 
EGLint type,
 void
 dri2_fini_surface(_EGLSurface *surf);
 
+EGLBoolean
+dri2_create_drawable(struct dri2_egl_display *dri2_dpy,
+ const __DRIconfig *config,
+ struct dri2_egl_surface *dri2_surf);
+
 static inline uint64_t
 combine_u32_into_u64(uint32_t hi, uint32_t lo)
 {
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index fece214d939..e7c7df7 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -341,7 +341,6 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
_EGLConfig *conf, void *native_window,
const EGLint *attrib_list)
 {
-   __DRIcreateNewDrawableFunc createNewDrawable;
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
struct dri2_egl_surface *dri2_surf;
@@ -386,17 +385,8 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
   goto cleanup_surface;
}
 
-   if (dri2_dpy->image_driver)
-  createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
-   else
-  createNewDrawable = dri2_dpy->dri2->createNewDrawable;
-
-   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen, config,
-  dri2_surf);
-   if (dri2_surf->dri_drawable == NULL) {
-  _eglError(EGL_BAD_ALLOC, "createNewDrawable");
+   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
   goto cleanup_surface;
-   }
 
if (window) {
   window->common.incRef(>common);
diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index 7d916fe92b7..c59b9341d87 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -171,23 +171,8 @@ dri2_drm_create_window_surface(_EGLDriver *drv, 
_EGLDisplay *disp,

[Mesa-dev] [PATCH 2/8] egl: handle the full attrib list in display::options

2019-05-16 Thread Emil Velikov
From: Adam Jackson 

Earlier spec is vague, although EGL 1.5 makes it clear:

 Multiple calls made to eglGetPlatformDisplay with the same
 parameters will return the same EGLDisplay handle.

With this commit we store and compare the full attrib list.

v2 (Emil):
 - Split into separate patches
 - Use EGLBoolean over int masked as such
 - Don't return free'd pointed on calloc failure

Reviewed-by: Mathias Fröhlich 
Reviewed-by: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/main/eglapi.c |  2 +-
 src/egl/main/egldisplay.c | 61 +--
 src/egl/main/egldisplay.h |  3 +-
 3 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index fb26c8a0888..05c529ba0a7 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -373,7 +373,7 @@ eglGetDisplay(EGLNativeDisplayType nativeDisplay)
native_display_ptr = (void*) nativeDisplay;
 
plat = _eglGetNativePlatform(native_display_ptr);
-   disp = _eglFindDisplay(plat, native_display_ptr);
+   disp = _eglFindDisplay(plat, native_display_ptr, NULL);
return _eglGetDisplayHandle(disp);
 }
 
diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index ba5f84510fe..9a2ac48e8bc 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -202,20 +202,49 @@ _eglFiniDisplay(void)
  }
   }
 
+  free(disp->Options.Attribs);
   free(disp);
}
_eglGlobal.DisplayList = NULL;
 }
 
+static EGLBoolean
+_eglSameAttribs(const EGLAttrib *a, const EGLAttrib *b)
+{
+   size_t na = _eglNumAttribs(a);
+   size_t nb = _eglNumAttribs(b);
+
+   /* different numbers of attributes must be different */
+   if (na != nb)
+  return EGL_FALSE;
+
+   /* both lists NULL are the same */
+   if (!a && !b)
+  return EGL_TRUE;
+
+   /* otherwise, compare the lists */
+   return memcmp(a, b, na) == 0 ? EGL_TRUE : EGL_FALSE;
+}
 
 /**
  * Find the display corresponding to the specified native display, or create a
- * new one.
+ * new one. EGL 1.5 says:
+ *
+ * Multiple calls made to eglGetPlatformDisplay with the same parameters
+ * will return the same EGLDisplay handle.
+ *
+ * We read this extremely strictly, and treat a call with NULL attribs as
+ * different from a call with attribs only equal to { EGL_NONE }. Similarly
+ * we do not sort the attribute list, so even if all attribute _values_ are
+ * identical, different attribute orders will be considered different
+ * parameters.
  */
 _EGLDisplay *
-_eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
+_eglFindDisplay(_EGLPlatformType plat, void *plat_dpy,
+const EGLAttrib *attrib_list)
 {
_EGLDisplay *disp;
+   size_t num_attribs;
 
if (plat == _EGL_INVALID_PLATFORM)
   return NULL;
@@ -225,7 +254,8 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
/* search the display list first */
disp = _eglGlobal.DisplayList;
while (disp) {
-  if (disp->Platform == plat && disp->PlatformDisplay == plat_dpy)
+  if (disp->Platform == plat && disp->PlatformDisplay == plat_dpy &&
+  _eglSameAttribs(disp->Options.Attribs, attrib_list))
  break;
   disp = disp->Next;
}
@@ -237,13 +267,24 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
  mtx_init(>Mutex, mtx_plain);
  disp->Platform = plat;
  disp->PlatformDisplay = plat_dpy;
-
- /* add to the display list */ 
+ num_attribs = _eglNumAttribs(attrib_list);
+ if (num_attribs) {
+disp->Options.Attribs = calloc(num_attribs, sizeof(EGLAttrib));
+if (!disp->Options.Attribs) {
+   free(disp);
+   disp = NULL;
+   goto out;
+}
+memcpy(disp->Options.Attribs, attrib_list,
+   num_attribs * sizeof(EGLAttrib));
+ }
+ /* add to the display list */
  disp->Next = _eglGlobal.DisplayList;
  _eglGlobal.DisplayList = disp;
   }
}
 
+out:
mtx_unlock(_eglGlobal.Mutex);
 
return disp;
@@ -477,7 +518,8 @@ _eglGetX11Display(Display *native_display,
   const EGLAttrib *attrib_list)
 {
_EGLDisplay *display = _eglFindDisplay(_EGL_PLATFORM_X11,
-  native_display);
+  native_display,
+  attrib_list);
 
if (!display) {
   _eglError(EGL_BAD_ALLOC, "eglGetPlatformDisplay");
@@ -503,7 +545,7 @@ _eglGetGbmDisplay(struct gbm_device *native_display,
   return NULL;
}
 
-   return _eglFindDisplay(_EGL_PLATFORM_DRM, native_display);
+   return _eglFindDisplay(_EGL_PLATFORM_DRM, native_display, attrib_list);
 }
 #endif /* HAVE_DRM_PLATFORM */
 
@@ -518,7 +560,7 @@ _eglGetWaylandDisplay(struct wl_display *native_display,
   return NULL;
 

[Mesa-dev] [PATCH 1/8] egl: flesh out a _eglNumAttribs() helper

2019-05-16 Thread Emil Velikov
From: Emil Velikov 

Reviewed-by: Mathias Fröhlich 
Reviewed-by: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/main/eglapi.c | 12 +++-
 src/egl/main/egldisplay.h | 13 +
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 61a3220705d..fb26c8a0888 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -340,22 +340,16 @@ _eglConvertIntsToAttribs(const EGLint *int_list, 
EGLAttrib **out_attrib_list)
 static EGLint *
 _eglConvertAttribsToInt(const EGLAttrib *attr_list)
 {
+   size_t size = _eglNumAttribs(attr_list);
EGLint *int_attribs = NULL;
 
/* Convert attributes from EGLAttrib[] to EGLint[] */
-   if (attr_list) {
-  int i, size = 0;
-
-  while (attr_list[size] != EGL_NONE)
- size += 2;
-
-  size += 1; /* add space for EGL_NONE */
-
+   if (size) {
   int_attribs = calloc(size, sizeof(int_attribs[0]));
   if (!int_attribs)
  return NULL;
 
-  for (i = 0; i < size; i++)
+  for (size_t i = 0; i < size; i++)
  int_attribs[i] = attr_list[i];
}
return int_attribs;
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index cfd0ff66d64..f37b633b466 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -274,6 +274,19 @@ _eglIsResourceLinked(_EGLResource *res)
return res->IsLinked;
 }
 
+static inline size_t
+_eglNumAttribs(const EGLAttrib *attribs)
+{
+   size_t len = 0;
+
+   if (attribs) {
+  while (attribs[len] != EGL_NONE)
+ len += 2;
+  len++;
+   }
+   return len;
+}
+
 #ifdef HAVE_X11_PLATFORM
 _EGLDisplay*
 _eglGetX11Display(Display *native_display, const EGLAttrib *attrib_list);
-- 
2.21.0

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

[Mesa-dev] [PATCH 5/8] egl: fold X11 attrib handling like other platforms

2019-05-16 Thread Emil Velikov
From: Emil Velikov 

Since we no longer need special handling for X11, refactor the code to
follow the style used by all other platforms.

Reviewed-by: Mathias Fröhlich 
Reviewed-by: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/main/egldisplay.c | 45 ++-
 1 file changed, 11 insertions(+), 34 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 418ab0ec9b8..422b473844e 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -487,45 +487,22 @@ _eglUnlinkResource(_EGLResource *res, _EGLResourceType 
type)
 }
 
 #ifdef HAVE_X11_PLATFORM
-static EGLBoolean
-_eglParseX11DisplayAttribList(_EGLDisplay *display,
-  const EGLAttrib *attrib_list)
-{
-   int i;
-
-   if (attrib_list == NULL) {
-  return EGL_TRUE;
-   }
-
-   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
-* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
-*/
-   for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
-  if (attrib_list[i] != EGL_PLATFORM_X11_SCREEN_EXT)
- return _eglError(EGL_BAD_ATTRIBUTE, "eglGetPlatformDisplay");
-   }
-
-   return EGL_TRUE;
-}
-
 _EGLDisplay*
 _eglGetX11Display(Display *native_display,
   const EGLAttrib *attrib_list)
 {
-   _EGLDisplay *display = _eglFindDisplay(_EGL_PLATFORM_X11,
-  native_display,
-  attrib_list);
-
-   if (!display) {
-  _eglError(EGL_BAD_ALLOC, "eglGetPlatformDisplay");
-  return NULL;
-   }
-
-   if (!_eglParseX11DisplayAttribList(display, attrib_list)) {
-  return NULL;
+   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
+* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
+*/
+   if (attrib_list != NULL) {
+  for (int i = 0; attrib_list[i] != EGL_NONE; i += 2) {
+ if (attrib_list[i] != EGL_PLATFORM_X11_SCREEN_EXT) {
+_eglError(EGL_BAD_ATTRIBUTE, "eglGetPlatformDisplay");
+return NULL;
+ }
+  }
}
-
-   return display;
+   return _eglFindDisplay(_EGL_PLATFORM_X11, native_display, attrib_list);
 }
 #endif /* HAVE_X11_PLATFORM */
 
-- 
2.21.0

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

[Mesa-dev] [PATCH 4/8] egl: remove Options::Platform handling

2019-05-16 Thread Emil Velikov
From: Adam Jackson 

The full set of attributes is already handled with previous patches.
Thus all this is not dead code.

v2 (Emil) - split from a larger patch.

Reviewed-by: Mathias Fröhlich 
Reviewed-by: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/main/egldisplay.c | 13 -
 src/egl/main/egldisplay.h |  1 -
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 9a2ac48e8bc..418ab0ec9b8 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -497,17 +497,12 @@ _eglParseX11DisplayAttribList(_EGLDisplay *display,
   return EGL_TRUE;
}
 
+   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
+* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
+*/
for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
-  EGLAttrib attrib = attrib_list[i];
-  EGLAttrib value = attrib_list[i + 1];
-
-  /* EGL_EXT_platform_x11 recognizes exactly one attribute,
-   * EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
-   */
-  if (attrib != EGL_PLATFORM_X11_SCREEN_EXT)
+  if (attrib_list[i] != EGL_PLATFORM_X11_SCREEN_EXT)
  return _eglError(EGL_BAD_ATTRIBUTE, "eglGetPlatformDisplay");
-
-  display->Options.Platform = (void *)(uintptr_t)value;
}
 
return EGL_TRUE;
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index 2c9fd6c3399..369ab31187f 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -167,7 +167,6 @@ struct _egl_display
/* options that affect how the driver initializes the display */
struct {
   EGLBoolean ForceSoftware; /**< Use software path only */
-  void *Platform; /**< Platform-specific options */
   EGLAttrib *Attribs; /**< Platform-specific options */
} Options;
 
-- 
2.21.0

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

[Mesa-dev] [PATCH 3/8] egl/x11: pick the user requested screen

2019-05-16 Thread Emil Velikov
From: Adam Jackson 

At the moment the user will pass the screen number via attribs, yet we
would throw that away. Reason being that the int *screen passed to
xcb_connect() is output only.

v2 (Emil):
 - split from a larger patch
 - use xcb_connect() returned screen, as fallback
 - use helper function only as needed

Reviewed-by: Mathias Fröhlich 
Reviewed-by: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/drivers/dri2/platform_x11.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index 1d5efc0126d..538cffd6a76 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1277,21 +1277,34 @@ static const __DRIextension *swrast_loader_extensions[] 
= {
NULL,
 };
 
+static int
+dri2_find_screen_for_display(const _EGLDisplay *disp, int fallback_screen)
+{
+   const EGLAttrib *attr;
+
+   for (attr = disp->Options.Attribs; attr; attr += 2) {
+  if (attr[0] == EGL_PLATFORM_X11_SCREEN_EXT)
+ return attr[1];
+   }
+
+   return fallback_screen;
+}
+
 static EGLBoolean
 dri2_get_xcb_connection(_EGLDriver *drv, _EGLDisplay *disp,
 struct dri2_egl_display *dri2_dpy)
 {
xcb_screen_iterator_t s;
-   int screen = (uintptr_t)disp->Options.Platform;
+   int screen;
const char *msg;
 
disp->DriverData = (void *) dri2_dpy;
if (disp->PlatformDisplay == NULL) {
   dri2_dpy->conn = xcb_connect(NULL, );
   dri2_dpy->own_device = true;
+  screen = dri2_find_screen_for_display(disp, screen);
} else {
   Display *dpy = disp->PlatformDisplay;
-
   dri2_dpy->conn = XGetXCBConnection(dpy);
   screen = DefaultScreen(dpy);
}
-- 
2.21.0

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

Re: [Mesa-dev] Move adriconf to mesa repositories

2019-05-16 Thread Emil Velikov
On Thu, 16 May 2019 at 10:59, Jean Hertel  wrote:
>
> On 8th May you Wrote:
> >Hi Emil,
> >
> >>This is the tricky part - wish I could find my notes they have better
> >>brain-dump.
> >>It's OK to have the library as both front (config tool) and backend
> >>(used by mesa) although:
> >> - special care on splitting and annotating the API is needed
> >> - handling this "extra" dependency would be fiddly for slower moving 
> >> distros
> >>
> >I'm not sure I get the whole picture of what you are suggesting, so I put 
> >some ideas on how I think such API would work [here][1].
> >
> >>> What about the current configuration files? Do you think there is a 
> >>> better way to handle them?
> >>> They are for in a xml format, which is far from optimal.
> >>>
> >>What seems to be the problem with XML? The files are meant to be
> >>read/written to $app.
> >>
> >
> >I dislike XML because it is too ugly. Something more natural and easy to 
> >read/write would be more interesting.
> >Like a YML format. Ideally I would like to use JSON, but then it becomes a 
> >lot harder for people to write this by hand in case the GUI tools don't 
> >offer >the options they want/need.
> >Of course this is just my point of view, and not necessarily a real issue.
> >
> >Kind Regards,
> >Jean Hertel
> >
> >[1]: https://github.com/jlHertel/libdriconfig/blob/master/USAGE.md
>
> Hey Emil,
> Did you get a chance to look in my proposed way of building this new API?
>
> Some comments would be welcome.
> Also,  CC'ing Rob, as I think he would also have interest in this.
>
Had a quick look and noticed a few architectural nits.

I am a bit split between opaque structs vs not - slightly leaning
towards the former.
Reason being is that doing internal re-design becomes much easier and
we're baking less ABI into the users.

The following seems reasonable flow:
 - enumerate devices
   - query specifics - pci id/platform, memory, driver (kernel and/or
mesa), supported APIs (GLX,EGL,GBM, GL,GLES, NINE, VDPAU/OMX? Vulkan)
   - set specifics - mesa driver at least (think amdgpu vs amdgpu-pro
on same kernel module)
   [backend specific]
   - create device - sets the details ... APIs will be a bit fiddly
   - delete device - do we really need it? figure out the details

 - enumerate_apps/profiles (device) - there's special handle for the
global/default profile
   - query - name, executable, options
   - set
   [not backend specific]
   - create/delete
   - create_from_{global,other_profile)

- enumerate_options(device, API_MASK /* say GL_THREAD is GLX|EGL */, profile )
   - query_option - type/value
   - set_option - type(?)/value
   [backend specific]
   - create/delete


The tricky part wrt the fronend/backend mentioned earlier is that we
don't want !misc application to say "hey device/driver X can support
feature Y, on API Z".
Only the backend (mesa) is allowed to do that ;-)

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

Re: [Mesa-dev] [RFC 0/2] Alternate default config mechanism

2019-05-15 Thread Emil Velikov
Hi all,

On Tue, 14 May 2019 at 08:18, Tapani Pälli  wrote:
>
>
> On 5/13/19 6:52 PM, Haehnle, Nicolai wrote:
> > This approach seems entirely incompatible with si_debug_options.h, and
> > will be an absolute maintenance nightmare going forward for adding /
> > removing options, because you're introducing a second location where
> > options are defined.
> >
> > Quite frankly, this seems like a terrible idea as-is.
> >
> > If you really can't use XML for whatever reason, then please find some
> > way of deriving both the tables here and the XML from the same single
> > source of truth.
>
> I was looking at this yesterday and came up with same conclusion. We
> should have the options in one place. Currently libexpat is statically
> linked with Android >=O, maybe for such restricted environments we could
> just inline the xml as is at compile time and parse that later or
> alternatively (maybe cleaner) parse and generate default option cache
> already during compilation?
>
I realise that jumping the "me too" train does not help much, so here
are some alternative ideas.

How about we first distil the reasons why this is a problem and what
kind. Then explore independent solution for each one - as-is this
seems like a one-size-fits-all approach.
Some examples:
 - XML file may be inaccessible - the in-driver defaults should work(tm)
Yes there are some app specific ones, yet neither(?) of these apps is
present on Android
 - libexpat is not available, but libFOO is - investigate into a compat wrapper
 - cannot use external libraries (libexpat or equivalent) - static link

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

Re: [Mesa-dev] [PATCH] mesa: Prevent classic swrast crash on a surfaceless context.

2019-05-15 Thread Emil Velikov
On Wed, 15 May 2019 at 08:26,  wrote:
>
> From: Mathias Fröhlich 
>
> Hi all,
>
> One small fix below.
>
> Please review!
>
> best
>
> Mathias
>
>
>
>
>
> Running swrast with the new device egl extensions piglit test
> brings up this failure. Fix that by adding some NULL pointer
> checks.
>
From a very quick look this seems to be off.

The driver does not set the buffers as incomplete, thus in MakeCurrent
(and handle_first_current in particular) things go south.

For context grep for _mesa_get_incomplete_framebuffer and see how
i9[16]5 use the function when both read/draw priv. are NULL.
The gallium drivers st/mesa (really) also do that, yet classic swrast
(alongside r100, r200, nouveau_vieux) don't do that. Which is
understandable since those drivers receive far less testing than the
rest.

Can I suggest fleshing out some helper and fixing the drivers instead?

Note !intel classic drivers may also struggle with (draw xor read)
surfaces. It could be handled in the upper layers (dri/common or DRI
loaders - GLX/EGL/GBM) but haven't checked.

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

Re: [Mesa-dev] [PATCH] etnaviv: fill missing offset in etna_resource_get_handle

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 14:53, Christian Gmeiner
 wrote:
>
> Am Fr., 3. Mai 2019 um 12:05 Uhr schrieb Philipp Zabel 
> :
> >
> > Without this gbm_bo_get_offset() can return 0 where it shouldn't.
>
> Reviewed-by: Christian Gmeiner 
>
Nice catch. Please include the stable tag so we get this in the 19.0
and 19.1 series.

Reviewed-by: Emil Velikov 
Cc: 

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

Re: [Mesa-dev] [PATCH] panfrost: Add CAPFs for conservative rasterization

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 13:09, Tomeu Vizoso  wrote:
>
> Just do what everybody else but Nouveau does and return 0.0f.
>
> This prevents the repeated logging of these messages on startup:
>
> Unexpected PIPE_CAPF 6 query
> Unexpected PIPE_CAPF 7 query
> Unexpected PIPE_CAPF 8 query
>
> Signed-off-by: Tomeu Vizoso 
> ---
Reviewed-by: Emil Velikov 

At some point we could consider adding a float version of
u_pipe_screen_get_param_defaults()

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

Re: [Mesa-dev] [PATCH 08/11] panfrost: ci: Skip running some tests

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 07:35, Tomeu Vizoso  wrote:
>
> These tests add too much time to the total run time, and some of them
> even hang the DUTs, even if I haven't been able to reproduce it locally.
>
> Signed-off-by: Tomeu Vizoso 
> ---
>  src/gallium/drivers/panfrost/ci/deqp-runner.sh | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/src/gallium/drivers/panfrost/ci/deqp-runner.sh 
> b/src/gallium/drivers/panfrost/ci/deqp-runner.sh
> index 68d9bc1fc878..8d30eae28b04 100644
> --- a/src/gallium/drivers/panfrost/ci/deqp-runner.sh
> +++ b/src/gallium/drivers/panfrost/ci/deqp-runner.sh
> @@ -27,9 +27,11 @@ sleep 1  # Give some time for Weston to start up
>
>  # Disable for now tests that are very slow, either by just using lots of CPU 
> or by crashing
>  sed -i '/dEQP-GLES2.performance/d' /tmp/case-list.txt
> +sed -i '/dEQP-GLES2.stress/d' /tmp/case-list.txt
IMHO not running stress/performance tests on each job makes sense. A
longer interval - say once a week/month - would be reasonable.

>  sed -i '/dEQP-GLES2.functional.texture.filtering.2d.linear_mipmap_linear_/d' 
> /tmp/case-list.txt
>  sed -i 
> '/dEQP-GLES2.functional.texture.filtering.cube.linear_mipmap_linear_/d' 
> /tmp/case-list.txt
>  sed -i 
> '/dEQP-GLES2.functional.texture.filtering.cube.linear_mipmap_nearest_/d' 
> /tmp/case-list.txt
> +sed -i '/dEQP-GLES2.functional.fbo.render.depth./d' /tmp/case-list.txt
>
Pretty sure you or Alyssa will get to fixing these.

Wondering if one cannot use a trivial visual reminder about these.
Say printing some FIXME + tests in the logs. Or using an amber
indication on successful pipeline - a green one when the list (modulo
perf & stress tests) is empty?
Perhaps even something better?

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

Re: [Mesa-dev] [PATCH 10/11] st/glsl_to_nir: Fix uninitialized access

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 15:49, Emil Velikov  wrote:
>
> On Thu, 9 May 2019 at 07:35, Tomeu Vizoso  wrote:
> >
> > NIR_PASS will only set lower_flrp_progress if there's progress, and if
> > there isn't its value will be undefined.
> >
> > Fixes this Valgrind error:
> >
> > ==6589== Conditional jump or move depends on uninitialised value(s)
> > ==6589==at 0x55CA7E6: st_nir_opts (st_glsl_to_nir.cpp:347)
> > ==6589==by 0x55CC1DD: st_nir_link_shaders(nir_shader**, nir_shader**, 
> > bool) (st_glsl_to_nir.cpp:667)
> > ==6589==by 0x55CCDAB: st_link_nir (st_glsl_to_nir.cpp:803)
> > ==6589==by 0x55C8C5F: st_link_shader (st_glsl_to_ir.cpp:167)
> > ==6589==by 0x5438B11: _mesa_glsl_link_shader (ir_to_mesa.cpp:3170)
> > ==6589==by 0x5371701: link_program (shaderapi.c:1216)
> > ==6589==by 0x5371701: link_program_error (shaderapi.c:1310)
> > ==6589==by 0x5372901: _mesa_LinkProgram (shaderapi.c:1802)
> > ==6589==by 0x509B459: shared_dispatch_stub_509 (glapi_mapi_tmp.h:21115)
> > ==6589==by 0x4799BF: glu::Program::link() (in 
> > /home/tomeu/deqp-build/modules/gles2/deqp-gles2)
> > ==6589==by 0x47A253: glu::ShaderProgram::init(glw::Functions const&, 
> > glu::ProgramSources const&) (in 
> > /home/tomeu/deqp-build/modules/gles2/deqp-gles2)
> >
> > Signed-off-by: Tomeu Vizoso 
> > Fixes: d41cdef2a591 ("nir: Use the flrp lowering pass instead of 
> > nir_opt_algebraic")
> > Cc: Ian Romanick 
>
> Reviewed-by: Emil Velikov 
>
Ian has a MR [1] which addresses all use-cases, so I guess you can
drop this in favour of this patch.

-Emil
[1] https://gitlab.freedesktop.org/mesa/mesa/merge_requests/839

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

Re: [Mesa-dev] [PATCH 11/11] panfrost: Only take the fast paths on buffers aligned to block size

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 07:36, Tomeu Vizoso  wrote:
>
> As the functions operate on 16-byte blocks.
>
> Fixes this Valgrind error:
>
> Invalid read of size 4
>at 0x5857568: swizzle_bpp1_align16 (pan_swizzle.c:85)
>by 0x585780F: panfrost_texture_swizzle (pan_swizzle.c:171)
>by 0x584F587: panfrost_tile_texture (pan_resource.c:489)
>by 0x584F641: panfrost_transfer_unmap (pan_resource.c:525)
>by 0x587718D: u_transfer_helper_transfer_unmap (u_transfer_helper.c:516)
>by 0x5875D85: pipe_transfer_unmap (u_inlines.h:515)
>by 0x5875F13: u_default_texture_subdata (u_transfer.c:80)
>by 0x53FFDC3: st_TexSubImage (st_cb_texture.c:1480)
>by 0x54005BB: st_TexImage (st_cb_texture.c:1709)
>by 0x5391353: teximage (teximage.c:3105)
>by 0x5391353: teximage_err (teximage.c:3132)
>by 0x5391B9B: _mesa_TexImage2D (teximage.c:3170)
>by 0x5097A77: shared_dispatch_stub_183 (glapi_mapi_tmp.h:18833)
>  Address 0x1e94f1e8 is 0 bytes after a block of size 16 alloc'd
>at 0x483F5C8: malloc (vg_replace_malloc.c:299)
>by 0x584F47D: panfrost_transfer_map (pan_resource.c:467)
>by 0x587694D: u_transfer_helper_transfer_map (u_transfer_helper.c:243)
>by 0x5875EA7: u_default_texture_subdata (u_transfer.c:59)
>by 0x53FFDC3: st_TexSubImage (st_cb_texture.c:1480)
>by 0x54005BB: st_TexImage (st_cb_texture.c:1709)
>by 0x5391353: teximage (teximage.c:3105)
>by 0x5391353: teximage_err (teximage.c:3132)
>by 0x5391B9B: _mesa_TexImage2D (teximage.c:3170)
>by 0x5097A77: shared_dispatch_stub_183 (glapi_mapi_tmp.h:18833)
>by 0x4DA8AB: glu::CallLogWrapper::glTexImage2D(unsigned int, int, int, 
> int, int, int, unsigned int, unsigned int, void const*) (in 
> /home/tomeu/deqp-build/modules/gles2/deqp-gles2)
>
> Signed-off-by: Tomeu Vizoso 

There is no clear commit for "fixes" - so I'd add a stable tag here.

CC: 19.1 
Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH 10/11] st/glsl_to_nir: Fix uninitialized access

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 07:35, Tomeu Vizoso  wrote:
>
> NIR_PASS will only set lower_flrp_progress if there's progress, and if
> there isn't its value will be undefined.
>
> Fixes this Valgrind error:
>
> ==6589== Conditional jump or move depends on uninitialised value(s)
> ==6589==at 0x55CA7E6: st_nir_opts (st_glsl_to_nir.cpp:347)
> ==6589==by 0x55CC1DD: st_nir_link_shaders(nir_shader**, nir_shader**, 
> bool) (st_glsl_to_nir.cpp:667)
> ==6589==by 0x55CCDAB: st_link_nir (st_glsl_to_nir.cpp:803)
> ==6589==by 0x55C8C5F: st_link_shader (st_glsl_to_ir.cpp:167)
> ==6589==by 0x5438B11: _mesa_glsl_link_shader (ir_to_mesa.cpp:3170)
> ==6589==by 0x5371701: link_program (shaderapi.c:1216)
> ==6589==by 0x5371701: link_program_error (shaderapi.c:1310)
> ==6589==by 0x5372901: _mesa_LinkProgram (shaderapi.c:1802)
> ==6589==by 0x509B459: shared_dispatch_stub_509 (glapi_mapi_tmp.h:21115)
> ==6589==by 0x4799BF: glu::Program::link() (in 
> /home/tomeu/deqp-build/modules/gles2/deqp-gles2)
> ==6589==by 0x47A253: glu::ShaderProgram::init(glw::Functions const&, 
> glu::ProgramSources const&) (in 
> /home/tomeu/deqp-build/modules/gles2/deqp-gles2)
>
> Signed-off-by: Tomeu Vizoso 
> Fixes: d41cdef2a591 ("nir: Use the flrp lowering pass instead of 
> nir_opt_algebraic")
> Cc: Ian Romanick 

Reviewed-by: Emil Velikov 

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

Re: [Mesa-dev] [PATCH 09/11] panfrost: Fix two uninitialized accesses in compiler

2019-05-09 Thread Emil Velikov
On Thu, 9 May 2019 at 07:35, Tomeu Vizoso  wrote:
>
> Valgrind was complaining of those.
>
> NIR_PASS only sets progress to TRUE if there was progress.
>
> nir_const_load_to_arr() only sets as many constants as components has
> the instruction.
>
> This was causing some dEQP tests to flip-flop, such as:
>
> dEQP-GLES2.functional.fragment_ops.blend.equation_src_func_dst_func.add_src_color_constant_color
>
Please add a Fixes tag or a cc: mesa-stable@ one at least. This way
we'll pick this fix for stable.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH] configure.ac: check for libdrm when using VL with X11

2019-05-09 Thread Emil Velikov
On Wed, 8 May 2019 at 00:48, Dylan Baker  wrote:
>
> Quoting Alyssa Ross (2019-05-07 06:17:15)
> > On Mon, May 06, 2019 at 04:38:20PM -0700, Alyssa Rosenzweig wrote:
> > > Wrong Alyssa, cc'ing the right one :)
> >
> > Thank you for the CC, fellow Alyssa! :)
> >
Ahem, totally meant to do that :-)

> > > On Mon, May 06, 2019 at 04:32:38PM +0100, Emil Velikov wrote:
> > > > Alyssa this should resolve the failure with minimal churn. Please let
> > > > me know if it works on your end or not.
> >
> > Emil, this works for me. Thank you.
> >
> > Tested-by: Alyssa Ross 
> >
> >
> > ___
> > mesa-stable mailing list
> > mesa-sta...@lists.freedesktop.org
> > https://lists.freedesktop.org/mailman/listinfo/mesa-stable
>
> Staged for 19.0, thanks.

Thanks Dylan. I suspect the meson build needs an equivalent patch, yet
haven't looked at it recently.

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

Re: [Mesa-dev] [PATCH] Revert "glx: Fix synthetic error generation in __glXSendError"

2019-05-07 Thread Emil Velikov
On Tue, 7 May 2019 at 09:27, Michel Dänzer  wrote:
>
> On 2019-05-07 5:55 a.m., Timothy Arceri wrote:
> > This reverts commit e91ee763c378d03883eb88cf0eadd8aa916f7878.
> >
> > This seems to have broken a number of wine games.
> >
> > Cc: Adam Jackson 
> > Cc: Ian Romanick 
> > Cc: Hal Gentz 
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110632
> > Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=110590
> > ---
> >  src/glx/glx_error.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/glx/glx_error.c b/src/glx/glx_error.c
> > index 712ecf8213d..653cbeb2d2a 100644
> > --- a/src/glx/glx_error.c
> > +++ b/src/glx/glx_error.c
> > @@ -54,7 +54,7 @@ __glXSendError(Display * dpy, int_fast8_t errorCode, 
> > uint_fast32_t resourceID,
> >error.errorCode = glx_dpy->codes->first_error + errorCode;
> > }
> >
> > -   error.sequenceNumber = dpy->last_request_read;
> > +   error.sequenceNumber = dpy->request;
> > error.resourceID = resourceID;
> > error.minorCode = minorCode;
> > error.majorCode = glx_dpy->majorOpcode;
> > @@ -73,7 +73,7 @@ __glXSendErrorForXcb(Display * dpy, const 
> > xcb_generic_error_t *err)
> >
> > error.type = X_Error;
> > error.errorCode = err->error_code;
> > -   error.sequenceNumber = dpy->last_request_read;
> > +   error.sequenceNumber = err->sequence;
> > error.resourceID = err->resource_id;
> > error.minorCode = err->minor_code;
> > error.majorCode = err->major_code;
> >
>
> As-is, this will re-introduce
> https://bugs.freedesktop.org/show_bug.cgi?id=99781 . That one was about
> __glXSendErrorForXcb, while the regressions are about __glXSendError, so
> maybe only revert the __glXSendError hunk for now?
>
Could not agree more.

Can I suggest adding inline comment + even a bugzilla link? Otherwise
we're bound to copy/paste and break it again.

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

Re: [Mesa-dev] [Mesa-stable] [PATCH 1/2] swr/rast: don't create wrapper for every Create LLVM call

2019-05-06 Thread Emil Velikov
On Fri, 19 Apr 2019 at 16:36, Hota, Alok  wrote:
>
> Just wanted to give a quick update on this. We have agreed on moving forward 
> with this patch.
> I'm working on it now to get it through our CI and run some basic tests. I'll 
> keep you updated.
> Thanks again for the patch, Emil!
>
Thanks Alok.

I'm struggling to see this or equivalent in master. What's happening?

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

Re: [Mesa-dev] Move adriconf to mesa repositories

2019-05-06 Thread Emil Velikov
On Sun, 28 Apr 2019 at 19:38, Jean Hertel  wrote:
>
> >Could not find my original notes, but the idea is roughly as follows:
>  >- introduce a separate (user only?) library - say libmesa-config.so
> > - ^^ provides an API to query/set attributes, via numerical tokens
> > - any localisation is built on top of ^^ as standalone files
> >
> >Reasoning:
> >- library reused by anyone to make a pretty config tool in their
> >toolkit and/or language
> > - numerical tokens are trivial to handle and cheap - can be
> >binned/deprecated easily
> > - translation lives outside of the driver - the driver doesn't care
> >about it, so don't bloat
> > - translators do not need access to mesa - one less hurdle/obstacle
> >
> >
> >Hope it makes sense, not sure if coffee has kicked in fully ;-)
> >-Emil
>
> Hey Emil,
>
> I really liked this idea, specially since right now I have a lot of issues to 
> query which option is exactly supported by each driver. Like in the scenarios 
> when you have multiple drivers that support the same GPU, or when you have a 
> difference between userspace and kernel space driver naming.
>
> Can you give me more details on the idea?
> If I got it right, this library would be independent and mesa will itself use 
> it to query the options it wants/needs.
>
This is the tricky part - wish I could find my notes they have better
brain-dump.
It's OK to have the library as both front (config tool) and backend
(used by mesa) although:
 - special care on splitting and annotating the API is needed
 - handling this "extra" dependency would be fiddly for slower moving distros

> What about the current configuration files? Do you think there is a better 
> way to handle them?
> They are for in a xml format, which is far from optimal.
>
What seems to be the problem with XML? The files are meant to be
read/written to $app.

> What about Vulkan?
> As far as I known the current setup only handles OpenGL driver configurations.
>
The current setup handles GLX, DRI and Nine IIRC. One of my goals was
to split and structure this in a more obvious way.

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

[Mesa-dev] [PATCH] configure.ac: check for libdrm when using VL with X11

2019-05-06 Thread Emil Velikov
From: Emil Velikov 

The X11 specific code uses libdrm, yet we are missing the dependency.
This has gone unnoticed since all drivers which use VL already mandate
the library.

Note: this is applicable only for the stable branches.

Cc: Alyssa Rosenzweig 
Cc: 
Signed-off-by: Emil Velikov 
---
Alyssa this should resolve the failure with minimal churn. Please let
me know if it works on your end or not.
---
 configure.ac | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/configure.ac b/configure.ac
index 1ef68fe68e6..b288ecbd265 100644
--- a/configure.ac
+++ b/configure.ac
@@ -2357,7 +2357,7 @@ if test "x$enable_xvmc" = xyes -o \
 "x$enable_omx_tizonia" = xyes -o \
 "x$enable_va" = xyes; then
 if echo $platforms | grep -q "x11"; then
-PKG_CHECK_MODULES([VL], [x11-xcb xcb xcb-dri2 >= $XCBDRI2_REQUIRED])
+PKG_CHECK_MODULES([VL], [x11-xcb xcb xcb-dri2 >= $XCBDRI2_REQUIRED 
libdrm >= $LIBDRM_REQUIRED])
 fi
 need_gallium_vl_winsys=yes
 fi
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

2019-05-06 Thread Emil Velikov
On Sat, 4 May 2019 at 04:18, Marek Olšák  wrote:
>
> On Fri, May 3, 2019 at 1:58 AM Mathias Fröhlich  
> wrote:
>>
>> Good Morning,
>>
>> On Wednesday, 1 May 2019 21:43:08 CEST Marek Olšák wrote:
>> > BTW, swrast doesn't have to exist on the system. It's not uncommon for me
>> > to have no swrast on my development system.
>>
>> Ok. I see. I use swrast regularly to test changes with different backend 
>> drivers.
>> Also especially classic swrast as something that is close to the good old 
>> swtnl
>> drivers - to catch bad interactions with those.
>>
>> Anyhow, with a very old swrast I think you will get test failures.
>> But else if the system swrast is found in the hopefully not so distant future
>> the tests should even pass - well depends on what Emil now does to get a
>> better overall swrast behavior.
>> On a production system with a full set of driver packages I do expect to
>> find swrast, right? At least on a workstation grade linux distribution.
>>
>> I start to see the actual problem for AMD there.
>> Not your test system at home, but the pro driver that needs to ship
>> and QA swrast then.
>>
>> Anyhow, I do not actually understand the way how we walk all
>> installed egl driver implementations - including closed drivers - finally
>> and present all those devices. In a perfect world *for the customer*
>> I could enumerate all devices - including oss i965 and the closed nvidia
>> bumblebee device - on my laptop for example.
>>
>> Means - if that works fine AMD could hook into that mechanism and
>> provide further devices. Well - in the long term.
>
>
> We include libGL and libEGL along with radeonsi in our binary driver 
> installer. We probably don't include swrast, but I'm not 100% sure.
>
The series I just sent out covers everything but the "don't expose the
software device". It does include a hack which can be toggled to
achieve that though ;-)

My line of thinking is as follows:

Preamble:
A software device is only listed when the user requests the full
device list via QueryDevices and even then, it's the last one in the
list.
Thus it's close to impossible to get it "by mistake".

Case A - average Joe:
Getting Mesa from their distribution - swrast is build and shipped.

Case B - tailored solution like AMDGPU-PRO, Yocto builders or others:
People doing the platform integration know if swrast will be
built/available. If listing the software device is not something
they're interested, the trivial hack can be applied locally.

This seems like a perfectly good middle-ground, don't you agree?

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

[Mesa-dev] [PATCH 6/9] egl/dri: flesh out and use dri2_create_drawable()

2019-05-06 Thread Emil Velikov
From: Emil Velikov 

Wrap the loader->createNewDrawable() dance into a helper and use it
throughout the codebase.

This addresses a cases like surfaceless (SL) on swrast (SL on kms_swrast
is fine) where we'd attempt using the wrong driver and crash out.

Cc: mesa-sta...@lists.freedesktop.org
Signed-off-by: Emil Velikov 
---
 src/egl/drivers/dri2/egl_dri2.c | 24 +
 src/egl/drivers/dri2/egl_dri2.h |  5 +
 src/egl/drivers/dri2/platform_android.c | 12 +--
 src/egl/drivers/dri2/platform_drm.c | 17 +--
 src/egl/drivers/dri2/platform_surfaceless.c |  7 +-
 src/egl/drivers/dri2/platform_wayland.c | 14 +---
 src/egl/drivers/dri2/platform_x11.c | 15 +
 7 files changed, 34 insertions(+), 60 deletions(-)

diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index d584bccdebe..586bdd3b046 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -1424,6 +1424,30 @@ dri2_surf_update_fence_fd(_EGLContext *ctx,
dri2_surface_set_out_fence_fd(surf, fence_fd);
 }
 
+EGLBoolean
+dri2_create_drawable(struct dri2_egl_display *dri2_dpy,
+ const __DRIconfig *config,
+ struct dri2_egl_surface *dri2_surf)
+{
+   __DRIcreateNewDrawableFunc createNewDrawable;
+
+   if (dri2_dpy->image_driver)
+  createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
+   else if (dri2_dpy->dri2)
+  createNewDrawable = dri2_dpy->dri2->createNewDrawable;
+   else if (dri2_dpy->swrast)
+  createNewDrawable = dri2_dpy->swrast->createNewDrawable;
+   else
+  return _eglError(EGL_BAD_ALLOC, "no createNewDrawable");
+
+   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen,
+  config, dri2_surf);
+   if (dri2_surf->dri_drawable == NULL)
+  return _eglError(EGL_BAD_ALLOC, "createNewDrawable");
+
+   return EGL_TRUE;
+}
+
 /**
  * Called via eglMakeCurrent(), drv->API.MakeCurrent().
  */
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index aa143deb867..e7ff5c9184b 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -540,6 +540,11 @@ dri2_init_surface(_EGLSurface *surf, _EGLDisplay *disp, 
EGLint type,
 void
 dri2_fini_surface(_EGLSurface *surf);
 
+EGLBoolean
+dri2_create_drawable(struct dri2_egl_display *dri2_dpy,
+ const __DRIconfig *config,
+ struct dri2_egl_surface *dri2_surf);
+
 static inline uint64_t
 combine_u32_into_u64(uint32_t hi, uint32_t lo)
 {
diff --git a/src/egl/drivers/dri2/platform_android.c 
b/src/egl/drivers/dri2/platform_android.c
index e9ea9e6002b..270d342ac6d 100644
--- a/src/egl/drivers/dri2/platform_android.c
+++ b/src/egl/drivers/dri2/platform_android.c
@@ -341,7 +341,6 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
_EGLConfig *conf, void *native_window,
const EGLint *attrib_list)
 {
-   __DRIcreateNewDrawableFunc createNewDrawable;
struct dri2_egl_display *dri2_dpy = dri2_egl_display(disp);
struct dri2_egl_config *dri2_conf = dri2_egl_config(conf);
struct dri2_egl_surface *dri2_surf;
@@ -385,17 +384,8 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay *disp, 
EGLint type,
   goto cleanup_surface;
}
 
-   if (dri2_dpy->image_driver)
-  createNewDrawable = dri2_dpy->image_driver->createNewDrawable;
-   else
-  createNewDrawable = dri2_dpy->dri2->createNewDrawable;
-
-   dri2_surf->dri_drawable = (*createNewDrawable)(dri2_dpy->dri_screen, config,
-  dri2_surf);
-   if (dri2_surf->dri_drawable == NULL) {
-  _eglError(EGL_BAD_ALLOC, "createNewDrawable");
+   if (!dri2_create_drawable(dri2_dpy, config, dri2_surf))
   goto cleanup_surface;
-   }
 
if (window) {
   window->common.incRef(>common);
diff --git a/src/egl/drivers/dri2/platform_drm.c 
b/src/egl/drivers/dri2/platform_drm.c
index c1ab1c9b0f6..ec66ac3866e 100644
--- a/src/egl/drivers/dri2/platform_drm.c
+++ b/src/egl/drivers/dri2/platform_drm.c
@@ -171,23 +171,8 @@ dri2_drm_create_window_surface(_EGLDriver *drv, 
_EGLDisplay *disp,
dri2_surf->base.Height = surf->base.height;
surf->dri_private = dri2_surf;
 
-   if (dri2_dpy->dri2) {
-  dri2_surf->dri_drawable =
- dri2_dpy->dri2->createNewDrawable(dri2_dpy->dri_screen, config,
-   dri2_surf->gbm_surf);
-
-   } else {
-  assert(dri2_dpy->swrast != NULL);
-
-  dri2_surf->dri_drawable =
- dri2_dpy->swrast->createNewDrawable(dri2_dpy->dri_screen, config,
- dri2_surf->gbm_surf);
-
-   }
-   if (dri2_surf->d

[Mesa-dev] [PATCH 8/9] egl: add EGL_platform_device support

2019-05-06 Thread Emil Velikov
This new 'platform' is added by default with no guards.

It is effectively a copy of the surfaceless one, with updated function
names and brand new probe function.

Due to the reuse, some of the ifdef HAVE_SURFACELESS_PLATFORM guards
have been dropped.

A worthy mention are the changes in _egFindDisplay, since the original
and dup'd fd are required, we make use of the plat_opt argument.

Note that no hacks for eglGetDisplay are added - the API works only with
the eglGetPlatformDisplay* API.

v2:
 - s/_eglCompareDeviceDisplay/_eglSameDeviceDisplay/ (Eric)
 - let ^^ return bool (Eric)
 - fixup meson build, move files() further up (Eric)
 - copy from plat. surfaceless w/o the visual cleanups
 - close and free when destroying the dpy
 - sprinkle a few _eglDeviceSupports
 - split fd handling into separate function
 - use directly the render node if no FD is given (Mathias)

v3:
 - s/dpy/disp/g
 - drop swap_buffers* callbacks
 - drop loader_set_logger()
 - drop local define
 - re-introduce _eglGetDRMDeviceRenderNode()
 - EGL_WARN on ForceSoftware with HW device - continue using the HW device
 - bail out for "EGL_MESA_device_software" until it's fixed
 - wire-up the Android build

v4:
 - use new style _eglFindDisplay()
 - split hw vs sw code paths
 - don't close the internal fd (already handled in FiniDisplay())
 - make swrast work (bit hacky bit will do for now)
 - Android for real, drop autotools
 - Correct HW + LIBGL_ALWAYS_SOFTWARE check
 - use the dri2_create_drawable() helper

Cc: Mathias Fröhlich 
Cc: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/Android.mk |   1 +
 src/egl/drivers/dri2/egl_dri2.c|   3 +
 src/egl/drivers/dri2/egl_dri2.h|  13 +-
 src/egl/drivers/dri2/platform_device.c | 432 +
 src/egl/main/eglapi.c  |  13 +-
 src/egl/main/egldevice.c   |  16 +
 src/egl/main/egldevice.h   |   3 +
 src/egl/main/egldisplay.c  |  64 
 src/egl/main/egldisplay.h  |   7 +-
 src/egl/main/eglglobals.c  |   1 +
 src/egl/meson.build|   1 +
 11 files changed, 543 insertions(+), 11 deletions(-)
 create mode 100644 src/egl/drivers/dri2/platform_device.c

diff --git a/src/egl/Android.mk b/src/egl/Android.mk
index a9319f56ae7..01c33298ee7 100644
--- a/src/egl/Android.mk
+++ b/src/egl/Android.mk
@@ -36,6 +36,7 @@ include $(CLEAR_VARS)
 LOCAL_SRC_FILES := \
$(LIBEGL_C_FILES) \
$(dri2_backend_core_FILES) \
+   drivers/dri2/platform_device.c \
drivers/dri2/platform_android.c
 
 LOCAL_CFLAGS := \
diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c
index 586bdd3b046..c4dcfc364ac 100644
--- a/src/egl/drivers/dri2/egl_dri2.c
+++ b/src/egl/drivers/dri2/egl_dri2.c
@@ -874,6 +874,9 @@ dri2_initialize(_EGLDriver *drv, _EGLDisplay *disp)
case _EGL_PLATFORM_SURFACELESS:
   ret = dri2_initialize_surfaceless(drv, disp);
   break;
+   case _EGL_PLATFORM_DEVICE:
+  ret = dri2_initialize_device(drv, disp);
+  break;
case _EGL_PLATFORM_X11:
   ret = dri2_initialize_x11(drv, disp);
   break;
diff --git a/src/egl/drivers/dri2/egl_dri2.h b/src/egl/drivers/dri2/egl_dri2.h
index e7ff5c9184b..cd17b0a7036 100644
--- a/src/egl/drivers/dri2/egl_dri2.h
+++ b/src/egl/drivers/dri2/egl_dri2.h
@@ -331,10 +331,10 @@ struct dri2_egl_surface
} color_buffers[3], *back;
 #endif
 
-#if defined(HAVE_SURFACELESS_PLATFORM)
-  __DRIimage   *front;
-  unsigned int visual;
-#endif
+   /* surfaceless and device */
+   __DRIimage   *front;
+   unsigned int visual;
+
int out_fence_fd;
EGLBoolean enable_out_fence;
 };
@@ -492,6 +492,11 @@ dri2_initialize_surfaceless(_EGLDriver *drv, _EGLDisplay 
*disp)
 }
 #endif
 
+EGLBoolean
+dri2_initialize_device(_EGLDriver *drv, _EGLDisplay *disp);
+static inline void
+dri2_teardown_device(struct dri2_egl_display *dri2_dpy) { /* noop */ }
+
 void
 dri2_flush_drawable_for_swapbuffers(_EGLDisplay *disp, _EGLSurface *draw);
 
diff --git a/src/egl/drivers/dri2/platform_device.c 
b/src/egl/drivers/dri2/platform_device.c
new file mode 100644
index 000..0d9046c91b2
--- /dev/null
+++ b/src/egl/drivers/dri2/platform_device.c
@@ -0,0 +1,432 @@
+/*
+ * Mesa 3-D graphics library
+ *
+ * Copyright 2018 Collabora
+ *
+ * Based on platform_surfaceless, which has:
+ *
+ * Copyright (c) 2014 The Chromium OS Authors.
+ * Copyright © 2011 Intel Corporation
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining a
+ * copy of this software and associated documentation files (the "Software"),
+ * to deal in the Software without restriction, including without limitation
+ * the rights to use, copy, modify, merge, publish, distribute, sublicense,
+ * and/or sell copies of the Software, and to permit persons to whom the
+ * Software is furnished to do so, subject to the following conditions:
+ *
+ *

[Mesa-dev] [PATCH 7/9] egl: keep the software device at the end of the list

2019-05-06 Thread Emil Velikov
From: Emil Velikov 

By default, the user is likely to pick the first device so it should
not be the least performant (aka software) one.

Suggested-by: Marek Olšák 
Signed-off-by: Emil Velikov 
---
 src/egl/main/egldevice.c | 15 ++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/src/egl/main/egldevice.c b/src/egl/main/egldevice.c
index c5c9a21273a..328d9ea08c5 100644
--- a/src/egl/main/egldevice.c
+++ b/src/egl/main/egldevice.c
@@ -293,13 +293,26 @@ _eglQueryDevicesEXT(EGLint max_devices,
   goto out;
}
 
+   /* Push the first device (the software one) to the end of the list.
+* Sending it to the user only if they've requested the full list.
+*
+* By default, the user is likely to pick the first device so having the
+* software (aka least performant) one is not a good idea.
+*/
*num_devices = MIN2(num_devs, max_devices);
 
-   for (i = 0, dev = devs; i < *num_devices; i++) {
+   for (i = 0, dev = devs->Next; dev && i < max_devices; i++) {
   devices[i] = dev;
   dev = dev->Next;
}
 
+   /* User requested the full device list, add the sofware device. */
+   if (max_devices >= num_devs) {
+  /* The first device is always software */
+  assert(_eglDeviceSupports(devs, _EGL_DEVICE_SOFTWARE));
+  devices[num_devs - 1] = devs;
+   }
+
 out:
mtx_unlock(_eglGlobal.Mutex);
 
-- 
2.21.0

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

[Mesa-dev] [PATCH 9/9] HACK: eg: hack to drop SOFTWARE egl device

2019-05-06 Thread Emil Velikov
For some extreme corner-cases where you'd want to do it.
---
 src/egl/main/egldevice.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/egl/main/egldevice.c b/src/egl/main/egldevice.c
index 3e8c0e5aeb5..4a3127d7d62 100644
--- a/src/egl/main/egldevice.c
+++ b/src/egl/main/egldevice.c
@@ -321,15 +321,20 @@ _eglQueryDevicesEXT(EGLint max_devices,
   devices[i] = dev;
   dev = dev->Next;
}
-
+#define HACK_DROP_SOFTWARE_DEVICE 0
+#if !HACK_DROP_SOFTWARE_DEVICE
/* User requested the full device list, add the sofware device. */
if (max_devices >= num_devs) {
   /* The first device is always software */
   assert(_eglDeviceSupports(devs, _EGL_DEVICE_SOFTWARE));
   devices[num_devs - 1] = devs;
}
+#endif
 
 out:
+#if HACK_DROP_SOFTWARE_DEVICE
+   *num_devices--;
+#endif
mtx_unlock(_eglGlobal.Mutex);
 
return EGL_TRUE;
-- 
2.21.0

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

[Mesa-dev] [PATCH 3/9] egl/x11: pick the user requested screen

2019-05-06 Thread Emil Velikov
From: Adam Jackson 

At the moment the user will pass the screen number via attribs, yet we
would throw that away. Reason being that the int *screen passed to
xcb_connect() is output only.

v2 (Emil):
 - split from a larger patch
 - use xcb_connect() returned screen, as fallback
 - use helper function only as needed

Signed-off-by: Emil Velikov 
---
 src/egl/drivers/dri2/platform_x11.c | 17 +++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/egl/drivers/dri2/platform_x11.c 
b/src/egl/drivers/dri2/platform_x11.c
index c8c676d2f00..ba0379c1177 100644
--- a/src/egl/drivers/dri2/platform_x11.c
+++ b/src/egl/drivers/dri2/platform_x11.c
@@ -1276,21 +1276,34 @@ static const __DRIextension *swrast_loader_extensions[] 
= {
NULL,
 };
 
+static int
+dri2_find_screen_for_display(const _EGLDisplay *disp, int fallback_screen)
+{
+   const EGLAttrib *attr;
+
+   for (attr = disp->Options.Attribs; attr; attr += 2) {
+  if (attr[0] == EGL_PLATFORM_X11_SCREEN_EXT)
+ return attr[1];
+   }
+
+   return fallback_screen;
+}
+
 static EGLBoolean
 dri2_get_xcb_connection(_EGLDriver *drv, _EGLDisplay *disp,
 struct dri2_egl_display *dri2_dpy)
 {
xcb_screen_iterator_t s;
-   int screen = (uintptr_t)disp->Options.Platform;
+   int screen;
const char *msg;
 
disp->DriverData = (void *) dri2_dpy;
if (disp->PlatformDisplay == NULL) {
   dri2_dpy->conn = xcb_connect(NULL, );
   dri2_dpy->own_device = true;
+  screen = dri2_find_screen_for_display(disp, screen);
} else {
   Display *dpy = disp->PlatformDisplay;
-
   dri2_dpy->conn = XGetXCBConnection(dpy);
   screen = DefaultScreen(dpy);
}
-- 
2.21.0

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

[Mesa-dev] [PATCH 4/9] egl: remove Options::Platform handling

2019-05-06 Thread Emil Velikov
From: Adam Jackson 

The full set of attributes is already handled with previous patches.
Thus all this is not dead code.

v2 (Emil) - split from a larger patch.

Signed-off-by: Emil Velikov 
---
 src/egl/main/egldisplay.c | 13 -
 src/egl/main/egldisplay.h |  1 -
 2 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 9a2ac48e8bc..418ab0ec9b8 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -497,17 +497,12 @@ _eglParseX11DisplayAttribList(_EGLDisplay *display,
   return EGL_TRUE;
}
 
+   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
+* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
+*/
for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
-  EGLAttrib attrib = attrib_list[i];
-  EGLAttrib value = attrib_list[i + 1];
-
-  /* EGL_EXT_platform_x11 recognizes exactly one attribute,
-   * EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
-   */
-  if (attrib != EGL_PLATFORM_X11_SCREEN_EXT)
+  if (attrib_list[i] != EGL_PLATFORM_X11_SCREEN_EXT)
  return _eglError(EGL_BAD_ATTRIBUTE, "eglGetPlatformDisplay");
-
-  display->Options.Platform = (void *)(uintptr_t)value;
}
 
return EGL_TRUE;
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index 2c9fd6c3399..369ab31187f 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -167,7 +167,6 @@ struct _egl_display
/* options that affect how the driver initializes the display */
struct {
   EGLBoolean ForceSoftware; /**< Use software path only */
-  void *Platform; /**< Platform-specific options */
   EGLAttrib *Attribs; /**< Platform-specific options */
} Options;
 
-- 
2.21.0

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

[Mesa-dev] [PATCH 5/9] egl: fold X11 attrib handling like other platforms

2019-05-06 Thread Emil Velikov
From: Emil Velikov 

Since we no longer need special handling for X11, refactor the code to
follow the style used by all other platforms.

Signed-off-by: Emil Velikov 
---
 src/egl/main/egldisplay.c | 45 ++-
 1 file changed, 11 insertions(+), 34 deletions(-)

diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index 418ab0ec9b8..422b473844e 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -487,45 +487,22 @@ _eglUnlinkResource(_EGLResource *res, _EGLResourceType 
type)
 }
 
 #ifdef HAVE_X11_PLATFORM
-static EGLBoolean
-_eglParseX11DisplayAttribList(_EGLDisplay *display,
-  const EGLAttrib *attrib_list)
-{
-   int i;
-
-   if (attrib_list == NULL) {
-  return EGL_TRUE;
-   }
-
-   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
-* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
-*/
-   for (i = 0; attrib_list[i] != EGL_NONE; i += 2) {
-  if (attrib_list[i] != EGL_PLATFORM_X11_SCREEN_EXT)
- return _eglError(EGL_BAD_ATTRIBUTE, "eglGetPlatformDisplay");
-   }
-
-   return EGL_TRUE;
-}
-
 _EGLDisplay*
 _eglGetX11Display(Display *native_display,
   const EGLAttrib *attrib_list)
 {
-   _EGLDisplay *display = _eglFindDisplay(_EGL_PLATFORM_X11,
-  native_display,
-  attrib_list);
-
-   if (!display) {
-  _eglError(EGL_BAD_ALLOC, "eglGetPlatformDisplay");
-  return NULL;
-   }
-
-   if (!_eglParseX11DisplayAttribList(display, attrib_list)) {
-  return NULL;
+   /* EGL_EXT_platform_x11 recognizes exactly one attribute,
+* EGL_PLATFORM_X11_SCREEN_EXT, which is optional.
+*/
+   if (attrib_list != NULL) {
+  for (int i = 0; attrib_list[i] != EGL_NONE; i += 2) {
+ if (attrib_list[i] != EGL_PLATFORM_X11_SCREEN_EXT) {
+_eglError(EGL_BAD_ATTRIBUTE, "eglGetPlatformDisplay");
+return NULL;
+ }
+  }
}
-
-   return display;
+   return _eglFindDisplay(_EGL_PLATFORM_X11, native_display, attrib_list);
 }
 #endif /* HAVE_X11_PLATFORM */
 
-- 
2.21.0

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

[Mesa-dev] [PATCH 2/9] egl: handle the full attrib list in display::options

2019-05-06 Thread Emil Velikov
From: Adam Jackson 

Earlier spec is vague, although EGL 1.5 makes it clear:

 Multiple calls made to eglGetPlatformDisplay with the same
 parameters will return the same EGLDisplay handle.

With this commit we store and compare the full attrib list.

v2 (Emil):
 - Split into separate patches
 - Use EGLBoolean over int masked as such
 - Don't return free'd pointed on calloc failure

Signed-off-by: Emil Velikov 
---
 src/egl/main/eglapi.c |  2 +-
 src/egl/main/egldisplay.c | 61 +--
 src/egl/main/egldisplay.h |  3 +-
 3 files changed, 55 insertions(+), 11 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 4481eb9cd0f..f77df8ff437 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -373,7 +373,7 @@ eglGetDisplay(EGLNativeDisplayType nativeDisplay)
native_display_ptr = (void*) nativeDisplay;
 
plat = _eglGetNativePlatform(native_display_ptr);
-   disp = _eglFindDisplay(plat, native_display_ptr);
+   disp = _eglFindDisplay(plat, native_display_ptr, NULL);
return _eglGetDisplayHandle(disp);
 }
 
diff --git a/src/egl/main/egldisplay.c b/src/egl/main/egldisplay.c
index ba5f84510fe..9a2ac48e8bc 100644
--- a/src/egl/main/egldisplay.c
+++ b/src/egl/main/egldisplay.c
@@ -202,20 +202,49 @@ _eglFiniDisplay(void)
  }
   }
 
+  free(disp->Options.Attribs);
   free(disp);
}
_eglGlobal.DisplayList = NULL;
 }
 
+static EGLBoolean
+_eglSameAttribs(const EGLAttrib *a, const EGLAttrib *b)
+{
+   size_t na = _eglNumAttribs(a);
+   size_t nb = _eglNumAttribs(b);
+
+   /* different numbers of attributes must be different */
+   if (na != nb)
+  return EGL_FALSE;
+
+   /* both lists NULL are the same */
+   if (!a && !b)
+  return EGL_TRUE;
+
+   /* otherwise, compare the lists */
+   return memcmp(a, b, na) == 0 ? EGL_TRUE : EGL_FALSE;
+}
 
 /**
  * Find the display corresponding to the specified native display, or create a
- * new one.
+ * new one. EGL 1.5 says:
+ *
+ * Multiple calls made to eglGetPlatformDisplay with the same parameters
+ * will return the same EGLDisplay handle.
+ *
+ * We read this extremely strictly, and treat a call with NULL attribs as
+ * different from a call with attribs only equal to { EGL_NONE }. Similarly
+ * we do not sort the attribute list, so even if all attribute _values_ are
+ * identical, different attribute orders will be considered different
+ * parameters.
  */
 _EGLDisplay *
-_eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
+_eglFindDisplay(_EGLPlatformType plat, void *plat_dpy,
+const EGLAttrib *attrib_list)
 {
_EGLDisplay *disp;
+   size_t num_attribs;
 
if (plat == _EGL_INVALID_PLATFORM)
   return NULL;
@@ -225,7 +254,8 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
/* search the display list first */
disp = _eglGlobal.DisplayList;
while (disp) {
-  if (disp->Platform == plat && disp->PlatformDisplay == plat_dpy)
+  if (disp->Platform == plat && disp->PlatformDisplay == plat_dpy &&
+  _eglSameAttribs(disp->Options.Attribs, attrib_list))
  break;
   disp = disp->Next;
}
@@ -237,13 +267,24 @@ _eglFindDisplay(_EGLPlatformType plat, void *plat_dpy)
  mtx_init(>Mutex, mtx_plain);
  disp->Platform = plat;
  disp->PlatformDisplay = plat_dpy;
-
- /* add to the display list */ 
+ num_attribs = _eglNumAttribs(attrib_list);
+ if (num_attribs) {
+disp->Options.Attribs = calloc(num_attribs, sizeof(EGLAttrib));
+if (!disp->Options.Attribs) {
+   free(disp);
+   disp = NULL;
+   goto out;
+}
+memcpy(disp->Options.Attribs, attrib_list,
+   num_attribs * sizeof(EGLAttrib));
+ }
+ /* add to the display list */
  disp->Next = _eglGlobal.DisplayList;
  _eglGlobal.DisplayList = disp;
   }
}
 
+out:
mtx_unlock(_eglGlobal.Mutex);
 
return disp;
@@ -477,7 +518,8 @@ _eglGetX11Display(Display *native_display,
   const EGLAttrib *attrib_list)
 {
_EGLDisplay *display = _eglFindDisplay(_EGL_PLATFORM_X11,
-  native_display);
+  native_display,
+  attrib_list);
 
if (!display) {
   _eglError(EGL_BAD_ALLOC, "eglGetPlatformDisplay");
@@ -503,7 +545,7 @@ _eglGetGbmDisplay(struct gbm_device *native_display,
   return NULL;
}
 
-   return _eglFindDisplay(_EGL_PLATFORM_DRM, native_display);
+   return _eglFindDisplay(_EGL_PLATFORM_DRM, native_display, attrib_list);
 }
 #endif /* HAVE_DRM_PLATFORM */
 
@@ -518,7 +560,7 @@ _eglGetWaylandDisplay(struct wl_display *native_display,
   return NULL;
}
 
-   return _eglFindDisplay(_EGL_PLATFORM_WAYLAND, native_dis

[Mesa-dev] [PATCH 0/9] Final round: EGL_EXT_platform_device

2019-05-06 Thread Emil Velikov
*** BLURB HERE ***
Hi all,

Here is, hopefully the final series, which add support for the extension.

Note that pbuffers are broken (afaict have been since day 1) so one need fixes
like the following [1].

Changes since last version:
 - split and fixup the EGLAttrib patch from Adam - less hacky eglFindDisplay

 - drop _eglParseX11DisplayAttribList() - minor polish

 - introduce dri2_create_drawable() helper - fixes a crash in
platform_surfaceless and by extension platform_device with swrast

 - list software device in QueryDevices only when the full list is requested
and even then - keep if the last device

 - get platform_device + swrast working - architecturally hacky, but will do
for now

 - HACK: POC drop the software devices from QueryDevices
Meant for Marek, if their stack does not build/ship swrast_dri.so


Review and comments are appreciated :-)

Thanks
Emil

Cc: Mathias Fröhlich 
Cc: Marek Olšák 

[1] https://patchwork.freedesktop.org/series/60018/ 


Adam Jackson (3):
  egl: handle the full attrib list in display::options
  egl/x11: pick the user requested screen
  egl: remove Options::Platform handling

Emil Velikov (6):
  egl: flesh out a _eglNumAttribs() helper
  egl: fold X11 attrib handling like other platforms
  egl/dri: flesh out and use dri2_create_drawable()
  egl: keep the software device at the end of the list
  egl: add EGL_platform_device support
  HACK: eg: hack to drop SOFTWARE egl device

 src/egl/Android.mk  |   1 +
 src/egl/drivers/dri2/egl_dri2.c |  27 ++
 src/egl/drivers/dri2/egl_dri2.h |  18 +-
 src/egl/drivers/dri2/platform_android.c |  12 +-
 src/egl/drivers/dri2/platform_device.c  | 432 
 src/egl/drivers/dri2/platform_drm.c |  17 +-
 src/egl/drivers/dri2/platform_surfaceless.c |   7 +-
 src/egl/drivers/dri2/platform_wayland.c |  14 +-
 src/egl/drivers/dri2/platform_x11.c |  32 +-
 src/egl/main/eglapi.c   |  27 +-
 src/egl/main/egldevice.c|  36 +-
 src/egl/main/egldevice.h|   3 +
 src/egl/main/egldisplay.c   | 171 +---
 src/egl/main/egldisplay.h   |  24 +-
 src/egl/main/eglglobals.c   |   1 +
 src/egl/meson.build |   1 +
 16 files changed, 691 insertions(+), 132 deletions(-)
 create mode 100644 src/egl/drivers/dri2/platform_device.c

-- 
2.21.0

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

[Mesa-dev] [PATCH 1/9] egl: flesh out a _eglNumAttribs() helper

2019-05-06 Thread Emil Velikov
From: Emil Velikov 

Signed-off-by: Emil Velikov 
---
 src/egl/main/eglapi.c | 12 +++-
 src/egl/main/egldisplay.h | 13 +
 2 files changed, 16 insertions(+), 9 deletions(-)

diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c
index 588c6a5f1eb..4481eb9cd0f 100644
--- a/src/egl/main/eglapi.c
+++ b/src/egl/main/eglapi.c
@@ -340,22 +340,16 @@ _eglConvertIntsToAttribs(const EGLint *int_list, 
EGLAttrib **out_attrib_list)
 static EGLint *
 _eglConvertAttribsToInt(const EGLAttrib *attr_list)
 {
+   size_t size = _eglNumAttribs(attr_list);
EGLint *int_attribs = NULL;
 
/* Convert attributes from EGLAttrib[] to EGLint[] */
-   if (attr_list) {
-  int i, size = 0;
-
-  while (attr_list[size] != EGL_NONE)
- size += 2;
-
-  size += 1; /* add space for EGL_NONE */
-
+   if (size) {
   int_attribs = calloc(size, sizeof(int_attribs[0]));
   if (!int_attribs)
  return NULL;
 
-  for (i = 0; i < size; i++)
+  for (size_t i = 0; i < size; i++)
  int_attribs[i] = attr_list[i];
}
return int_attribs;
diff --git a/src/egl/main/egldisplay.h b/src/egl/main/egldisplay.h
index cfd0ff66d64..f37b633b466 100644
--- a/src/egl/main/egldisplay.h
+++ b/src/egl/main/egldisplay.h
@@ -274,6 +274,19 @@ _eglIsResourceLinked(_EGLResource *res)
return res->IsLinked;
 }
 
+static inline size_t
+_eglNumAttribs(const EGLAttrib *attribs)
+{
+   size_t len = 0;
+
+   if (attribs) {
+  while (attribs[len] != EGL_NONE)
+ len += 2;
+  len++;
+   }
+   return len;
+}
+
 #ifdef HAVE_X11_PLATFORM
 _EGLDisplay*
 _eglGetX11Display(Display *native_display, const EGLAttrib *attrib_list);
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH 2/2] egl: add EGL_platform_device support

2019-04-30 Thread Emil Velikov
On Mon, 29 Apr 2019 at 22:50, Marek Olšák  wrote:
>
> On Mon, Apr 29, 2019 at 4:00 AM Pekka Paalanen  wrote:
>>
>> On Sat, 27 Apr 2019 09:38:27 -0400
>> Marek Olšák  wrote:
>>
>> > Those are all valid reasons, but I don't wanna expose swrast for AMD's
>> > customers.
>>
>> Hi Marek,
>>
>> is you objection that you will never want to see any software renderer
>> in the list, or that you don't want to see a software renderer only as
>> long as it doesn't actually work?
>>
>> Why do you not "wanna expose swrast for AMD's customers"?
>>
>> Are you talking about swrast specifically or all the software renderers
>> in Mesa?
>>
>> I'm not an AMD customer if you mean someone paying for support rather
>> than just buying their hardware, so I cannot speak for them. However, I
>> would be very happy to have a software renderer available to be picked
>> the same way as any other hardware renderer, so that I can use it in
>> graphical test suites (a point from Anholt) testing also the EGL glue
>> in addition to the pixels produced.
>>
>> The thing will be run on boxes with AMD GPUs, and in those cases there
>> must be a way to *not* use the AMD GPU, and use the software renderer
>> instead when wanted. Not by environment variables or anything hacky
>> like that, but by deliberately choosing the software renderer in the
>> program. It will need an EGLSurface too.
>>
>> How would this be made to work in the future?
>
>
> A software renderer could be exposed by adding a new EGL function on top of 
> EGL_EXT_device_base, for example:
>
> // EGL_MESA_device_software
>
> EGLBoolean eglQuerySoftwareDeviceMESA(EGLDeviceEXT *swdevice);
>
>
> You would get the swrast device from that function instead of 
> eglQueryDevicesEXT. It clearly separates hw and sw devices but keeps 
> everything else the same.
>
IMHO the current EGL_MESA_device_software approach is perfectly valid.
The Query API provides the devices and one can query their
capabilities/device extensions.

As far as I can see you have valid concerns:
 - HW devices should be the first/default
 - the SW device should work, if exposed
 - one may want to omit the SW one all together

At the same time:
 - the device_base extension is explicit that at least one device must
be present
 - manipulating the EGL client extension string, before we determine
the driver is a PITA

How about:
 - whip a quick (admittedly gross) hack that makes SW work
 - flip the order so SW is always the last one in the list
 - add a hack that disables SW all-together?

The first two should be OK to upstream, although I'd suggest keeping
the last one in AMDGPU-PRO.
Reason being is that the pro packaging can enforce that radeonsi is
always present. While we cannot do that for distributions :-\

/me starts working on the patches

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

[Mesa-dev] [PATCH] vulkan/wsi: document libdrm version introducing drmIsMaster()

2019-04-30 Thread Emil Velikov
From: Emil Velikov 

To avoid pulling the latest libdrm we copy the helper locally. Document
when it was introduced, making it easier to purge in the future.

Cc: Eric Engestrom 
Suggested-by: Eric Engestrom 
Signed-off-by: Emil Velikov 
---
Thanks for the suggestion Eric.
---
 src/vulkan/wsi/wsi_common_display.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c
index 7df1e1fd384..803e7f7e9dd 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1815,7 +1815,7 @@ fail_attr_init:
 
 /*
  * Local version fo the libdrm helper. Added to avoid depending on bleeding
- * edge version of the library.
+ * edge version of the library - libdrm v2.4.98, released 19 Apr 2019.
  */
 static int
 local_drmIsMaster(int fd)
-- 
2.21.0

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

Re: [Mesa-dev] Mesa 3D

2019-04-29 Thread Emil Velikov
On Fri, 26 Apr 2019 at 08:13, Rain_Kuper  wrote:
>
> Hello,does Mesa 3D have support for NVIDIA Tegra K1 SoC and Android?
>
Tegra K1 should work with the nouveau kernel and mesa drivers. Android
is also amongst the platforms where Mesa runs.
IIRC since Android makes heavy use of MT, you might need to
rebase/apply these [1] nouveau patches.

That said, best thing is to try if yourself - yet YMMV.
I would start with Android-x86.

HTH
Emil

[1] https://patchwork.freedesktop.org/series/53595/
Note: there might be a newer revision, haven't checked.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH 0/3] Pbuffer fixes

2019-04-29 Thread Emil Velikov
On Sat, 27 Apr 2019 at 04:31, Marek Olšák  wrote:
>
> Hi,
>
> This series fixes pbuffers for EGL as exercised by the egl_ext_device-
> _base piglit test.
>
> It passes piglit, GL-CTS, dEQP, and The Hitchhiker's Guide to the Galaxy,
> but I didn't test GLX, so things might still break horribly there.
>
> Rbs welcome,
>
Thanks for sorting these our Marek.

With Mathias' comment addressed the series is:
Reviewed-by: Emil Velikov 

Can I ask you to run these against the QT test suite - it did
high-light a large number of issues in the past.

Please let Adam and others have a look before merging these.

Thanks
-Emil
Note to self: nominate these for stable, if we don't have any
regressions after a few weeks.
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH] vulkan/wsi: don't use DUMB_CLOSE for normal GEM handles

2019-04-26 Thread Emil Velikov
On Thu, 25 Apr 2019 at 11:44, Bas Nieuwenhuizen  
wrote:
>
> r-b
>
Thank you Bas. Pushed both fixes to master.

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

Re: [Mesa-dev] [PATCH v2 1/3] llvmpipe: add lp_fence_timedwait() helper

2019-04-26 Thread Emil Velikov
On Thu, 25 Apr 2019 at 19:24, Gustaw Smolarczyk  wrote:
>
> czw., 25 kwi 2019 o 20:11 Gustaw Smolarczyk  napisał(a):
> >
> > czw., 25 kwi 2019 o 19:42 Emil Velikov  
> > napisał(a):
> > >
> > > The function is analogous to lp_fence_wait() while taking at timeout
> > > (ns) parameter, as needed for EGL fence/sync.
> > >
> > > v2:
> > >  - use absolute UTC time, as per spec (Gustaw)
> > >  - bail out on cnd_timedwait() failure (Gustaw)
> > >
> > > Cc: Gustaw Smolarczyk 
> > > Cc: Roland Scheidegger 
> > > Signed-off-by: Emil Velikov 
> > > Reviewed-by: Roland Scheidegger  (v1)
> > > ---
> > >  src/gallium/drivers/llvmpipe/lp_fence.c | 30 +
> > >  src/gallium/drivers/llvmpipe/lp_fence.h |  3 +++
> > >  2 files changed, 33 insertions(+)
> > >
> > > diff --git a/src/gallium/drivers/llvmpipe/lp_fence.c 
> > > b/src/gallium/drivers/llvmpipe/lp_fence.c
> > > index 20cd91cd63d..b79d773bf6c 100644
> > > --- a/src/gallium/drivers/llvmpipe/lp_fence.c
> > > +++ b/src/gallium/drivers/llvmpipe/lp_fence.c
> > > @@ -125,3 +125,33 @@ lp_fence_wait(struct lp_fence *f)
> > >  }
> > >
> > >
> > > +boolean
> > > +lp_fence_timedwait(struct lp_fence *f, uint64_t timeout)
> > > +{
> > > +   struct timespec ts;
> > > +   int ret;
> > > +
> > > +   timespec_get(, TIME_UTC);
> > > +
> > > +   ts.tv_nsec += timeout % 10L;
> > > +   ts.tv_sec += timeout / 10L;
> > > +   if (ts.tv_nsec >= 10L) {
> > > +  ts.tv_sec++;
> > > +  ts.tv_nsec -= 10L;
> > > +   }
> > > +
> > > +   if (LP_DEBUG & DEBUG_FENCE)
> > > +  debug_printf("%s %d\n", __FUNCTION__, f->id);
> > > +
> > > +   mtx_lock(>mutex);
> > > +   assert(f->issued);
> > > +   while (f->count < f->rank) {
> > > +  ret = cnd_timedwait(>signalled, >mutex, );
> > > +  if (ret != thrd_success)
> > > + break;
> > > +   }
> > > +   mtx_unlock(>mutex);
> > > +   return (f->count >= f->rank && ret == thrd_success);
>
> Hmm, you are reading from the fence object outside of the critical
> section, which doesn't sound safe. Maybe compute the return value
> before the mutex is unlocked?
>
>const boolean result = (f->count >= f->rank);
>mtx_unlock(>mutex);
>return result;
>
> Since f->rank is immutable and f->count never decreases, it might
> still be ok without this change, though it is racy.
>
In all fairness it shouldn't matters that much but it is the correct
thing regardless.
Will fixup and push in a bit. Thanks for the help Gustaw!

Aside: nearly all of mesa get this and the while loop around
cnd_timedwait wrong :-\
We should really fix that one of these days.

Bonus points: our c11 thread.h has a typo in thrd_timeDout (missing d)
and doesn't set/use it where needed.

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

[Mesa-dev] [PATCH v2 2/3] llvmpipe: correctly handle waiting in llvmpipe_fence_finish

2019-04-25 Thread Emil Velikov
Currently if the timeout differs from 0, we'll end up with infinite
wait... even if the user is perfectly clear they don't want that.

Use the new lp_fence_timedwait() helper guarding both waits in an
!lp_fence_signalled block like the rest of llvmpipe.

Signed-off-by: Emil Velikov 
Reviewed-by: Roland Scheidegger 
---
 src/gallium/drivers/llvmpipe/lp_screen.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c 
b/src/gallium/drivers/llvmpipe/lp_screen.c
index 8426427e397..510346d2abf 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -637,7 +637,12 @@ llvmpipe_fence_finish(struct pipe_screen *screen,
if (!timeout)
   return lp_fence_signalled(f);
 
-   lp_fence_wait(f);
+   if (!lp_fence_signalled(f)) {
+  if (timeout != PIPE_TIMEOUT_INFINITE)
+ return lp_fence_timedwait(f, timeout);
+
+  lp_fence_wait(f);
+   }
return TRUE;
 }
 
-- 
2.20.1

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

[Mesa-dev] [PATCH v2 1/3] llvmpipe: add lp_fence_timedwait() helper

2019-04-25 Thread Emil Velikov
The function is analogous to lp_fence_wait() while taking at timeout
(ns) parameter, as needed for EGL fence/sync.

v2:
 - use absolute UTC time, as per spec (Gustaw)
 - bail out on cnd_timedwait() failure (Gustaw)

Cc: Gustaw Smolarczyk 
Cc: Roland Scheidegger 
Signed-off-by: Emil Velikov 
Reviewed-by: Roland Scheidegger  (v1)
---
 src/gallium/drivers/llvmpipe/lp_fence.c | 30 +
 src/gallium/drivers/llvmpipe/lp_fence.h |  3 +++
 2 files changed, 33 insertions(+)

diff --git a/src/gallium/drivers/llvmpipe/lp_fence.c 
b/src/gallium/drivers/llvmpipe/lp_fence.c
index 20cd91cd63d..b79d773bf6c 100644
--- a/src/gallium/drivers/llvmpipe/lp_fence.c
+++ b/src/gallium/drivers/llvmpipe/lp_fence.c
@@ -125,3 +125,33 @@ lp_fence_wait(struct lp_fence *f)
 }
 
 
+boolean
+lp_fence_timedwait(struct lp_fence *f, uint64_t timeout)
+{
+   struct timespec ts;
+   int ret;
+
+   timespec_get(, TIME_UTC);
+
+   ts.tv_nsec += timeout % 10L;
+   ts.tv_sec += timeout / 10L;
+   if (ts.tv_nsec >= 10L) {
+  ts.tv_sec++;
+  ts.tv_nsec -= 10L;
+   }
+
+   if (LP_DEBUG & DEBUG_FENCE)
+  debug_printf("%s %d\n", __FUNCTION__, f->id);
+
+   mtx_lock(>mutex);
+   assert(f->issued);
+   while (f->count < f->rank) {
+  ret = cnd_timedwait(>signalled, >mutex, );
+  if (ret != thrd_success)
+ break;
+   }
+   mtx_unlock(>mutex);
+   return (f->count >= f->rank && ret == thrd_success);
+}
+
+
diff --git a/src/gallium/drivers/llvmpipe/lp_fence.h 
b/src/gallium/drivers/llvmpipe/lp_fence.h
index b72026492c6..5ba746d22d1 100644
--- a/src/gallium/drivers/llvmpipe/lp_fence.h
+++ b/src/gallium/drivers/llvmpipe/lp_fence.h
@@ -65,6 +65,9 @@ lp_fence_signalled(struct lp_fence *fence);
 void
 lp_fence_wait(struct lp_fence *fence);
 
+boolean
+lp_fence_timedwait(struct lp_fence *fence, uint64_t timeout);
+
 void
 llvmpipe_init_screen_fence_funcs(struct pipe_screen *screen);
 
-- 
2.20.1

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

[Mesa-dev] [PATCH v2 3/3] llvmpipe: Always return some fence in flush (v2)

2019-04-25 Thread Emil Velikov
From: Tomasz Figa 

If there is no last fence, due to no rendering happening yet, just
create a new signaled fence and return it, to match the expectations of
the EGL sync fence API.

Fixes random "Could not create sync fence 0x3003" assertion failures from
Skia on Android, coming from the following code:

https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp#427

Reproducible especially with thread count >= 4.

One could make the driver always keep the reference to the last fence,
but:

 - the driver seems to explicitly destroy the fence whenever a rendering
   pass completes and changing that would require a significant functional
   change to the code. (Specifically, in lp_scene_end_rasterization().)

 - it still wouldn't solve the problem of an EGL sync fence being created
   and waited on without any rendering happening at all, which is
   also likely to happen with Android code pointed to in the commit.

Therefore, the simple approach of always creating a fence is taken,
similarly to other drivers, such as radeonsi.

Tested with piglit llvmpipe suite with no regressions and following
tests fixed:

egl_khr_fence_sync
 conformance
  eglclientwaitsynckhr_flag_sync_flush
  eglclientwaitsynckhr_nonzero_timeout
  eglclientwaitsynckhr_zero_timeout
  eglcreatesynckhr_default_attributes
  eglgetsyncattribkhr_invalid_attrib
  eglgetsyncattribkhr_sync_status

v2:
 - remove the useless lp_fence_reference() dance (Nicolai),
 - explain why creating the dummy fence is the right approach.

Signed-off-by: Tomasz Figa 
Reviewed-by: Roland Scheidegger 
---
 src/gallium/drivers/llvmpipe/lp_setup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c 
b/src/gallium/drivers/llvmpipe/lp_setup.c
index b0873694732..e72e119c8a1 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup.c
@@ -361,6 +361,8 @@ lp_setup_flush( struct lp_setup_context *setup,
 
if (fence) {
   lp_fence_reference((struct lp_fence **)fence, setup->last_fence);
+  if (!*fence)
+ *fence = (struct pipe_fence_handle *)lp_fence_create(0);
}
 }
 
-- 
2.20.1

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

Re: [Mesa-dev] [PATCH 1/3] llvmpipe: add lp_fence_timedwait() helper

2019-04-25 Thread Emil Velikov
On Tue, 16 Apr 2019 at 11:50, Gustaw Smolarczyk  wrote:
>
> wt., 16 kwi 2019 o 12:11 Emil Velikov  napisał(a):
> >
> > On Thu, 11 Apr 2019 at 17:55, Gustaw Smolarczyk  
> > wrote:
> > >
> > > czw., 11 kwi 2019 o 18:06 Emil Velikov  
> > > napisał(a):
> > > >
> > > > The function is analogous to lp_fence_wait() while taking at timeout
> > > > (ns) parameter, as needed for EGL fence/sync.
> > > >
> > > > Cc: Roland Scheidegger 
> > > > Signed-off-by: Emil Velikov 
> > > > ---
> > > >  src/gallium/drivers/llvmpipe/lp_fence.c | 22 ++
> > > >  src/gallium/drivers/llvmpipe/lp_fence.h |  3 +++
> > > >  2 files changed, 25 insertions(+)
> > > >
> > > > diff --git a/src/gallium/drivers/llvmpipe/lp_fence.c 
> > > > b/src/gallium/drivers/llvmpipe/lp_fence.c
> > > > index 20cd91cd63d..f8b31a9d6a5 100644
> > > > --- a/src/gallium/drivers/llvmpipe/lp_fence.c
> > > > +++ b/src/gallium/drivers/llvmpipe/lp_fence.c
> > > > @@ -125,3 +125,25 @@ lp_fence_wait(struct lp_fence *f)
> > > >  }
> > > >
> > > >
> > > > +boolean
> > > > +lp_fence_timedwait(struct lp_fence *f, uint64_t timeout)
> > > > +{
> > > > +   struct timespec ts = {
> > > > +  .tv_nsec = timeout % 10L,
> > > > +  .tv_sec = timeout / 10L,
> > > > +   };
> > >
> > > According to the documentation [1] and looking at the implementation
> > > in mesa [2], cnd_timedwait accepts an absolute time in UTC, not
> > > duration. It seems that the fence_finish callback accepts duration.
> > >
> > Agreed, not sure how I missed that one.
> >
> > > [1] https://en.cppreference.com/w/c/thread/cnd_timedwait
> > > [2] 
> > > https://gitlab.freedesktop.org/mesa/mesa/blob/master/include/c11/threads_posix.h#L135
> > >
> > > > +   int ret;
> > > > +
> > > > +   if (LP_DEBUG & DEBUG_FENCE)
> > > > +  debug_printf("%s %d\n", __FUNCTION__, f->id);
> > > > +
> > > > +   mtx_lock(>mutex);
> > > > +   assert(f->issued);
> > > > +   while (f->count < f->rank) {
> > > > +  ret = cnd_timedwait(>signalled, >mutex, );
> > >
> > > Shouldn't ret be checked for thrd_busy here as well? Otherwise, the
> > > function will busy-wait after the timeout is reached instead of
> > > returning.
> > >
> > Actually this should be tweaked to:
> >  - only wait for the requested timeout
> >  - _regardless_ of how meany threads (and hence ranks) llvmpipe has
> >
> > Something like the following (warning pre-coffee code) should work.
> >
> >mtx_lock(>mutex);
> >assert(f->issued);
> >if (f->count < f->rank)
> >   ret = cnd_timedwait(>signalled, >mutex, );
> >
> >mtx_unlock(>mutex);
> >return (f->count >= f->rank && ret == thrd_success);
> >
>
> Is it a good idea not to perform cnd_timedwait in a loop? A spurious
> wake up could could shorten the wait time.
>
Thanks Gustaw - had no idea cnd_timedwait can spuriously return.

For anyone else wondering - the C11 spec does not mention anything,
but there's a bug [1] to fix that.
Underlying implementations (such as pthreads [2]) are already pretty
clear about this "interesting" behaviour.

Will send out and updated revision in a moment.

Thanks again.
Emil

[1] http://www.open-std.org/jtc1/sc22/wg14/www/docs/summary.htm#dr_480
[2] https://linux.die.net/man/3/pthread_cond_timedwait
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] vulkan/wsi: check if the display_fd given is master

2019-04-25 Thread Emil Velikov
On Fri, 19 Apr 2019 at 16:01, Emil Velikov  wrote:
>
> From: Emil Velikov 
>
> As effectively required by the extension, we need to ensure we're master
>
> Currently drivers employ vendor specific solutions, which check if the
> device behind the fd is capable*, yet none of them do the master check.
>
> *In the radv case, if acceleration is available.
>
> Instead of duplicating the check in each driver, keep it where it's
> needed and used.
>
> Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
> edge version of the library.
>
> v2: set the fd to -1 if not master (Bas)
>
> Cc: Keith Packard 
> Cc: Jason Ekstrand 
> Cc: Bas Nieuwenhuizen 
> Cc: Andres Rodriguez 
> Reported-by: Andres Rodriguez 
> Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
> Signed-off-by: Emil Velikov 
> ---
>  src/vulkan/wsi/wsi_common_display.c | 27 +++
>  1 file changed, 27 insertions(+)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c 
> b/src/vulkan/wsi/wsi_common_display.c
> index 74ed36ed646..2be20e85046 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -1812,6 +1812,30 @@ fail_attr_init:
> return ret;
>  }
>
> +
> +/*
> + * Local version fo the libdrm helper. Added to avoid depending on bleeding
> + * edge version of the library.
> + */
> +static int
> +local_drmIsMaster(int fd)
> +{
> +   /* Detect master by attempting something that requires master.
> +*
> +* Authenticating magic tokens requires master and 0 is an
> +* internal kernel detail which we could use. Attempting this on
> +* a master fd would fail therefore fail with EINVAL because 0
> +* is invalid.
> +*
> +* A non-master fd will fail with EACCES, as the kernel checks
> +* for master before attempting to do anything else.
> +*
> +* Since we don't want to leak implementation details, use
> +* EACCES.
> +*/
> +   return drmAuthMagic(fd, 0) != -EACCES;
> +}
> +
>  VkResult
>  wsi_display_init_wsi(struct wsi_device *wsi_device,
>   const VkAllocationCallbacks *alloc,
> @@ -1827,6 +1851,9 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
> }
>
> wsi->fd = display_fd;
> +   if (wsi->fd != -1 && !local_drmIsMaster(wsi->fd))
> +  wsi->fd = -1;
> +
> wsi->alloc = alloc;
>
Humble ping?

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

Re: [Mesa-dev] [PATCH] vulkan/wsi: don't use DUMB_CLOSE for normal GEM handles

2019-04-25 Thread Emil Velikov
On Fri, 19 Apr 2019 at 16:03, Emil Velikov  wrote:
>
> From: Emil Velikov 
>
> Currently we get normal GEM handles from PrimeFDToHandle, yet we close
> then with DUMB_CLOSE. Use GEM_CLOSE instead.
>
> Cc: Keith Packard 
> Cc: Jason Ekstrand 
> Cc: Bas Nieuwenhuizen 
> Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
> Signed-off-by: Emil Velikov 
> ---
>  src/vulkan/wsi/wsi_common_display.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/src/vulkan/wsi/wsi_common_display.c 
> b/src/vulkan/wsi/wsi_common_display.c
> index 2be20e85046..66e191906fc 100644
> --- a/src/vulkan/wsi/wsi_common_display.c
> +++ b/src/vulkan/wsi/wsi_common_display.c
> @@ -974,8 +974,8 @@ static void
>  wsi_display_destroy_buffer(struct wsi_display *wsi,
> uint32_t buffer)
>  {
> -   (void) drmIoctl(wsi->fd, DRM_IOCTL_MODE_DESTROY_DUMB,
> -   &((struct drm_mode_destroy_dumb) { .handle = buffer }));
> +   (void) drmIoctl(wsi->fd, DRM_IOCTL_GEM_CLOSE,
> +   &((struct drm_gem_close) { .handle = buffer }));
>  }
>
Humble ping anyone?

AFAICT closing handles from PrimeFDToHandle() with DUMB_CLOSE is a
violation, even if it somehow works today.

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

Re: [Mesa-dev] [PATCH v2] st: require compatible driver in autotools

2019-04-24 Thread Emil Velikov
On Wed, 24 Apr 2019 at 13:09, Emil Velikov  wrote:
>
> On Tue, 23 Apr 2019 at 23:10, Alyssa Ross  wrote:
> >
> > > Off the top of my head - none of the VL code should be build when we
> > > have only a swrast driver.
> > > Can you try and kill that bug, or shall I?
> >
> > Isn't that what this patch does?
> >
> > If there's only swrast, this patch prevents enabling any of the state
> > trackers, so need_gallium_vl won't be set, and VL won't be built.
>
> How about instead of tracking each driver and combination do somethings like:
>
> if no_gallium_drivers or gallium_drivers=swrast; then
>  all_vl_state_trackers=off
>
Having a closer look such code exists already at [1]

if test -n "$with_gallium_drivers" -a "x$with_gallium_drivers" != xswrast; then
if test "x$enable_xvmc" = xauto -a "x$have_xvmc_platform" = xyes; then
PKG_CHECK_EXISTS([xvmc >= $XVMC_REQUIRED], [enable_xvmc=yes],
[enable_xvmc=no])
fi
...
fi

Thus auto-detection will disable xvmc and other VL state-trackers,
when no gallium drivers or swrast only is set.
Thus the NEED_GALLIUM_VL_WINSYS shortly afterwords is set to
no/disabled, and vl_winsys_dri.c et al is not build.

A random old checkout commit 7be26976b8e8bc34fa7d0014197ed2af488f
seems happy with the following:

mkdir aa; cd aa;
../autogen.sh  --enable-autotools \
 --with-platforms=x11 \
 --with-dri-drivers= \
 --with-gallium-drivers=swrast \
 --disable-glx \
 --disable-dri3 \
 --disable-gbm

Am I missing something?
-Emil

[1] 
https://cgit.freedesktop.org/mesa/mesa/tree/configure.ac?h=19.0=d41acb4c9e46306e3e9cebe9c23de77c6f26ff93#n2322
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Re: [Mesa-dev] [PATCH v2] st: require compatible driver in autotools

2019-04-24 Thread Emil Velikov
On Tue, 23 Apr 2019 at 23:10, Alyssa Ross  wrote:
>
> > Off the top of my head - none of the VL code should be build when we
> > have only a swrast driver.
> > Can you try and kill that bug, or shall I?
>
> Isn't that what this patch does?
>
> If there's only swrast, this patch prevents enabling any of the state
> trackers, so need_gallium_vl won't be set, and VL won't be built.

How about instead of tracking each driver and combination do somethings like:

if no_gallium_drivers or gallium_drivers=swrast; then
 all_vl_state_trackers=off

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

Re: [Mesa-dev] [PATCH v2] st: require compatible driver in autotools

2019-04-23 Thread Emil Velikov
On Fri, 19 Apr 2019 at 19:53, Alyssa Ross  wrote:
>
> The meson build system already has these checks. I've just copied them
> to autotools.
>
> Without this, state trackers could be enabled when building with the
> following set of options, which resulted in a compile error due to VL
> being built without DRM.
>
>  --enable-autotools
>  --with-platforms=x11
>  --with-dri-drivers=
>  --with-gallium-drivers=swrast
>  --disable-glx
>  --disable-dri3
>  --disable-gbm
>
> The compile error was:
>
> vl/vl_winsys_dri.c:36:10: fatal error: xf86drm.h: No such file or 
> directory
>  #include 
>   ^~~
> compilation terminated.
>
I was in favour of the the explicit driver/st tracking yet it was
nuked with commit 28e84c93bb33dfc8ac5219480b274c84dbd58762
To address the above use-case, we don't need to reinstate the tracking
- it causes a lot of churn.

Off the top of my head - none of the VL code should be build when we
have only a swrast driver.
Can you try and kill that bug, or shall I?

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

[Mesa-dev] [PATCH] vulkan/wsi: don't use DUMB_CLOSE for normal GEM handles

2019-04-19 Thread Emil Velikov
From: Emil Velikov 

Currently we get normal GEM handles from PrimeFDToHandle, yet we close
then with DUMB_CLOSE. Use GEM_CLOSE instead.

Cc: Keith Packard 
Cc: Jason Ekstrand 
Cc: Bas Nieuwenhuizen 
Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
Signed-off-by: Emil Velikov 
---
 src/vulkan/wsi/wsi_common_display.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c
index 2be20e85046..66e191906fc 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -974,8 +974,8 @@ static void
 wsi_display_destroy_buffer(struct wsi_display *wsi,
uint32_t buffer)
 {
-   (void) drmIoctl(wsi->fd, DRM_IOCTL_MODE_DESTROY_DUMB,
-   &((struct drm_mode_destroy_dumb) { .handle = buffer }));
+   (void) drmIoctl(wsi->fd, DRM_IOCTL_GEM_CLOSE,
+   &((struct drm_gem_close) { .handle = buffer }));
 }
 
 static VkResult
-- 
2.21.0

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

[Mesa-dev] [PATCH] turnip: drop dead close(master_fd)

2019-04-19 Thread Emil Velikov
From: Emil Velikov 

The fd is -1, thus the block of if (fd != -1) close(fd) is dead code.

Cc: Chad Versace 
Cc: Chia-I Wu 
Signed-off-by: Emil Velikov 
---
 src/freedreno/vulkan/tu_device.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/freedreno/vulkan/tu_device.c b/src/freedreno/vulkan/tu_device.c
index c7f4d0b26eb..1268624bd8d 100644
--- a/src/freedreno/vulkan/tu_device.c
+++ b/src/freedreno/vulkan/tu_device.c
@@ -198,8 +198,6 @@ tu_physical_device_init(struct tu_physical_device *device,
 
if (strcmp(version->name, "msm")) {
   drmFreeVersion(version);
-  if (master_fd != -1)
- close(master_fd);
   close(fd);
   return vk_errorf(instance, VK_ERROR_INCOMPATIBLE_DRIVER,
"device %s does not use the msm kernel driver", path);
-- 
2.21.0

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

[Mesa-dev] [PATCH v2] vulkan/wsi: check if the display_fd given is master

2019-04-19 Thread Emil Velikov
From: Emil Velikov 

As effectively required by the extension, we need to ensure we're master

Currently drivers employ vendor specific solutions, which check if the
device behind the fd is capable*, yet none of them do the master check.

*In the radv case, if acceleration is available.

Instead of duplicating the check in each driver, keep it where it's
needed and used.

Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
edge version of the library.

v2: set the fd to -1 if not master (Bas)

Cc: Keith Packard 
Cc: Jason Ekstrand 
Cc: Bas Nieuwenhuizen 
Cc: Andres Rodriguez 
Reported-by: Andres Rodriguez 
Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
Signed-off-by: Emil Velikov 
---
 src/vulkan/wsi/wsi_common_display.c | 27 +++
 1 file changed, 27 insertions(+)

diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c
index 74ed36ed646..2be20e85046 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1812,6 +1812,30 @@ fail_attr_init:
return ret;
 }
 
+
+/*
+ * Local version fo the libdrm helper. Added to avoid depending on bleeding
+ * edge version of the library.
+ */
+static int
+local_drmIsMaster(int fd)
+{
+   /* Detect master by attempting something that requires master.
+*
+* Authenticating magic tokens requires master and 0 is an
+* internal kernel detail which we could use. Attempting this on
+* a master fd would fail therefore fail with EINVAL because 0
+* is invalid.
+*
+* A non-master fd will fail with EACCES, as the kernel checks
+* for master before attempting to do anything else.
+*
+* Since we don't want to leak implementation details, use
+* EACCES.
+*/
+   return drmAuthMagic(fd, 0) != -EACCES;
+}
+
 VkResult
 wsi_display_init_wsi(struct wsi_device *wsi_device,
  const VkAllocationCallbacks *alloc,
@@ -1827,6 +1851,9 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
}
 
wsi->fd = display_fd;
+   if (wsi->fd != -1 && !local_drmIsMaster(wsi->fd))
+  wsi->fd = -1;
+
wsi->alloc = alloc;
 
list_inithead(>connectors);
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH] egl: Fix EGL_PLATFORM_X11_SCREEN_KHR handling

2019-04-18 Thread Emil Velikov
Hi Adam,

On Wed, 17 Apr 2019 at 19:57, Adam Jackson  wrote:
>
> If this was specified and a non-NULL display was passed to
> eglGetPlatformDisplay, we would ignore the attribute and instead use
> whatever xcb thought the default screen would be.
>
> To fix this, store a copy of the attribute list in the _EGLDisplay, and
> use that to look up any non-default screen number if we need it.
> ---
>  src/egl/drivers/dri2/platform_x11.c | 28 --
>  src/egl/main/eglapi.c   |  2 +-
>  src/egl/main/egldisplay.c   | 85 +++--
>  src/egl/main/egldisplay.h   |  4 +-
>  4 files changed, 96 insertions(+), 23 deletions(-)
>
We have multiple different things happening here. Can you split things up a bit?

1. Flesh out a simpler version of _eglNumAttribs based off
_eglConvertIntsToAttribs. For example:

len = 0;
if (attribs) {
   while (attribs[len] != EGL_NONE)
  len += 2;
   len++;
}
return len;

2. Optional: update the codebase to use ^^ helper
3. Introduce Options::Attribs, populate and compare
Note: add extra wording why compare is safe - as-is it will cause
functional changes in some corner-cases
4. update platform_x11.c to honour the attribs
5. Kill off Options::Platform - _eglParseX11DisplayAttribList and elsewhere.
6. Optional: move _eglParseX11DisplayAttribList validation before the
_eglFindDisplay() ... just like all !X11 platforms do.

Nit: Make _eglSameAttribs return a bool - an c99 or EGL one.

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

Re: [Mesa-dev] [PATCH] vulkan/wsi: check if the display_fd given is master

2019-04-17 Thread Emil Velikov
On Wed, 17 Apr 2019 at 19:25, Bas Nieuwenhuizen  
wrote:
>
> This will not work as-is for radv, as failure to initialize from
> wsi_display_init_wsi (->wsi_device_init -> radv_init_wsi) will cause
> us to fail initializing the whole device.
>
Indeed - same applies across the board.

I'll send out v2 tomorrow - after testing it on ANV and RADV.

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

[Mesa-dev] [PATCH] vulkan/wsi: check if the display_fd given is master

2019-04-17 Thread Emil Velikov
From: Emil Velikov 

As effectively required by the extension, we need to ensure we're master

Currently drivers employ vendor specific solutions, which check if the
device behind the fd is capable*, yet none of them do the master check.

*In the radv case, if acceleration is available.

Instead of duplicating the check in each driver, keep it where it's
needed and used.

Note this copies libdrm's drmIsMaster() to avoid depending on bleeding
edge version of the library.

Cc: Keith Packard 
Cc: Jason Ekstrand 
Cc: Bas Nieuwenhuizen 
Cc: Andres Rodriguez 
Reported-by: Andres Rodriguez 
Fixes: da997ebec92 ("vulkan: Add KHR_display extension using DRM [v10]")
Signed-off-by: Emil Velikov 
---
 src/vulkan/wsi/wsi_common_display.c | 30 +
 1 file changed, 30 insertions(+)

diff --git a/src/vulkan/wsi/wsi_common_display.c 
b/src/vulkan/wsi/wsi_common_display.c
index 74ed36ed646..d6b2ae004ce 100644
--- a/src/vulkan/wsi/wsi_common_display.c
+++ b/src/vulkan/wsi/wsi_common_display.c
@@ -1812,6 +1812,30 @@ fail_attr_init:
return ret;
 }
 
+
+/*
+ * Local version fo the libdrm helper. Added to avoid depending on bleeding
+ * edge version of the library.
+ */
+static int
+local_drmIsMaster(int fd)
+{
+   /* Detect master by attempting something that requires master.
+*
+* Authenticating magic tokens requires master and 0 is an
+* internal kernel detail which we could use. Attempting this on
+* a master fd would fail therefore fail with EINVAL because 0
+* is invalid.
+*
+* A non-master fd will fail with EACCES, as the kernel checks
+* for master before attempting to do anything else.
+*
+* Since we don't want to leak implementation details, use
+* EACCES.
+*/
+   return drmAuthMagic(fd, 0) != -EACCES;
+}
+
 VkResult
 wsi_display_init_wsi(struct wsi_device *wsi_device,
  const VkAllocationCallbacks *alloc,
@@ -1826,6 +1850,11 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
   goto fail;
}
 
+   if (!local_drmIsMaster(display_fd)) {
+  result = VK_ERROR_INITIALIZATION_FAILED;
+  goto fail_fd;
+   }
+
wsi->fd = display_fd;
wsi->alloc = alloc;
 
@@ -1857,6 +1886,7 @@ wsi_display_init_wsi(struct wsi_device *wsi_device,
 fail_cond:
pthread_mutex_destroy(>wait_mutex);
 fail_mutex:
+fail_fd:
vk_free(alloc, wsi);
 fail:
return result;
-- 
2.21.0

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

Re: [Mesa-dev] [PATCH 1/3] llvmpipe: add lp_fence_timedwait() helper

2019-04-16 Thread Emil Velikov
On Thu, 11 Apr 2019 at 17:55, Gustaw Smolarczyk  wrote:
>
> czw., 11 kwi 2019 o 18:06 Emil Velikov  napisał(a):
> >
> > The function is analogous to lp_fence_wait() while taking at timeout
> > (ns) parameter, as needed for EGL fence/sync.
> >
> > Cc: Roland Scheidegger 
> > Signed-off-by: Emil Velikov 
> > ---
> >  src/gallium/drivers/llvmpipe/lp_fence.c | 22 ++
> >  src/gallium/drivers/llvmpipe/lp_fence.h |  3 +++
> >  2 files changed, 25 insertions(+)
> >
> > diff --git a/src/gallium/drivers/llvmpipe/lp_fence.c 
> > b/src/gallium/drivers/llvmpipe/lp_fence.c
> > index 20cd91cd63d..f8b31a9d6a5 100644
> > --- a/src/gallium/drivers/llvmpipe/lp_fence.c
> > +++ b/src/gallium/drivers/llvmpipe/lp_fence.c
> > @@ -125,3 +125,25 @@ lp_fence_wait(struct lp_fence *f)
> >  }
> >
> >
> > +boolean
> > +lp_fence_timedwait(struct lp_fence *f, uint64_t timeout)
> > +{
> > +   struct timespec ts = {
> > +  .tv_nsec = timeout % 10L,
> > +  .tv_sec = timeout / 10L,
> > +   };
>
> According to the documentation [1] and looking at the implementation
> in mesa [2], cnd_timedwait accepts an absolute time in UTC, not
> duration. It seems that the fence_finish callback accepts duration.
>
Agreed, not sure how I missed that one.

> [1] https://en.cppreference.com/w/c/thread/cnd_timedwait
> [2] 
> https://gitlab.freedesktop.org/mesa/mesa/blob/master/include/c11/threads_posix.h#L135
>
> > +   int ret;
> > +
> > +   if (LP_DEBUG & DEBUG_FENCE)
> > +  debug_printf("%s %d\n", __FUNCTION__, f->id);
> > +
> > +   mtx_lock(>mutex);
> > +   assert(f->issued);
> > +   while (f->count < f->rank) {
> > +  ret = cnd_timedwait(>signalled, >mutex, );
>
> Shouldn't ret be checked for thrd_busy here as well? Otherwise, the
> function will busy-wait after the timeout is reached instead of
> returning.
>
Actually this should be tweaked to:
 - only wait for the requested timeout
 - _regardless_ of how meany threads (and hence ranks) llvmpipe has

Something like the following (warning pre-coffee code) should work.

   mtx_lock(>mutex);
   assert(f->issued);
   if (f->count < f->rank)
  ret = cnd_timedwait(>signalled, >mutex, );

   mtx_unlock(>mutex);
   return (f->count >= f->rank && ret == thrd_success);

Thanks for the review :-)
-Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

[Mesa-dev] [PATCH 1/3] llvmpipe: add lp_fence_timedwait() helper

2019-04-11 Thread Emil Velikov
The function is analogous to lp_fence_wait() while taking at timeout
(ns) parameter, as needed for EGL fence/sync.

Cc: Roland Scheidegger 
Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/llvmpipe/lp_fence.c | 22 ++
 src/gallium/drivers/llvmpipe/lp_fence.h |  3 +++
 2 files changed, 25 insertions(+)

diff --git a/src/gallium/drivers/llvmpipe/lp_fence.c 
b/src/gallium/drivers/llvmpipe/lp_fence.c
index 20cd91cd63d..f8b31a9d6a5 100644
--- a/src/gallium/drivers/llvmpipe/lp_fence.c
+++ b/src/gallium/drivers/llvmpipe/lp_fence.c
@@ -125,3 +125,25 @@ lp_fence_wait(struct lp_fence *f)
 }
 
 
+boolean
+lp_fence_timedwait(struct lp_fence *f, uint64_t timeout)
+{
+   struct timespec ts = {
+  .tv_nsec = timeout % 10L,
+  .tv_sec = timeout / 10L,
+   };
+   int ret;
+
+   if (LP_DEBUG & DEBUG_FENCE)
+  debug_printf("%s %d\n", __FUNCTION__, f->id);
+
+   mtx_lock(>mutex);
+   assert(f->issued);
+   while (f->count < f->rank) {
+  ret = cnd_timedwait(>signalled, >mutex, );
+   }
+   mtx_unlock(>mutex);
+   return ret == thrd_success;
+}
+
+
diff --git a/src/gallium/drivers/llvmpipe/lp_fence.h 
b/src/gallium/drivers/llvmpipe/lp_fence.h
index b72026492c6..5ba746d22d1 100644
--- a/src/gallium/drivers/llvmpipe/lp_fence.h
+++ b/src/gallium/drivers/llvmpipe/lp_fence.h
@@ -65,6 +65,9 @@ lp_fence_signalled(struct lp_fence *fence);
 void
 lp_fence_wait(struct lp_fence *fence);
 
+boolean
+lp_fence_timedwait(struct lp_fence *fence, uint64_t timeout);
+
 void
 llvmpipe_init_screen_fence_funcs(struct pipe_screen *screen);
 
-- 
2.20.1

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

[Mesa-dev] [PATCH 3/3] llvmpipe: Always return some fence in flush (v2)

2019-04-11 Thread Emil Velikov
From: Tomasz Figa 

If there is no last fence, due to no rendering happening yet, just
create a new signaled fence and return it, to match the expectations of
the EGL sync fence API.

Fixes random "Could not create sync fence 0x3003" assertion failures from
Skia on Android, coming from the following code:

https://android.googlesource.com/platform/frameworks/base/+/master/libs/hwui/pipeline/skia/SkiaOpenGLPipeline.cpp#427

Reproducible especially with thread count >= 4.

One could make the driver always keep the reference to the last fence,
but:

 - the driver seems to explicitly destroy the fence whenever a rendering
   pass completes and changing that would require a significant functional
   change to the code. (Specifically, in lp_scene_end_rasterization().)

 - it still wouldn't solve the problem of an EGL sync fence being created
   and waited on without any rendering happening at all, which is
   also likely to happen with Android code pointed to in the commit.

Therefore, the simple approach of always creating a fence is taken,
similarly to other drivers, such as radeonsi.

Tested with piglit llvmpipe suite with no regressions and following
tests fixed:

egl_khr_fence_sync
 conformance
  eglclientwaitsynckhr_flag_sync_flush
  eglclientwaitsynckhr_nonzero_timeout
  eglclientwaitsynckhr_zero_timeout
  eglcreatesynckhr_default_attributes
  eglgetsyncattribkhr_invalid_attrib
  eglgetsyncattribkhr_sync_status

v2:
 - remove the useless lp_fence_reference() dance (Nicolai),
 - explain why creating the dummy fence is the right approach.

Cc: Roland Scheidegger 
Signed-off-by: Tomasz Figa 
---
 src/gallium/drivers/llvmpipe/lp_setup.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/gallium/drivers/llvmpipe/lp_setup.c 
b/src/gallium/drivers/llvmpipe/lp_setup.c
index b0873694732..e72e119c8a1 100644
--- a/src/gallium/drivers/llvmpipe/lp_setup.c
+++ b/src/gallium/drivers/llvmpipe/lp_setup.c
@@ -361,6 +361,8 @@ lp_setup_flush( struct lp_setup_context *setup,
 
if (fence) {
   lp_fence_reference((struct lp_fence **)fence, setup->last_fence);
+  if (!*fence)
+ *fence = (struct pipe_fence_handle *)lp_fence_create(0);
}
 }
 
-- 
2.20.1

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

[Mesa-dev] [PATCH 2/3] llvmpipe: correctly handle waiting in llvmpipe_fence_finish

2019-04-11 Thread Emil Velikov
Currently if the timeout differs from 0, we'll end up with infinite
wait... even if the user is perfectly clear they don't want that.

Use the new lp_fence_timedwait() helper guarding both waits in an
!lp_fence_signalled block like the rest of llvmpipe.

Cc: Roland Scheidegger 
Signed-off-by: Emil Velikov 
---
 src/gallium/drivers/llvmpipe/lp_screen.c | 7 ++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/gallium/drivers/llvmpipe/lp_screen.c 
b/src/gallium/drivers/llvmpipe/lp_screen.c
index b55b4a3c4fe..3aa8b9fbcc9 100644
--- a/src/gallium/drivers/llvmpipe/lp_screen.c
+++ b/src/gallium/drivers/llvmpipe/lp_screen.c
@@ -636,7 +636,12 @@ llvmpipe_fence_finish(struct pipe_screen *screen,
if (!timeout)
   return lp_fence_signalled(f);
 
-   lp_fence_wait(f);
+   if (!lp_fence_signalled(f)) {
+  if (timeout != PIPE_TIMEOUT_INFINITE)
+ return lp_fence_timedwait(f, timeout);
+
+  lp_fence_wait(f);
+   }
return TRUE;
 }
 
-- 
2.20.1

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

Re: [Mesa-dev] [PATCH 1/2] egl: add optional plat_opt to _eglFindDisplay()

2019-04-08 Thread Emil Velikov
On Thu, 4 Apr 2019 at 18:05, Adam Jackson  wrote:
>
> On Thu, 2019-04-04 at 16:25 +0100, Emil Velikov wrote:
>
> > I'm not a huge fan of this. Yet again the other platform (x11) that uses
> > these has a number of issues, see below for details. Once those are resolved
> > we could try and make this more uniform.
> >
> >  a) spec does not mention if a new (or the existing) EGLDislay should be
> > given for same native_dpy/screen_no combo.
>
> An EGLDisplay maps to an X11 screen. EGL_KHR_platform_x11 says:
>
> > On the X11 platform, an EGLDisplay refers to a specific X11 screen 
> > rather
> > than an X11 display connection.
>
> And EGL 1.5 says (with similar text in EGL_EXT_platform_base):
>
> > Multiple calls made to eglGetPlatformDisplay with the same parameters will
> > return the same EGLDisplay handle.
>
> EGL_KHR_platform_x11 is a little confusing on this point in one of the
> issues, because it says a new display connection is created for
> EGL_DEFAULT_DISPLAY; presumably that's only for the first EGLDisplay
> you create though.
>
> >  b) the implementation has number of issues:
> >  - screen 0 is hardcoded when using DEFAULT_DISPLAY
>
> False, but:
>
> >  - screen is _ignored_ when using !DEFAULT_DISPLAY
>
> true, but easily fixed. We definitely store the screen number in
> disp->Options.Platform in _eglParseX11DisplayAttribList, and then:
>
> > static EGLBoolean
> > dri2_get_xcb_connection(_EGLDriver *drv, _EGLDisplay *disp,
> > struct dri2_egl_display *dri2_dpy)
> > {
> >xcb_screen_iterator_t s;
> >int screen = (uintptr_t)disp->Options.Platform;
> >const char *msg;
> >
> >disp->DriverData = (void *) dri2_dpy;
> >if (disp->PlatformDisplay == NULL) {
> >   dri2_dpy->conn = xcb_connect(NULL, );
> >   dri2_dpy->own_device = true;
>
> If no display was given, we connect however xcb would, and save what it
> decided was the default screen number.
>
> >} else {
> >   Display *dpy = disp->PlatformDisplay;
> >
> >   dri2_dpy->conn = XGetXCBConnection(dpy);
> >   screen = DefaultScreen(dpy);
> >}
>
> If not, we use what the display already had set; this is where we
> ignore the user's screen number and instead use the display's default.
>
> I think the fix is to delete that DefaultScreen line above, and in
> _eglFindDisplay call the appropriate platform's vtable to check for
> identical option lists (this is your _eglSameDeviceDisplay in the next
> patch, just abstracted out a bit).
>
Thanks for having a look Adam.

Can I interest you in writing a fix for the X11 code ;-)
It'll be great if we have inline comments with the spec quotes and
reasoning you mentioned.

-Emil
___
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   10   >