Re: [Mesa-dev] Mesa Querying Devices Filter
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
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]
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
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
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
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
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.
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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
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.
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
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.
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.
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.
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
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
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
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
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
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.
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()
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
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
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()
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.
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
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
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()
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
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
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
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
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
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
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
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.
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
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
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
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
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
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
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
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
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"
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
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
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
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
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()
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
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
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
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
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
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
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
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
*** 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
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
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()
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
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
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
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
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
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
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)
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
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
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
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
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
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
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
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)
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
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
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
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
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
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
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)
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
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()
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