Re: [Mesa-dev] [PATCH 15/15] ac/nir: fix saturate emission
This patch is Reviewed-by: Bas Nieuwenhuizen On Tue, Aug 8, 2017 at 3:45 AM, Connor Abbott wrote: > From: Connor Abbott > > The .f32 was already getting added by emit_intrin_2f_param(). Noticed > when enabling LLVM module verification. > --- > src/amd/common/ac_nir_to_llvm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/src/amd/common/ac_nir_to_llvm.c b/src/amd/common/ac_nir_to_llvm.c > index bafe4d3..46e15c9 100644 > --- a/src/amd/common/ac_nir_to_llvm.c > +++ b/src/amd/common/ac_nir_to_llvm.c > @@ -5337,8 +5337,8 @@ static LLVMValueRef > emit_float_saturate(struct ac_llvm_context *ctx, LLVMValueRef v, float lo, > float hi) > { > v = ac_to_float(ctx, v); > - v = emit_intrin_2f_param(ctx, "llvm.maxnum.f32", ctx->f32, v, > LLVMConstReal(ctx->f32, lo)); > - return emit_intrin_2f_param(ctx, "llvm.minnum.f32", ctx->f32, v, > LLVMConstReal(ctx->f32, hi)); > + v = emit_intrin_2f_param(ctx, "llvm.maxnum", ctx->f32, v, > LLVMConstReal(ctx->f32, lo)); > + return emit_intrin_2f_param(ctx, "llvm.minnum", ctx->f32, v, > LLVMConstReal(ctx->f32, hi)); > } > > > -- > 2.9.4 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/2] glx/dri: Attempt to fix GLX_OML_swap_method
The current implementation was suffering from the following problems: 1) The driGetConfigAttribIndex function was not returning any value for the driconfig swapMethod. 2) The X server GLX code is using the value obtained from the AIGLX dri driver driGetConfigAttribIndex to populate its GLX fbconfig. 3) The client side GLX code was assuming fbconfig and driconfig match regardless of whether there actually was a swapMethod match. 4) We were using GLX tokens in the dri code. 5) dri3 does currently not support GLX_SWAP_COPY_OML, even if that's advertized by the gallium dri2 state tracker. Attempt to fix 1-4. Some highlights: Because of 1) in combination with 2), if the X server uses an AIGLX dri driver that doesn't have this fix, the GLX transmitted fbconfig swapMethod value will be bogus. So if we, on the client side detect a bogus value, change it to GLX_SWAP_UNDEFINED_OML. We define new __DRI_ATTRIB_XXX tokens for the swap method to fix 4). But due to 2) they need to take the same values as the GLX tokens. Signed-off-by: Thomas Hellstrom --- include/GL/internal/dri_interface.h | 17 - src/gallium/state_trackers/dri/dri_screen.c | 3 ++- src/glx/dri_common.c | 17 ++--- src/glx/glxext.c | 12 +++- src/mesa/drivers/dri/common/dri_util.c| 2 +- src/mesa/drivers/dri/common/utils.c | 8 ++-- src/mesa/drivers/dri/i915/intel_screen.c | 2 +- src/mesa/drivers/dri/i965/intel_screen.c | 2 +- src/mesa/drivers/dri/nouveau/nouveau_screen.c | 2 +- src/mesa/drivers/dri/radeon/radeon_screen.c | 6 ++ src/mesa/drivers/dri/swrast/swrast.c | 2 +- 11 files changed, 52 insertions(+), 21 deletions(-) diff --git a/include/GL/internal/dri_interface.h b/include/GL/internal/dri_interface.h index f676ac5..2cbd738 100644 --- a/include/GL/internal/dri_interface.h +++ b/include/GL/internal/dri_interface.h @@ -724,11 +724,26 @@ struct __DRIuseInvalidateExtensionRec { #define __DRI_ATTRIB_TEXTURE_2D_BIT0x02 #define __DRI_ATTRIB_TEXTURE_RECTANGLE_BIT 0x04 +/* __DRI_ATTRIB_SWAP_METHOD */ +/* Note that with the exception of __DRI_ATTRIB_SWAP_NONE, we need to define + * the same tokens as GLX. This is because old and current X servers will + * transmit the driconf value grabbed from the AIGLX driver untranslated as + * the GLX fbconfig value. __DRI_ATTRIB_SWAP_NONE is only used by dri drivers + * to signal to the dri core that the driconfig is single-buffer. + */ +#define __DRI_ATTRIB_SWAP_NONE 0x +#define __DRI_ATTRIB_SWAP_EXCHANGE 0x8061 +#define __DRI_ATTRIB_SWAP_COPY 0x8062 +#define __DRI_ATTRIB_SWAP_UNDEFINED 0x8063 + /** * This extension defines the core DRI functionality. + * + * Version >= 2 indicates that getConfigAttrib with __DRI_ATTRIB_SWAP_METHOD + * returns a reliable value. */ #define __DRI_CORE "DRI_Core" -#define __DRI_CORE_VERSION 1 +#define __DRI_CORE_VERSION 2 struct __DRIcoreExtensionRec { __DRIextension base; diff --git a/src/gallium/state_trackers/dri/dri_screen.c b/src/gallium/state_trackers/dri/dri_screen.c index 406e97d..1d9f441 100644 --- a/src/gallium/state_trackers/dri/dri_screen.c +++ b/src/gallium/state_trackers/dri/dri_screen.c @@ -156,7 +156,8 @@ dri_fill_in_modes(struct dri_screen *screen) boolean mixed_color_depth; static const GLenum back_buffer_modes[] = { - GLX_NONE, GLX_SWAP_UNDEFINED_OML, GLX_SWAP_COPY_OML + __DRI_ATTRIB_SWAP_NONE, __DRI_ATTRIB_SWAP_UNDEFINED, + __DRI_ATTRIB_SWAP_COPY }; if (driQueryOptionb(&screen->dev->option_cache, "always_have_depth_buffer")) { diff --git a/src/glx/dri_common.c b/src/glx/dri_common.c index 854733a..e2bbd48 100644 --- a/src/glx/dri_common.c +++ b/src/glx/dri_common.c @@ -242,10 +242,8 @@ static const struct __ATTRIB(__DRI_ATTRIB_MAX_PBUFFER_PIXELS, maxPbufferPixels), __ATTRIB(__DRI_ATTRIB_OPTIMAL_PBUFFER_WIDTH, optimalPbufferWidth), __ATTRIB(__DRI_ATTRIB_OPTIMAL_PBUFFER_HEIGHT, optimalPbufferHeight), -#if 0 __ATTRIB(__DRI_ATTRIB_SWAP_METHOD, swapMethod), -#endif -__ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb), + __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGB, bindToTextureRgb), __ATTRIB(__DRI_ATTRIB_BIND_TO_TEXTURE_RGBA, bindToTextureRgba), __ATTRIB(__DRI_ATTRIB_BIND_TO_MIPMAP_TEXTURE, bindToMipmapTexture), @@ -319,6 +317,19 @@ driConfigEqual(const __DRIcoreExtension *core, return GL_FALSE; break; + case __DRI_ATTRIB_SWAP_METHOD: + if (value == __DRI_ATTRIB_SWAP_EXCHANGE) +glxValue = GLX_SWAP_EXCHANGE_OML; + else if (value == __DRI_ATTRIB_SWAP_COPY) +glxValue = GLX_SWAP_COPY_OML; + else +glxValue = GLX_SWAP_UNDEFINED_OML; + + if (!scalarEqual(config, attrib, glxValue)) +return GL_FALSE;
Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon
On 08/05/2017 02:25 AM, Emil Velikov wrote: From: Emil Velikov As mentioned in previous commit the negative tests in dEQP expect the arguments to be evaluated in particular order. Namely - first the dpy, then the config, followed by the surface/window. Move the check further down or executing the test below will produce the following error. dEQP-EGL.functional.negative_api.create_pbuffer_surface eglCreateWindowSurface(0x9bfff0f150, 0x, 0x, { EGL_NONE }); // 0x returned // ERROR expected: EGL_BAD_CONFIG, Got: EGL_BAD_NATIVE_WINDOW Cc: Cc: Mark Janes Cc: Chad Versace Signed-off-by: Emil Velikov --- Mark, IMHO the CI does the impossible and passes the test. Perhaps it's worth looking into how/why it does so - I don't know. For me the above test is passing fine on x11 (x11_egl). I'll pipe the series through Jenkins tomorrow - don't want to stall things for the guys still working. Chad, I see that in the EGL_MESA_surfaceless implementation you explicitly mentioned that the surface is checked prior to the config. Wouldn't it be better to stay consistent and move those, as per the above? AFAICT the spec does not explicitly dictates the order. --- src/egl/main/eglapi.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c index 3ca3dd4c7c1..3b0f896f74c 100644 --- a/src/egl/main/eglapi.c +++ b/src/egl/main/eglapi.c @@ -872,10 +872,6 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, EGLConfig config, _EGLSurface *surf; EGLSurface ret; - - if (native_window == NULL) - RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE); - #ifdef HAVE_SURFACELESS_PLATFORM if (disp && disp->Platform == _EGL_PLATFORM_SURFACELESS) { /* From the EGL_MESA_platform_surfaceless spec (v1): @@ -899,6 +895,9 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, EGLConfig config, if ((conf->SurfaceType & EGL_WINDOW_BIT) == 0) RETURN_EGL_ERROR(disp, EGL_BAD_MATCH, EGL_NO_SURFACE); + if (native_window == NULL) + RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE); + surf = drv->API.CreateWindowSurface(drv, disp, conf, native_window, attrib_list); ret = (surf) ? _eglLinkSurface(surf) : EGL_NO_SURFACE; ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] configure: Trust LLVM >= 4.0 llvm-config --libs for shared libraries
From: Michel Dänzer No need to manually look for the library files anymore with current LLVM. This sidesteps the manual method failing when LLVM was built with -DLLVM_APPEND_VC_REV=ON. (This might already work with older versions of LLVM) Signed-off-by: Michel Dänzer --- configure.ac | 58 ++ 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/configure.ac b/configure.ac index 5b12dd8506..4992e9538f 100644 --- a/configure.ac +++ b/configure.ac @@ -2623,35 +2623,37 @@ if test "x$enable_llvm" = xyes; then fi LLVM_LIBS="`$LLVM_CONFIG --libs ${LLVM_COMPONENTS}`" -dnl llvm-config may not give the right answer when llvm is a built as a -dnl single shared library, so we must work the library name out for -dnl ourselves. -dnl (See https://llvm.org/bugs/show_bug.cgi?id=6823) if test "x$enable_llvm_shared_libs" = xyes; then -dnl We can't use $LLVM_VERSION because it has 'svn' stripped out, -LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version` -AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], [llvm_have_one_so=yes]) - -if test "x$llvm_have_one_so" = xyes; then -dnl LLVM was built using auto*, so there is only one shared object. -LLVM_LIBS="-l$LLVM_SO_NAME" -else -dnl If LLVM was built with CMake, there will be one shared object per -dnl component. -AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"], -[AC_MSG_ERROR([Could not find llvm shared libraries: - Please make sure you have built llvm with the --enable-shared option - and that your llvm libraries are installed in $LLVM_LIBDIR - If you have installed your llvm libraries to a different directory you - can use the --with-llvm-prefix= configure flag to specify this directory. - NOTE: Mesa is attempting to use llvm shared libraries by default. - If you do not want to build with llvm shared libraries and instead want to - use llvm static libraries then add --disable-llvm-shared-libs to your configure - invocation and rebuild.])]) - - dnl We don't need to update LLVM_LIBS in this case because the LLVM - dnl install uses a shared object for each component and we have - dnl already added all of these objects to LLVM_LIBS. +if test $LLVM_VERSION_MAJOR -lt 4; then +dnl llvm-config may not give the right answer when llvm is a built as a +dnl single shared library, so we must work the library name out for +dnl ourselves. +dnl (See https://llvm.org/bugs/show_bug.cgi?id=6823) +dnl We can't use $LLVM_VERSION because it has 'svn' stripped out, +LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version` +AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], [llvm_have_one_so=yes]) + +if test "x$llvm_have_one_so" = xyes; then + dnl LLVM was built using auto*, so there is only one shared object. + LLVM_LIBS="-l$LLVM_SO_NAME" +else +dnl If LLVM was built with CMake, there will be one shared object per +dnl component. +AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"], + [AC_MSG_ERROR([Could not find llvm shared libraries: + Please make sure you have built llvm with the --enable-shared option + and that your llvm libraries are installed in $LLVM_LIBDIR + If you have installed your llvm libraries to a different directory you + can use the --with-llvm-prefix= configure flag to specify this directory. + NOTE: Mesa is attempting to use llvm shared libraries by default. + If you do not want to build with llvm shared libraries and instead want to + use llvm static libraries then add --disable-llvm-shared-libs to your configure + invocation and rebuild.])]) + +dnl We don't need to update LLVM_LIBS in this case because the LLVM +dnl install uses a shared object for each component and we have +dnl already added all of these objects to LLVM_LIBS. +fi fi else AC_MSG_WARN([Building mesa with statically linked LLVM may cause compilation issues]) -- 2.13.3 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] i965: Drop uniform buffer alignment back to 16 on Gen6.
We don't push UBOs on Gen6 currently, so there's no need for the larger alignment value. Cc: "17.2" --- src/mesa/drivers/dri/i965/brw_context.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c index 60b14571ed0..9c4e91fe3cd 100644 --- a/src/mesa/drivers/dri/i965/brw_context.c +++ b/src/mesa/drivers/dri/i965/brw_context.c @@ -616,7 +616,7 @@ brw_initialize_context_constants(struct brw_context *brw) * In order to push UBO data, 3DSTATE_CONSTANT_XS imposes an additional * restriction: the start of the buffer needs to be 32B aligned. */ - ctx->Const.UniformBufferOffsetAlignment = 32; + ctx->Const.UniformBufferOffsetAlignment = brw->gen >= 7 ? 32 : 16; /* ShaderStorageBufferOffsetAlignment should be a cacheline (64 bytes) so * that we can safely have the CPU and GPU writing the same SSBO on -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] Mesa support GLES 3.1 status with compute shaders
> On 08/08/2017 08:23 AM, Yuan, Feng wrote: > > Hi, > > > >> On Mon, Aug 7, 2017 at 9:45 AM, Emil Velikov > >> wrote: > >>> On 7 August 2017 at 12:56, Tapani Pälli wrote: > Hi; > > On 08/07/2017 02:15 PM, Yuan, Feng wrote: > > > > Hi, > > > > What’s the status of GLES 3.1 compute shaders support in > > Linux and Android. > > > It is supported. > > > Which branch/version start this support? and which Intel > > platforms are enabled. Is there any benchmark data in SKL/APL? > > > GLES 3.1 landed somewhere during Mesa versions 11.x so you very > likely have it on your machine. I'm running Fedora Linux on Haswell > and it is > >> supported. > It is also supported in Android-IA. > > > > Any news about Android-O? which Mesa version will be there? > > Is Android version same as Linux on GLES/SL? > > Android-IA tracks Mesa master branch and is being rebased once a week (so > currently at 17.3.0-devel). Plan is to support any new Android release. Thanks for the info. > > I'm not sure about specific benchmark for compute shaders but I do > know that starting from version 3.1 GFXBench utilizes compute. > > >>> Different generations got GLES3.1 support at separate Mesa versions. > >>> > >>> For example, in Mesa 12.0 we had > >>> - OpenGL ES 3.1 on i965/bdw+ (Broadwell and later) > >>> > >>> while with Mesa 13.0 > >>> - OpenGL ES 3.1 on i965/hsw > >>> - OpenGL ES 3.2 on i965/gen9+ (Skylake and later) > > > > Any Linux kernel or libdrm version dependency for build? > > Do I need special options for configure? > > I have a hsw with Ubuntu 16.04/kernel 4.10.0, which mesa version is better > > to try? > > Not sure what your goals or requirements are here but have a peek in > configure.ac > to see what options are available. My suggestion is that you pull Mesa git and > compile/use that instead of the rather old version of Mesa in Ubuntu 16.04 > (12.x?). I'm trying to port some image processing features from OpenCL to GLES compute shaders. But not sure about Mesa performance compare to OCL drivers in different IA platforms. I'll try build branch 17.2, looks like it's the latest release. Thank you. > > >> > >> You might also be interested in > >> > >> https://people.freedesktop.org/~imirkin/glxinfo/#b=version&g=Intel%20 > >> Skylake%20 > >> (HD%205xx)&p=es > >> > >> (use the top navigation to select different hardware, or change the > >> breakdown back to gpu to see multiple gpu's for a single mesa > >> version) > > > > That's very helpful for me. Thank you:) > > > > Thanks, > > Wind > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/dri2: Allow modifiers to add FDs to imports
On Tue, 2017-08-08 at 07:29 +0300, Tapani Pälli wrote: > > On 08/07/2017 03:05 PM, Philipp Zabel wrote: > > On Mon, 2017-07-31 at 18:35 +0100, Daniel Stone wrote: > >> When using dmabuf import, make sure that the modifier is actually > >> allowed to add planes to the base format, as implied by the comment. > >> > >> Signed-off-by: Daniel Stone > >> --- > >> src/egl/drivers/dri2/egl_dri2.c | 4 +++- > >> 1 file changed, 3 insertions(+), 1 deletion(-) > >> > >> diff --git a/src/egl/drivers/dri2/egl_dri2.c > >> b/src/egl/drivers/dri2/egl_dri2.c > >> index b73dcd72b6..76294897a5 100644 > >> --- a/src/egl/drivers/dri2/egl_dri2.c > >> +++ b/src/egl/drivers/dri2/egl_dri2.c > >> @@ -2166,8 +2166,10 @@ dri2_check_dma_buf_format(const _EGLImageAttribs > >> *attrs) > >> * this extension." > >> */ > >>if (attrs->DMABufPlaneModifiersLo[i].IsPresent && > >> - attrs->DMABufPlaneModifiersHi[i].IsPresent) > >> + attrs->DMABufPlaneModifiersHi[i].IsPresent) { > >> +plane_n = i + 1; > > > > Since this increments plane_n, Should a check be added that the > > corresponding DMABufPlanFds[i] is present? > > Check for the fd is right above this check. I see this right above: if (attrs->DMABufPlaneFds[i].IsPresent || attrs->DMABufPlaneOffsets[i].IsPresent || attrs->DMABufPlanePitches[i].IsPresent || attrs->DMABufPlaneModifiersLo[i].IsPresent || attrs->DMABufPlaneModifiersHi[i].IsPresent) { If modifiers are present, this is always true, regardless of whether the fd is present. The loop that checks for fd presence even before that only loops up to the number of planes determined by the non-modified fourcc. regards Philipp ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/2] dri3: Swapbuffer update
Implement back-to-fake-front flips, Fix EGL_BUFFER_PRESERVED path. Implement dri3 support for GLX_SWAP_EXCHANGE_OML and GLX_SWAP_COPY_OML. The back-to-fake-front flips will save a full buffer copy in the case of a fake front being enabled and GLX_SWAP_UNDEFINED_OML. Support for EGL_BUFFER_PRESERVED and GLX_SWAP_X_OML are mostly useful for things like glretrace if traces are capured with applications relying on a specific swapbuffer behavior. The EGL_BUFFER_PRESERVED path previously made sure the present was done as a copy, but there was nothing making sure that after the present, the same back buffer was chosen. This has now been changed so that if the previous back buffer is idle, we reuse it. Otherwise we grab a new and copy the contents and buffer age from the previous back buffer. Server side flips are allowed. GLX_SWAP_COPY_OML will behave like EGL_BUFFER_PRESERVED. GLX_SWAP_EXCHANGE_OML will behave similarly, except that we try to reuse the previous fake front as the new back buffer if it's idle. If not, we grab a new back buffer and copy the contents and buffer age from the old fake front. Signed-off-by: Thomas Hellstrom --- src/loader/loader_dri3_helper.c | 126 src/loader/loader_dri3_helper.h | 4 ++ 2 files changed, 105 insertions(+), 25 deletions(-) diff --git a/src/loader/loader_dri3_helper.c b/src/loader/loader_dri3_helper.c index 9d24130..3323706 100644 --- a/src/loader/loader_dri3_helper.c +++ b/src/loader/loader_dri3_helper.c @@ -40,6 +40,9 @@ #define DRI_CONF_VBLANK_DEF_INTERVAL_1 2 #define DRI_CONF_VBLANK_ALWAYS_SYNC 3 +static void +dri3_flush_present_events(struct loader_dri3_drawable *draw); + static inline void dri3_fence_reset(xcb_connection_t *c, struct loader_dri3_buffer *buffer) { @@ -148,7 +151,9 @@ loader_dri3_drawable_init(xcb_connection_t *conn, draw->have_back = 0; draw->have_fake_front = 0; draw->first_init = true; - + draw->cur_blit_source = -1; + draw->have_image_blit = draw->ext->image->base.version >= 9 && + draw->ext->image->blitImage != NULL; if (draw->ext->config) draw->ext->config->configQueryi(draw->dri_screen, "vblank_mode", &vblank_mode); @@ -190,6 +195,13 @@ loader_dri3_drawable_init(xcb_connection_t *conn, draw->vtable->set_drawable_size(draw, draw->width, draw->height); free(reply); + draw->swap_method = __DRI_ATTRIB_SWAP_UNDEFINED; + if (draw->ext->core->base.version >= 2) { + (void )draw->ext->core->getConfigAttrib(dri_config, + __DRI_ATTRIB_SWAP_METHOD, + &draw->swap_method); + } + /* * Make sure server has the same swap interval we do for the new * drawable. @@ -363,11 +375,24 @@ static int dri3_find_back(struct loader_dri3_drawable *draw) { int b; + int num_to_consider = draw->num_back; xcb_generic_event_t *ev; xcb_present_generic_event_t *ge; + /* Increase the likelyhood of reusing current buffer */ + dri3_flush_present_events(draw); + + /* Check whether we need to reuse the current back buffer as new back. +* In that case, wait until it's not busy anymore, +* only considering that particular buffer. +*/ + if (!draw->have_image_blit && draw->cur_blit_source != -1) { + num_to_consider = 1; + draw->cur_blit_source = -1; + } + for (;;) { - for (b = 0; b < draw->num_back; b++) { + for (b = 0; b < num_to_consider; b++) { int id = LOADER_DRI3_BACK_ID((b + draw->cur_back) % draw->num_back); struct loader_dri3_buffer *buffer = draw->buffers[id]; @@ -649,14 +674,27 @@ loader_dri3_swap_buffers_msc(struct loader_dri3_drawable *draw, back->height, 0, 0, back->width, back->height, __BLIT_FLAG_FLUSH); - /* Update the fake front */ - if (draw->have_fake_front) - draw->ext->image->blitImage(dri_context, - draw->buffers[LOADER_DRI3_FRONT_ID]->image, - back->image, - 0, 0, draw->width, draw->height, - 0, 0, draw->width, draw->height, - __BLIT_FLAG_FLUSH); + } + + /* If we need to preload the new back buffer, remember the source. +* The force_copy parameter is used by EGL to attempt to preserve +* the back buffer across a call to this function. +*/ + if (draw->swap_method != __DRI_ATTRIB_SWAP_UNDEFINED || force_copy) + draw->cur_blit_source = LOADER_DRI3_BACK_ID(draw->cur_back); + + /* Flip the back and fake front. Even though the server knows about these +* buffers, it has no notion of back and fake front. +*/ + if (back && draw->have_fake_front) { + struct loader_dri3_buffer *tmp; + + tmp = dri3
[Mesa-dev] [PATCH] egl/dri2: refactor dri2_query_surface, swrastGetDrawableInfo
Currently swrastGetDrawableInfo always initializes w and h, patch refactors function as x11_get_drawable_info that returns success and sets the values only if no error happened. Add swrastGetDrawableInfo wrapper function as expected by DRI extension. Signed-off-by: Tapani Pälli Reported-by: Emil Velikov --- src/egl/drivers/dri2/platform_x11.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c index 4ce819f..80f2477 100644 --- a/src/egl/drivers/dri2/platform_x11.c +++ b/src/egl/drivers/dri2/platform_x11.c @@ -99,8 +99,8 @@ swrastDestroyDrawable(struct dri2_egl_display * dri2_dpy, xcb_free_gc(dri2_dpy->conn, dri2_surf->swapgc); } -static void -swrastGetDrawableInfo(__DRIdrawable * draw, +static bool +x11_get_drawable_info(__DRIdrawable * draw, int *x, int *y, int *w, int *h, void *loaderPrivate) { @@ -110,12 +110,12 @@ swrastGetDrawableInfo(__DRIdrawable * draw, xcb_get_geometry_cookie_t cookie; xcb_get_geometry_reply_t *reply; xcb_generic_error_t *error; + bool ret = false; - *x = *y = *w = *h = 0; cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable); reply = xcb_get_geometry_reply (dri2_dpy->conn, cookie, &error); if (reply == NULL) - return; + return false; if (error != NULL) { _eglLog(_EGL_WARNING, "error in xcb_get_geometry"); @@ -125,8 +125,18 @@ swrastGetDrawableInfo(__DRIdrawable * draw, *y = reply->y; *w = reply->width; *h = reply->height; + ret = true; } free(reply); + return ret; +} + +static void +swrastGetDrawableInfo(__DRIdrawable * draw, + int *x, int *y, int *w, int *h, + void *loaderPrivate) +{ + x11_get_drawable_info(draw, x, y, w, h, loaderPrivate); } static void @@ -412,15 +422,14 @@ dri2_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, { struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); - int x, y, w = -1, h = -1; + int x, y, w, h; __DRIdrawable *drawable = dri2_dpy->vtbl->get_dri_drawable(surf); switch (attribute) { case EGL_WIDTH: case EGL_HEIGHT: - swrastGetDrawableInfo(drawable, &x, &y, &w, &h, dri2_surf); - if (w != -1 && h != -1) { + if (x11_get_drawable_info(drawable, &x, &y, &w, &h, dri2_surf)) { surf->Width = w; surf->Height = h; } -- 2.9.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl: do not initialize w, h in dri2_query_surface
On 08/07/2017 04:33 PM, Emil Velikov wrote: On 7 August 2017 at 12:59, Tapani Pälli wrote: They always get initialized to zero by swrastGetDrawableInfo. Valid X11 Drawable minimum size is 1x1, so we can detect success/change by checking against 0. Signed-off-by: Tapani Pälli Reported-by: Emil Velikov --- src/egl/drivers/dri2/platform_x11.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c index 61e700f..4917004 100644 --- a/src/egl/drivers/dri2/platform_x11.c +++ b/src/egl/drivers/dri2/platform_x11.c @@ -412,7 +412,7 @@ dri2_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, { struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); - int x, y, w = -1, h = -1; + int x, y, w, h; __DRIdrawable *drawable = dri2_dpy->vtbl->get_dri_drawable(surf); @@ -420,7 +420,7 @@ dri2_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, case EGL_WIDTH: case EGL_HEIGHT: swrastGetDrawableInfo(drawable, &x, &y, &w, &h, dri2_surf); - if (w != -1 && h != -1) { + if (w != 0 && h != 0) { Thanks Tapani. I'm wondering if it's not better to rename swrastGetDrawableInfo to a more generic one, which sets the output pointers only when it succeeds. This way we won't have this "hack", plus we won't be calling a "swrast" function from a non-swrast path. For swrastGetDrawableInfo we'will need a simple wrapper. I've sent now a proposal on this. // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/dri2: Allow modifiers to add FDs to imports
On 08/08/2017 10:37 AM, Philipp Zabel wrote: On Tue, 2017-08-08 at 07:29 +0300, Tapani Pälli wrote: On 08/07/2017 03:05 PM, Philipp Zabel wrote: On Mon, 2017-07-31 at 18:35 +0100, Daniel Stone wrote: When using dmabuf import, make sure that the modifier is actually allowed to add planes to the base format, as implied by the comment. Signed-off-by: Daniel Stone --- src/egl/drivers/dri2/egl_dri2.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index b73dcd72b6..76294897a5 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -2166,8 +2166,10 @@ dri2_check_dma_buf_format(const _EGLImageAttribs *attrs) * this extension." */ if (attrs->DMABufPlaneModifiersLo[i].IsPresent && - attrs->DMABufPlaneModifiersHi[i].IsPresent) + attrs->DMABufPlaneModifiersHi[i].IsPresent) { +plane_n = i + 1; Since this increments plane_n, Should a check be added that the corresponding DMABufPlanFds[i] is present? Check for the fd is right above this check. I see this right above: if (attrs->DMABufPlaneFds[i].IsPresent || attrs->DMABufPlaneOffsets[i].IsPresent || attrs->DMABufPlanePitches[i].IsPresent || attrs->DMABufPlaneModifiersLo[i].IsPresent || attrs->DMABufPlaneModifiersHi[i].IsPresent) { If modifiers are present, this is always true, regardless of whether the fd is present. The loop that checks for fd presence even before that only loops up to the number of planes determined by the non-modified fourcc. Ah that is correct, my bad. It would be possible to swim through with having modifiers and not fd present. // Tapani ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] i965: Don't use ggtt_bo for Gen8+ streamout offset buffer.
Quoting Kenneth Graunke (2017-08-07 22:50:42) > RELOC_NEEDS_GGTT is only meaningful on Sandybridge - it's skipped on > other generations - so this has no purpose. Just use rw_bo(). Furthermore the w/a is for a pipecontrol, so we should be able to trim it down further. Though it shouldn't make any difference other than documentation. Series is Reviewed-by: Chris Wilson -Chris ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/dri2: refactor dri2_query_surface, swrastGetDrawableInfo
On Tuesday, 2017-08-08 11:31:36 +0300, Tapani Pälli wrote: > Currently swrastGetDrawableInfo always initializes w and h, patch > refactors function as x11_get_drawable_info that returns success and > sets the values only if no error happened. Add swrastGetDrawableInfo > wrapper function as expected by DRI extension. > > Signed-off-by: Tapani Pälli > Reported-by: Emil Velikov Couple notes below, but with that: Reviewed-by: Eric Engestrom > --- > src/egl/drivers/dri2/platform_x11.c | 23 --- > 1 file changed, 16 insertions(+), 7 deletions(-) > > diff --git a/src/egl/drivers/dri2/platform_x11.c > b/src/egl/drivers/dri2/platform_x11.c > index 4ce819f..80f2477 100644 > --- a/src/egl/drivers/dri2/platform_x11.c > +++ b/src/egl/drivers/dri2/platform_x11.c > @@ -99,8 +99,8 @@ swrastDestroyDrawable(struct dri2_egl_display * dri2_dpy, > xcb_free_gc(dri2_dpy->conn, dri2_surf->swapgc); > } > > -static void > -swrastGetDrawableInfo(__DRIdrawable * draw, > +static bool > +x11_get_drawable_info(__DRIdrawable * draw, >int *x, int *y, int *w, int *h, >void *loaderPrivate) > { > @@ -110,12 +110,12 @@ swrastGetDrawableInfo(__DRIdrawable * draw, > xcb_get_geometry_cookie_t cookie; > xcb_get_geometry_reply_t *reply; > xcb_generic_error_t *error; > + bool ret = false; Don't initialise `ret` here... > > - *x = *y = *w = *h = 0; > cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable); > reply = xcb_get_geometry_reply (dri2_dpy->conn, cookie, &error); > if (reply == NULL) > - return; > + return false; > > if (error != NULL) { >_eglLog(_EGL_WARNING, "error in xcb_get_geometry"); but set it here: + ret = false; > @@ -125,8 +125,18 @@ swrastGetDrawableInfo(__DRIdrawable * draw, >*y = reply->y; >*w = reply->width; >*h = reply->height; > + ret = true; > } > free(reply); > + return ret; > +} > + > +static void > +swrastGetDrawableInfo(__DRIdrawable * draw, > + int *x, int *y, int *w, int *h, > + void *loaderPrivate) > +{ + *x = *y = *w = *h = 0; All the callers (swrast & drisw) expect these to be set unconditionally, so we should initialise them here. (Note that __glXDRIGetDrawableInfo() is the only impl to not honour that; might need to be fixed.) > + x11_get_drawable_info(draw, x, y, w, h, loaderPrivate); > } > > static void > @@ -412,15 +422,14 @@ dri2_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, > { > struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); > struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); > - int x, y, w = -1, h = -1; > + int x, y, w, h; > > __DRIdrawable *drawable = dri2_dpy->vtbl->get_dri_drawable(surf); > > switch (attribute) { > case EGL_WIDTH: > case EGL_HEIGHT: > - swrastGetDrawableInfo(drawable, &x, &y, &w, &h, dri2_surf); > - if (w != -1 && h != -1) { > + if (x11_get_drawable_info(drawable, &x, &y, &w, &h, dri2_surf)) { > surf->Width = w; > surf->Height = h; >} > -- > 2.9.4 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] st/mesa: fix CTS regression caused by fcbb93e86024
On 08/08/2017 07:34 AM, Timothy Arceri wrote: On 01/08/17 19:37, Timothy Arceri wrote: On 01/08/17 18:07, Samuel Pitoiset wrote: Don't you think it's just safer to revert the bad commit for 17.2 and fix it later on? I'm not overly worried. If you really want to go that way we can, but I don't think it's necessary. So how do we move forward on this? Can we have the revert applied to 17.2 and commit this to master? Sorry, missed that. I would prefer to revert yes, but opinions are welcome. :-) Ccing Emil for answers :) On 08/01/2017 09:35 AM, Timothy Arceri wrote: When generation the storage offset for struct members we need to skip opaque types as they no longer have backing storage. Fixes: fcbb93e86024 ("mesa: stop assigning unused storage for non-bindless opaque types") Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101983 --- src/mesa/Makefile.sources | 2 + src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 16 - src/mesa/state_tracker/st_glsl_types.cpp | 105 + src/mesa/state_tracker/st_glsl_types.h | 44 4 files changed, 165 insertions(+), 2 deletions(-) create mode 100644 src/mesa/state_tracker/st_glsl_types.cpp create mode 100644 src/mesa/state_tracker/st_glsl_types.h diff --git a/src/mesa/Makefile.sources b/src/mesa/Makefile.sources index 97c8287acb..2c557c3952 100644 --- a/src/mesa/Makefile.sources +++ b/src/mesa/Makefile.sources @@ -500,20 +500,22 @@ STATETRACKER_FILES = \ state_tracker/st_extensions.c \ state_tracker/st_extensions.h \ state_tracker/st_format.c \ state_tracker/st_format.h \ state_tracker/st_gen_mipmap.c \ state_tracker/st_gen_mipmap.h \ state_tracker/st_gl_api.h \ state_tracker/st_glsl_to_nir.cpp \ state_tracker/st_glsl_to_tgsi.cpp \ state_tracker/st_glsl_to_tgsi.h \ +- state_tracker/st_glsl_types.cpp \ +- state_tracker/st_glsl_types.h \ state_tracker/st_manager.c \ state_tracker/st_manager.h \ state_tracker/st_mesa_to_tgsi.c \ state_tracker/st_mesa_to_tgsi.h \ state_tracker/st_nir.h \ state_tracker/st_nir_lower_builtin.c \ state_tracker/st_nir_lower_tex_src_plane.c \ state_tracker/st_pbo.c \ state_tracker/st_pbo.h \ state_tracker/st_program.c \ diff --git a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp index 2fca398a51..54a749d388 100644 --- a/src/mesa/state_tracker/st_glsl_to_tgsi.cpp +++ b/src/mesa/state_tracker/st_glsl_to_tgsi.cpp @@ -42,20 +42,21 @@ #include "main/shaderapi.h" #include "main/shaderimage.h" #include "program/prog_instruction.h" #include "pipe/p_context.h" #include "pipe/p_screen.h" #include "tgsi/tgsi_ureg.h" #include "tgsi/tgsi_info.h" #include "util/u_math.h" #include "util/u_memory.h" +#include "st_glsl_types.h" #include "st_program.h" #include "st_mesa_to_tgsi.h" #include "st_format.h" #include "st_nir.h" #include "st_shader_cache.h" #include "util/hash_table.h" #include #define PROGRAM_ANY_CONST ((1 << PROGRAM_STATE_VAR) |\ @@ -2825,22 +2826,30 @@ shrink_array_declarations(struct inout_decl *decls, unsigned count, *usage_mask |= BITFIELD64_BIT(decl->mesa_index + j); } } } void glsl_to_tgsi_visitor::visit(ir_dereference_array *ir) { ir_constant *index; st_src_reg src; - int element_size = type_size(ir->type); bool is_2D = false; + ir_variable *var = ir->variable_referenced(); + + /* We only need the logic provided by st_glsl_storage_type_size() +* for arrays of structs. Indirect sampler and image indexing is handled +* elsewhere. +*/ + int element_size = ir->type->without_array()->is_record() ? + st_glsl_storage_type_size(ir->type, var->data.bindless) : + type_size(ir->type); index = ir->array_index->constant_expression_value(); ir->array->accept(this); src = this->result; if (ir->array->ir_type != ir_type_dereference_array) { switch (this->prog->Target) { case GL_TESS_CONTROL_PROGRAM_NV: is_2D = (src.file == PROGRAM_INPUT || src.file == PROGRAM_OUTPUT) && @@ -2916,28 +2925,31 @@ glsl_to_tgsi_visitor::visit(ir_dereference_array *ir) src.type = ir->type->base_type; this->result = src; } void glsl_to_tgsi_visitor::visit(ir_dereference_record *ir) { unsigned int i; const glsl_type *struct_type = ir->record->type; + ir_variable *var = ir->record->variable_referenced(); int offset = 0; ir->record->accept(this); + assert(var); for (i = 0; i < struct_type->length; i++) { if (strcmp(struct_type->fields.structure[i].name, ir->field) == 0) break; - offset += type_size(struct_type->fields.structure[i].type); + const glsl_type *member_type = struct_type->fields.structure[i].type; + offset += st_glsl_storage_type_size(member_type,
Re: [Mesa-dev] [PATCH v5 06/12] etnaviv: use common pipe_screen ref counting
On Tue, Aug 8, 2017 at 12:58 AM, Rob Herring wrote: > Use the common pipe_screen ref counting and fd hashing functions. > The mutex can be dropped as the pipe loader serializes the > create_screen() and destroy() calls. > > Signed-off-by: Rob Herring > Cc: Christian Gmeiner > Cc: Wladimir J. van der Laan > Cc: Lucas Stach This sure cleans up a lot. Reviewed-by: Wladimir J. van der Laan (will test) > --- > src/gallium/drivers/etnaviv/etnaviv_screen.c | 1 - > src/gallium/drivers/etnaviv/etnaviv_screen.h | 4 -- > .../winsys/etnaviv/drm/etnaviv_drm_winsys.c| 81 > +++--- > 3 files changed, 8 insertions(+), 78 deletions(-) > > diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.c > b/src/gallium/drivers/etnaviv/etnaviv_screen.c > index 9aca90642c37..92950c3bfbb5 100644 > --- a/src/gallium/drivers/etnaviv/etnaviv_screen.c > +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.c > @@ -806,7 +806,6 @@ etna_screen_create(struct etna_device *dev, struct > etna_gpu *gpu, > screen->dev = dev; > screen->gpu = gpu; > screen->ro = renderonly_dup(ro); > - screen->refcnt = 1; > > if (!screen->ro) { >DBG("could not create renderonly object"); > diff --git a/src/gallium/drivers/etnaviv/etnaviv_screen.h > b/src/gallium/drivers/etnaviv/etnaviv_screen.h > index dc57a38dbb80..74b8e98d1695 100644 > --- a/src/gallium/drivers/etnaviv/etnaviv_screen.h > +++ b/src/gallium/drivers/etnaviv/etnaviv_screen.h > @@ -30,7 +30,6 @@ > > #include "etnaviv_internal.h" > > -#include "os/os_thread.h" > #include "pipe/p_screen.h" > #include "renderonly/renderonly.h" > #include "util/slab.h" > @@ -59,9 +58,6 @@ enum viv_features_word { > struct etna_screen { > struct pipe_screen base; > > - int refcnt; > - void *winsys_priv; > - > struct etna_device *dev; > struct etna_gpu *gpu; > struct etna_pipe *pipe; > diff --git a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c > b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c > index 8e3f7a06a9a0..09b20389810b 100644 > --- a/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c > +++ b/src/gallium/winsys/etnaviv/drm/etnaviv_drm_winsys.c > @@ -28,6 +28,7 @@ > > #include "util/u_hash_table.h" > #include "util/u_memory.h" > +#include "util/u_screen.h" > > #include "etnaviv/etnaviv_screen.h" > #include "etnaviv/hw/common.xml.h" > @@ -67,85 +68,19 @@ screen_create(struct renderonly *ro) > return etna_screen_create(dev, gpu, ro); > } > > -static struct util_hash_table *etna_tab = NULL; > - > -static mtx_t etna_screen_mutex = _MTX_INITIALIZER_NP; > - > -static void > -etna_drm_screen_destroy(struct pipe_screen *pscreen) > -{ > - struct etna_screen *screen = etna_screen(pscreen); > - boolean destroy; > - > - mtx_lock(&etna_screen_mutex); > - destroy = --screen->refcnt == 0; > - if (destroy) { > - int fd = etna_device_fd(screen->dev); > - util_hash_table_remove(etna_tab, intptr_to_pointer(fd)); > - } > - mtx_unlock(&etna_screen_mutex); > - > - if (destroy) { > - pscreen->destroy = screen->winsys_priv; > - pscreen->destroy(pscreen); > - } > -} > - > -static unsigned hash_fd(void *key) > -{ > - int fd = pointer_to_intptr(key); > - struct stat stat; > - > - fstat(fd, &stat); > - > - return stat.st_dev ^ stat.st_ino ^ stat.st_rdev; > -} > - > -static int compare_fd(void *key1, void *key2) > -{ > - int fd1 = pointer_to_intptr(key1); > - int fd2 = pointer_to_intptr(key2); > - struct stat stat1, stat2; > - > - fstat(fd1, &stat1); > - fstat(fd2, &stat2); > - > - return stat1.st_dev != stat2.st_dev || > - stat1.st_ino != stat2.st_ino || > - stat1.st_rdev != stat2.st_rdev; > -} > - > struct pipe_screen * > etna_drm_screen_create_renderonly(struct renderonly *ro) > { > - struct pipe_screen *pscreen = NULL; > + struct etna_screen *screen; > + struct pipe_screen *pscreen = pipe_screen_reference(ro->gpu_fd); > > - mtx_lock(&etna_screen_mutex); > - if (!etna_tab) { > - etna_tab = util_hash_table_create(hash_fd, compare_fd); > - if (!etna_tab) > - goto unlock; > - } > + if (pscreen) > + return pscreen; > > - pscreen = util_hash_table_get(etna_tab, intptr_to_pointer(ro->gpu_fd)); > - if (pscreen) { > - etna_screen(pscreen)->refcnt++; > - } else { > - pscreen = screen_create(ro); > - if (pscreen) { > - int fd = etna_device_fd(etna_screen(pscreen)->dev); > - util_hash_table_set(etna_tab, intptr_to_pointer(fd), pscreen); > - > - /* Bit of a hack, to avoid circular linkage dependency, > - * ie. pipe driver having to call in to winsys, we > - * override the pipe drivers screen->destroy() */ > - etna_screen(pscreen)->winsys_priv = pscreen->destroy; > - pscreen->destroy = etna_drm_screen_destroy; > - } > - } > + pscreen = screen_create(ro); > > -unlock: > - mtx_unlock(&etna_screen_mutex); > + screen = etna_screen(pscreen)
Re: [Mesa-dev] [PATCH] egl/dri2: refactor dri2_query_surface, swrastGetDrawableInfo
On 8 August 2017 at 10:00, Eric Engestrom wrote: > On Tuesday, 2017-08-08 11:31:36 +0300, Tapani Pälli wrote: >> Currently swrastGetDrawableInfo always initializes w and h, patch >> refactors function as x11_get_drawable_info that returns success and >> sets the values only if no error happened. Add swrastGetDrawableInfo >> wrapper function as expected by DRI extension. >> >> Signed-off-by: Tapani Pälli >> Reported-by: Emil Velikov > > Couple notes below, but with that: > Reviewed-by: Eric Engestrom > >> --- >> src/egl/drivers/dri2/platform_x11.c | 23 --- >> 1 file changed, 16 insertions(+), 7 deletions(-) >> >> diff --git a/src/egl/drivers/dri2/platform_x11.c >> b/src/egl/drivers/dri2/platform_x11.c >> index 4ce819f..80f2477 100644 >> --- a/src/egl/drivers/dri2/platform_x11.c >> +++ b/src/egl/drivers/dri2/platform_x11.c >> @@ -99,8 +99,8 @@ swrastDestroyDrawable(struct dri2_egl_display * dri2_dpy, >> xcb_free_gc(dri2_dpy->conn, dri2_surf->swapgc); >> } >> >> -static void >> -swrastGetDrawableInfo(__DRIdrawable * draw, >> +static bool >> +x11_get_drawable_info(__DRIdrawable * draw, >>int *x, int *y, int *w, int *h, >>void *loaderPrivate) >> { >> @@ -110,12 +110,12 @@ swrastGetDrawableInfo(__DRIdrawable * draw, >> xcb_get_geometry_cookie_t cookie; >> xcb_get_geometry_reply_t *reply; >> xcb_generic_error_t *error; >> + bool ret = false; > > Don't initialise `ret` here... > >> >> - *x = *y = *w = *h = 0; >> cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable); >> reply = xcb_get_geometry_reply (dri2_dpy->conn, cookie, &error); >> if (reply == NULL) >> - return; >> + return false; >> >> if (error != NULL) { >>_eglLog(_EGL_WARNING, "error in xcb_get_geometry"); > > but set it here: > > + ret = false; > >> @@ -125,8 +125,18 @@ swrastGetDrawableInfo(__DRIdrawable * draw, >>*y = reply->y; >>*w = reply->width; >>*h = reply->height; >> + ret = true; >> } >> free(reply); >> + return ret; >> +} >> + >> +static void >> +swrastGetDrawableInfo(__DRIdrawable * draw, >> + int *x, int *y, int *w, int *h, >> + void *loaderPrivate) >> +{ > > + *x = *y = *w = *h = 0; > > All the callers (swrast & drisw) expect these to be set unconditionally, > so we should initialise them here. > Initialize preemptively or set when x11_get_drawable_info() fails - either one will do. With Eric's suggestions Reviewed-by: Emil Velikov > (Note that __glXDRIGetDrawableInfo() is the only impl to not honour that; > might need to be fixed.) > AFAICT the function returns false on error, we should be fine. Plus it's a legacy DRI1 codepath, so personally I will be a bit cautious about subtle changes in behaviour -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/2] st/mesa: fix CTS regression caused by fcbb93e86024
On 8 August 2017 at 10:21, Samuel Pitoiset wrote: > > > On 08/08/2017 07:34 AM, Timothy Arceri wrote: >> >> On 01/08/17 19:37, Timothy Arceri wrote: >>> >>> On 01/08/17 18:07, Samuel Pitoiset wrote: Don't you think it's just safer to revert the bad commit for 17.2 and fix it later on? >>> >>> >>> I'm not overly worried. If you really want to go that way we can, but I >>> don't think it's necessary. >> >> >> So how do we move forward on this? Can we have the revert applied to 17.2 >> and commit this to master? > > > Sorry, missed that. I would prefer to revert yes, but opinions are welcome. > :-) > Thanks guys. I think we're fine to have the revert for 17.2 and this series in master. When you send the revert please note that it's 17.2 only, and ideally reference the [comprehensive] fixes in master. Couple of suggestions: - the "+-" in the Makefile seems bogus, I think it should be "+" only? - s/When generation the/When generating the/ in the summary - reword the commit title to say what it does - "correctly calculate the storage offset" ? -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] Revert "mesa: stop assigning unused storage for non-bindless opaque types"
On 07/31/2017 10:59 PM, Samuel Pitoiset wrote: This reverts commit fcbb93e860246375d03f280f927f79d3645a8988 and also commit 7c5b204e38d8cae70f5bf26e7223da5bc448bb5c to avoid compilation errors. Basically, the parameter indexes look wrong when a non-bindless sampler is declared inside a nested struct (because it is skipped). I think it's safer to just restore the previous behaviour which is here since ages and also because the initial attempt is only a little performance improvement. This fixes a regression with ES2-CTS.functional.shaders.struct.uniform.sampler_nested*. Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101983 Fixes: fcbb93e860 ("mesa: stop assigning unused storage for non-bindless opaque types") Emil, the patch is this one. Cc: 17.2 Signed-off-by: Samuel Pitoiset --- src/mesa/program/ir_to_mesa.cpp | 56 + 1 file changed, 45 insertions(+), 11 deletions(-) diff --git a/src/mesa/program/ir_to_mesa.cpp b/src/mesa/program/ir_to_mesa.cpp index ac12b59d07..775211cefb 100644 --- a/src/mesa/program/ir_to_mesa.cpp +++ b/src/mesa/program/ir_to_mesa.cpp @@ -2409,8 +2409,10 @@ namespace { class add_uniform_to_shader : public program_resource_visitor { public: add_uniform_to_shader(struct gl_shader_program *shader_program, -struct gl_program_parameter_list *params) - : shader_program(shader_program), params(params), idx(-1) +struct gl_program_parameter_list *params, + gl_shader_stage shader_type) + : shader_program(shader_program), params(params), idx(-1), +shader_type(shader_type) { /* empty */ } @@ -2433,6 +2435,7 @@ private: struct gl_program_parameter_list *params; int idx; ir_variable *var; + gl_shader_stage shader_type; }; } /* anonymous namespace */ @@ -2444,18 +2447,49 @@ add_uniform_to_shader::visit_field(const glsl_type *type, const char *name, const enum glsl_interface_packing, bool /* last_field */) { - /* opaque types don't use storage in the param list unless they are -* bindless samplers or images. -*/ - if (type->contains_opaque() && !var->data.bindless) + /* atomics don't get real storage */ + if (type->contains_atomic()) return; - assert(_mesa_lookup_parameter_index(params, name) < 0); + gl_register_file file; + if (type->without_array()->is_sampler() && !var->data.bindless) { + file = PROGRAM_SAMPLER; + } else { + file = PROGRAM_UNIFORM; + } + + int index = _mesa_lookup_parameter_index(params, name); + if (index < 0) { + unsigned size = type_size(type) * 4; + + index = _mesa_add_parameter(params, file, name, size, type->gl_type, + NULL, NULL); - unsigned size = type_size(type) * 4; + /* Sampler uniform values are stored in prog->SamplerUnits, + * and the entry in that array is selected by this index we + * store in ParameterValues[]. + */ + if (file == PROGRAM_SAMPLER) { +unsigned location; +const bool found = + this->shader_program->UniformHash->get(location, + params->Parameters[index].Name); +assert(found); + +if (!found) + return; + +struct gl_uniform_storage *storage = +&this->shader_program->data->UniformStorage[location]; - int index = _mesa_add_parameter(params, PROGRAM_UNIFORM, name, size, - type->gl_type, NULL, NULL); + assert(storage->type->is_sampler() && +storage->opaque[shader_type].active); + +for (unsigned int j = 0; j < size / 4; j++) +params->ParameterValues[index + j][0].f = + storage->opaque[shader_type].index + j; + } + } /* The first part of the uniform that's processed determines the base * location of the whole uniform (for structures). @@ -2479,7 +2513,7 @@ _mesa_generate_parameters_list_for_uniforms(struct gl_shader_program struct gl_program_parameter_list *params) { - add_uniform_to_shader add(shader_program, params); + add_uniform_to_shader add(shader_program, params, sh->Stage); foreach_in_list(ir_instruction, node, sh->ir) { ir_variable *var = node->as_variable(); ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] egl/dri2: refactor dri2_query_surface, swrastGetDrawableInfo
On 08/08/2017 12:00 PM, Eric Engestrom wrote: On Tuesday, 2017-08-08 11:31:36 +0300, Tapani Pälli wrote: Currently swrastGetDrawableInfo always initializes w and h, patch refactors function as x11_get_drawable_info that returns success and sets the values only if no error happened. Add swrastGetDrawableInfo wrapper function as expected by DRI extension. Signed-off-by: Tapani Pälli Reported-by: Emil Velikov Couple notes below, but with that: Reviewed-by: Eric Engestrom --- src/egl/drivers/dri2/platform_x11.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/egl/drivers/dri2/platform_x11.c b/src/egl/drivers/dri2/platform_x11.c index 4ce819f..80f2477 100644 --- a/src/egl/drivers/dri2/platform_x11.c +++ b/src/egl/drivers/dri2/platform_x11.c @@ -99,8 +99,8 @@ swrastDestroyDrawable(struct dri2_egl_display * dri2_dpy, xcb_free_gc(dri2_dpy->conn, dri2_surf->swapgc); } -static void -swrastGetDrawableInfo(__DRIdrawable * draw, +static bool +x11_get_drawable_info(__DRIdrawable * draw, int *x, int *y, int *w, int *h, void *loaderPrivate) { @@ -110,12 +110,12 @@ swrastGetDrawableInfo(__DRIdrawable * draw, xcb_get_geometry_cookie_t cookie; xcb_get_geometry_reply_t *reply; xcb_generic_error_t *error; + bool ret = false; Don't initialise `ret` here... - *x = *y = *w = *h = 0; cookie = xcb_get_geometry (dri2_dpy->conn, dri2_surf->drawable); reply = xcb_get_geometry_reply (dri2_dpy->conn, cookie, &error); if (reply == NULL) - return; + return false; if (error != NULL) { _eglLog(_EGL_WARNING, "error in xcb_get_geometry"); but set it here: + ret = false; ok, will change @@ -125,8 +125,18 @@ swrastGetDrawableInfo(__DRIdrawable * draw, *y = reply->y; *w = reply->width; *h = reply->height; + ret = true; } free(reply); + return ret; +} + +static void +swrastGetDrawableInfo(__DRIdrawable * draw, + int *x, int *y, int *w, int *h, + void *loaderPrivate) +{ + *x = *y = *w = *h = 0; All the callers (swrast & drisw) expect these to be set unconditionally, so we should initialise them here. argh this initialization was the plan but I forgot it, thanks! :) (Note that __glXDRIGetDrawableInfo() is the only impl to not honour that; might need to be fixed.) + x11_get_drawable_info(draw, x, y, w, h, loaderPrivate); } static void @@ -412,15 +422,14 @@ dri2_query_surface(_EGLDriver *drv, _EGLDisplay *dpy, { struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); - int x, y, w = -1, h = -1; + int x, y, w, h; __DRIdrawable *drawable = dri2_dpy->vtbl->get_dri_drawable(surf); switch (attribute) { case EGL_WIDTH: case EGL_HEIGHT: - swrastGetDrawableInfo(drawable, &x, &y, &w, &h, dri2_surf); - if (w != -1 && h != -1) { + if (x11_get_drawable_info(drawable, &x, &y, &w, &h, dri2_surf)) { surf->Width = w; surf->Height = h; } -- 2.9.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v6] egl: Allow creation of per surface out fence
From: Zhongmin Wu Add plumbing to allow creation of per display surface out fence. Currently enabled only on android, since the system expects a valid fd in ANativeWindow::{queue,cancel}Buffer. We pass a fd of -1 with which native applications such as flatland fail. The patch enables explicit sync on android and fixes one of the functional issue for apps or buffer consumers which depend upon fence and its timestamp. v2: a) Also implement the fence in cancelBuffer. b) The last sync fence is stored in drawable object rather than brw context. c) format clear. v3: a) Save the last fence fd in DRI Context object. b) Return the last fence if the batch buffer is empty and nothing to be flushed when _intel_batchbuffer_flush_fence c) Add the new interface in vbtl to set the retrieve fence v3.1 a) close fd in the new vbtl interface on none Android platform v4: a) The last fence is saved in brw context. b) The retrieve fd is for all the platform but not just Android c) Add a uniform dri2 interface to initialize the surface. v4.1: a) make some changes of variable name. b) the patch is broken into two patches. v4.2: a) Add a deinit interface for surface to clear the out fence v5: a) Add enable_out_fence to init, platform sets it true or false b) Change get fd to update fd and check for fence c) Commit description updated v6: a) Heading and commit description updated b) enable_out_fence is set only if fence is supported c) Review comments on function names d) Test with standalone patch, resolves the bug Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=101655 Signed-off-by: Zhongmin Wu Signed-off-by: Yogesh Marathe --- src/egl/drivers/dri2/egl_dri2.c | 65 + src/egl/drivers/dri2/egl_dri2.h | 9 src/egl/drivers/dri2/platform_android.c | 29 +++-- src/egl/drivers/dri2/platform_drm.c | 3 +- src/egl/drivers/dri2/platform_surfaceless.c | 3 +- src/egl/drivers/dri2/platform_wayland.c | 3 +- src/egl/drivers/dri2/platform_x11.c | 3 +- src/egl/drivers/dri2/platform_x11_dri3.c| 3 +- 8 files changed, 100 insertions(+), 18 deletions(-) diff --git a/src/egl/drivers/dri2/egl_dri2.c b/src/egl/drivers/dri2/egl_dri2.c index f0d1ded..e9430a8 100644 --- a/src/egl/drivers/dri2/egl_dri2.c +++ b/src/egl/drivers/dri2/egl_dri2.c @@ -1316,6 +1316,44 @@ dri2_destroy_context(_EGLDriver *drv, _EGLDisplay *disp, _EGLContext *ctx) return EGL_TRUE; } +EGLBoolean +dri2_init_surface(_EGLSurface *surf, _EGLDisplay *dpy, EGLint type, +_EGLConfig *conf, const EGLint *attrib_list, EGLBoolean enable_out_fence) +{ + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); + struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); + + dri2_surf->out_fence_fd = -1; + if (dri2_dpy->fence && dri2_dpy->fence->base.version >= 2 && + dri2_dpy->fence->get_capabilities && + (dri2_dpy->fence->get_capabilities(dri2_dpy->dri_screen) & +__DRI_FENCE_CAP_NATIVE_FD)) { + dri2_surf->enable_out_fence = enable_out_fence; + } + + return _eglInitSurface(surf, dpy, type, conf, attrib_list); +} + +static void +dri2_surface_set_out_fence_fd( _EGLSurface *surf, int fence_fd) +{ + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); + + if (dri2_surf->out_fence_fd >=0) + close(dri2_surf->out_fence_fd); + + dri2_surf->out_fence_fd = fence_fd; +} + +void +dri2_deinit_surface(_EGLSurface *surf) +{ + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); + + dri2_surface_set_out_fence_fd(surf, -1); + dri2_surf->enable_out_fence = false; +} + static EGLBoolean dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf) { @@ -1327,6 +1365,27 @@ dri2_destroy_surface(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf) return dri2_dpy->vtbl->destroy_surface(drv, dpy, surf); } +static void +dri2_surf_update_fence_fd(_EGLContext *ctx, + _EGLDisplay *dpy, _EGLSurface *surf) +{ + __DRIcontext *dri_ctx = dri2_egl_context(ctx)->dri_context; + struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); + struct dri2_egl_surface *dri2_surf = dri2_egl_surface(surf); + int fence_fd = -1; + void *fence; + + if (dri2_surf->enable_out_fence) { + fence = dri2_dpy->fence->create_fence_fd(dri_ctx, -1); + if (fence) { + fence_fd = dri2_dpy->fence->get_fence_fd(dri2_dpy->dri_screen, + fence); + dri2_dpy->fence->destroy_fence(dri2_dpy->dri_screen, fence); + } + } + dri2_surface_set_out_fence_fd(surf, fence_fd); +} + /** * Called via eglMakeCurrent(), drv->API.MakeCurrent(). */ @@ -1501,6 +1560,9 @@ static EGLBoolean dri2_swap_buffers(_EGLDriver *drv, _EGLDisplay *dpy, _EGLSurface *surf) { struct dri2_egl_display *dri2_dpy = dri2_egl_display(dpy); + _EGLContext *ctx = _egl
[Mesa-dev] [Bug 102030] private memory overflow in openCL
https://bugs.freedesktop.org/show_bug.cgi?id=102030 --- Comment #3 from Janpieter Sollie --- confirmed on -RC3 (initial report on RC2) -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure.ac: Check for expat21 if expat is not found.
On 7 August 2017 at 19:13, Vinson Lee wrote: > On Mon, Aug 7, 2017 at 8:18 AM, Emil Velikov wrote: >> On 5 August 2017 at 21:53, Vinson Lee wrote: >>> Fixes build error on CentOS 6.9. >>> >>> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=102052 >>> Fixes: 5c007203b73d ("configure.ac: drop manual detection of expat >>> header/library") >>> Signed-off-by: Vinson Lee >>> --- >>> configure.ac |4 +++- >>> 1 files changed, 3 insertions(+), 1 deletions(-) >>> >>> diff --git a/configure.ac b/configure.ac >>> index 5b12dd8..0dfd47b 100644 >>> --- a/configure.ac >>> +++ b/configure.ac >>> @@ -1792,7 +1792,9 @@ if test "x$with_dri_drivers" = xno; then >>> fi >>> >>> # Check for expat >>> -PKG_CHECK_MODULES([EXPAT], [expat]) >>> +PKG_CHECK_MODULES([EXPAT], [expat],, >>> +[PKG_CHECK_MODULES([EXPAT], [expat21])] >> Hmm... the CenOS guys have gone "interesting" ways with this. >> >> I doubt we have any actual users for such setups, so I'm wondering if >> you can keep a WA for this locally. >> Say simply create a link/copy? >> >> Thanks >> Emil > > CentOS 6 and RHEL 6 have expat 2.0 by default. expat 2.0 does not > provide expat.pc. No arguments there - it's their decision to use 10+ year old software. Do we have any actual RHEL/CentOS 6 users, apart from the build testing that you do? If yes, sure let's have the workaround - otherwise, please keep it locally. Some solutions include: - cp/symlink the file - set EXPAT_CFLAGS and EXPAT_LIBS alongside/prior to calling configure -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 102077] multilib mesa fail to build with "egl/drivers/dri2/platform_drm.c:542: undefined reference to `gbm_bo_get_bpp'"
https://bugs.freedesktop.org/show_bug.cgi?id=102077 Laurent carlier changed: What|Removed |Added Status|NEW |RESOLVED Resolution|--- |NOTOURBUG --- Comment #2 from Laurent carlier --- All my packages are built in a chroot :) I somehow followed your suggestion, i've installed a dummy package in the chroot that replaced mesa/lib32-mesa, and that fixed the build failure (so the problem is related to libtool that try to link with libgbm library in the system) So i can close it as NOTOURBUG -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon
On 8 August 2017 at 08:21, Tapani Pälli wrote: > > > On 08/05/2017 02:25 AM, Emil Velikov wrote: >> >> From: Emil Velikov >> >> As mentioned in previous commit the negative tests in dEQP expect the >> arguments to be evaluated in particular order. >> >> Namely - first the dpy, then the config, followed by the surface/window. >> >> Move the check further down or executing the test below will produce >> the following error. >> >> dEQP-EGL.functional.negative_api.create_pbuffer_surface >> >> >> >>eglCreateWindowSurface(0x9bfff0f150, 0x, >> 0x, { EGL_NONE }); >>// 0x returned >>// ERROR expected: EGL_BAD_CONFIG, Got: >> EGL_BAD_NATIVE_WINDOW >> >> >> Cc: >> Cc: Mark Janes >> Cc: Chad Versace >> Signed-off-by: Emil Velikov >> --- >> Mark, >> >> IMHO the CI does the impossible and passes the test. Perhaps it's worth >> looking into how/why it does so - I don't know. > > > For me the above test is passing fine on x11 (x11_egl). > Barring local changes i cannot see how that's possible. The test itself is as below, while Mesa does the window (NULL) check first. expectNoSurface(eglCreateWindowSurface(display, (EGLConfig)-1, DE_NULL, s_emptyAttribList)); expectError(EGL_BAD_CONFIG); Then again, I've spotted a few extra bits that we're not handling correctly. More patches coming shortly. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/8] egl: drop unreachable BAD_NATIVE_WINDOW conditions
On Tuesday, 2017-08-08 08:02:27 +0300, Tapani Pälli wrote: > > > On 08/05/2017 02:25 AM, Emil Velikov wrote: > > From: Emil Velikov > > > > The code in _eglCreatePixmapSurfaceCommon() already has a NULL check > > _eglCreateWindowSurfaceCommon() > > > which handles the condition. There's no point in checkin again further "checking" With the two typos Tapani noticed fixed, Reviewed-by: Eric Engestrom > > down the stack. > > > > v2: Split the WINDOW vs PIXMAP into separate patches > > > > Cc: Eric Engestrom > > Signed-off-by: Emil Velikov > > --- > > src/egl/drivers/dri2/platform_android.c | 2 +- > > src/egl/drivers/dri2/platform_drm.c | 5 - > > src/egl/drivers/dri2/platform_wayland.c | 5 - > > src/egl/drivers/dri2/platform_x11.c | 6 ++ > > 4 files changed, 3 insertions(+), 15 deletions(-) > > > > diff --git a/src/egl/drivers/dri2/platform_android.c > > b/src/egl/drivers/dri2/platform_android.c > > index 50a82486956..beb474025f7 100644 > > --- a/src/egl/drivers/dri2/platform_android.c > > +++ b/src/egl/drivers/dri2/platform_android.c > > @@ -329,7 +329,7 @@ droid_create_surface(_EGLDriver *drv, _EGLDisplay > > *disp, EGLint type, > > if (type == EGL_WINDOW_BIT) { > > int format; > > - if (!window || window->common.magic != ANDROID_NATIVE_WINDOW_MAGIC) { > > + if (window->common.magic != ANDROID_NATIVE_WINDOW_MAGIC) { > >_eglError(EGL_BAD_NATIVE_WINDOW, "droid_create_surface"); > >goto cleanup_surface; > > } > > diff --git a/src/egl/drivers/dri2/platform_drm.c > > b/src/egl/drivers/dri2/platform_drm.c > > index a952aa54560..7ea43e62010 100644 > > --- a/src/egl/drivers/dri2/platform_drm.c > > +++ b/src/egl/drivers/dri2/platform_drm.c > > @@ -115,11 +115,6 @@ dri2_drm_create_surface(_EGLDriver *drv, _EGLDisplay > > *disp, EGLint type, > > switch (type) { > > case EGL_WINDOW_BIT: > > - if (!window) { > > - _eglError(EGL_BAD_NATIVE_WINDOW, "dri2_create_surface"); > > - goto cleanup_surf; > > - } > > - > > surf = gbm_dri_surface(window); > > dri2_surf->gbm_surf = surf; > > dri2_surf->base.Width = surf->base.width; > > diff --git a/src/egl/drivers/dri2/platform_wayland.c > > b/src/egl/drivers/dri2/platform_wayland.c > > index 38fdfe974fa..dcc777a3e8b 100644 > > --- a/src/egl/drivers/dri2/platform_wayland.c > > +++ b/src/egl/drivers/dri2/platform_wayland.c > > @@ -162,11 +162,6 @@ dri2_wl_create_window_surface(_EGLDriver *drv, > > _EGLDisplay *disp, > >dri2_surf->format = WL_SHM_FORMAT_ARGB; > > } > > - if (!window) { > > - _eglError(EGL_BAD_NATIVE_WINDOW, "dri2_create_surface"); > > - goto cleanup_surf; > > - } > > - > > dri2_surf->wl_win = window; > > dri2_surf->wl_queue = wl_display_create_queue(dri2_dpy->wl_dpy); > > if (!dri2_surf->wl_queue) { > > diff --git a/src/egl/drivers/dri2/platform_x11.c > > b/src/egl/drivers/dri2/platform_x11.c > > index 7b5a1770bd7..00cab577b77 100644 > > --- a/src/egl/drivers/dri2/platform_x11.c > > +++ b/src/egl/drivers/dri2/platform_x11.c > > @@ -235,10 +235,8 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay > > *disp, EGLint type, > > dri2_surf->base.Width, dri2_surf->base.Height); > > } else { > > if (!drawable) { > > - if (type == EGL_WINDOW_BIT) > > -_eglError(EGL_BAD_NATIVE_WINDOW, "dri2_create_surface"); > > - else > > -_eglError(EGL_BAD_NATIVE_PIXMAP, "dri2_create_surface"); > > + assert(type == EGL_PIXMAP_BIT_BIT) > > no BIT_BIT but EGL_PIXMAP_BIT :) > > > + _eglError(EGL_BAD_NATIVE_PIXMAP, "dri2_create_surface"); > >goto cleanup_surf; > > } > > dri2_surf->drawable = drawable; > > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/8] egl: handle BAD_NATIVE_PIXMAP further up the stack
On Saturday, 2017-08-05 00:25:48 +0100, Emil Velikov wrote: > From: Emil Velikov > > The basic (null) check is identical across all backends. > Just move it to the top. > > v2: > - Split the WINDOW vs PIXMAP into separate patches > - Move check after the dpy and config - dEQP expects so > > Cc: Eric Engestrom > Signed-off-by: Emil Velikov Reviewed-by: Eric Engestrom > --- > src/egl/drivers/dri2/platform_x11.c | 5 - > src/egl/main/eglapi.c | 3 +++ > 2 files changed, 3 insertions(+), 5 deletions(-) > > diff --git a/src/egl/drivers/dri2/platform_x11.c > b/src/egl/drivers/dri2/platform_x11.c > index 00cab577b77..e2007a0313e 100644 > --- a/src/egl/drivers/dri2/platform_x11.c > +++ b/src/egl/drivers/dri2/platform_x11.c > @@ -234,11 +234,6 @@ dri2_x11_create_surface(_EGLDriver *drv, _EGLDisplay > *disp, EGLint type, > dri2_surf->drawable, dri2_dpy->screen->root, > dri2_surf->base.Width, dri2_surf->base.Height); > } else { > - if (!drawable) { > - assert(type == EGL_PIXMAP_BIT_BIT) > - _eglError(EGL_BAD_NATIVE_PIXMAP, "dri2_create_surface"); > - goto cleanup_surf; > - } >dri2_surf->drawable = drawable; > } > > diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c > index c5e3955c48c..3ca3dd4c7c1 100644 > --- a/src/egl/main/eglapi.c > +++ b/src/egl/main/eglapi.c > @@ -1021,6 +1021,9 @@ _eglCreatePixmapSurfaceCommon(_EGLDisplay *disp, > EGLConfig config, > if ((conf->SurfaceType & EGL_PIXMAP_BIT) == 0) >RETURN_EGL_ERROR(disp, EGL_BAD_MATCH, EGL_NO_SURFACE); > > + if (native_pixmap == NULL) > + RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_PIXMAP, EGL_NO_SURFACE); > + > surf = drv->API.CreatePixmapSurface(drv, disp, conf, native_pixmap, > attrib_list); > ret = (surf) ? _eglLinkSurface(surf) : EGL_NO_SURFACE; > -- > 2.13.3 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] egl: avoid eglCreatePlatform*Surface{EXT, } crash with invalid dpy
From: Emil Velikov If we have an invalid display fed into the functions, the display lookup will return NULL. Thus as we attempt to get the platform type, we'll deref. it leading to a crash. Keep in mind that this will not happen if Mesa is built without X11 or when the legacy eglCreate*Surface codepaths are used. An similar check was added with earlier commit 5e97b8f5ce9 ("egl: Fix crashes in eglCreate*Surface), although it was only applicable when the surfaceless platform is built. Cc: mesa-sta...@lists.freedesktop.org Cc: Tapani Pälli Cc: Chad Versace Signed-off-by: Emil Velikov --- src/egl/main/eglapi.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c index c5e3955c48c..8146009da4f 100644 --- a/src/egl/main/eglapi.c +++ b/src/egl/main/eglapi.c @@ -923,7 +923,7 @@ static void * _fixupNativeWindow(_EGLDisplay *disp, void *native_window) { #ifdef HAVE_X11_PLATFORM - if (disp->Platform == _EGL_PLATFORM_X11 && native_window != NULL) { + if (disp && disp->Platform == _EGL_PLATFORM_X11 && native_window != NULL) { /* The `native_window` parameter for the X11 platform differs between * eglCreateWindowSurface() and eglCreatePlatformPixmapSurfaceEXT(). In * eglCreateWindowSurface(), the type of `native_window` is an Xlib @@ -985,7 +985,7 @@ _fixupNativePixmap(_EGLDisplay *disp, void *native_pixmap) * `Pixmap*`. Convert `Pixmap*` to `Pixmap` because that's what * dri2_x11_create_pixmap_surface() expects. */ - if (disp->Platform == _EGL_PLATFORM_X11 && native_pixmap != NULL) + if (disp && disp->Platform == _EGL_PLATFORM_X11 && native_pixmap != NULL) return (void *)(* (Pixmap*) native_pixmap); #endif return native_pixmap; -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 102067] Is it possible select a driver/implementation in an application? (like in OpenCL)
https://bugs.freedesktop.org/show_bug.cgi?id=102067 Emil Velikov changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED --- Comment #1 from Emil Velikov --- (In reply to myocyt...@sina.com from comment #0) > Is it possible for a application to enumerate and select the GL > implementation, > like in OpenCL? > > In case of: > 1. Mesa EGL. >How to enumerate and select a GL vendor/implementation? There is the EGL device set of extensions. There are some WIP patches for mesa, but none are merged yet. > 2. GLX. >Is it possible to enumerate and select the GL drivers? AFAICT no extensions that provide the functionality are available. > 3. Wayland. >Is Mesa EGL/DRI the only frontend/backend? >(Or is there some way to load proprietary drivers?) Yes there are proprietary drivers that have Wayland support. Regardless, Wayland is used in conjunction with EGL, which can use the above extensions. -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon
On Saturday, 2017-08-05 00:25:49 +0100, Emil Velikov wrote: > From: Emil Velikov > > As mentioned in previous commit the negative tests in dEQP expect the > arguments to be evaluated in particular order. The spec doesn't say that, so the test is wrong. Changing it in Mesa doesn't hurt though, so I have nothing against it, except for the fact it hide the dEQP bug. Does anyone know the preferred way to report dEQP bugs? The README [1] is rather... bare. I have about a dozen known dEQP bugs that I'd love to report, if only so that I don't have to keep telling people "ignore that test, it's broken" but instead point them at the report of why & how it's broken. [1] https://android.googlesource.com/platform/external/deqp/+/master/README.md > > Namely - first the dpy, then the config, followed by the surface/window. > > Move the check further down or executing the test below will produce > the following error. > >dEQP-EGL.functional.negative_api.create_pbuffer_surface > > > > eglCreateWindowSurface(0x9bfff0f150, 0x, > 0x, { EGL_NONE }); > // 0x returned > // ERROR expected: EGL_BAD_CONFIG, Got: > EGL_BAD_NATIVE_WINDOW > > > Cc: > Cc: Mark Janes > Cc: Chad Versace > Signed-off-by: Emil Velikov > --- > Mark, > > IMHO the CI does the impossible and passes the test. Perhaps it's worth > looking into how/why it does so - I don't know. > > I'll pipe the series through Jenkins tomorrow - don't want to stall > things for the guys still working. > > Chad, I see that in the EGL_MESA_surfaceless implementation you > explicitly mentioned that the surface is checked prior to the config. > > Wouldn't it be better to stay consistent and move those, as per the > above? AFAICT the spec does not explicitly dictates the order. > --- > src/egl/main/eglapi.c | 7 +++ > 1 file changed, 3 insertions(+), 4 deletions(-) > > diff --git a/src/egl/main/eglapi.c b/src/egl/main/eglapi.c > index 3ca3dd4c7c1..3b0f896f74c 100644 > --- a/src/egl/main/eglapi.c > +++ b/src/egl/main/eglapi.c > @@ -872,10 +872,6 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, > EGLConfig config, > _EGLSurface *surf; > EGLSurface ret; > > - > - if (native_window == NULL) > - RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE); > - > #ifdef HAVE_SURFACELESS_PLATFORM > if (disp && disp->Platform == _EGL_PLATFORM_SURFACELESS) { >/* From the EGL_MESA_platform_surfaceless spec (v1): > @@ -899,6 +895,9 @@ _eglCreateWindowSurfaceCommon(_EGLDisplay *disp, > EGLConfig config, > if ((conf->SurfaceType & EGL_WINDOW_BIT) == 0) >RETURN_EGL_ERROR(disp, EGL_BAD_MATCH, EGL_NO_SURFACE); > > + if (native_window == NULL) > + RETURN_EGL_ERROR(disp, EGL_BAD_NATIVE_WINDOW, EGL_NO_SURFACE); > + > surf = drv->API.CreateWindowSurface(drv, disp, conf, native_window, > attrib_list); > ret = (surf) ? _eglLinkSurface(surf) : EGL_NO_SURFACE; > -- > 2.13.3 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/main: Advertise ARB_texture_filter_anisotropic
On Wed, 2017-06-28 at 16:50 +0300, Plamena Manolova wrote: > ARB_texture_filter_anisotropic is the ARB variation of > EXT_texture_fitter_anisotropic and it operates in the > same way, so there's no reason not to advertise it. Nak. The ARB version bups the minmax anisotropy to 16x, see issue 2 in the spec. This patch would wrongly enable the ARB version for intel < gen4 and nouveau < nv40. - ajax ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/main: Advertise ARB_texture_filter_anisotropic
On Tue, Aug 8, 2017 at 11:16 AM, Adam Jackson wrote: > On Wed, 2017-06-28 at 16:50 +0300, Plamena Manolova wrote: >> ARB_texture_filter_anisotropic is the ARB variation of >> EXT_texture_fitter_anisotropic and it operates in the >> same way, so there's no reason not to advertise it. > > Nak. The ARB version bups the minmax anisotropy to 16x, see issue 2 in > the spec. This patch would wrongly enable the ARB version for intel < > gen4 and nouveau < nv40. Yeah, this needs a new ext enable bit which is flipped on in st_extensions.c and the relevant i965 file, conditional on the driver being able to handle >= 16x aniso. (There should be a PIPE_CAP for it on the gallium end of it. If you don't want to hook it up for st/mesa that's fine too, I or someone else can handle that one-liner.) Well-noticed, ajax. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/main: Advertise ARB_texture_filter_anisotropic
On Tue, Aug 8, 2017 at 11:34 AM, Roland Scheidegger wrote: > Am 08.08.2017 um 17:21 schrieb Ilia Mirkin: >> On Tue, Aug 8, 2017 at 11:16 AM, Adam Jackson wrote: >>> On Wed, 2017-06-28 at 16:50 +0300, Plamena Manolova wrote: ARB_texture_filter_anisotropic is the ARB variation of EXT_texture_fitter_anisotropic and it operates in the same way, so there's no reason not to advertise it. >>> >>> Nak. The ARB version bups the minmax anisotropy to 16x, see issue 2 in >>> the spec. This patch would wrongly enable the ARB version for intel < >>> gen4 and nouveau < nv40. >> >> Yeah, this needs a new ext enable bit which is flipped on in >> st_extensions.c and the relevant i965 file, conditional on the driver >> being able to handle >= 16x aniso. (There should be a PIPE_CAP for it >> on the gallium end of it. If you don't want to hook it up for st/mesa >> that's fine too, I or someone else can handle that one-liner.) >> >> Well-noticed, ajax. >> >> -ilia >> > > Does it really matter, though? > The spec does not require any specific implementation. About the > strongest wording I can find is that "However, when the texture's value > of TEXTURE_MAX_ANISOTROPY is greater than 1.0, the GL implementation > should use a texture filtering scheme that accounts for a degree of > anisotropy up to..." > > So it's only "should". > Therefore implementations not really being able to handle 16xAF would > probably still be conformant with a lesser AF degree - they can just > pretend it's 16xAF with a crappy scheme :-). > > FWIW d3d10 also requires 16xAF, but is quite fine with any filtering as > long as the filtering is no worse than simple trilinear (which is > exactly what llvmpipe does if you give it a sampler with AF filtering...). In practice though, MAX_TEXTURE_MAX_ANISOTROPY will continue to be reported as 4 or whatever it was, against the spec, without additional changes. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/main: Advertise ARB_texture_filter_anisotropic
Am 08.08.2017 um 17:21 schrieb Ilia Mirkin: > On Tue, Aug 8, 2017 at 11:16 AM, Adam Jackson wrote: >> On Wed, 2017-06-28 at 16:50 +0300, Plamena Manolova wrote: >>> ARB_texture_filter_anisotropic is the ARB variation of >>> EXT_texture_fitter_anisotropic and it operates in the >>> same way, so there's no reason not to advertise it. >> >> Nak. The ARB version bups the minmax anisotropy to 16x, see issue 2 in >> the spec. This patch would wrongly enable the ARB version for intel < >> gen4 and nouveau < nv40. > > Yeah, this needs a new ext enable bit which is flipped on in > st_extensions.c and the relevant i965 file, conditional on the driver > being able to handle >= 16x aniso. (There should be a PIPE_CAP for it > on the gallium end of it. If you don't want to hook it up for st/mesa > that's fine too, I or someone else can handle that one-liner.) > > Well-noticed, ajax. > > -ilia > Does it really matter, though? The spec does not require any specific implementation. About the strongest wording I can find is that "However, when the texture's value of TEXTURE_MAX_ANISOTROPY is greater than 1.0, the GL implementation should use a texture filtering scheme that accounts for a degree of anisotropy up to..." So it's only "should". Therefore implementations not really being able to handle 16xAF would probably still be conformant with a lesser AF degree - they can just pretend it's 16xAF with a crappy scheme :-). FWIW d3d10 also requires 16xAF, but is quite fine with any filtering as long as the filtering is no worse than simple trilinear (which is exactly what llvmpipe does if you give it a sampler with AF filtering...). Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] mesa/main: Advertise ARB_texture_filter_anisotropic
Am 08.08.2017 um 17:45 schrieb Ilia Mirkin: > On Tue, Aug 8, 2017 at 11:34 AM, Roland Scheidegger > wrote: >> Am 08.08.2017 um 17:21 schrieb Ilia Mirkin: >>> On Tue, Aug 8, 2017 at 11:16 AM, Adam Jackson wrote: On Wed, 2017-06-28 at 16:50 +0300, Plamena Manolova wrote: > ARB_texture_filter_anisotropic is the ARB variation of > EXT_texture_fitter_anisotropic and it operates in the > same way, so there's no reason not to advertise it. Nak. The ARB version bups the minmax anisotropy to 16x, see issue 2 in the spec. This patch would wrongly enable the ARB version for intel < gen4 and nouveau < nv40. >>> >>> Yeah, this needs a new ext enable bit which is flipped on in >>> st_extensions.c and the relevant i965 file, conditional on the driver >>> being able to handle >= 16x aniso. (There should be a PIPE_CAP for it >>> on the gallium end of it. If you don't want to hook it up for st/mesa >>> that's fine too, I or someone else can handle that one-liner.) >>> >>> Well-noticed, ajax. >>> >>> -ilia >>> >> >> Does it really matter, though? >> The spec does not require any specific implementation. About the >> strongest wording I can find is that "However, when the texture's value >> of TEXTURE_MAX_ANISOTROPY is greater than 1.0, the GL implementation >> should use a texture filtering scheme that accounts for a degree of >> anisotropy up to..." >> >> So it's only "should". >> Therefore implementations not really being able to handle 16xAF would >> probably still be conformant with a lesser AF degree - they can just >> pretend it's 16xAF with a crappy scheme :-). >> >> FWIW d3d10 also requires 16xAF, but is quite fine with any filtering as >> long as the filtering is no worse than simple trilinear (which is >> exactly what llvmpipe does if you give it a sampler with AF filtering...). > > In practice though, MAX_TEXTURE_MAX_ANISOTROPY will continue to be > reported as 4 or whatever it was, against the spec, without additional > changes. > Ahh right. So you either have to lie about the max anisotropy or not advertize the ARB version indeed. Unless you want to violate the spec there, but there's probably no good reason to (unlike for instance the min multisample level required for GL 3.0). Roland ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 0/3] Implement more 4.6 extensions
I hadn't seen Plamena's patches for these before writing them, and I think I got right most of the issues that were caught in review (namely: 16x minmax for aniso, updated headers and xml for new dispatch for polygon offset clamp), so in the interest of saving time... - ajax ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 1/3] include: Sync Khronos headers for OpenGL 4.6
Taken from c21e602b9fda1d3bbaecb08194592f67e6a0649b from OpenGL-Registry. Signed-off-by: Adam Jackson --- include/GL/glcorearb.h | 66 ++- include/GL/glext.h | 82 +++-- include/GL/glxext.h| 2 +- include/GL/wglext.h| 2 +- src/mapi/glapi/registry/gl.xml | 261 + 5 files changed, 350 insertions(+), 63 deletions(-) diff --git a/include/GL/glcorearb.h b/include/GL/glcorearb.h index 1f4d64e83f..071b0c5613 100644 --- a/include/GL/glcorearb.h +++ b/include/GL/glcorearb.h @@ -2893,6 +2893,42 @@ GLAPI void APIENTRY glTextureBarrier (void); #endif #endif /* GL_VERSION_4_5 */ +#ifndef GL_VERSION_4_6 +#define GL_VERSION_4_6 1 +#define GL_SHADER_BINARY_FORMAT_SPIR_V0x9551 +#define GL_SPIR_V_BINARY 0x9552 +#define GL_PARAMETER_BUFFER 0x80EE +#define GL_PARAMETER_BUFFER_BINDING 0x80EF +#define GL_CONTEXT_FLAG_NO_ERROR_BIT 0x0008 +#define GL_VERTICES_SUBMITTED 0x82EE +#define GL_PRIMITIVES_SUBMITTED 0x82EF +#define GL_VERTEX_SHADER_INVOCATIONS 0x82F0 +#define GL_TESS_CONTROL_SHADER_PATCHES0x82F1 +#define GL_TESS_EVALUATION_SHADER_INVOCATIONS 0x82F2 +#define GL_GEOMETRY_SHADER_PRIMITIVES_EMITTED 0x82F3 +#define GL_FRAGMENT_SHADER_INVOCATIONS0x82F4 +#define GL_COMPUTE_SHADER_INVOCATIONS 0x82F5 +#define GL_CLIPPING_INPUT_PRIMITIVES 0x82F6 +#define GL_CLIPPING_OUTPUT_PRIMITIVES 0x82F7 +#define GL_POLYGON_OFFSET_CLAMP 0x8E1B +#define GL_SPIR_V_EXTENSIONS 0x9553 +#define GL_NUM_SPIR_V_EXTENSIONS 0x9554 +#define GL_TEXTURE_MAX_ANISOTROPY 0x84FE +#define GL_MAX_TEXTURE_MAX_ANISOTROPY 0x84FF +#define GL_TRANSFORM_FEEDBACK_OVERFLOW0x82EC +#define GL_TRANSFORM_FEEDBACK_STREAM_OVERFLOW 0x82ED +typedef void (APIENTRYP PFNGLSPECIALIZESHADERPROC) (GLuint shader, const GLchar *pEntryPoint, GLuint numSpecializationConstants, const GLuint *pConstantIndex, const GLuint *pConstantValue); +typedef void (APIENTRYP PFNGLMULTIDRAWARRAYSINDIRECTCOUNTPROC) (GLenum mode, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +typedef void (APIENTRYP PFNGLMULTIDRAWELEMENTSINDIRECTCOUNTPROC) (GLenum mode, GLenum type, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +typedef void (APIENTRYP PFNGLPOLYGONOFFSETCLAMPPROC) (GLfloat factor, GLfloat units, GLfloat clamp); +#ifdef GL_GLEXT_PROTOTYPES +GLAPI void APIENTRY glSpecializeShader (GLuint shader, const GLchar *pEntryPoint, GLuint numSpecializationConstants, const GLuint *pConstantIndex, const GLuint *pConstantValue); +GLAPI void APIENTRY glMultiDrawArraysIndirectCount (GLenum mode, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +GLAPI void APIENTRY glMultiDrawElementsIndirectCount (GLenum mode, GLenum type, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +GLAPI void APIENTRY glPolygonOffsetClamp (GLfloat factor, GLfloat units, GLfloat clamp); +#endif +#endif /* GL_VERSION_4_6 */ + #ifndef GL_ARB_ES2_compatibility #define GL_ARB_ES2_compatibility 1 #endif /* GL_ARB_ES2_compatibility */ @@ -3314,11 +3350,11 @@ GLAPI void APIENTRY glProgramUniform4ui64vARB (GLuint program, GLint location, G #define GL_ARB_indirect_parameters 1 #define GL_PARAMETER_BUFFER_ARB 0x80EE #define GL_PARAMETER_BUFFER_BINDING_ARB 0x80EF -typedef void (APIENTRYP PFNGLMULTIDRAWARRAYSINDIRECTCOUNTARBPROC) (GLenum mode, GLintptr indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); -typedef void (APIENTRYP PFNGLMULTIDRAWELEMENTSINDIRECTCOUNTARBPROC) (GLenum mode, GLenum type, GLintptr indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +typedef void (APIENTRYP PFNGLMULTIDRAWARRAYSINDIRECTCOUNTARBPROC) (GLenum mode, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +typedef void (APIENTRYP PFNGLMULTIDRAWELEMENTSINDIRECTCOUNTARBPROC) (GLenum mode, GLenum type, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); #ifdef GL_GLEXT_PROTOTYPES -GLAPI void APIENTRY glMultiDrawArraysIndirectCountARB (GLenum mode, GLintptr indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); -GLAPI void APIENTRY glMultiDrawElementsIndirectCountARB (GLenum mode, GLenum type, GLintptr indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +GLAPI void APIENTRY glMultiDrawArraysIndirectCountARB (GLenum mode, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); +GLAPI void APIENTRY glMultiDrawElementsIndirectCountARB (GLenum mode, GLenum type, const void *indirect, GLintptr drawcount, GLsizei maxdrawcount, GLsizei stride); #endif #endif /* GL_ARB_indirect_parameters */ @@ -3396,6 +3432,10 @@ GLAPI void APIENTRY glMaxShaderCompilerThreadsARB (GLuint count); #define GL_PIXEL_UNPACK_BUFFE
[Mesa-dev] [PATCH 3/3] mesa: Implement GL_ARB_polygon_offset_clamp
Identical to the EXT version. Signed-off-by: Adam Jackson --- docs/features.txt| 2 +- src/mapi/glapi/gen/GL4x.xml | 9 + src/mesa/drivers/dri/i965/intel_extensions.c | 1 + src/mesa/main/dlist.c| 20 src/mesa/main/extensions_table.h | 1 + src/mesa/main/get_hash_params.py | 2 +- src/mesa/main/mtypes.h | 1 + src/mesa/main/polygon.c | 18 +- src/mesa/main/polygon.h | 3 +++ src/mesa/main/version.c | 2 +- src/mesa/state_tracker/st_extensions.c | 1 + 11 files changed, 56 insertions(+), 4 deletions(-) diff --git a/docs/features.txt b/docs/features.txt index b9cbd8818f..5de50d485c 100644 --- a/docs/features.txt +++ b/docs/features.txt @@ -226,7 +226,7 @@ GL 4.6, GLSL 4.60 GL_ARB_gl_spirv in progress (Nicolai Hähnle, Ian Romanick) GL_ARB_indirect_parametersDONE (nvc0, radeonsi) GL_ARB_pipeline_statistics_query DONE (i965, nvc0, radeonsi, softpipe, swr) - GL_ARB_polygon_offset_clamp not started + GL_ARB_polygon_offset_clamp DONE (i965, nv50, nvc0, r600, radeonsi, llvmpipe, swr) GL_ARB_shader_atomic_counter_ops DONE (i965/gen7+, nvc0, radeonsi, softpipe) GL_ARB_shader_draw_parameters DONE (i965, nvc0, radeonsi) GL_ARB_shader_group_vote DONE (i965, nvc0, radeonsi) diff --git a/src/mapi/glapi/gen/GL4x.xml b/src/mapi/glapi/gen/GL4x.xml index e958ee70c7..9e7685e5fd 100644 --- a/src/mapi/glapi/gen/GL4x.xml +++ b/src/mapi/glapi/gen/GL4x.xml @@ -66,4 +66,13 @@ + + + + + + + + + diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c index c3cd8004a1..c94c1b8309 100644 --- a/src/mesa/drivers/dri/i965/intel_extensions.c +++ b/src/mesa/drivers/dri/i965/intel_extensions.c @@ -66,6 +66,7 @@ intelInitExtensions(struct gl_context *ctx) ctx->Extensions.ARB_occlusion_query = true; ctx->Extensions.ARB_occlusion_query2 = true; ctx->Extensions.ARB_point_sprite = true; + ctx->Extensions.ARB_polygon_offset_clamp = true; ctx->Extensions.ARB_seamless_cube_map = true; ctx->Extensions.ARB_shader_bit_encoding = true; ctx->Extensions.ARB_shader_draw_parameters = true; diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c index 208471aca7..7875f20ba7 100644 --- a/src/mesa/main/dlist.c +++ b/src/mesa/main/dlist.c @@ -3521,6 +3521,23 @@ save_PolygonOffsetClampEXT(GLfloat factor, GLfloat units, GLfloat clamp) } static void GLAPIENTRY +save_PolygonOffsetClamp(GLfloat factor, GLfloat units, GLfloat clamp) +{ + GET_CURRENT_CONTEXT(ctx); + Node *n; + ASSERT_OUTSIDE_SAVE_BEGIN_END_AND_FLUSH(ctx); + n = alloc_instruction(ctx, OPCODE_POLYGON_OFFSET_CLAMP, 3); + if (n) { + n[1].f = factor; + n[2].f = units; + n[3].f = clamp; + } + if (ctx->ExecuteFlag) { + CALL_PolygonOffsetClamp(ctx->Exec, (factor, units, clamp)); + } +} + +static void GLAPIENTRY save_PopAttrib(void) { GET_CURRENT_CONTEXT(ctx); @@ -10067,6 +10084,9 @@ _mesa_initialize_save_table(const struct gl_context *ctx) /* GL_EXT_window_rectangles */ SET_WindowRectanglesEXT(table, save_WindowRectanglesEXT); + + /* GL_ARB_polygon_offset_clamp */ + SET_PolygonOffsetClamp(table, save_PolygonOffsetClamp); } diff --git a/src/mesa/main/extensions_table.h b/src/mesa/main/extensions_table.h index d096260891..6f2310abab 100644 --- a/src/mesa/main/extensions_table.h +++ b/src/mesa/main/extensions_table.h @@ -94,6 +94,7 @@ EXT(ARB_pipeline_statistics_query , ARB_pipeline_statistics_query EXT(ARB_pixel_buffer_object , EXT_pixel_buffer_object , GLL, GLC, x , x , 2004) EXT(ARB_point_parameters, EXT_point_parameters , GLL, x , x , x , 1997) EXT(ARB_point_sprite, ARB_point_sprite , GLL, GLC, x , x , 2003) +EXT(ARB_polygon_offset_clamp, ARB_polygon_offset_clamp , GLL, GLC, x , x , 2017) EXT(ARB_post_depth_coverage , ARB_post_depth_coverage , x , GLC, x , x, 2015) EXT(ARB_program_interface_query , dummy_true , GLL, GLC, x , x , 2012) EXT(ARB_provoking_vertex, EXT_provoking_vertex , GLL, GLC, x , x , 2009) diff --git a/src/mesa/main/get_hash_params.py b/src/mesa/main/get_hash_params.py index 9d67ca49f8..761ce0e0c8 100644 --- a/src/mesa/main/get_hash_params.py +++ b/src/mesa/main/get_hash_params.py @@ -136,7 +136,7 @@ descriptor=[ [ "MAX_DEBUG_GROUP_STACK_DEPTH", "CONST(MAX_D
[Mesa-dev] [PATCH 2/3] mesa: Implement GL_ARB_texture_filter_anisotropic
The only difference from the EXT version is bumping the minmax to 16, so just hit all the drivers at once. Signed-off-by: Adam Jackson --- docs/features.txt| 4 +++- src/glx/glxextensions.c | 1 + src/glx/glxextensions.h | 1 + src/mesa/drivers/dri/i965/intel_extensions.c | 1 + src/mesa/drivers/dri/r200/r200_context.c | 1 + src/mesa/drivers/dri/radeon/radeon_context.c | 1 + src/mesa/main/extensions.c | 1 + src/mesa/main/extensions_table.h | 1 + src/mesa/main/mtypes.h | 1 + src/mesa/main/version.c | 2 +- src/mesa/state_tracker/st_extensions.c | 4 11 files changed, 16 insertions(+), 2 deletions(-) diff --git a/docs/features.txt b/docs/features.txt index ac7645d069..b9cbd8818f 100644 --- a/docs/features.txt +++ b/docs/features.txt @@ -231,10 +231,12 @@ GL 4.6, GLSL 4.60 GL_ARB_shader_draw_parameters DONE (i965, nvc0, radeonsi) GL_ARB_shader_group_vote DONE (i965, nvc0, radeonsi) GL_ARB_spirv_extensions in progress (Nicolai Hähnle, Ian Romanick) - GL_ARB_texture_filter_anisotropic not started + GL_ARB_texture_filter_anisotropic DONE (i965, nv40+, all radeons, softpipe (*), llvmpipe (*)) GL_ARB_transform_feedback_overflow_query DONE (i965/gen6+, radeonsi) GL_KHR_no_error started (Timothy Arceri) +(*) softpipe and llvmpipe advertise 16x anisotropy but simply ignore the setting + These are the extensions cherry-picked to make GLES 3.1 GLES3.1, GLSL ES 3.1 -- all DONE: i965/hsw+, nvc0, radeonsi diff --git a/src/glx/glxextensions.c b/src/glx/glxextensions.c index 22b078ce48..88bf0de3e6 100644 --- a/src/glx/glxextensions.c +++ b/src/glx/glxextensions.c @@ -190,6 +190,7 @@ static const struct extension_info known_gl_extensions[] = { { GL(ARB_texture_env_combine),VER(1,3), Y, N, N, N }, { GL(ARB_texture_env_crossbar), VER(1,4), Y, N, N, N }, { GL(ARB_texture_env_dot3), VER(1,3), Y, N, N, N }, + { GL(ARB_texture_filter_anisotropic), VER(0,0), Y, N, N, N }, { GL(ARB_texture_mirrored_repeat),VER(1,4), Y, N, N, N }, { GL(ARB_texture_non_power_of_two), VER(1,5), Y, N, N, N }, { GL(ARB_texture_rectangle), VER(0,0), Y, N, N, N }, diff --git a/src/glx/glxextensions.h b/src/glx/glxextensions.h index 21ad02a44b..2a595516ee 100644 --- a/src/glx/glxextensions.h +++ b/src/glx/glxextensions.h @@ -101,6 +101,7 @@ enum GL_ARB_texture_env_combine_bit, GL_ARB_texture_env_crossbar_bit, GL_ARB_texture_env_dot3_bit, + GL_ARB_texture_filter_anisotropic_bit, GL_ARB_texture_mirrored_repeat_bit, GL_ARB_texture_non_power_of_two_bit, GL_ARB_texture_rectangle_bit, diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c b/src/mesa/drivers/dri/i965/intel_extensions.c index b91bbdc8d9..c3cd8004a1 100644 --- a/src/mesa/drivers/dri/i965/intel_extensions.c +++ b/src/mesa/drivers/dri/i965/intel_extensions.c @@ -80,6 +80,7 @@ intelInitExtensions(struct gl_context *ctx) ctx->Extensions.ARB_texture_env_combine = true; ctx->Extensions.ARB_texture_env_crossbar = true; ctx->Extensions.ARB_texture_env_dot3 = true; + ctx->Extensions.ARB_texture_filter_anisotropic = true; ctx->Extensions.ARB_texture_float = true; ctx->Extensions.ARB_texture_mirror_clamp_to_edge = true; ctx->Extensions.ARB_texture_non_power_of_two = true; diff --git a/src/mesa/drivers/dri/r200/r200_context.c b/src/mesa/drivers/dri/r200/r200_context.c index ca1023c5c3..0a27985de7 100644 --- a/src/mesa/drivers/dri/r200/r200_context.c +++ b/src/mesa/drivers/dri/r200/r200_context.c @@ -339,6 +339,7 @@ GLboolean r200CreateContext( gl_api api, ctx->Extensions.ARB_texture_env_combine = true; ctx->Extensions.ARB_texture_env_dot3 = true; ctx->Extensions.ARB_texture_env_crossbar = true; + ctx->Extensions.ARB_texture_filter_anisotropic = true; ctx->Extensions.ARB_texture_mirror_clamp_to_edge = true; ctx->Extensions.ARB_vertex_program = true; ctx->Extensions.ATI_fragment_shader = (ctx->Const.MaxTextureUnits == 6); diff --git a/src/mesa/drivers/dri/radeon/radeon_context.c b/src/mesa/drivers/dri/radeon/radeon_context.c index 0c016b4da4..28a79860d7 100644 --- a/src/mesa/drivers/dri/radeon/radeon_context.c +++ b/src/mesa/drivers/dri/radeon/radeon_context.c @@ -300,6 +300,7 @@ r100CreateContext( gl_api api, ctx->Extensions.ARB_texture_env_combine = true; ctx->Extensions.ARB_texture_env_crossbar = true; ctx->Extensions.ARB_texture_env_dot3 = true; + ctx->Extensions.ARB_texture_filter_anisotropic = true; ctx->Extensions.ARB_texture_mirror_clamp_to_edge = true; ctx->Extensions.ATI_texture_env_combine3 = true; ctx->Extensions.ATI_texture_mirror_once = true; diff --git a/src/mesa
Re: [Mesa-dev] [PATCH 2/3] mesa: Implement GL_ARB_texture_filter_anisotropic
On Tue, Aug 8, 2017 at 11:52 AM, Adam Jackson wrote: > The only difference from the EXT version is bumping the minmax to 16, so > just hit all the drivers at once. > > Signed-off-by: Adam Jackson > --- > docs/features.txt| 4 +++- > src/glx/glxextensions.c | 1 + > src/glx/glxextensions.h | 1 + > src/mesa/drivers/dri/i965/intel_extensions.c | 1 + > src/mesa/drivers/dri/r200/r200_context.c | 1 + > src/mesa/drivers/dri/radeon/radeon_context.c | 1 + > src/mesa/main/extensions.c | 1 + > src/mesa/main/extensions_table.h | 1 + > src/mesa/main/mtypes.h | 1 + > src/mesa/main/version.c | 2 +- > src/mesa/state_tracker/st_extensions.c | 4 > 11 files changed, 16 insertions(+), 2 deletions(-) > > diff --git a/docs/features.txt b/docs/features.txt > index ac7645d069..b9cbd8818f 100644 > --- a/docs/features.txt > +++ b/docs/features.txt > @@ -231,10 +231,12 @@ GL 4.6, GLSL 4.60 >GL_ARB_shader_draw_parameters DONE (i965, nvc0, > radeonsi) >GL_ARB_shader_group_vote DONE (i965, nvc0, > radeonsi) >GL_ARB_spirv_extensions in progress (Nicolai > Hähnle, Ian Romanick) > - GL_ARB_texture_filter_anisotropic not started > + GL_ARB_texture_filter_anisotropic DONE (i965, nv40+, > all radeons, softpipe (*), llvmpipe (*)) Normally this is listed as nv30, nv50, nvc0, r300, r600, radeonsi as those are treated as separate drivers. Unfortunately there are scripts that parse these things and expect fixed strings. Also: src/gallium/drivers/etnaviv/etnaviv_screen.c: case PIPE_CAPF_MAX_TEXTURE_ANISOTROPY: src/gallium/drivers/etnaviv/etnaviv_screen.c- return 16.0f; src/gallium/drivers/freedreno/freedreno_screen.c: case PIPE_CAPF_MAX_TEXTURE_ANISOTROPY: src/gallium/drivers/freedreno/freedreno_screen.c- return 16.0f; src/gallium/drivers/virgl/virgl_screen.c: case PIPE_CAPF_MAX_TEXTURE_ANISOTROPY: src/gallium/drivers/virgl/virgl_screen.c- return 16.0; Also add it to relnotes/17.3.0.html Reviewed-by: Ilia Mirkin ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/3] include: Sync Khronos headers for OpenGL 4.6
On Tue, Aug 8, 2017 at 11:52 AM, Adam Jackson wrote: > Taken from c21e602b9fda1d3bbaecb08194592f67e6a0649b from > OpenGL-Registry. > > Signed-off-by: Adam Jackson Acked-by: Ilia Mirkin I glanced through the diff, and looks like you carefully preserved mesa's modifications to some of those files. -ilia ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] mesa: Implement GL_ARB_polygon_offset_clamp
IMHO this should be implemented differently. There is no change of functionality vis-a-vis the EXT version, just a name change. So a single enable flag should be preserved, and a single function entrypoint. I'd rename it to the plain name, and make the EXT one an alias of the non-suffixed one. Cheers, -ilia On Tue, Aug 8, 2017 at 11:52 AM, Adam Jackson wrote: > Identical to the EXT version. > > Signed-off-by: Adam Jackson > --- > docs/features.txt| 2 +- > src/mapi/glapi/gen/GL4x.xml | 9 + > src/mesa/drivers/dri/i965/intel_extensions.c | 1 + > src/mesa/main/dlist.c| 20 > src/mesa/main/extensions_table.h | 1 + > src/mesa/main/get_hash_params.py | 2 +- > src/mesa/main/mtypes.h | 1 + > src/mesa/main/polygon.c | 18 +- > src/mesa/main/polygon.h | 3 +++ > src/mesa/main/version.c | 2 +- > src/mesa/state_tracker/st_extensions.c | 1 + > 11 files changed, 56 insertions(+), 4 deletions(-) > > diff --git a/docs/features.txt b/docs/features.txt > index b9cbd8818f..5de50d485c 100644 > --- a/docs/features.txt > +++ b/docs/features.txt > @@ -226,7 +226,7 @@ GL 4.6, GLSL 4.60 >GL_ARB_gl_spirv in progress (Nicolai > Hähnle, Ian Romanick) >GL_ARB_indirect_parametersDONE (nvc0, radeonsi) >GL_ARB_pipeline_statistics_query DONE (i965, nvc0, > radeonsi, softpipe, swr) > - GL_ARB_polygon_offset_clamp not started > + GL_ARB_polygon_offset_clamp DONE (i965, nv50, > nvc0, r600, radeonsi, llvmpipe, swr) >GL_ARB_shader_atomic_counter_ops DONE (i965/gen7+, > nvc0, radeonsi, softpipe) >GL_ARB_shader_draw_parameters DONE (i965, nvc0, > radeonsi) >GL_ARB_shader_group_vote DONE (i965, nvc0, > radeonsi) > diff --git a/src/mapi/glapi/gen/GL4x.xml b/src/mapi/glapi/gen/GL4x.xml > index e958ee70c7..9e7685e5fd 100644 > --- a/src/mapi/glapi/gen/GL4x.xml > +++ b/src/mapi/glapi/gen/GL4x.xml > @@ -66,4 +66,13 @@ > > > > + > + > + > + > + > + > + > + > + > > diff --git a/src/mesa/drivers/dri/i965/intel_extensions.c > b/src/mesa/drivers/dri/i965/intel_extensions.c > index c3cd8004a1..c94c1b8309 100644 > --- a/src/mesa/drivers/dri/i965/intel_extensions.c > +++ b/src/mesa/drivers/dri/i965/intel_extensions.c > @@ -66,6 +66,7 @@ intelInitExtensions(struct gl_context *ctx) > ctx->Extensions.ARB_occlusion_query = true; > ctx->Extensions.ARB_occlusion_query2 = true; > ctx->Extensions.ARB_point_sprite = true; > + ctx->Extensions.ARB_polygon_offset_clamp = true; > ctx->Extensions.ARB_seamless_cube_map = true; > ctx->Extensions.ARB_shader_bit_encoding = true; > ctx->Extensions.ARB_shader_draw_parameters = true; > diff --git a/src/mesa/main/dlist.c b/src/mesa/main/dlist.c > index 208471aca7..7875f20ba7 100644 > --- a/src/mesa/main/dlist.c > +++ b/src/mesa/main/dlist.c > @@ -3521,6 +3521,23 @@ save_PolygonOffsetClampEXT(GLfloat factor, GLfloat > units, GLfloat clamp) > } > > static void GLAPIENTRY > +save_PolygonOffsetClamp(GLfloat factor, GLfloat units, GLfloat clamp) > +{ > + GET_CURRENT_CONTEXT(ctx); > + Node *n; > + ASSERT_OUTSIDE_SAVE_BEGIN_END_AND_FLUSH(ctx); > + n = alloc_instruction(ctx, OPCODE_POLYGON_OFFSET_CLAMP, 3); > + if (n) { > + n[1].f = factor; > + n[2].f = units; > + n[3].f = clamp; > + } > + if (ctx->ExecuteFlag) { > + CALL_PolygonOffsetClamp(ctx->Exec, (factor, units, clamp)); > + } > +} > + > +static void GLAPIENTRY > save_PopAttrib(void) > { > GET_CURRENT_CONTEXT(ctx); > @@ -10067,6 +10084,9 @@ _mesa_initialize_save_table(const struct gl_context > *ctx) > > /* GL_EXT_window_rectangles */ > SET_WindowRectanglesEXT(table, save_WindowRectanglesEXT); > + > + /* GL_ARB_polygon_offset_clamp */ > + SET_PolygonOffsetClamp(table, save_PolygonOffsetClamp); > } > > > diff --git a/src/mesa/main/extensions_table.h > b/src/mesa/main/extensions_table.h > index d096260891..6f2310abab 100644 > --- a/src/mesa/main/extensions_table.h > +++ b/src/mesa/main/extensions_table.h > @@ -94,6 +94,7 @@ EXT(ARB_pipeline_statistics_query , > ARB_pipeline_statistics_query > EXT(ARB_pixel_buffer_object , EXT_pixel_buffer_object > , GLL, GLC, x , x , 2004) > EXT(ARB_point_parameters, EXT_point_parameters > , GLL, x , x , x , 1997) > EXT(ARB_point_sprite, ARB_point_sprite > , GLL, GLC, x , x , 2003) > +EXT(ARB_polygon_offset_clamp, ARB_polygon_offset_clamp > , GLL, GLC, x , x , 2017) > EXT(ARB_post_dep
Re: [Mesa-dev] [PATCH 7/8] egl/drm: remove unreachable code in dri2_drm_create_surface()
On Saturday, 2017-08-05 00:25:52 +0100, Emil Velikov wrote: > From: Emil Velikov > > The function can be called only when the type is EGL_WINDOW_BIT. > Remove the unneeded switch statement. > > Signed-off-by: Emil Velikov > --- > src/egl/drivers/dri2/platform_drm.c | 20 +++- > 1 file changed, 7 insertions(+), 13 deletions(-) > > diff --git a/src/egl/drivers/dri2/platform_drm.c > b/src/egl/drivers/dri2/platform_drm.c > index 7ea43e62010..8d56fcb7698 100644 > --- a/src/egl/drivers/dri2/platform_drm.c > +++ b/src/egl/drivers/dri2/platform_drm.c > @@ -92,13 +92,13 @@ has_free_buffers(struct gbm_surface *_surf) > > static _EGLSurface * > dri2_drm_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, > -_EGLConfig *conf, void *native_window, > +_EGLConfig *conf, void *native_surface, > const EGLint *attrib_list) > { > 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; > - struct gbm_surface *window = native_window; > + struct gbm_surface *window = native_surface; Why not rename `window` too? Regardless: Reviewed-by: Eric Engestrom > struct gbm_dri_surface *surf; > const __DRIconfig *config; > > @@ -113,17 +113,11 @@ dri2_drm_create_surface(_EGLDriver *drv, _EGLDisplay > *disp, EGLint type, > if (!_eglInitSurface(&dri2_surf->base, disp, type, conf, attrib_list)) >goto cleanup_surf; > > - switch (type) { > - case EGL_WINDOW_BIT: > - surf = gbm_dri_surface(window); > - dri2_surf->gbm_surf = surf; > - dri2_surf->base.Width = surf->base.width; > - dri2_surf->base.Height = surf->base.height; > - surf->dri_private = dri2_surf; > - break; > - default: > - goto cleanup_surf; > - } > + surf = gbm_dri_surface(window); > + dri2_surf->gbm_surf = surf; > + dri2_surf->base.Width = surf->base.width; > + dri2_surf->base.Height = surf->base.height; > + surf->dri_private = dri2_surf; > > config = dri2_get_dri_config(dri2_conf, EGL_WINDOW_BIT, > dri2_surf->base.GLColorspace); > -- > 2.13.3 > ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/8] egl/drm: rename dri2_drm_create_surface()
On Saturday, 2017-08-05 02:30:51 +0100, Emil Velikov wrote: > On 5 August 2017 at 00:25, Emil Velikov wrote: > > From: Emil Velikov > > > > The function can handle only window surfaces, so let's rename it > > accordingly, killing the wrapper around it. > > > > Suggested-by: Eric Engestrom > > Signed-off-by: Emil Velikov > > --- > > New patch > > --- > > src/egl/drivers/dri2/platform_drm.c | 17 - > > 1 file changed, 4 insertions(+), 13 deletions(-) > > > > diff --git a/src/egl/drivers/dri2/platform_drm.c > > b/src/egl/drivers/dri2/platform_drm.c > > index 8d56fcb7698..89ad9e0d10c 100644 > > --- a/src/egl/drivers/dri2/platform_drm.c > > +++ b/src/egl/drivers/dri2/platform_drm.c > > @@ -91,9 +91,9 @@ has_free_buffers(struct gbm_surface *_surf) > > } > > > > static _EGLSurface * > > -dri2_drm_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, > > -_EGLConfig *conf, void *native_surface, > > -const EGLint *attrib_list) > > +dri2_drm_create_window_surface(_EGLDriver *drv, _EGLDisplay *disp, > > + _EGLConfig *conf, void *native_window, > > Fixed this locally to read native_surface, instead of native_window. With that fixed: Reviewed-by: Eric Engestrom ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] st/dri2: fix kms_swrast driconf option handling
Commit e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader") moved the option cache to the pipe_loader_device. However, the screen->dev pointer is not set when dri_init_options() is called. Move the call to after the pipe_loader_sw_probe_kms() call so screen->dev is set. This mirrors the code flow for dri2_init_screen(). Fixes: e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader") Cc: Nicolai Hähnle Cc: Marek Olšák Signed-off-by: Rob Herring --- src/gallium/state_trackers/dri/dri2.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/gallium/state_trackers/dri/dri2.c b/src/gallium/state_trackers/dri/dri2.c index 3555107856c8..680826f58144 100644 --- a/src/gallium/state_trackers/dri/dri2.c +++ b/src/gallium/state_trackers/dri/dri2.c @@ -2164,9 +2164,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) goto free_screen; - dri_init_options(screen); - if (pipe_loader_sw_probe_kms(&screen->dev, fd)) + dri_init_options(screen); pscreen = pipe_loader_create_screen(screen->dev); if (!pscreen) -- 2.11.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/dri2: fix kms_swrast driconf option handling
On Tue, Aug 8, 2017 at 12:50 PM, Rob Herring wrote: > Commit e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader") > moved the option cache to the pipe_loader_device. However, the > screen->dev pointer is not set when dri_init_options() is called. Move > the call to after the pipe_loader_sw_probe_kms() call so screen->dev is > set. This mirrors the code flow for dri2_init_screen(). > > Fixes: e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader") > Cc: Nicolai Hähnle > Cc: Marek Olšák > Signed-off-by: Rob Herring > --- > src/gallium/state_trackers/dri/dri2.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/src/gallium/state_trackers/dri/dri2.c > b/src/gallium/state_trackers/dri/dri2.c > index 3555107856c8..680826f58144 100644 > --- a/src/gallium/state_trackers/dri/dri2.c > +++ b/src/gallium/state_trackers/dri/dri2.c > @@ -2164,9 +2164,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) > if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) >goto free_screen; > > - dri_init_options(screen); > - > if (pipe_loader_sw_probe_kms(&screen->dev, fd)) { > + dri_init_options(screen); >pscreen = pipe_loader_create_screen(screen->dev); } > > if (!pscreen) > -- > 2.11.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH] gallium/util: add new module that allocate "numbers"
Will be used for allocating bindless descriptor slots for RadeonSI. Signed-off-by: Samuel Pitoiset --- src/gallium/auxiliary/Makefile.sources | 1 + src/gallium/auxiliary/util/u_idalloc.h | 103 + 2 files changed, 104 insertions(+) create mode 100644 src/gallium/auxiliary/util/u_idalloc.h diff --git a/src/gallium/auxiliary/Makefile.sources b/src/gallium/auxiliary/Makefile.sources index 9ae8e6c8ca..10101d45a4 100644 --- a/src/gallium/auxiliary/Makefile.sources +++ b/src/gallium/auxiliary/Makefile.sources @@ -253,6 +253,7 @@ C_SOURCES := \ util/u_hash_table.h \ util/u_helpers.c \ util/u_helpers.h \ + util/u_idalloc.h \ util/u_index_modify.c \ util/u_index_modify.h \ util/u_inlines.h \ diff --git a/src/gallium/auxiliary/util/u_idalloc.h b/src/gallium/auxiliary/util/u_idalloc.h new file mode 100644 index 00..e98f47ccb1 --- /dev/null +++ b/src/gallium/auxiliary/util/u_idalloc.h @@ -0,0 +1,103 @@ +/** + * + * Copyright 2017 Valve Corporation + * All Rights Reserved. + * + * 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, sub license, 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: + * + * The above copyright notice and this permission notice (including the + * next paragraph) shall be included in all copies or substantial portions + * of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS + * OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF + * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NON-INFRINGEMENT. + * IN NO EVENT SHALL THE AUTHORS AND/OR ITS SUPPLIERS BE LIABLE FOR + * ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, + * TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE + * SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + * + **/ + +#ifndef U_IDALLOC_H +#define U_IDALLOC_H + +/* A simple allocator that allocates and release "numbers". */ + +#include "util/u_memory.h" + +#ifdef __cplusplus +extern "C" { +#endif + +struct util_idalloc +{ + uint32_t *data; + unsigned num_elements; +}; + +static inline void +util_idalloc_init(struct util_idalloc *buf) +{ + memset(buf, 0, sizeof(*buf)); +} + +static inline void +util_idalloc_fini(struct util_idalloc *buf) +{ + if (buf->data) + free(buf->data); +} + +static inline void +util_idalloc_resize(struct util_idalloc *buf, unsigned new_num_elements) +{ + assert(!(new_num_elements % 32)); + + if (new_num_elements > buf->num_elements) { + unsigned i; + + buf->data = realloc(buf->data, + (new_num_elements / 32) * sizeof(*buf->data)); + + for (i = buf->num_elements / 32; i < new_num_elements / 32; i++) + buf->data[i] = 0; + buf->num_elements = new_num_elements; + } +} + +static inline int32_t +util_idalloc_get_next_free(struct util_idalloc *buf) +{ + for (unsigned i = 0; i < buf->num_elements; i++) { + if (!(buf->data[i / 32] & (1 << (i % 32 + return i; + } + return -1; +} + +static inline void +util_idalloc_lock(struct util_idalloc *buf, unsigned id) +{ + assert(id < buf->num_elements); + buf->data[id / 32] |= 1 << (id % 32); +} + +static inline void +util_idalloc_unlock(struct util_idalloc *buf, unsigned id) +{ + assert(id < buf->num_elements); + buf->data[id / 32] &= ~(1 << (id % 32)); +} + +#ifdef __cplusplus +} +#endif + +#endif /* U_IDALLOC_H */ -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 1/6] radeonsi: declare new user SGPR indices for bindless samplers/images
A new pair of user SGPR is needed for loading the bindless descriptors from shaders. Because the descriptors are global for all stages, there is no need to add separate indices for GFX9. v3: - fix merged shaders on GFX9 v2: - fix declaring new bindless parameter Signed-off-by: Samuel Pitoiset --- src/gallium/drivers/radeonsi/si_shader.c | 21 + src/gallium/drivers/radeonsi/si_shader.h | 4 +++- src/gallium/drivers/radeonsi/si_shader_internal.h | 1 + 3 files changed, 21 insertions(+), 5 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_shader.c b/src/gallium/drivers/radeonsi/si_shader.c index 09053c355e..035e36fbab 100644 --- a/src/gallium/drivers/radeonsi/si_shader.c +++ b/src/gallium/drivers/radeonsi/si_shader.c @@ -2900,6 +2900,9 @@ static void si_set_ls_return_value_for_tcs(struct si_shader_context *ctx) ret = si_insert_input_ret(ctx, ret, ctx->param_merged_wave_info, 3); ret = si_insert_input_ret(ctx, ret, ctx->param_tcs_factor_offset, 4); ret = si_insert_input_ret(ctx, ret, ctx->param_merged_scratch_offset, 5); + ret = si_insert_input_ptr_as_2xi32(ctx, ret, + ctx->param_bindless_samplers_and_images, + 8 + SI_SGPR_BINDLESS_SAMPLERS_AND_IMAGES); ret = si_insert_input_ret(ctx, ret, ctx->param_vs_state_bits, 8 + SI_SGPR_VS_STATE_BITS); @@ -2938,6 +2941,9 @@ static void si_set_es_return_value_for_gs(struct si_shader_context *ctx) ret = si_insert_input_ret(ctx, ret, ctx->param_merged_wave_info, 3); ret = si_insert_input_ret(ctx, ret, ctx->param_merged_scratch_offset, 5); + ret = si_insert_input_ptr_as_2xi32(ctx, ret, + ctx->param_bindless_samplers_and_images, + 8 + SI_SGPR_BINDLESS_SAMPLERS_AND_IMAGES); unsigned desc_param = ctx->param_vs_state_bits + 1; ret = si_insert_input_ptr_as_2xi32(ctx, ret, desc_param, @@ -4249,6 +4255,8 @@ static void declare_default_desc_pointers(struct si_shader_context *ctx, { ctx->param_rw_buffers = add_arg(fninfo, ARG_SGPR, si_const_array(ctx->v4i32, SI_NUM_RW_BUFFERS)); + ctx->param_bindless_samplers_and_images = add_arg(fninfo, ARG_SGPR, + si_const_array(ctx->v8i32, 0)); declare_per_stage_desc_pointers(ctx, fninfo, true); } @@ -4388,8 +4396,9 @@ static void create_function(struct si_shader_context *ctx) add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused */ add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused */ - add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused */ - add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused */ + ctx->param_bindless_samplers_and_images = + add_arg(&fninfo, ARG_SGPR, si_const_array(ctx->v8i32, 0)); + declare_per_stage_desc_pointers(ctx, &fninfo, ctx->type == PIPE_SHADER_VERTEX); declare_vs_specific_input_sgprs(ctx, &fninfo); @@ -4442,8 +4451,9 @@ static void create_function(struct si_shader_context *ctx) add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused (SPI_SHADER_PGM_LO/HI_GS << 8) */ add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused (SPI_SHADER_PGM_LO/HI_GS >> 24) */ - add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused */ - add_arg(&fninfo, ARG_SGPR, ctx->i32); /* unused */ + ctx->param_bindless_samplers_and_images = + add_arg(&fninfo, ARG_SGPR, si_const_array(ctx->v8i32, 0)); + declare_per_stage_desc_pointers(ctx, &fninfo, (ctx->type == PIPE_SHADER_VERTEX || ctx->type == PIPE_SHADER_TESS_EVAL)); @@ -6888,6 +6898,7 @@ static void si_build_tcs_epilog_function(struct si_shader_context *ctx, add_arg(&fninfo, ARG_SGPR, ctx->i64); add_arg(&fninfo, ARG_SGPR, ctx->i64); add_arg(&fninfo, ARG_SGPR, ctx->i64); + add_arg(&fninfo, ARG_SGPR, ctx->i64); add_arg(&fninfo, ARG_SGPR, ctx->i32); add_arg(&fninfo, ARG_SGPR, ctx->i32); add_arg(&fninfo, ARG_SGPR, ctx->i32); @@ -6898,6 +6909,7 @@ static void si_build_tcs_epilog_function(struct si_shader_context *ctx, ctx->param_tcs_offchip_addr_base64k = add_arg(&fninfo, ARG_SGPR, ctx->i32); ctx->param_tcs_factor_addr_base64k = add_arg(&fninfo, ARG_SGPR, ctx->i32); } else { + add_arg(&fninfo, ARG_SGPR, ctx->i64); add_arg(&fninfo, ARG_SGPR, ctx->i64); add_arg(&fninfo, ARG_SGPR, ctx->i64); add_arg(&fninfo, ARG_SGPR, ctx->i64); @@ -7249,6 +7261,7 @@ static void si_build_ps_epilog_function(struct si_shader_context *ctx, /* Declare input SGPRs. */
[Mesa-dev] [PATCH v3 3/6] radeonsi: only initialize dirty_mask when CE is used
Looks like it's useless to initialize that field when CE is unused. This will also allow to declare more than 64 elements for the array of bindless descriptors. Signed-off-by: Samuel Pitoiset Reviewed-by: Marek Olšák --- src/gallium/drivers/radeonsi/si_descriptors.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c index 1e0c422fb4..586310c168 100644 --- a/src/gallium/drivers/radeonsi/si_descriptors.c +++ b/src/gallium/drivers/radeonsi/si_descriptors.c @@ -125,19 +125,20 @@ static void si_init_descriptors(struct si_context *sctx, unsigned num_ce_slots, unsigned *ce_offset) { - assert(num_elements <= sizeof(desc->dirty_mask)*8); - desc->list = CALLOC(num_elements, element_dw_size * 4); desc->element_dw_size = element_dw_size; desc->num_elements = num_elements; desc->first_ce_slot = sctx->ce_ib ? first_ce_slot : 0; desc->num_ce_slots = sctx->ce_ib ? num_ce_slots : 0; - desc->dirty_mask = u_bit_consecutive64(0, num_elements); + desc->dirty_mask = 0; desc->shader_userdata_offset = shader_userdata_index * 4; if (desc->num_ce_slots) { + assert(num_elements <= sizeof(desc->dirty_mask)*8); + desc->uses_ce = true; desc->ce_offset = *ce_offset; + desc->dirty_mask = u_bit_consecutive64(0, num_elements); *ce_offset += element_dw_size * desc->num_ce_slots * 4; } -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 2/6] radeonsi: make some si_descriptors fields 32-bit
The number of bindless descriptors is dynamic and we definitely have to support more than 256 slots. Signed-off-by: Samuel Pitoiset Reviewed-by: Marek Olšák --- src/gallium/drivers/radeonsi/si_state.h | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_state.h b/src/gallium/drivers/radeonsi/si_state.h index bce4066308..2b3c37fa16 100644 --- a/src/gallium/drivers/radeonsi/si_state.h +++ b/src/gallium/drivers/radeonsi/si_state.h @@ -234,7 +234,7 @@ struct si_descriptors { /* The size of one descriptor. */ ubyte element_dw_size; /* The maximum number of descriptors. */ - ubyte num_elements; + uint32_t num_elements; /* Offset in CE RAM */ uint16_t ce_offset; @@ -243,16 +243,16 @@ struct si_descriptors { * range, direct uploads to memory will be used instead. This basically * governs switching between onchip (CE) and offchip (upload) modes. */ - ubyte first_ce_slot; - ubyte num_ce_slots; + uint32_t first_ce_slot; + uint32_t num_ce_slots; /* Slots that are used by currently-bound shaders. * With CE: It determines which slots are dumped to L2. * It doesn't skip uploads to CE RAM. * Without CE: It determines which slots are uploaded. */ - ubyte first_active_slot; - ubyte num_active_slots; + uint32_t first_active_slot; + uint32_t num_active_slots; /* Whether CE is used to upload this descriptor array. */ bool uses_ce; -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 5/6] radeonsi: use slot indexes for bindless handles
Using VRAM address as bindless handles is not a good idea because we have to use LLVMIntToPTr and the LLVM CSE pass can't optimize because it has no information about the pointer. Instead, use slots indexes like the existing descriptors. Note that we use fixed 16-dword slots for both samplers and images. This doesn't really matter because no real apps use image handles. This improves performance with DOW3 by +7%. v3: - fix si_emit_global_shader_pointers() for merged GFX9 shaders - always re-upload the array of descriptors at creation time v2: - inline si_release_bindless_descriptors() - fix overwriting sampler and image slots - use fixed 16-dword slots for images Signed-off-by: Samuel Pitoiset Reviewed-by: Marek Olšák (v2) --- src/gallium/drivers/radeonsi/si_descriptors.c | 350 ++ src/gallium/drivers/radeonsi/si_pipe.c| 12 - src/gallium/drivers/radeonsi/si_pipe.h| 23 +- src/gallium/drivers/radeonsi/si_shader_tgsi_mem.c | 35 ++- 4 files changed, 193 insertions(+), 227 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c index 799a53eefb..2e8f1320a1 100644 --- a/src/gallium/drivers/radeonsi/si_descriptors.c +++ b/src/gallium/drivers/radeonsi/si_descriptors.c @@ -1875,16 +1875,20 @@ static void si_rebind_buffer(struct pipe_context *ctx, struct pipe_resource *buf /* Bindless texture handles */ if (rbuffer->texture_handle_allocated) { + struct si_descriptors *descs = &sctx->bindless_descriptors; + util_dynarray_foreach(&sctx->resident_tex_handles, struct si_texture_handle *, tex_handle) { struct pipe_sampler_view *view = (*tex_handle)->view; - struct si_bindless_descriptor *desc = (*tex_handle)->desc; + unsigned desc_slot = (*tex_handle)->desc_slot; if (view->texture == buf) { si_set_buf_desc_address(rbuffer, view->u.buf.offset, - &desc->desc_list[4]); - desc->dirty = true; + descs->list + + desc_slot * 16 + 4); + + (*tex_handle)->desc_dirty = true; sctx->bindless_descriptors_dirty = true; radeon_add_to_buffer_list_check_mem( @@ -1897,10 +1901,12 @@ static void si_rebind_buffer(struct pipe_context *ctx, struct pipe_resource *buf /* Bindless image handles */ if (rbuffer->image_handle_allocated) { + struct si_descriptors *descs = &sctx->bindless_descriptors; + util_dynarray_foreach(&sctx->resident_img_handles, struct si_image_handle *, img_handle) { struct pipe_image_view *view = &(*img_handle)->view; - struct si_bindless_descriptor *desc = (*img_handle)->desc; + unsigned desc_slot = (*img_handle)->desc_slot; if (view->resource == buf) { if (view->access & PIPE_IMAGE_ACCESS_WRITE) @@ -1908,8 +1914,10 @@ static void si_rebind_buffer(struct pipe_context *ctx, struct pipe_resource *buf si_set_buf_desc_address(rbuffer, view->u.buf.offset, - &desc->desc_list[4]); - desc->dirty = true; + descs->list + + desc_slot * 16 + 4); + + (*img_handle)->desc_dirty = true; sctx->bindless_descriptors_dirty = true; radeon_add_to_buffer_list_check_mem( @@ -1941,11 +1949,19 @@ static void si_invalidate_buffer(struct pipe_context *ctx, struct pipe_resource } static void si_upload_bindless_descriptor(struct si_context *sctx, - struct si_bindless_descriptor *desc) + unsigned desc_slot, + unsigned num_dwords) { + struct si_descriptors *desc = &sctx->bindless_descriptors; struct radeon_winsys_cs *cs = sctx->b.gfx.cs; - uint64_t va = desc->buffer->gpu_address + desc->offset; - unsigned num_dwords = sizeof(desc->desc_list) / 4; + unsigned desc_slot_offset = desc_slot * 16; + uint32_t *data; + uint64_t va; + + data = desc->list + desc_slot_offset; + + va = desc->buffer->gpu_address + desc->buffe
[Mesa-dev] [PATCH v3 4/6] radeonsi: add si_emit_global_shader_pointers() helper
To share common code between rw buffers and bindless descriptors. v3: - rename to si_emit_global_shader_pointers() Signed-off-by: Samuel Pitoiset --- src/gallium/drivers/radeonsi/si_descriptors.c | 57 +++ 1 file changed, 31 insertions(+), 26 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c index 586310c168..799a53eefb 100644 --- a/src/gallium/drivers/radeonsi/si_descriptors.c +++ b/src/gallium/drivers/radeonsi/si_descriptors.c @@ -2185,6 +2185,35 @@ static void si_emit_shader_pointer(struct si_context *sctx, radeon_emit(cs, va >> 32); } +static void si_emit_global_shader_pointers(struct si_context *sctx, + struct si_descriptors *descs) +{ + si_emit_shader_pointer(sctx, descs, + R_00B030_SPI_SHADER_USER_DATA_PS_0); + si_emit_shader_pointer(sctx, descs, + R_00B130_SPI_SHADER_USER_DATA_VS_0); + + if (sctx->b.chip_class >= GFX9) { + /* GFX9 merged LS-HS and ES-GS. +* Set RW_BUFFERS in the special registers, so that +* it's preloaded into s[0:1] instead of s[8:9]. +*/ + si_emit_shader_pointer(sctx, descs, + R_00B208_SPI_SHADER_USER_DATA_ADDR_LO_GS); + si_emit_shader_pointer(sctx, descs, + R_00B408_SPI_SHADER_USER_DATA_ADDR_LO_HS); + } else { + si_emit_shader_pointer(sctx, descs, + R_00B230_SPI_SHADER_USER_DATA_GS_0); + si_emit_shader_pointer(sctx, descs, + R_00B330_SPI_SHADER_USER_DATA_ES_0); + si_emit_shader_pointer(sctx, descs, + R_00B430_SPI_SHADER_USER_DATA_HS_0); + si_emit_shader_pointer(sctx, descs, + R_00B530_SPI_SHADER_USER_DATA_LS_0); + } +} + void si_emit_graphics_shader_pointers(struct si_context *sctx, struct r600_atom *atom) { @@ -2194,32 +2223,8 @@ void si_emit_graphics_shader_pointers(struct si_context *sctx, descs = &sctx->descriptors[SI_DESCS_RW_BUFFERS]; - if (sctx->shader_pointers_dirty & (1 << SI_DESCS_RW_BUFFERS)) { - si_emit_shader_pointer(sctx, descs, - R_00B030_SPI_SHADER_USER_DATA_PS_0); - si_emit_shader_pointer(sctx, descs, - R_00B130_SPI_SHADER_USER_DATA_VS_0); - - if (sctx->b.chip_class >= GFX9) { - /* GFX9 merged LS-HS and ES-GS. -* Set RW_BUFFERS in the special registers, so that -* it's preloaded into s[0:1] instead of s[8:9]. -*/ - si_emit_shader_pointer(sctx, descs, - R_00B208_SPI_SHADER_USER_DATA_ADDR_LO_GS); - si_emit_shader_pointer(sctx, descs, - R_00B408_SPI_SHADER_USER_DATA_ADDR_LO_HS); - } else { - si_emit_shader_pointer(sctx, descs, - R_00B230_SPI_SHADER_USER_DATA_GS_0); - si_emit_shader_pointer(sctx, descs, - R_00B330_SPI_SHADER_USER_DATA_ES_0); - si_emit_shader_pointer(sctx, descs, - R_00B430_SPI_SHADER_USER_DATA_HS_0); - si_emit_shader_pointer(sctx, descs, - R_00B530_SPI_SHADER_USER_DATA_LS_0); - } - } + if (sctx->shader_pointers_dirty & (1 << SI_DESCS_RW_BUFFERS)) + si_emit_global_shader_pointers(sctx, descs); mask = sctx->shader_pointers_dirty & u_bit_consecutive(SI_DESCS_FIRST_SHADER, -- 2.14.0 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH v3 6/6] radeonsi: try to re-use previously deleted bindless descriptor slots
Currently, when the array is full it is resized but it can grow over and over because we don't try to re-use descriptor slots. v3: - use new idalloc gallium module Signed-off-by: Samuel Pitoiset --- src/gallium/drivers/radeonsi/si_descriptors.c | 57 +-- src/gallium/drivers/radeonsi/si_pipe.h| 2 + 2 files changed, 47 insertions(+), 12 deletions(-) diff --git a/src/gallium/drivers/radeonsi/si_descriptors.c b/src/gallium/drivers/radeonsi/si_descriptors.c index 2e8f1320a1..29eb5bf3d1 100644 --- a/src/gallium/drivers/radeonsi/si_descriptors.c +++ b/src/gallium/drivers/radeonsi/si_descriptors.c @@ -61,6 +61,7 @@ #include "gfx9d.h" #include "util/hash_table.h" +#include "util/u_idalloc.h" #include "util/u_format.h" #include "util/u_memory.h" #include "util/u_upload_mgr.h" @@ -2333,33 +2334,62 @@ static void si_init_bindless_descriptors(struct si_context *sctx, * considered to be a valid handle. */ sctx->num_bindless_descriptors = 1; + + /* Keep track of which bindless slots are used (or not). */ + util_idalloc_init(&sctx->bindless_used_slots); + util_idalloc_resize(&sctx->bindless_used_slots, num_elements); + + /* Lock slot 0 because it's an invalid handle for bindless. */ + util_idalloc_lock(&sctx->bindless_used_slots, 0); } static inline void si_release_bindless_descriptors(struct si_context *sctx) { si_release_descriptors(&sctx->bindless_descriptors); + util_idalloc_fini(&sctx->bindless_used_slots); } -static unsigned -si_create_bindless_descriptor(struct si_context *sctx, uint32_t *desc_list, - unsigned size) +static unsigned si_get_next_free_bindless_slot(struct si_context *sctx) { - struct si_descriptors *desc = &sctx->bindless_descriptors; - unsigned desc_slot, desc_slot_offset; - - /* Reserve a new slot for this bindless descriptor. */ - desc_slot = sctx->num_bindless_descriptors++; + int desc_slot; - if (desc_slot >= desc->num_elements) { - /* The array of bindless descriptors is full, resize it. */ + desc_slot = util_idalloc_get_next_free(&sctx->bindless_used_slots); + if (desc_slot < 0) { + /* No slots are available. */ + struct si_descriptors *desc = &sctx->bindless_descriptors; unsigned slot_size = desc->element_dw_size * 4; - unsigned new_num_elements = desc->num_elements * 2; + unsigned old_num_elements = desc->num_elements; + unsigned new_num_elements = old_num_elements * 2; - desc->list = REALLOC(desc->list, desc->num_elements * slot_size, + /* Resize the array of descriptors. */ + desc->list = REALLOC(desc->list, old_num_elements * slot_size, new_num_elements * slot_size); desc->num_elements = new_num_elements; desc->num_active_slots = new_num_elements; + + /* Resize the array of slots. */ + util_idalloc_resize(&sctx->bindless_used_slots, new_num_elements); + + /* Get a new slot. */ + desc_slot = util_idalloc_get_next_free(&sctx->bindless_used_slots); } + assert(desc_slot); + + /* Prevent this bindless descriptor slot to be re-used. */ + util_idalloc_lock(&sctx->bindless_used_slots, desc_slot); + + return desc_slot; +} + +static unsigned +si_create_bindless_descriptor(struct si_context *sctx, uint32_t *desc_list, + unsigned size) +{ + struct si_descriptors *desc = &sctx->bindless_descriptors; + unsigned desc_slot, desc_slot_offset; + + /* Find a free slot. */ + desc_slot = si_get_next_free_bindless_slot(sctx); /* For simplicity, sampler and image bindless descriptors use fixed * 16-dword slots for now. Image descriptors only need 8-dword but this @@ -2473,6 +2503,9 @@ static void si_delete_texture_handle(struct pipe_context *ctx, uint64_t handle) tex_handle = (struct si_texture_handle *)entry->data; + /* Unlock this descriptor slot. */ + util_idalloc_unlock(&sctx->bindless_used_slots, tex_handle->desc_slot); + pipe_sampler_view_reference(&tex_handle->view, NULL); _mesa_hash_table_remove(sctx->tex_handles, entry); FREE(tex_handle); diff --git a/src/gallium/drivers/radeonsi/si_pipe.h b/src/gallium/drivers/radeonsi/si_pipe.h index c44d6d5dff..447795d97b 100644 --- a/src/gallium/drivers/radeonsi/si_pipe.h +++ b/src/gallium/drivers/radeonsi/si_pipe.h @@ -29,6 +29,7 @@ #include "si_shader.h" #include "util/u_dynarray.h" +#include "util/u_idalloc.h" #ifdef PIPE_ARCH_BIG_ENDIAN #define SI_BIG_ENDIAN 1 @@ -429,6 +430,7 @@ struct si_context { /* Bindless descriptors. */ struct si_descriptors bindless_descriptors; + struct util_idalloc bindless_used_s
Re: [Mesa-dev] [PATCH 7/8] egl/drm: remove unreachable code in dri2_drm_create_surface()
On 8 August 2017 at 17:34, Eric Engestrom wrote: > On Saturday, 2017-08-05 00:25:52 +0100, Emil Velikov wrote: >> From: Emil Velikov >> >> The function can be called only when the type is EGL_WINDOW_BIT. >> Remove the unneeded switch statement. >> >> Signed-off-by: Emil Velikov >> --- >> src/egl/drivers/dri2/platform_drm.c | 20 +++- >> 1 file changed, 7 insertions(+), 13 deletions(-) >> >> diff --git a/src/egl/drivers/dri2/platform_drm.c >> b/src/egl/drivers/dri2/platform_drm.c >> index 7ea43e62010..8d56fcb7698 100644 >> --- a/src/egl/drivers/dri2/platform_drm.c >> +++ b/src/egl/drivers/dri2/platform_drm.c >> @@ -92,13 +92,13 @@ has_free_buffers(struct gbm_surface *_surf) >> >> static _EGLSurface * >> dri2_drm_create_surface(_EGLDriver *drv, _EGLDisplay *disp, EGLint type, >> -_EGLConfig *conf, void *native_window, >> +_EGLConfig *conf, void *native_surface, >> const EGLint *attrib_list) >> { >> 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; >> - struct gbm_surface *window = native_window; >> + struct gbm_surface *window = native_surface; > > Why not rename `window` too? > Fixed-up locally. > Regardless: > Reviewed-by: Eric Engestrom > Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 4/8] egl: rework input validation order in _eglCreateWindowSurfaceCommon
On 8 August 2017 at 16:10, Eric Engestrom wrote: > On Saturday, 2017-08-05 00:25:49 +0100, Emil Velikov wrote: >> From: Emil Velikov >> >> As mentioned in previous commit the negative tests in dEQP expect the >> arguments to be evaluated in particular order. > > The spec doesn't say that, so the test is wrong. > Changing it in Mesa doesn't hurt though, so I have nothing against it, > except for the fact it hide the dEQP bug. > I agree, the spec does not say anything on the topic. I think it makes sense to have the patch regardless, since it provides a bit more consistency. Although fixing dEQP is also a good idea. I think Tapani/Chad have some experience/pointers on the topic. -Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] st/dri2: fix kms_swrast driconf option handling
On Tue, Aug 8, 2017 at 11:56 AM, Ilia Mirkin wrote: > On Tue, Aug 8, 2017 at 12:50 PM, Rob Herring wrote: >> Commit e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader") >> moved the option cache to the pipe_loader_device. However, the >> screen->dev pointer is not set when dri_init_options() is called. Move >> the call to after the pipe_loader_sw_probe_kms() call so screen->dev is >> set. This mirrors the code flow for dri2_init_screen(). >> >> Fixes: e794f8bf8bdb ("gallium: move loading of drirc to pipe-loader") >> Cc: Nicolai Hähnle >> Cc: Marek Olšák >> Signed-off-by: Rob Herring >> --- >> src/gallium/state_trackers/dri/dri2.c | 3 +-- >> 1 file changed, 1 insertion(+), 2 deletions(-) >> >> diff --git a/src/gallium/state_trackers/dri/dri2.c >> b/src/gallium/state_trackers/dri/dri2.c >> index 3555107856c8..680826f58144 100644 >> --- a/src/gallium/state_trackers/dri/dri2.c >> +++ b/src/gallium/state_trackers/dri/dri2.c >> @@ -2164,9 +2164,8 @@ dri_kms_init_screen(__DRIscreen * sPriv) >> if (screen->fd < 0 || (fd = fcntl(screen->fd, F_DUPFD_CLOEXEC, 3)) < 0) >>goto free_screen; >> >> - dri_init_options(screen); >> - >> if (pipe_loader_sw_probe_kms(&screen->dev, fd)) > > { > >> + dri_init_options(screen); >>pscreen = pipe_loader_create_screen(screen->dev); > > } Good catch. By luck it worked for the non-error case. Rob ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 3/3] specs: Add EGL_MESA_platform_device
On Mon, 2017-08-07 at 09:24 +0100, Daniel Stone wrote: > On 2 August 2017 at 21:24, Adam Jackson wrote: > > +Overview > > + > > +A system may support multiple devices and multiple window systems. For > > +example, a Wayland environment may drive multiple GPUs and support both > > +GLX and EGL clients. In order to realize this, the implementation must > > +allow the client to specify both device and platform explicitly. > > s/GLX/X11/ > > > +New Behavior > > + > > +If EGL_DEVICE_EXT is specified as an attribute for > > eglGetPlatformDisplay, > > +the value of the attribute is interpreted as an EGLDeviceEXT as > > returned > > +by eglQueryDevicesEXT. The platform will attempt to initialize using > > +exactly the specified device. If the attribute does not name a known > > +EGLDeviceEXT, EGL_BAD_DEVICE_EXT is generated. If the device and > > platform > > +are not compatible for any reason, EGL_BAD_MATCH is generated. > > + > > +If EGL_EXT_platform_device is supported, passing EGL_DEVICE_EXT as an > > +attribute to eglGetPlatformDisplay(EGL_PLATFORM_DEVICE_EXT) is > > +undefined. > > Might as well make it an error, else someone will come to rely on > those semantics. > > Also, given it seems like something other people will want to support, > we should definitely make this EXT. Word, thanks. I've folded in the review suggestions and opened an EGL registry PR for this to (hopefully) get non-Mesa feedback: https://github.com/KhronosGroup/EGL-Registry/pull/23 - ajax ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] radeonsi: drop two unused variables in create_function()
Reviewed-by: Marek Olšák Marek On Mon, Aug 7, 2017 at 12:41 PM, Samuel Pitoiset wrote: > Signed-off-by: Samuel Pitoiset > --- > src/gallium/drivers/radeonsi/si_shader.c | 2 -- > 1 file changed, 2 deletions(-) > > diff --git a/src/gallium/drivers/radeonsi/si_shader.c > b/src/gallium/drivers/radeonsi/si_shader.c > index 035e36fbab..b2765b5089 100644 > --- a/src/gallium/drivers/radeonsi/si_shader.c > +++ b/src/gallium/drivers/radeonsi/si_shader.c > @@ -4313,8 +4313,6 @@ enum { > > static void create_function(struct si_shader_context *ctx) > { > - struct lp_build_tgsi_context *bld_base = &ctx->bld_base; > - struct gallivm_state *gallivm = &ctx->gallivm; > struct si_shader *shader = ctx->shader; > struct si_function_info fninfo; > LLVMTypeRef returns[16+32*4]; > -- > 2.14.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: Trust LLVM >= 4.0 llvm-config --libs for shared libraries
Acked-by: Marek Olšák Marek On Tue, Aug 8, 2017 at 9:23 AM, Michel Dänzer wrote: > From: Michel Dänzer > > No need to manually look for the library files anymore with current > LLVM. This sidesteps the manual method failing when LLVM was built with > -DLLVM_APPEND_VC_REV=ON. > > (This might already work with older versions of LLVM) > > Signed-off-by: Michel Dänzer > --- > configure.ac | 58 ++ > 1 file changed, 30 insertions(+), 28 deletions(-) > > diff --git a/configure.ac b/configure.ac > index 5b12dd8506..4992e9538f 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -2623,35 +2623,37 @@ if test "x$enable_llvm" = xyes; then > fi > LLVM_LIBS="`$LLVM_CONFIG --libs ${LLVM_COMPONENTS}`" > > -dnl llvm-config may not give the right answer when llvm is a built as a > -dnl single shared library, so we must work the library name out for > -dnl ourselves. > -dnl (See https://llvm.org/bugs/show_bug.cgi?id=6823) > if test "x$enable_llvm_shared_libs" = xyes; then > -dnl We can't use $LLVM_VERSION because it has 'svn' stripped out, > -LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version` > -AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], > [llvm_have_one_so=yes]) > - > -if test "x$llvm_have_one_so" = xyes; then > -dnl LLVM was built using auto*, so there is only one shared > object. > -LLVM_LIBS="-l$LLVM_SO_NAME" > -else > -dnl If LLVM was built with CMake, there will be one shared > object per > -dnl component. > -AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"], > -[AC_MSG_ERROR([Could not find llvm shared libraries: > - Please make sure you have built llvm with the --enable-shared option > - and that your llvm libraries are installed in $LLVM_LIBDIR > - If you have installed your llvm libraries to a different directory you > - can use the --with-llvm-prefix= configure flag to specify this > directory. > - NOTE: Mesa is attempting to use llvm shared libraries by default. > - If you do not want to build with llvm shared libraries and instead > want to > - use llvm static libraries then add --disable-llvm-shared-libs to your > configure > - invocation and rebuild.])]) > - > - dnl We don't need to update LLVM_LIBS in this case because the > LLVM > - dnl install uses a shared object for each component and we have > - dnl already added all of these objects to LLVM_LIBS. > +if test $LLVM_VERSION_MAJOR -lt 4; then > +dnl llvm-config may not give the right answer when llvm is a > built as a > +dnl single shared library, so we must work the library name out > for > +dnl ourselves. > +dnl (See https://llvm.org/bugs/show_bug.cgi?id=6823) > +dnl We can't use $LLVM_VERSION because it has 'svn' stripped out, > +LLVM_SO_NAME=LLVM-`$LLVM_CONFIG --version` > +AS_IF([test -f "$LLVM_LIBDIR/lib$LLVM_SO_NAME.$IMP_LIB_EXT"], > [llvm_have_one_so=yes]) > + > +if test "x$llvm_have_one_so" = xyes; then > + dnl LLVM was built using auto*, so there is only one shared > object. > + LLVM_LIBS="-l$LLVM_SO_NAME" > +else > +dnl If LLVM was built with CMake, there will be one shared > object per > +dnl component. > +AS_IF([test ! -f "$LLVM_LIBDIR/libLLVMTarget.$IMP_LIB_EXT"], > + [AC_MSG_ERROR([Could not find llvm shared libraries: > + Please make sure you have built llvm with the > --enable-shared option > + and that your llvm libraries are installed in > $LLVM_LIBDIR > + If you have installed your llvm libraries to a > different directory you > + can use the --with-llvm-prefix= configure flag to > specify this directory. > + NOTE: Mesa is attempting to use llvm shared libraries > by default. > + If you do not want to build with llvm shared > libraries and instead want to > + use llvm static libraries then add > --disable-llvm-shared-libs to your configure > + invocation and rebuild.])]) > + > +dnl We don't need to update LLVM_LIBS in this case because > the LLVM > +dnl install uses a shared object for each component and we > have > +dnl already added all of these objects to LLVM_LIBS. > +fi > fi > else > AC_MSG_WARN([Building mesa with statically linked LLVM may cause > compilation issues]) > -- > 2.13.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/
Re: [Mesa-dev] [PATCH 2/3] mesa: Implement GL_ARB_texture_filter_anisotropic
On Tue, Aug 8, 2017 at 6:01 PM, Ilia Mirkin wrote: > On Tue, Aug 8, 2017 at 11:52 AM, Adam Jackson wrote: >> The only difference from the EXT version is bumping the minmax to 16, so >> just hit all the drivers at once. >> >> Signed-off-by: Adam Jackson >> --- >> docs/features.txt| 4 +++- >> src/glx/glxextensions.c | 1 + >> src/glx/glxextensions.h | 1 + >> src/mesa/drivers/dri/i965/intel_extensions.c | 1 + >> src/mesa/drivers/dri/r200/r200_context.c | 1 + >> src/mesa/drivers/dri/radeon/radeon_context.c | 1 + >> src/mesa/main/extensions.c | 1 + >> src/mesa/main/extensions_table.h | 1 + >> src/mesa/main/mtypes.h | 1 + >> src/mesa/main/version.c | 2 +- >> src/mesa/state_tracker/st_extensions.c | 4 >> 11 files changed, 16 insertions(+), 2 deletions(-) >> >> diff --git a/docs/features.txt b/docs/features.txt >> index ac7645d069..b9cbd8818f 100644 >> --- a/docs/features.txt >> +++ b/docs/features.txt >> @@ -231,10 +231,12 @@ GL 4.6, GLSL 4.60 >>GL_ARB_shader_draw_parameters DONE (i965, nvc0, >> radeonsi) >>GL_ARB_shader_group_vote DONE (i965, nvc0, >> radeonsi) >>GL_ARB_spirv_extensions in progress >> (Nicolai Hähnle, Ian Romanick) >> - GL_ARB_texture_filter_anisotropic not started >> + GL_ARB_texture_filter_anisotropic DONE (i965, nv40+, >> all radeons, softpipe (*), llvmpipe (*)) > > Normally this is listed as nv30, nv50, nvc0, r300, r600, radeonsi as r300 shouldn't be listed in features.txt. Marek ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] specs: Add EGL_MESA_device_software
On 2 August 2017 at 21:24, Adam Jackson wrote: > The device extension string is expected to contain the name of the > extension defining what kind of device it is, so the caller can know > what kinds of operations it can perform with it. So that string had > better be non-empty, hence this trivial extension. > Is there something that forbids a EGL device to advertise an empty string? I don't recall such text. Regardless, I think this (esp. combined with the extension in 3/3) is a good idea. One minor suggestion below. > +Overview > + > +This extension defines a software EGL "device". The device is not backed > by > +any actual device node and simply renders into client memory. > + > +By defining this as an extension, EGL_EXT_device_enumeration is able to > +sanely enumerate a software fallback device. > + Drop "fallback"? Despite how crazy it sounds it's not impossible to have a system with only software device(s). Hopefully we won't see too many. Thanks Emil ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: Trust LLVM >= 4.0 llvm-config --libs for shared libraries
On 8 August 2017 at 08:23, Michel Dänzer wrote: > From: Michel Dänzer > > No need to manually look for the library files anymore with current > LLVM. This sidesteps the manual method failing when LLVM was built with > -DLLVM_APPEND_VC_REV=ON. > IIRC you recently spotted that the Suse packaging was building LLVM with separate shared libraries. This is something which gets flagged with this (admittedly) quirky test. Quick look [1] shows that Gentoo still uses separate shared libs :-\ I think we'd want to keep the best of both - just not sure how to exactly do that. --libs/--libfiles was completely busted with LLVM 3.9 and is back to normal with 4.0. Could we use that somehow? -Emil [1] https://gitweb.gentoo.org/repo/gentoo.git/tree/sys-devel/llvm/llvm-4.0.1.ebuild ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] i965: Drop uniform buffer alignment back to 16 on Gen6.
On Tuesday, August 8, 2017 12:26:41 AM PDT Kenneth Graunke wrote: > We don't push UBOs on Gen6 currently, so there's no need for the > larger alignment value. > > Cc: "17.2" > --- > src/mesa/drivers/dri/i965/brw_context.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/mesa/drivers/dri/i965/brw_context.c > b/src/mesa/drivers/dri/i965/brw_context.c > index 60b14571ed0..9c4e91fe3cd 100644 > --- a/src/mesa/drivers/dri/i965/brw_context.c > +++ b/src/mesa/drivers/dri/i965/brw_context.c > @@ -616,7 +616,7 @@ brw_initialize_context_constants(struct brw_context *brw) > * In order to push UBO data, 3DSTATE_CONSTANT_XS imposes an additional > * restriction: the start of the buffer needs to be 32B aligned. > */ > - ctx->Const.UniformBufferOffsetAlignment = 32; > + ctx->Const.UniformBufferOffsetAlignment = brw->gen >= 7 ? 32 : 16; > > /* ShaderStorageBufferOffsetAlignment should be a cacheline (64 bytes) so > * that we can safely have the CPU and GPU writing the same SSBO on > Ilia made a good point on IRC - making this different across generations would make it impossible to take an apitrace on Sandybridge and replay it on anything newer. That's not a deal breaker, but it's kind of a bummer. NVIDIA apparently requires an alignment of 256, so 32 should be fine. I'll just drop this patch unless someone thinks this is a good idea. signature.asc Description: This is a digitally signed message part. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/3] specs: Add EGL_MESA_device_software
On Tue, 2017-08-08 at 19:11 +0100, Emil Velikov wrote: > On 2 August 2017 at 21:24, Adam Jackson wrote: > > The device extension string is expected to contain the name of the > > extension defining what kind of device it is, so the caller can know > > what kinds of operations it can perform with it. So that string had > > better be non-empty, hence this trivial extension. > > > > Is there something that forbids a EGL device to advertise an empty string? > I don't recall such text. Not strictly "forbids" I suppose, but it does seem to be implied. EGL_EXT_device_query says: > The EGL_EXTENSIONS string describes which device extensions are > supported by . The string is of the same format specified > for display and client extension strings in section 3.4. Note that > device extensions are properties of the device, and are distinct > from other extension strings. It's not clear to me what a device that exposes zero device extensions would mean. If you had two such devices, how would you distinguish between them? Probably the device_query spec should make this more explicit. NVIDIA's EGL returns EGL_EXT_device_drm and EGL_NV_device_cuda in this string; I haven't tested on any other implementations (in fact am not aware of other implementations of EXT_device_query). I imagine llvmpipe would say both MESA_device_software and EXT_device_drm if it was rendering to vgem. > Drop "fallback"? Despite how crazy it sounds it's not impossible to > have a system with only software device(s). Good point, fixed. I think I also want to keep this one as a MESA extension, there could be other software devices with meaningfully different capabilities. - ajax ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 1/2] Update TextureParameter* error for incompatible texture targets
On 2017-08-06 21:18:23, Iago Toral Quiroga wrote: > The OpenGL 4.6 specs have been updated so that GetTextureParameter* > with a texture object with an incompatible TEXTURE_TARGET should now > report INVALID_OPERATION instead of INVALID_ENUM. > > Fixes: > KHR-GL45.direct_state_access.textures_parameter_errors Assuming there is no regression with the GLES CTS, both patches: Reviewed-by: Jordan Justen > --- > src/mesa/main/texparam.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/mesa/main/texparam.c b/src/mesa/main/texparam.c > index b6e91503ea..039b93349e 100644 > --- a/src/mesa/main/texparam.c > +++ b/src/mesa/main/texparam.c > @@ -174,7 +174,7 @@ get_texobj_by_name(struct gl_context *ctx, GLuint > texture, const char *name) > case GL_TEXTURE_RECTANGLE: >return texObj; > default: > - _mesa_error(ctx, GL_INVALID_ENUM, "%s(target)", name); > + _mesa_error(ctx, GL_INVALID_OPERATION, "%s(target)", name); >return NULL; > } > > -- > 2.11.0 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/15] radv: Support for VK_AMD_shader_ballot
On Mon, Aug 7, 2017 at 6:32 PM, Connor Abbott wrote: > From: Connor Abbott > > This series implements VK_AMD_shader_ballot for radv. This extension > builds on VK_EXT_shader_subgroup_ballot and VK_EXT_shader_subgroup_vote > by adding a number of reductions across a subgroup (or wavefront in AMD > terminology). Previously, shaders had to use shared memory to compute, > say, the average across all threads in a workgroup, or the minimum and > maximum values across a workgroup. But that requires a lot of accesses > to LDS memory, which is (relatively) slow. This extension allows the > shader to do part of the reduction directly in registers, as long as it > stays within a single wavefront, reducing the amount of traffic to the > LDS that has to happen. It also adds a few AMD-specific instructions, > like mbcnt. To get an idea of what exactly is in the extension, and what > inclusive scan, exclusive scan, etc. mean, you can look at the GL > extension which exposes mostly the same things [1]. > > Why should you care? It turns out that with this extension enabled, plus > a few other AMD-specific extensions that are mostly trivial, DOOM will > take a different path that uses shaders that were tuned specifically for > AMD hardware. I haven't actually tested DOOM yet, since a few more > things need to be wired up, but it's a lot less work than this extension > and I'm sure Dave or Bas will be do it for me when they get around to it > :). > > It uses a few new features of the AMDGPU LLVM backend that I just > landed, as well as one more small change that still needs review: > https://reviews.llvm.org/D34718, so it's going to require LLVM 6.0. It > also uses the DPP modifier that was only added on VI since that was > easier than using ds_swizzle (which is available on all GCN cards). It > should be possible to implement support for older cards using > ds_swizzle, but I haven't gotten to it yet. A note to those reviewing: > it might be helpful to look at the LLVM changes that this series uses, > in particular: > > https://reviews.llvm.org/rL310087 > https://reviews.llvm.org/rL310088 > https://reviews.llvm.org/D34718 > > in order to get the complete picture. I've just pushed the last LLVM change required as https://reviews.llvm.org/rL310399, so this series should now work with upstream LLVM master. > > This series depends on my previous series [2] to implement > VK_EXT_shader_subgroup_vote and VK_EXT_shader_subgroup_ballot, if > nothing else in order to be able to test the implementation. I think > DOOM also uses the latter two extensions. I've also based on my series > adding cross-thread semantics to NIR [3], which Jason needs to review, > since I was hoping that would land first, although with a little effort > it should be possible to land this first (it would require changing > PATCH 01 a little). The whole thing is available at: > > git://people.freedesktop.org/~cwabbott0/mesa radv-amd-shader-ballot > > and the LLVM branch that I've been using to test, with the one patch > added is at: > > https://github.com/cwabbott0/llvm.git dpp-intrinsics-v4 I've also forced-pushed all three Mesa branches (nir-divergence-v4, radv-shader-ballot-v4, and radv-amd-shader-ballot) with trivial rebasing after pushing the last patch in this series. I've also pushed my Crucible tests to git://people.freedesktop.org/~cwabbott0/crucible amd-shader-ballot although I haven't yet cleaned things up. At least it'll be useful for making sure this code still works. > > I've got some Crucible tests for exercising the various different parts > of the implementation, although I didn't bother to test all the possible > combinations of reductions, since they didn't really require any special > code to implement anyways. I'll try and get that cleaned up and sent out > soon. Maybe I should just push the tests? > > Finally, I'm leaving Valve soon (this week) to go back to school, and I > suspect that I won't have too much time to work on this afterwards, so > someone else will probably have to pick it up. I've been working on this > for most of the summer, since it turned out to be a way more complicated > beast to implement than I thought. It's required changes across the > entire stack, from spirv-to-nir all the way down to register allocation > in the LLVM backend. Thankfully, though, most of the tricky LLVM > changes have landed (thanks Nicolai for reviewing!) and what's left is a > lot more straightforward. I should still be around to answer questions, > though. Whew! > > [1] > https://www.khronos.org/registry/OpenGL/extensions/AMD/AMD_shader_ballot.txt > [2] https://lists.freedesktop.org/archives/mesa-dev/2017-August/164903.html > [3] https://lists.freedesktop.org/archives/mesa-dev/2017-August/164898.html > > Connor Abbott (15): > nir: define intrinsics needed for AMD_shader_ballot > spirv: import AMD extensions header > spirv: add plumbing for SPV_AMD_shader_ballot and Groups > nir: rename and generalize nir_lower_read_invocatio
Re: [Mesa-dev] [PATCH 1/8] glsl: calculate number of operands in an expression once
I'm not quite sure if the increase in size of the base ir class versus the reduced overhead and added code is worth it? Some numbers would be nice. A comment below 2017-08-07 2:18 GMT+00:00 Timothy Arceri : > Extra validation is added to ir_validate to make sure this is > always updated to the correct numer of operands, as passes like > lower_instructions modify the instructions directly rather then > generating a new one. > --- > src/compiler/glsl/glsl_to_nir.cpp | 4 +-- > src/compiler/glsl/ir.cpp | 20 +- > src/compiler/glsl/ir.h | 13 + > src/compiler/glsl/ir_builder_print_visitor.cpp | 8 +++--- > src/compiler/glsl/ir_clone.cpp | 2 +- > src/compiler/glsl/ir_constant_expression.cpp | 2 +- > src/compiler/glsl/ir_equals.cpp| 2 +- > src/compiler/glsl/ir_hv_accept.cpp | 2 +- > src/compiler/glsl/ir_print_visitor.cpp | 2 +- > src/compiler/glsl/ir_rvalue_visitor.cpp| 2 +- > src/compiler/glsl/ir_validate.cpp | 8 ++ > src/compiler/glsl/lower_instructions.cpp | 32 > ++ > src/compiler/glsl/lower_int64.cpp | 4 +-- > src/compiler/glsl/lower_mat_op_to_vec.cpp | 8 +++--- > src/compiler/glsl/lower_ubo_reference.cpp | 2 +- > .../glsl/lower_vec_index_to_cond_assign.cpp| 2 +- > src/compiler/glsl/lower_vector.cpp | 2 +- > src/compiler/glsl/opt_algebraic.cpp| 4 +-- > src/compiler/glsl/opt_constant_folding.cpp | 2 +- > src/compiler/glsl/opt_tree_grafting.cpp| 2 +- > src/mesa/program/ir_to_mesa.cpp| 4 +-- > src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 ++-- > 22 files changed, 102 insertions(+), 31 deletions(-) > > diff --git a/src/compiler/glsl/glsl_to_nir.cpp > b/src/compiler/glsl/glsl_to_nir.cpp > index 331438a183..e5166855e8 100644 > --- a/src/compiler/glsl/glsl_to_nir.cpp > +++ b/src/compiler/glsl/glsl_to_nir.cpp > @@ -1487,25 +1487,25 @@ nir_visitor::visit(ir_expression *ir) >} > >return; > } > > default: >break; > } > > nir_ssa_def *srcs[4]; > - for (unsigned i = 0; i < ir->get_num_operands(); i++) > + for (unsigned i = 0; i < ir->num_operands; i++) >srcs[i] = evaluate_rvalue(ir->operands[i]); > > glsl_base_type types[4]; > - for (unsigned i = 0; i < ir->get_num_operands(); i++) > + for (unsigned i = 0; i < ir->num_operands; i++) >if (supports_ints) > types[i] = ir->operands[i]->type->base_type; >else > types[i] = GLSL_TYPE_FLOAT; > > glsl_base_type out_type; > if (supports_ints) >out_type = ir->type->base_type; > else >out_type = GLSL_TYPE_FLOAT; > diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp > index 78889bd6d3..d501e19c01 100644 > --- a/src/compiler/glsl/ir.cpp > +++ b/src/compiler/glsl/ir.cpp > @@ -196,38 +196,46 @@ ir_expression::ir_expression(int op, const struct > glsl_type *type, > ir_rvalue *op0, ir_rvalue *op1, > ir_rvalue *op2, ir_rvalue *op3) > : ir_rvalue(ir_type_expression) > { > this->type = type; > this->operation = ir_expression_operation(op); > this->operands[0] = op0; > this->operands[1] = op1; > this->operands[2] = op2; > this->operands[3] = op3; > + init_num_operands(); > + > #ifndef NDEBUG > - int num_operands = get_num_operands(this->operation); > for (int i = num_operands; i < 4; i++) { >assert(this->operands[i] == NULL); > } > + > + for (unsigned i = 0; i < num_operands; i++) { > + assert(this->operands[i] != NULL); > + } > #endif > } > It would be nice if the upper for loop iterator used unsigned for consistency with the variable and the other loop. The same accounts for the other occurence further down. But otherwise things looks ok and the patch is: Reviewed-by: Thomas Helland > ir_expression::ir_expression(int op, ir_rvalue *op0) > : ir_rvalue(ir_type_expression) > { > this->operation = ir_expression_operation(op); > this->operands[0] = op0; > this->operands[1] = NULL; > this->operands[2] = NULL; > this->operands[3] = NULL; > > assert(op <= ir_last_unop); > + init_num_operands(); > + assert(num_operands == 1); > + assert(this->operands[0]); > > switch (this->operation) { > case ir_unop_bit_not: > case ir_unop_logic_not: > case ir_unop_neg: > case ir_unop_abs: > case ir_unop_sign: > case ir_unop_rcp: > case ir_unop_rsq: > case ir_unop_sqrt: > @@ -418,20 +426,25 @@ ir_expression::ir_expression(int op, ir_rvalue *op0) > ir_expression::ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1) > : ir_rvalue(ir_type_expression) > { > this->operation = ir_expression_op
Re: [Mesa-dev] [PATCH v2 0/2] glsl: interpolateAt*() fixes
Ping On 01.08.2017 12:49, Nicolai Hähnle wrote: Hi all, I sent a v1 of this around ~6 weeks ago. Since then, I brought some related issues up with the OpenGL WG, and we now have a rule clarification in GLSL 4.60 (which is intended to apply to earlier versions as well). This updated series implements that rule clarification. It passes on radeonsi with some additional piglit tests that I'm going to send around in a moment. Please review! Thanks, Nicolai -- Lerne, wie die Welt wirklich ist, Aber vergiss niemals, wie sie sein sollte. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 2/8] glsl: tidy up get_num_operands()
2017-08-07 2:18 GMT+00:00 Timothy Arceri : > Also add a comment that this should only be used by the ir_reader > interface for testing purposes. > --- > src/compiler/glsl/ir.cpp | 8 ++-- > src/compiler/glsl/ir.h | 14 +++--- > 2 files changed, 9 insertions(+), 13 deletions(-) > > diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp > index d501e19c01..194d535790 100644 > --- a/src/compiler/glsl/ir.cpp > +++ b/src/compiler/glsl/ir.cpp > @@ -549,38 +549,42 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, > ir_rvalue *op1, > case ir_triop_csel: >this->type = op1->type; >break; > > default: >assert(!"not reached: missing automatic type setup for ir_expression"); >this->type = glsl_type::float_type; > } > } > > -unsigned int > +/** > + * This is only here for ir_reader to used for testing purposes please use > + * the precomputed num_operands field if you need the number of operands. > + */ purposes. Please use . > +unsigned > ir_expression::get_num_operands(ir_expression_operation op) > { > assert(op <= ir_last_opcode); > > if (op <= ir_last_unop) >return 1; > > if (op <= ir_last_binop) >return 2; > > if (op <= ir_last_triop) >return 3; > > if (op <= ir_last_quadop) >return 4; > > - assert(false); > + assert(!"could not calculate number of operands"); Can we do: unreachable("Could not calculate number of operands"); The patch is: Reviewed-by: Thomas Helland > return 0; > } > > #include "ir_expression_operation_strings.h" > > const char* > depth_layout_string(ir_depth_layout layout) > { > switch(layout) { > case ir_depth_layout_none: return ""; > diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h > index 377e03657d..70b5716965 100644 > --- a/src/compiler/glsl/ir.h > +++ b/src/compiler/glsl/ir.h > @@ -1531,32 +1531,24 @@ public: > * The "variable_context" hash table links ir_variable * to ir_constant * > * that represent the variables' values. \c NULL represents an empty > * context. > * > * If the expression cannot be constant folded, this method will return > * \c NULL. > */ > virtual ir_constant *constant_expression_value(struct hash_table > *variable_context = NULL); > > /** > -* Determine the number of operands used by an expression > +* This is only here for ir_reader to used for testing purposes please use > +* the precomputed num_operands field if you need the number of operands. > */ > - static unsigned int get_num_operands(ir_expression_operation); > - > - /** > -* Determine the number of operands used by an expression > -*/ > - unsigned int get_num_operands() const > - { > - return (this->operation == ir_quadop_vector) > -? this->type->vector_elements : get_num_operands(operation); > - } > + static unsigned get_num_operands(ir_expression_operation); > > /** > * Return whether the expression operates on vectors horizontally. > */ > bool is_horizontal() const > { >return operation == ir_binop_all_equal || > operation == ir_binop_any_nequal || > operation == ir_binop_dot || > operation == ir_binop_vector_extract || > -- > 2.13.3 > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 8/8] glsl: use ralloc_str_append() rather than ralloc_asprintf_rewrite_tail()
2017-08-07 2:18 GMT+00:00 Timothy Arceri : > The Deus Ex: Mankind Divided shaders go from spending ~20 seconds > in the GLSL IR compilers front-end down to ~18.5 seconds on a > Ryzen 1800X. > > Tested by compiling once with shader-db then deleting the index file > from the shader cache and compiling again. The changes themselves look OK, but you are changing the append function that was added in the previous patch. Rebase failure? I'd like to see the changes here rebased onto the previous patch. I think that should be everything except for patch 6 and 7. I'll try to look into those more carefully tomorrow, as they're a bit tricky. > --- > src/compiler/glsl/glcpp/glcpp-parse.y | 144 > ++ > src/util/ralloc.c | 12 +-- > src/util/ralloc.h | 4 +- > 3 files changed, 122 insertions(+), 38 deletions(-) > > diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y > b/src/compiler/glsl/glcpp/glcpp-parse.y > index f1719f90b1..898a26044f 100644 > --- a/src/compiler/glsl/glcpp/glcpp-parse.y > +++ b/src/compiler/glsl/glcpp/glcpp-parse.y > @@ -202,21 +202,26 @@ add_builtin_define(glcpp_parser_t *parser, const char > *name, int value); > input: > /* empty */ > | input line > ; > > line: > control_line > | SPACE control_line > | text_line { > _glcpp_parser_print_expanded_token_list (parser, $1); > - ralloc_asprintf_rewrite_tail (&parser->output, > &parser->output_length, "\n"); > + const char *newline_str = "\n"; > + size_t size = strlen(newline_str); > + > + ralloc_str_append(&parser->output, newline_str, > + parser->output_length, size); > + parser->output_length += size; > } > | expanded_line > ; > > expanded_line: > IF_EXPANDED expression NEWLINE { > if (parser->is_gles && $2.undefined_macro) > glcpp_error(& @1, parser, "undefined macro %s in > expression (illegal in GLES)", $2.undefined_macro); > _glcpp_parser_skip_stack_push_if (parser, & @1, $2.value); > } > @@ -252,21 +257,26 @@ define: > | FUNC_IDENTIFIER '(' ')' replacement_list NEWLINE { > _define_function_macro (parser, & @1, $1, NULL, $4); > } > | FUNC_IDENTIFIER '(' identifier_list ')' replacement_list NEWLINE { > _define_function_macro (parser, & @1, $1, $3, $5); > } > ; > > control_line: > control_line_success { > - ralloc_asprintf_rewrite_tail (&parser->output, > &parser->output_length, "\n"); > + const char *newline_str = "\n"; > + size_t size = strlen(newline_str); > + > + ralloc_str_append(&parser->output, newline_str, > + parser->output_length, size); > + parser->output_length += size; > } > | control_line_error > | HASH_TOKEN LINE pp_tokens NEWLINE { > > if (parser->skip_stack == NULL || > parser->skip_stack->type == SKIP_NO_SKIP) > { > _glcpp_parser_expand_and_lex_from (parser, >LINE_EXPANDED, $3, > > EXPANSION_MODE_IGNORE_DEFINED); > @@ -1123,72 +1133,144 @@ _token_list_equal_ignoring_space(token_list_t *a, > token_list_t *b) >node_b = node_b->next; > } > > return 1; > } > > static void > _token_print(char **out, size_t *len, token_t *token) > { > if (token->type < 256) { > - ralloc_asprintf_rewrite_tail (out, len, "%c", token->type); > + size_t size = sizeof(char); > + > + ralloc_str_append(out, (char *) &token->type, *len, size); > + *len += size; >return; > } > > switch (token->type) { > case INTEGER: >ralloc_asprintf_rewrite_tail (out, len, "%" PRIiMAX, > token->value.ival); >break; > case IDENTIFIER: > case INTEGER_STRING: > - case OTHER: > - ralloc_asprintf_rewrite_tail (out, len, "%s", token->value.str); > + case OTHER: { > + size_t size = strlen(token->value.str); > + > + ralloc_str_append(out, token->value.str, *len, size); > + *len += size; >break; > - case SPACE: > - ralloc_asprintf_rewrite_tail (out, len, " "); > + } > + case SPACE: { > + const char *token_str = " "; > + size_t size = strlen(token_str); > + > + ralloc_str_append(out, token_str, *len, size); > + *len += size; >break; > - case LEFT_SHIFT: > - ralloc_asprintf_rewrite_tail (out, len, "<<"); > + } > + case LEFT_SHIFT: { > + const char *token_str = "<<"; > + size_t size = strlen(token_str); > + > + ralloc_str_append(out, token_str, *len, size); > + *len += size; >break; > - case
[Mesa-dev] AMD RXxxx/Vega: amd-staging-4.12 or amd-staging-drm-next
Hello Alex, which is the 'right' way to switch to (from amd-staging-4.11)? 4.13-rx UP (like Phoronix 'found';-)) show nice improvements. Sadly I'm hit by the BAD 'e1000e' nic regression. NetworkManager switch it on and off over and over again, since 4.11 UP. --- Daniel/Intel??? 06:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network Connection Subsystem: Fujitsu Technology Solutions Device 1192 Flags: bus master, fast devsel, latency 0, IRQ 18 Memory at b032 (32-bit, non-prefetchable) [size=128K] I/O ports at 4000 [size=32] Memory at b030 (32-bit, non-prefetchable) [size=16K] Capabilities: [c8] Power Management version 2 Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+ Capabilities: [e0] Express Endpoint, MSI 00 Capabilities: [a0] MSI-X: Enable+ Count=5 Masked- Capabilities: [100] Advanced Error Reporting Capabilities: [140] Device Serial Number 00-19-99-ff-ff-77-b8-0c Kernel driver in use: e1000e Kernel modules: e1000e [5.230215] e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k [5.230216] e1000e: Copyright(c) 1999 - 2015 Intel Corporation. [5.230474] e1000e :06:00.0: Interrupt Throttling Rate (ints/sec) set to dynamic conservative mode [5.290455] e1000e :06:00.0 :06:00.0 (uninitialized): registered PHC clock [5.346925] e1000e :06:00.0 eth0: (PCI Express:2.5GT/s:Width x1) 00:19:99:77:b8:0c [5.346927] e1000e :06:00.0 eth0: Intel(R) PRO/1000 Network Connection [5.347384] e1000e :06:00.0 eth0: MAC: 3, PHY: 8, PBA No: 313132-030 [ 10.820260] e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None [ 10.829857] e1000e :06:00.0 eth0: changing MTU from 1500 to 7152 [ 15.006499] e1000e :06:00.0 eth0: changing MTU from 7152 to 1500 [ 19.812377] e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow Control: None [ 19.820006] e1000e :06:00.0 eth0: changing MTU from 1500 to 7152 [ 24.007317] e1000e :06:00.0 eth0: changing MTU from 7152 to 1500 [to be continued] Thanks, Dieter ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] AMD RXxxx/Vega: amd-staging-4.12 or amd-staging-drm-next
On Tue, Aug 8, 2017 at 5:48 PM, Dieter Nützel wrote: > Hello Alex, > > which is the 'right' way to switch to (from amd-staging-4.11)? > 4.13-rx UP (like Phoronix 'found';-)) show nice improvements. amd-staging-4.12 is our current internal development branch. amd-staging-drm-next, is just the amd-staging-4.12 branch rebased on drm-next. We are in the process of moving our internal development to track drm-next. Both branches should work equally well for the most part. Alex > > Sadly I'm hit by the BAD 'e1000e' nic regression. > NetworkManager switch it on and off over and over again, since 4.11 UP. --- > Daniel/Intel??? > > > 06:00.0 Ethernet controller: Intel Corporation 82574L Gigabit Network > Connection > Subsystem: Fujitsu Technology Solutions Device 1192 > Flags: bus master, fast devsel, latency 0, IRQ 18 > Memory at b032 (32-bit, non-prefetchable) [size=128K] > I/O ports at 4000 [size=32] > Memory at b030 (32-bit, non-prefetchable) [size=16K] > Capabilities: [c8] Power Management version 2 > Capabilities: [d0] MSI: Enable- Count=1/1 Maskable- 64bit+ > Capabilities: [e0] Express Endpoint, MSI 00 > Capabilities: [a0] MSI-X: Enable+ Count=5 Masked- > Capabilities: [100] Advanced Error Reporting > Capabilities: [140] Device Serial Number 00-19-99-ff-ff-77-b8-0c > Kernel driver in use: e1000e > Kernel modules: e1000e > > > [5.230215] e1000e: Intel(R) PRO/1000 Network Driver - 3.2.6-k > [5.230216] e1000e: Copyright(c) 1999 - 2015 Intel Corporation. > [5.230474] e1000e :06:00.0: Interrupt Throttling Rate (ints/sec) set > to dynamic conservative mode > [5.290455] e1000e :06:00.0 :06:00.0 (uninitialized): registered > PHC clock > [5.346925] e1000e :06:00.0 eth0: (PCI Express:2.5GT/s:Width x1) > 00:19:99:77:b8:0c > [5.346927] e1000e :06:00.0 eth0: Intel(R) PRO/1000 Network > Connection > [5.347384] e1000e :06:00.0 eth0: MAC: 3, PHY: 8, PBA No: 313132-030 > [ 10.820260] e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow > Control: None > [ 10.829857] e1000e :06:00.0 eth0: changing MTU from 1500 to 7152 > [ 15.006499] e1000e :06:00.0 eth0: changing MTU from 7152 to 1500 > [ 19.812377] e1000e: eth0 NIC Link is Up 1000 Mbps Full Duplex, Flow > Control: None > [ 19.820006] e1000e :06:00.0 eth0: changing MTU from 1500 to 7152 > [ 24.007317] e1000e :06:00.0 eth0: changing MTU from 7152 to 1500 > > [to be continued] > > Thanks, > Dieter ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 03/10] anv: Pull the guts of anv_fence into anv_fence_impl
This is just a refactor, similar to what we did for semaphores, in preparation for handling VK_KHR_external_fence. --- src/intel/vulkan/anv_batch_chain.c | 22 -- src/intel/vulkan/anv_private.h | 42 ++- src/intel/vulkan/anv_queue.c | 144 ++--- 3 files changed, 159 insertions(+), 49 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 661d7ae..d5af354 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1545,10 +1545,20 @@ anv_cmd_buffer_execbuf(struct anv_device *device, } if (fence) { - result = anv_execbuf_add_bo(&execbuf, &fence->bo, NULL, - EXEC_OBJECT_WRITE, &device->alloc); - if (result != VK_SUCCESS) - return result; + assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); + struct anv_fence_impl *impl = &fence->permanent; + + switch (impl->type) { + case ANV_FENCE_TYPE_BO: + result = anv_execbuf_add_bo(&execbuf, &impl->bo.bo, NULL, + EXEC_OBJECT_WRITE, &device->alloc); + if (result != VK_SUCCESS) +return result; + break; + + default: + unreachable("Invalid fence type"); + } } if (cmd_buffer) { @@ -1593,7 +1603,7 @@ anv_cmd_buffer_execbuf(struct anv_device *device, anv_semaphore_reset_temporary(device, semaphore); } - if (fence) { + if (fence && fence->permanent.type == ANV_FENCE_TYPE_BO) { /* Once the execbuf has returned, we need to set the fence state to * SUBMITTED. We can't do this before calling execbuf because * anv_GetFenceStatus does take the global device lock before checking @@ -1604,7 +1614,7 @@ anv_cmd_buffer_execbuf(struct anv_device *device, * vkGetFenceStatus() return a valid result (VK_ERROR_DEVICE_LOST or * VK_SUCCESS) in a finite amount of time even if execbuf fails. */ - fence->state = ANV_FENCE_STATE_SUBMITTED; + fence->permanent.bo.state = ANV_FENCE_STATE_SUBMITTED; } if (result == VK_SUCCESS && need_out_fence) { diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index e47aea5..f0410ab 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1715,6 +1715,12 @@ anv_cmd_buffer_alloc_blorp_binding_table(struct anv_cmd_buffer *cmd_buffer, void anv_cmd_buffer_dump(struct anv_cmd_buffer *cmd_buffer); +enum anv_fence_type { + ANV_FENCE_TYPE_NONE = 0, + ANV_FENCE_TYPE_BO, + ANV_FENCE_TYPE_SYNCOBJ, +}; + enum anv_fence_state { /** Indicates that this is a new (or newly reset fence) */ ANV_FENCE_STATE_RESET, @@ -1727,9 +1733,41 @@ enum anv_fence_state { ANV_FENCE_STATE_SIGNALED, }; +struct anv_fence_impl { + enum anv_fence_type type; + + union { + /** Fence implementation for BO fences + * + * These fences use a BO and a set of CPU-tracked state flags. The BO + * is added to the object list of the last execbuf call in a QueueSubmit + * and is marked EXEC_WRITE. The state flags track when the BO has been + * submitted to the kernel. We need to do this because Vulkan lets you + * wait on a fence that has not yet been submitted and I915_GEM_BUSY + * will say it's idle in this case. + */ + struct { + struct anv_bo bo; + enum anv_fence_state state; + } bo; + }; +}; + struct anv_fence { - struct anv_bo bo; - enum anv_fence_state state; + /* Permanent fence state. Every fence has some form of permanent state +* (type != ANV_SEMAPHORE_TYPE_NONE). This may be a BO to fence on (for +* cross-process fences0 or it could just be a dummy for use internally. +*/ + struct anv_fence_impl permanent; + + /* Temporary fence state. A fence *may* have temporary state. That state +* is added to the fence by an import operation and is reset back to +* ANV_SEMAPHORE_TYPE_NONE when the fence is reset. A fence with temporary +* state cannot be signaled because the fence must already be signaled +* before the temporary state can be exported from the fence in the other +* process and imported here. +*/ + struct anv_fence_impl temporary; }; struct anv_event { diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index 04d6972..a3f4cd8 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -262,23 +262,26 @@ VkResult anv_CreateFence( VkFence*pFence) { ANV_FROM_HANDLE(anv_device, device, _device); - struct anv_bo fence_bo; struct anv_fence *fence; assert(pCreateInfo->sType == VK_STRUCTURE_TYPE_FENCE_CREATE_INFO); - VkResult result = anv_bo_pool_alloc(&device->batch_bo_pool, &fence_bo, 4096); + fence = vk_zalloc2(&device->alloc, pAllocator, sizeof(*fence), 8, +
[Mesa-dev] [PATCH 06/10] drm-uapi/drm: Add DRM_IOCTL_SYNCOBJ_WAIT and RESET
--- include/drm-uapi/drm.h | 19 +++ 1 file changed, 19 insertions(+) diff --git a/include/drm-uapi/drm.h b/include/drm-uapi/drm.h index bf3674a..06d1d0f 100644 --- a/include/drm-uapi/drm.h +++ b/include/drm-uapi/drm.h @@ -712,6 +712,23 @@ struct drm_syncobj_handle { __u32 pad; }; +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL (1 << 0) +#define DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT (1 << 1) +struct drm_syncobj_wait { + __u64 handles; + /* absolute timeout */ + __s64 timeout_nsec; + __u32 count_handles; + __u32 flags; + __u32 first_signaled; /* only valid when not waiting all */ + __u32 pad; +}; + +struct drm_syncobj_reset { + __u32 handle; + __u32 flags; +}; + #if defined(__cplusplus) } #endif @@ -834,6 +851,8 @@ extern "C" { #define DRM_IOCTL_SYNCOBJ_DESTROY DRM_IOWR(0xC0, struct drm_syncobj_destroy) #define DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD DRM_IOWR(0xC1, struct drm_syncobj_handle) #define DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE DRM_IOWR(0xC2, struct drm_syncobj_handle) +#define DRM_IOCTL_SYNCOBJ_WAIT DRM_IOWR(0xC3, struct drm_syncobj_wait) +#define DRM_IOCTL_SYNCOBJ_RESETDRM_IOWR(0xC4, struct drm_syncobj_reset) /** * Device specific ioctls should only be in their respective headers -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 05/10] vulkan/util: Add a vk_zalloc helper
--- src/vulkan/util/vk_alloc.h | 14 ++ 1 file changed, 14 insertions(+) diff --git a/src/vulkan/util/vk_alloc.h b/src/vulkan/util/vk_alloc.h index 2915021..f58a806 100644 --- a/src/vulkan/util/vk_alloc.h +++ b/src/vulkan/util/vk_alloc.h @@ -37,6 +37,20 @@ vk_alloc(const VkAllocationCallbacks *alloc, } static inline void * +vk_zalloc(const VkAllocationCallbacks *alloc, + size_t size, size_t align, + VkSystemAllocationScope scope) +{ + void *mem = vk_alloc(alloc, size, align, scope); + if (mem == NULL) + return NULL; + + memset(mem, 0, size); + + return mem; +} + +static inline void * vk_realloc(const VkAllocationCallbacks *alloc, void *ptr, size_t size, size_t align, VkSystemAllocationScope scope) -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 01/10] anv: Rework fences to work more like BO semaphores
This commit changes fences to work a bit more like BO semaphores. Instead of the fence being a batch, it's simply a BO that gets added to the validation list for the last execbuf call in the QueueSubmit operation. It's a bit annoying finding the last submit in the execbuf but this allows us to avoid the dummy execbuf. --- src/intel/vulkan/anv_batch_chain.c | 26 ++- src/intel/vulkan/anv_private.h | 5 +-- src/intel/vulkan/anv_queue.c | 88 +++--- 3 files changed, 51 insertions(+), 68 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index e670ad7..661d7ae 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1447,8 +1447,11 @@ anv_cmd_buffer_execbuf(struct anv_device *device, const VkSemaphore *in_semaphores, uint32_t num_in_semaphores, const VkSemaphore *out_semaphores, - uint32_t num_out_semaphores) + uint32_t num_out_semaphores, + VkFence _fence) { + ANV_FROM_HANDLE(anv_fence, fence, _fence); + struct anv_execbuf execbuf; anv_execbuf_init(&execbuf); @@ -1541,6 +1544,13 @@ anv_cmd_buffer_execbuf(struct anv_device *device, } } + if (fence) { + result = anv_execbuf_add_bo(&execbuf, &fence->bo, NULL, + EXEC_OBJECT_WRITE, &device->alloc); + if (result != VK_SUCCESS) + return result; + } + if (cmd_buffer) { result = setup_execbuf_for_cmd_buffer(&execbuf, cmd_buffer); if (result != VK_SUCCESS) @@ -1583,6 +1593,20 @@ anv_cmd_buffer_execbuf(struct anv_device *device, anv_semaphore_reset_temporary(device, semaphore); } + if (fence) { + /* Once the execbuf has returned, we need to set the fence state to + * SUBMITTED. We can't do this before calling execbuf because + * anv_GetFenceStatus does take the global device lock before checking + * fence->state. + * + * We set the fence state to SUBMITTED regardless of whether or not the + * execbuf succeeds because we need to ensure that vkWaitForFences() and + * vkGetFenceStatus() return a valid result (VK_ERROR_DEVICE_LOST or + * VK_SUCCESS) in a finite amount of time even if execbuf fails. + */ + fence->state = ANV_FENCE_STATE_SUBMITTED; + } + if (result == VK_SUCCESS && need_out_fence) { int out_fence = execbuf.execbuf.rsvd2 >> 32; for (uint32_t i = 0; i < num_out_semaphores; i++) { diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index de74637..e47aea5 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1650,7 +1650,8 @@ VkResult anv_cmd_buffer_execbuf(struct anv_device *device, const VkSemaphore *in_semaphores, uint32_t num_in_semaphores, const VkSemaphore *out_semaphores, -uint32_t num_out_semaphores); +uint32_t num_out_semaphores, +VkFence fence); VkResult anv_cmd_buffer_reset(struct anv_cmd_buffer *cmd_buffer); @@ -1728,8 +1729,6 @@ enum anv_fence_state { struct anv_fence { struct anv_bo bo; - struct drm_i915_gem_execbuffer2 execbuf; - struct drm_i915_gem_exec_object2 exec2_objects[1]; enum anv_fence_state state; }; diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index 0a40ebc..04d6972 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -114,10 +114,9 @@ VkResult anv_QueueSubmit( VkQueue _queue, uint32_tsubmitCount, const VkSubmitInfo* pSubmits, -VkFence _fence) +VkFence fence) { ANV_FROM_HANDLE(anv_queue, queue, _queue); - ANV_FROM_HANDLE(anv_fence, fence, _fence); struct anv_device *device = queue->device; /* Query for device status prior to submitting. Technically, we don't need @@ -158,7 +157,20 @@ VkResult anv_QueueSubmit( */ pthread_mutex_lock(&device->mutex); + if (fence && submitCount == 0) { + /* If we don't have any command buffers, we need to submit a dummy + * batch to give GEM something to wait on. We could, potentially, + * come up with something more efficient but this shouldn't be a + * common case. + */ + result = anv_cmd_buffer_execbuf(device, NULL, NULL, 0, NULL, 0, fence); + goto out; + } + for (uint32_t i = 0; i < submitCount; i++) { + /* Fence for this submit. NULL for all but the last one */ + VkFence submit_fence = (i == submitCount - 1) ? fence : NULL; + if (pSub
[Mesa-dev] [PATCH 02/10] anv/wsi: Use QueueSubmit to trigger the fence in AcquireNextImage
--- src/intel/vulkan/anv_wsi.c | 9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/intel/vulkan/anv_wsi.c b/src/intel/vulkan/anv_wsi.c index 9369f26..00edb22 100644 --- a/src/intel/vulkan/anv_wsi.c +++ b/src/intel/vulkan/anv_wsi.c @@ -364,22 +364,25 @@ VkResult anv_GetSwapchainImagesKHR( } VkResult anv_AcquireNextImageKHR( -VkDevice device, +VkDevice _device, VkSwapchainKHR _swapchain, uint64_t timeout, VkSemaphore semaphore, VkFence _fence, uint32_t*pImageIndex) { + ANV_FROM_HANDLE(anv_device, device, _device); ANV_FROM_HANDLE(wsi_swapchain, swapchain, _swapchain); ANV_FROM_HANDLE(anv_fence, fence, _fence); VkResult result = swapchain->acquire_next_image(swapchain, timeout, semaphore, pImageIndex); - /* Thanks to implicit sync, the image is ready immediately. */ + /* Thanks to implicit sync, the image is ready immediately. However, we +* should wait for the current GPU state to finish. +*/ if (fence) - fence->state = ANV_FENCE_STATE_SIGNALED; + anv_QueueSubmit(anv_queue_to_handle(&device->queue), 0, NULL, _fence); return result; } -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 07/10] anv/gem: Add support for syncobj wait and reset
--- src/intel/vulkan/anv_gem.c | 61 src/intel/vulkan/anv_gem_stubs.c | 20 + src/intel/vulkan/anv_private.h | 5 3 files changed, 86 insertions(+) diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c index 9e6b2bb..4f46d2a 100644 --- a/src/intel/vulkan/anv_gem.c +++ b/src/intel/vulkan/anv_gem.c @@ -488,3 +488,64 @@ anv_gem_syncobj_fd_to_handle(struct anv_device *device, int fd) return args.handle; } + +void +anv_gem_syncobj_reset(struct anv_device *device, uint32_t handle) +{ + struct drm_syncobj_reset args = { + .handle = handle, + }; + + anv_ioctl(device->fd, DRM_IOCTL_SYNCOBJ_RESET, &args); +} + +bool +anv_gem_supports_syncobj_wait(int fd) +{ + int ret; + + struct drm_syncobj_create create = { + .flags = 0, + }; + ret = anv_ioctl(fd, DRM_IOCTL_SYNCOBJ_CREATE, &create); + if (ret) + return false; + + uint32_t syncobj = create.handle; + + struct drm_syncobj_wait wait = { + .handles = (uint64_t)(uintptr_t)&create, + .count_handles = 1, + .timeout_nsec = 0, + .flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, + }; + ret = anv_ioctl(fd, DRM_IOCTL_SYNCOBJ_WAIT, &wait); + + struct drm_syncobj_destroy destroy = { + .handle = syncobj, + }; + anv_ioctl(fd, DRM_IOCTL_SYNCOBJ_DESTROY, &destroy); + + /* If it timed out, then we have the ioctl and it supports the +* DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT flag. +*/ + return ret == -1 && errno == ETIME; +} + +int +anv_gem_syncobj_wait(struct anv_device *device, + uint32_t *handles, uint32_t num_handles, + int64_t abs_timeout_ns, bool wait_all) +{ + struct drm_syncobj_wait args = { + .handles = (uint64_t)(uintptr_t)handles, + .count_handles = num_handles, + .timeout_nsec = abs_timeout_ns, + .flags = DRM_SYNCOBJ_WAIT_FLAGS_WAIT_FOR_SUBMIT, + }; + + if (wait_all) + args.flags |= DRM_SYNCOBJ_WAIT_FLAGS_WAIT_ALL; + + return anv_ioctl(device->fd, DRM_IOCTL_SYNCOBJ_WAIT, &args); +} diff --git a/src/intel/vulkan/anv_gem_stubs.c b/src/intel/vulkan/anv_gem_stubs.c index 842efb3..6c97730 100644 --- a/src/intel/vulkan/anv_gem_stubs.c +++ b/src/intel/vulkan/anv_gem_stubs.c @@ -204,3 +204,23 @@ anv_gem_syncobj_fd_to_handle(struct anv_device *device, int fd) { unreachable("Unused"); } + +void +anv_gem_syncobj_reset(struct anv_device *device, uint32_t handle) +{ + unreachable("Unused"); +} + +bool +anv_gem_supports_syncobj_wait(int fd) +{ + return false; +} + +int +anv_gem_syncobj_wait(struct anv_device *device, + uint32_t *handles, uint32_t num_handles, + int64_t abs_timeout_ns, bool wait_all) +{ + unreachable("Unused"); +} diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 5f49153..2f89d3f 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -817,6 +817,11 @@ uint32_t anv_gem_syncobj_create(struct anv_device *device); void anv_gem_syncobj_destroy(struct anv_device *device, uint32_t handle); int anv_gem_syncobj_handle_to_fd(struct anv_device *device, uint32_t handle); uint32_t anv_gem_syncobj_fd_to_handle(struct anv_device *device, int fd); +void anv_gem_syncobj_reset(struct anv_device *device, uint32_t handle); +bool anv_gem_supports_syncobj_wait(int fd); +int anv_gem_syncobj_wait(struct anv_device *device, + uint32_t *handles, uint32_t num_handles, + int64_t abs_timeout_ns, bool wait_all); VkResult anv_bo_init_new(struct anv_bo *bo, struct anv_device *device, uint64_t size); -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 04/10] anv: Rename anv_fence_state to anv_bo_fence_state
It only applies to legacy BO fences. --- src/intel/vulkan/anv_batch_chain.c | 2 +- src/intel/vulkan/anv_private.h | 10 +- src/intel/vulkan/anv_queue.c | 24 3 files changed, 18 insertions(+), 18 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index d5af354..5d876e4 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1614,7 +1614,7 @@ anv_cmd_buffer_execbuf(struct anv_device *device, * vkGetFenceStatus() return a valid result (VK_ERROR_DEVICE_LOST or * VK_SUCCESS) in a finite amount of time even if execbuf fails. */ - fence->permanent.bo.state = ANV_FENCE_STATE_SUBMITTED; + fence->permanent.bo.state = ANV_BO_FENCE_STATE_SUBMITTED; } if (result == VK_SUCCESS && need_out_fence) { diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index f0410ab..5f49153 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -1721,16 +1721,16 @@ enum anv_fence_type { ANV_FENCE_TYPE_SYNCOBJ, }; -enum anv_fence_state { +enum anv_bo_fence_state { /** Indicates that this is a new (or newly reset fence) */ - ANV_FENCE_STATE_RESET, + ANV_BO_FENCE_STATE_RESET, /** Indicates that this fence has been submitted to the GPU but is still * (as far as we know) in use by the GPU. */ - ANV_FENCE_STATE_SUBMITTED, + ANV_BO_FENCE_STATE_SUBMITTED, - ANV_FENCE_STATE_SIGNALED, + ANV_BO_FENCE_STATE_SIGNALED, }; struct anv_fence_impl { @@ -1748,7 +1748,7 @@ struct anv_fence_impl { */ struct { struct anv_bo bo; - enum anv_fence_state state; + enum anv_bo_fence_state state; } bo; }; }; diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index a3f4cd8..7348e15 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -279,9 +279,9 @@ VkResult anv_CreateFence( return result; if (pCreateInfo->flags & VK_FENCE_CREATE_SIGNALED_BIT) { - fence->permanent.bo.state = ANV_FENCE_STATE_SIGNALED; + fence->permanent.bo.state = ANV_BO_FENCE_STATE_SIGNALED; } else { - fence->permanent.bo.state = ANV_FENCE_STATE_RESET; + fence->permanent.bo.state = ANV_BO_FENCE_STATE_RESET; } *pFence = anv_fence_to_handle(fence); @@ -336,7 +336,7 @@ VkResult anv_ResetFences( switch (impl->type) { case ANV_FENCE_TYPE_BO: - impl->bo.state = ANV_FENCE_STATE_RESET; + impl->bo.state = ANV_BO_FENCE_STATE_RESET; break; default: @@ -363,18 +363,18 @@ VkResult anv_GetFenceStatus( switch (impl->type) { case ANV_FENCE_TYPE_BO: switch (impl->bo.state) { - case ANV_FENCE_STATE_RESET: + case ANV_BO_FENCE_STATE_RESET: /* If it hasn't even been sent off to the GPU yet, it's not ready */ return VK_NOT_READY; - case ANV_FENCE_STATE_SIGNALED: + case ANV_BO_FENCE_STATE_SIGNALED: /* It's been signaled, return success */ return VK_SUCCESS; - case ANV_FENCE_STATE_SUBMITTED: { + case ANV_BO_FENCE_STATE_SUBMITTED: { VkResult result = anv_device_bo_busy(device, &impl->bo.bo); if (result == VK_SUCCESS) { -impl->bo.state = ANV_FENCE_STATE_SIGNALED; +impl->bo.state = ANV_BO_FENCE_STATE_SIGNALED; return VK_SUCCESS; } else { return result; @@ -427,7 +427,7 @@ anv_wait_for_bo_fences(struct anv_device *device, struct anv_fence_impl *impl = &fence->permanent; switch (impl->bo.state) { - case ANV_FENCE_STATE_RESET: + case ANV_BO_FENCE_STATE_RESET: /* This fence hasn't been submitted yet, we'll catch it the next * time around. Yes, this may mean we dead-loop but, short of * lots of locking and a condition variable, there's not much that @@ -436,7 +436,7 @@ anv_wait_for_bo_fences(struct anv_device *device, pending_fences++; continue; - case ANV_FENCE_STATE_SIGNALED: + case ANV_BO_FENCE_STATE_SIGNALED: /* This fence is not pending. If waitAll isn't set, we can return * early. Otherwise, we have to keep going. */ @@ -446,14 +446,14 @@ anv_wait_for_bo_fences(struct anv_device *device, } continue; - case ANV_FENCE_STATE_SUBMITTED: + case ANV_BO_FENCE_STATE_SUBMITTED: /* These are the fences we really care about. Go ahead and wait * on it until we hit a timeout. */ result = anv_device_wait(device, &impl->bo.bo, timeout); switch (result) { case VK_SUCCESS: - impl->bo.state = ANV_FENCE_STATE_SIGNALED; + impl->bo.state = ANV_BO_FENCE_STATE_SIGNALED; signaled_fences =
[Mesa-dev] [PATCH 00/10] anv: Implement VK_KHR_external_fence*
This little series adds support for the VK_KHR_external_fence family of extensions. Most of the real work in implementing these extensions is actually in the kernel. Once we have a DRM_SYNCOBJ_IOCTL_WAIT that does what we need, the userspace bits are fairly straightforward. This series can be found as a branch here: https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/anv-external-fence The required kernel bits can be found here: https://cgit.freedesktop.org/~jekstrand/linux/log/?h=drm-syncobj-wait-submit-v1 Cc: Chad Versace Cc: Chris Wilson Jason Ekstrand (10): anv: Rework fences to work more like BO semaphores anv/wsi: Use QueueSubmit to trigger the fence in AcquireNextImage anv: Pull the guts of anv_fence into anv_fence_impl anv: Rename anv_fence_state to anv_bo_fence_state vulkan/util: Add a vk_zalloc helper drm-uapi/drm: Add DRM_IOCTL_SYNCOBJ_WAIT and RESET anv/gem: Add support for syncobj wait and reset anv: Use DRM sync objects to back fences whenever possible anv: Implement VK_KHR_external_fence anv: Add support for the SYNC_FD handle type for fences include/drm-uapi/drm.h | 19 ++ src/intel/vulkan/anv_batch_chain.c | 59 - src/intel/vulkan/anv_device.c | 1 + src/intel/vulkan/anv_extensions.py | 5 + src/intel/vulkan/anv_gem.c | 56 src/intel/vulkan/anv_gem_stubs.c | 27 ++ src/intel/vulkan/anv_private.h | 67 - src/intel/vulkan/anv_queue.c | 513 + src/intel/vulkan/anv_wsi.c | 9 +- src/vulkan/util/vk_alloc.h | 14 + 10 files changed, 651 insertions(+), 119 deletions(-) -- 2.5.0.400.gff86faf ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 09/10] anv: Implement VK_KHR_external_fence
--- src/intel/vulkan/anv_batch_chain.c | 19 - src/intel/vulkan/anv_extensions.py | 5 ++ src/intel/vulkan/anv_queue.c | 142 - 3 files changed, 161 insertions(+), 5 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 15082b5..7d16403 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1545,8 +1545,20 @@ anv_cmd_buffer_execbuf(struct anv_device *device, } if (fence) { - assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); - struct anv_fence_impl *impl = &fence->permanent; + /* Under most circumstances, out fences won't be temporary. However, + * the spec does allow it for opaque_fd. From the Vulkan 1.0.53 spec: + * + *"If the import is temporary, the implementation must restore the + *semaphore to its prior permanent state after submitting the next + *semaphore wait operation." + * + * The spec says nothing whatsoever about signal operations on + * temporarily imported semaphores so it appears they are allowed. + * There are also CTS tests that require this to work. + */ + struct anv_fence_impl *impl = + fence->temporary.type != ANV_FENCE_TYPE_NONE ? + &fence->temporary : &fence->permanent; switch (impl->type) { case ANV_FENCE_TYPE_BO: @@ -1612,6 +1624,9 @@ anv_cmd_buffer_execbuf(struct anv_device *device, } if (fence && fence->permanent.type == ANV_FENCE_TYPE_BO) { + /* BO fences can't be shared, so they can't be temporary. */ + assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); + /* Once the execbuf has returned, we need to set the fence state to * SUBMITTED. We can't do this before calling execbuf because * anv_GetFenceStatus does take the global device lock before checking diff --git a/src/intel/vulkan/anv_extensions.py b/src/intel/vulkan/anv_extensions.py index 3252e0f..6b3d72e 100644 --- a/src/intel/vulkan/anv_extensions.py +++ b/src/intel/vulkan/anv_extensions.py @@ -47,6 +47,11 @@ class Extension: EXTENSIONS = [ Extension('VK_KHR_dedicated_allocation', 1, True), Extension('VK_KHR_descriptor_update_template',1, True), +Extension('VK_KHR_external_fence',1, + 'device->has_syncobj_wait'), +Extension('VK_KHR_external_fence_capabilities', 1, True), +Extension('VK_KHR_external_fence_fd', 1, + 'device->has_syncobj_wait'), Extension('VK_KHR_external_memory', 1, True), Extension('VK_KHR_external_memory_capabilities', 1, True), Extension('VK_KHR_external_memory_fd',1, True), diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index 8e45bb2..ebd62ca 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -345,7 +345,18 @@ VkResult anv_ResetFences( for (uint32_t i = 0; i < fenceCount; i++) { ANV_FROM_HANDLE(anv_fence, fence, pFences[i]); - assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); + /* From the Vulkan 1.0.53 spec: + * + *"If any member of pFences currently has its payload imported with + *temporary permanence, that fence’s prior permanent payload is + *first restored. The remaining operations described therefore + *operate on the restored payload. + */ + if (fence->temporary.type != ANV_FENCE_TYPE_NONE) { + anv_fence_impl_cleanup(device, &fence->temporary); + fence->temporary.type = ANV_FENCE_TYPE_NONE; + } + struct anv_fence_impl *impl = &fence->permanent; switch (impl->type) { @@ -375,11 +386,14 @@ VkResult anv_GetFenceStatus( if (unlikely(device->lost)) return VK_ERROR_DEVICE_LOST; - assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); - struct anv_fence_impl *impl = &fence->permanent; + struct anv_fence_impl *impl = + fence->temporary.type != ANV_FENCE_TYPE_NONE ? + &fence->temporary : &fence->permanent; switch (impl->type) { case ANV_FENCE_TYPE_BO: + /* BO fences don't support import/export */ + assert(fence->temporary.type == ANV_FENCE_TYPE_NONE); switch (impl->bo.state) { case ANV_BO_FENCE_STATE_RESET: /* If it hasn't even been sent off to the GPU yet, it's not ready */ @@ -661,6 +675,128 @@ VkResult anv_WaitForFences( } } +void anv_GetPhysicalDeviceExternalFencePropertiesKHR( +VkPhysicalDevicephysicalDevice, +const VkPhysicalDeviceExternalFenceInfoKHR* pExternalFenceInfo, +VkExternalFencePropertiesKHR* pExternalFenceProperties) +{ + ANV_FROM_HANDLE(anv_physical_device, device, physicalDevice); + + switch (pExternalFenceInfo->handleType) { + case VK_EXTERNAL_FENCE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR: + if (dev
[Mesa-dev] [PATCH 08/10] anv: Use DRM sync objects to back fences whenever possible
In order to implement VK_KHR_external_fence, we need to back our fences with something that's shareable. Since the kernel wait interface for sync objects already supports waiting for multiple fences in one go, it makes anv_WaitForFences much simpler if we only have one type of fence. --- src/intel/vulkan/anv_batch_chain.c | 8 +++ src/intel/vulkan/anv_device.c | 2 + src/intel/vulkan/anv_private.h | 4 ++ src/intel/vulkan/anv_queue.c | 132 ++--- 4 files changed, 136 insertions(+), 10 deletions(-) diff --git a/src/intel/vulkan/anv_batch_chain.c b/src/intel/vulkan/anv_batch_chain.c index 5d876e4..15082b5 100644 --- a/src/intel/vulkan/anv_batch_chain.c +++ b/src/intel/vulkan/anv_batch_chain.c @@ -1556,6 +1556,14 @@ anv_cmd_buffer_execbuf(struct anv_device *device, return result; break; + case ANV_FENCE_TYPE_SYNCOBJ: + result = anv_execbuf_add_syncobj(&execbuf, impl->syncobj, + I915_EXEC_FENCE_SIGNAL, + &device->alloc); + if (result != VK_SUCCESS) +return result; + break; + default: unreachable("Invalid fence type"); } diff --git a/src/intel/vulkan/anv_device.c b/src/intel/vulkan/anv_device.c index a6d5215..2e0fa19 100644 --- a/src/intel/vulkan/anv_device.c +++ b/src/intel/vulkan/anv_device.c @@ -339,6 +339,8 @@ anv_physical_device_init(struct anv_physical_device *device, device->has_exec_async = anv_gem_get_param(fd, I915_PARAM_HAS_EXEC_ASYNC); device->has_exec_fence = anv_gem_get_param(fd, I915_PARAM_HAS_EXEC_FENCE); device->has_syncobj = anv_gem_get_param(fd, I915_PARAM_HAS_EXEC_FENCE_ARRAY); + device->has_syncobj_wait = device->has_syncobj && + anv_gem_supports_syncobj_wait(fd); bool swizzled = anv_gem_get_bit6_swizzle(fd, I915_TILING_X); diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 2f89d3f..430652d 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -654,6 +654,7 @@ struct anv_physical_device { boolhas_exec_async; boolhas_exec_fence; boolhas_syncobj; +boolhas_syncobj_wait; uint32_teu_total; uint32_tsubslice_total; @@ -1755,6 +1756,9 @@ struct anv_fence_impl { struct anv_bo bo; enum anv_bo_fence_state state; } bo; + + /** DRM syncobj handle for syncobj-based fences */ + uint32_t syncobj; }; }; diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index 7348e15..8e45bb2 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -271,17 +271,25 @@ VkResult anv_CreateFence( if (fence == NULL) return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY); - fence->permanent.type = ANV_FENCE_TYPE_BO; + if (device->instance->physicalDevice.has_syncobj_wait) { + fence->permanent.type = ANV_FENCE_TYPE_SYNCOBJ; - VkResult result = anv_bo_pool_alloc(&device->batch_bo_pool, - &fence->permanent.bo.bo, 4096); - if (result != VK_SUCCESS) - return result; - - if (pCreateInfo->flags & VK_FENCE_CREATE_SIGNALED_BIT) { - fence->permanent.bo.state = ANV_BO_FENCE_STATE_SIGNALED; + fence->permanent.syncobj = anv_gem_syncobj_create(device); + if (!fence->permanent.syncobj) + return vk_error(VK_ERROR_OUT_OF_HOST_MEMORY); } else { - fence->permanent.bo.state = ANV_BO_FENCE_STATE_RESET; + fence->permanent.type = ANV_FENCE_TYPE_BO; + + VkResult result = anv_bo_pool_alloc(&device->batch_bo_pool, + &fence->permanent.bo.bo, 4096); + if (result != VK_SUCCESS) + return result; + + if (pCreateInfo->flags & VK_FENCE_CREATE_SIGNALED_BIT) { + fence->permanent.bo.state = ANV_BO_FENCE_STATE_SIGNALED; + } else { + fence->permanent.bo.state = ANV_BO_FENCE_STATE_RESET; + } } *pFence = anv_fence_to_handle(fence); @@ -301,6 +309,10 @@ anv_fence_impl_cleanup(struct anv_device *device, case ANV_FENCE_TYPE_BO: anv_bo_pool_free(&device->batch_bo_pool, &impl->bo.bo); return; + + case ANV_FENCE_TYPE_SYNCOBJ: + anv_gem_syncobj_destroy(device, impl->syncobj); + return; } unreachable("Invalid fence type"); @@ -328,6 +340,8 @@ VkResult anv_ResetFences( uint32_tfenceCount, const VkFence* pFences) { + ANV_FROM_HANDLE(anv_device, device, _device); + for (uint32_t i = 0; i < fenceCount; i++) { ANV_FROM_HANDLE(anv_fence, fence, pFences[i]); @@ -339,6 +353,10 @
[Mesa-dev] [PATCH 10/10] anv: Add support for the SYNC_FD handle type for fences
--- src/intel/vulkan/anv_gem.c | 28 + src/intel/vulkan/anv_gem_stubs.c | 13 ++ src/intel/vulkan/anv_private.h | 4 +++ src/intel/vulkan/anv_queue.c | 53 +++- 4 files changed, 87 insertions(+), 11 deletions(-) diff --git a/src/intel/vulkan/anv_gem.c b/src/intel/vulkan/anv_gem.c index 4f46d2a..d46c223 100644 --- a/src/intel/vulkan/anv_gem.c +++ b/src/intel/vulkan/anv_gem.c @@ -489,6 +489,34 @@ anv_gem_syncobj_fd_to_handle(struct anv_device *device, int fd) return args.handle; } +int +anv_gem_syncobj_export_sync_file(struct anv_device *device, uint32_t handle) +{ + struct drm_syncobj_handle args = { + .handle = handle, + .flags = DRM_SYNCOBJ_HANDLE_TO_FD_FLAGS_EXPORT_SYNC_FILE, + }; + + int ret = anv_ioctl(device->fd, DRM_IOCTL_SYNCOBJ_HANDLE_TO_FD, &args); + if (ret) + return -1; + + return args.fd; +} + +int +anv_gem_syncobj_import_sync_file(struct anv_device *device, + uint32_t handle, int fd) +{ + struct drm_syncobj_handle args = { + .handle = handle, + .fd = fd, + .flags = DRM_SYNCOBJ_FD_TO_HANDLE_FLAGS_IMPORT_SYNC_FILE, + }; + + return anv_ioctl(device->fd, DRM_IOCTL_SYNCOBJ_FD_TO_HANDLE, &args); +} + void anv_gem_syncobj_reset(struct anv_device *device, uint32_t handle) { diff --git a/src/intel/vulkan/anv_gem_stubs.c b/src/intel/vulkan/anv_gem_stubs.c index 6c97730..d094d12 100644 --- a/src/intel/vulkan/anv_gem_stubs.c +++ b/src/intel/vulkan/anv_gem_stubs.c @@ -181,6 +181,19 @@ anv_gem_fd_to_handle(struct anv_device *device, int fd) unreachable("Unused"); } +int +anv_gem_syncobj_export_sync_file(struct anv_device *device, uint32_t handle) +{ + unreachable("Unused"); +} + +int +anv_gem_syncobj_import_sync_file(struct anv_device *device, + uint32_t handle, int fd) +{ + unreachable("Unused"); +} + uint32_t anv_gem_syncobj_create(struct anv_device *device) { diff --git a/src/intel/vulkan/anv_private.h b/src/intel/vulkan/anv_private.h index 430652d..bc19d79 100644 --- a/src/intel/vulkan/anv_private.h +++ b/src/intel/vulkan/anv_private.h @@ -818,6 +818,10 @@ uint32_t anv_gem_syncobj_create(struct anv_device *device); void anv_gem_syncobj_destroy(struct anv_device *device, uint32_t handle); int anv_gem_syncobj_handle_to_fd(struct anv_device *device, uint32_t handle); uint32_t anv_gem_syncobj_fd_to_handle(struct anv_device *device, int fd); +int anv_gem_syncobj_export_sync_file(struct anv_device *device, + uint32_t handle); +int anv_gem_syncobj_import_sync_file(struct anv_device *device, + uint32_t handle, int fd); void anv_gem_syncobj_reset(struct anv_device *device, uint32_t handle); bool anv_gem_supports_syncobj_wait(int fd); int anv_gem_syncobj_wait(struct anv_device *device, diff --git a/src/intel/vulkan/anv_queue.c b/src/intel/vulkan/anv_queue.c index ebd62ca..8b2f3a3 100644 --- a/src/intel/vulkan/anv_queue.c +++ b/src/intel/vulkan/anv_queue.c @@ -684,11 +684,14 @@ void anv_GetPhysicalDeviceExternalFencePropertiesKHR( switch (pExternalFenceInfo->handleType) { case VK_EXTERNAL_FENCE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR: + case VK_EXTERNAL_FENCE_HANDLE_TYPE_SYNC_FD_BIT_KHR: if (device->has_syncobj_wait) { pExternalFenceProperties->exportFromImportedHandleTypes = -VK_EXTERNAL_FENCE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR; +VK_EXTERNAL_FENCE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR | +VK_EXTERNAL_FENCE_HANDLE_TYPE_SYNC_FD_BIT_KHR; pExternalFenceProperties->compatibleHandleTypes = -VK_EXTERNAL_FENCE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR; +VK_EXTERNAL_FENCE_HANDLE_TYPE_OPAQUE_FD_BIT_KHR | +VK_EXTERNAL_FENCE_HANDLE_TYPE_SYNC_FD_BIT_KHR; pExternalFenceProperties->externalFenceFeatures = VK_EXTERNAL_FENCE_FEATURE_EXPORTABLE_BIT_KHR | VK_EXTERNAL_FENCE_FEATURE_IMPORTABLE_BIT_KHR; @@ -728,22 +731,41 @@ VkResult anv_ImportFenceFdKHR( if (!new_impl.syncobj) return vk_error(VK_ERROR_INVALID_EXTERNAL_HANDLE_KHR); - /* From the Vulkan 1.0.53 spec: - * - *"Importing a fence payload from a file descriptor transfers - *ownership of the file descriptor from the application to the - *Vulkan implementation. The application must not perform any - *operations on the file descriptor after a successful import." - * - * If the import fails, we leave the file descriptor open. + break; + + case VK_EXTERNAL_FENCE_HANDLE_TYPE_SYNC_FD_BIT_KHR: + /* Sync files are a bit tricky. Because we want to continue using the + * syncobj implementation of WaitForFences, we don't use the sync file + * directly but instead import it into a syncobj. */ - close(fd); + new_impl.type = ANV_FENCE_TYPE_SYNCOBJ; + + new_impl.synco
[Mesa-dev] [Bug 102122] [softpipe] piglit fdo10370 regression
https://bugs.freedesktop.org/show_bug.cgi?id=102122 Bug ID: 102122 Summary: [softpipe] piglit fdo10370 regression Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Keywords: bisected, regression Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org CC: mar...@gmail.com, nhaeh...@gmail.com mesa: c12c2e40a36f707f733c0d6ad90160472b7a3cf6 (master 17.3.0-devel) $ ./bin/fdo10370 -auto data[0x8f], foreground: expected RGBA (1.0, 0.0 0.0 1.0) First execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 CallList execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 data[0xff], foreground: expected RGBA (1.0, 0.0 0.0 1.0) First execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 CallList execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 data[0x7f], background: expected RGBA (0.0, 0.0 1.0 1.0) First execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 CallList execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 data[0x70], background: expected RGBA (0.0, 0.0 1.0 1.0) First execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 CallList execution, Readback RGBA: pixel[0, 0]: 1.0 0.0 0.0 1.0 max delta: 1.00 PIGLIT: {"result": "fail" } 678dadf1237a3fb492ee2d8daa32d0205dea59ba is the first bad commit commit 678dadf1237a3fb492ee2d8daa32d0205dea59ba Author: Nicolai Hähnle Date: Thu Jun 29 17:37:18 2017 +0200 gallium: move driinfo XML to pipe_loader We will switch to the pipe_loader loading the configuration options, so that they can be passed to the driver independently of the state tracker. Put the description into its own file so that it can be merged easily with driver-specific options in future commits. Reviewed-by: Marek Olšák :04 04 f973edf38d4f55ed798bfd5e534c14fa79d7a030 c8d20373c9716a640f9ef4ea0c928b40912dda37 M src bisect run success -- You are receiving this mail because: You are the assignee for the bug. You are the QA Contact for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 09/25] i965: Reorder brw_reg_type enum values
Matt Turner writes: > These vaguely corresponded to the hardware encodings, but that is purely > historical at this point. Reorder them so we stop making things "almost > work" when mixing enums. > > The ordering has been closen so that no enum value is the same as a > compatible hardware encoding. > --- > src/intel/compiler/brw_eu.c | 1 - > src/intel/compiler/brw_eu_emit.c | 6 -- > src/intel/compiler/brw_fs.cpp| 1 + > src/intel/compiler/brw_reg.h | 32 ++-- > src/intel/compiler/brw_vec4.cpp | 3 ++- > 5 files changed, 17 insertions(+), 26 deletions(-) > > diff --git a/src/intel/compiler/brw_eu.c b/src/intel/compiler/brw_eu.c > index 0ef52e219c..700a1badd4 100644 > --- a/src/intel/compiler/brw_eu.c > +++ b/src/intel/compiler/brw_eu.c > @@ -62,7 +62,6 @@ brw_reg_type_letters(unsigned type) >[BRW_REGISTER_TYPE_UQ] = "UQ", >[BRW_REGISTER_TYPE_Q] = "Q", > }; > - assert(type <= BRW_REGISTER_TYPE_Q); > return names[type]; > } > > diff --git a/src/intel/compiler/brw_eu_emit.c > b/src/intel/compiler/brw_eu_emit.c > index 6673e0741a..b59fc33a54 100644 > --- a/src/intel/compiler/brw_eu_emit.c > +++ b/src/intel/compiler/brw_eu_emit.c > @@ -112,7 +112,6 @@ brw_reg_type_to_hw_type(const struct gen_device_info > *devinfo, >}; >assert(type < ARRAY_SIZE(imm_hw_types)); >assert(imm_hw_types[type] != -1); > - assert(devinfo->gen >= 8 || type < BRW_REGISTER_TYPE_DF); >return imm_hw_types[type]; > } else { >/* Non-immediate registers */ > @@ -134,8 +133,6 @@ brw_reg_type_to_hw_type(const struct gen_device_info > *devinfo, >}; >assert(type < ARRAY_SIZE(hw_types)); >assert(hw_types[type] != -1); > - assert(devinfo->gen >= 7 || type < BRW_REGISTER_TYPE_DF); > - assert(devinfo->gen >= 8 || type < BRW_REGISTER_TYPE_Q); It might be good to keep these asserts around, but phrased as something like assert(gen_has_reg_type(gen, file, type)) or something. >return hw_types[type]; > } > } > @@ -184,9 +181,6 @@ brw_hw_reg_type_to_size(const struct gen_device_info > *devinfo, > [GEN8_HW_REG_NON_IMM_TYPE_HF] = 2, >}; >assert(type < ARRAY_SIZE(hw_sizes)); > - assert(devinfo->gen >= 7 || > - (type < GEN7_HW_REG_NON_IMM_TYPE_DF || type == > BRW_HW_REG_TYPE_F)); > - assert(devinfo->gen >= 8 || type <= BRW_HW_REG_TYPE_F); >return hw_sizes[type]; > } > } > diff --git a/src/intel/compiler/brw_fs.cpp b/src/intel/compiler/brw_fs.cpp > index fdc30d450c..0ea4c4f1cc 100644 > --- a/src/intel/compiler/brw_fs.cpp > +++ b/src/intel/compiler/brw_fs.cpp > @@ -403,6 +403,7 @@ void > fs_reg::init() > { > memset(this, 0, sizeof(*this)); > + type = BRW_REGISTER_TYPE_UD; > stride = 1; > } > > diff --git a/src/intel/compiler/brw_reg.h b/src/intel/compiler/brw_reg.h > index 17a51fbd65..48e6fd7b7d 100644 > --- a/src/intel/compiler/brw_reg.h > +++ b/src/intel/compiler/brw_reg.h > @@ -203,29 +203,25 @@ brw_mask_for_swizzle(unsigned swz) > } > > enum PACKED brw_reg_type { And maybe add a comment here like this part of the commit message: * The ordering has been chosen so that no enum value is the same as a * compatible hardware encoding. So it's clear that matching some hardware encoding is an anti-goal. fwiw: Reviewed-by: Scott D Phillips > - BRW_REGISTER_TYPE_UD = 0, > - BRW_REGISTER_TYPE_D, > - BRW_REGISTER_TYPE_UW, > - BRW_REGISTER_TYPE_W, > + /** Floating-point types: @{ */ > + BRW_REGISTER_TYPE_DF, > BRW_REGISTER_TYPE_F, > - > - /** Non-immediates only: @{ */ > - BRW_REGISTER_TYPE_UB, > - BRW_REGISTER_TYPE_B, > - /** @} */ > - > - /** Immediates only: @{ */ > - BRW_REGISTER_TYPE_UV, /* Gen6+ */ > - BRW_REGISTER_TYPE_V, > + BRW_REGISTER_TYPE_HF, > BRW_REGISTER_TYPE_VF, > /** @} */ > > - BRW_REGISTER_TYPE_DF, /* Gen7+ (no immediates until Gen8+) */ > - > - /* Gen8+ */ > - BRW_REGISTER_TYPE_HF, > - BRW_REGISTER_TYPE_UQ, > + /** Integer types: @{ */ > BRW_REGISTER_TYPE_Q, > + BRW_REGISTER_TYPE_UQ, > + BRW_REGISTER_TYPE_D, > + BRW_REGISTER_TYPE_UD, > + BRW_REGISTER_TYPE_W, > + BRW_REGISTER_TYPE_UW, > + BRW_REGISTER_TYPE_B, > + BRW_REGISTER_TYPE_UB, > + BRW_REGISTER_TYPE_V, > + BRW_REGISTER_TYPE_UV, > + /** @} */ > }; > > unsigned brw_reg_type_to_hw_type(const struct gen_device_info *devinfo, > diff --git a/src/intel/compiler/brw_vec4.cpp b/src/intel/compiler/brw_vec4.cpp > index 410922c62b..bf9a271900 100644 > --- a/src/intel/compiler/brw_vec4.cpp > +++ b/src/intel/compiler/brw_vec4.cpp > @@ -42,8 +42,8 @@ void > src_reg::init() > { > memset(this, 0, sizeof(*this)); > - > this->file = BAD_FILE; > + this->type = BRW_REGISTER_TYPE_UD; > } > > src_reg::src_reg(enum brw_reg_file file, int nr, const glsl_type *type) > @@ -85,6 +85,7 @@ dst_reg::init() > { > memset(this, 0, sizeof(*this)); > this->fi
Re: [Mesa-dev] [PATCH 10/25] i965: Use separate enums for register vs immediate types
Matt Turner writes: > The hardware encodings often mean different things depending on whether > the source is an immediate. > --- > src/intel/compiler/brw_disasm.c | 46 --- > src/intel/compiler/brw_eu_compact.c | 8 +-- > src/intel/compiler/brw_eu_defines.h | 48 +-- > src/intel/compiler/brw_eu_emit.c | 109 > +-- > src/intel/compiler/brw_eu_validate.c | 60 +-- > src/intel/compiler/brw_reg.h | 2 + > 6 files changed, 144 insertions(+), 129 deletions(-) > > diff --git a/src/intel/compiler/brw_disasm.c b/src/intel/compiler/brw_disasm.c > index 3a33614523..b5c283058a 100644 > --- a/src/intel/compiler/brw_disasm.c > +++ b/src/intel/compiler/brw_disasm.c > @@ -238,17 +238,18 @@ static const char *const access_mode[2] = { > }; > > static const char * const reg_encoding[] = { > - [BRW_HW_REG_TYPE_UD] = "UD", > - [BRW_HW_REG_TYPE_D] = "D", > - [BRW_HW_REG_TYPE_UW] = "UW", > - [BRW_HW_REG_TYPE_W] = "W", > - [BRW_HW_REG_NON_IMM_TYPE_UB] = "UB", > - [BRW_HW_REG_NON_IMM_TYPE_B] = "B", > - [GEN7_HW_REG_NON_IMM_TYPE_DF] = "DF", > - [BRW_HW_REG_TYPE_F] = "F", > - [GEN8_HW_REG_TYPE_UQ] = "UQ", > - [GEN8_HW_REG_TYPE_Q] = "Q", > - [GEN8_HW_REG_NON_IMM_TYPE_HF] = "HF", > + [BRW_HW_REG_TYPE_UD] = "UD", > + [BRW_HW_REG_TYPE_D] = "D", > + [BRW_HW_REG_TYPE_UW] = "UW", > + [BRW_HW_REG_TYPE_W] = "W", > + [BRW_HW_REG_TYPE_F] = "F", > + [GEN8_HW_REG_TYPE_UQ] = "UQ", > + [GEN8_HW_REG_TYPE_Q] = "Q", > + > + [BRW_HW_REG_TYPE_UB] = "UB", > + [BRW_HW_REG_TYPE_B] = "B", > + [GEN7_HW_REG_TYPE_DF] = "DF", > + [GEN8_HW_REG_TYPE_HF] = "HF", > }; > > static const char *const three_source_reg_encoding[] = { > @@ -1024,41 +1025,42 @@ src2_3src(FILE *file, const struct gen_device_info > *devinfo, const brw_inst *ins > } > > static int > -imm(FILE *file, const struct gen_device_info *devinfo, unsigned type, const > brw_inst *inst) > +imm(FILE *file, const struct gen_device_info *devinfo, enum hw_imm_type type, > +const brw_inst *inst) > { > switch (type) { > - case BRW_HW_REG_TYPE_UD: > + case BRW_HW_IMM_TYPE_UD: >format(file, "0x%08xUD", brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_D: > + case BRW_HW_IMM_TYPE_D: >format(file, "%dD", brw_inst_imm_d(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_UW: > + case BRW_HW_IMM_TYPE_UW: >format(file, "0x%04xUW", (uint16_t) brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_W: > + case BRW_HW_IMM_TYPE_W: >format(file, "%dW", (int16_t) brw_inst_imm_d(devinfo, inst)); >break; > - case BRW_HW_REG_IMM_TYPE_UV: > + case BRW_HW_IMM_TYPE_UV: >format(file, "0x%08xUV", brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_IMM_TYPE_VF: > + case BRW_HW_IMM_TYPE_VF: >format(file, "[%-gF, %-gF, %-gF, %-gF]VF", > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst)), > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 8), > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 16), > brw_vf_to_float(brw_inst_imm_ud(devinfo, inst) >> 24)); >break; > - case BRW_HW_REG_IMM_TYPE_V: > + case BRW_HW_IMM_TYPE_V: >format(file, "0x%08xV", brw_inst_imm_ud(devinfo, inst)); >break; > - case BRW_HW_REG_TYPE_F: > + case BRW_HW_IMM_TYPE_F: >format(file, "%-gF", brw_inst_imm_f(devinfo, inst)); >break; > - case GEN8_HW_REG_IMM_TYPE_DF: > + case GEN8_HW_IMM_TYPE_DF: >format(file, "%-gDF", brw_inst_imm_df(devinfo, inst)); >break; > - case GEN8_HW_REG_IMM_TYPE_HF: > + case GEN8_HW_IMM_TYPE_HF: >string(file, "Half Float IMM"); >break; > } > diff --git a/src/intel/compiler/brw_eu_compact.c > b/src/intel/compiler/brw_eu_compact.c > index 79103d7883..bca526f592 100644 > --- a/src/intel/compiler/brw_eu_compact.c > +++ b/src/intel/compiler/brw_eu_compact.c > @@ -995,9 +995,9 @@ precompact(const struct gen_device_info *devinfo, > brw_inst inst) > !(devinfo->is_haswell && > brw_inst_opcode(devinfo, &inst) == BRW_OPCODE_DIM) && > !(devinfo->gen >= 8 && > - (brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_IMM_TYPE_DF > || > - brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_TYPE_UQ || > - brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_REG_TYPE_Q))) { > + (brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_DF || > + brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_UQ || > + brw_inst_src0_reg_type(devinfo, &inst) == GEN8_HW_IMM_TYPE_Q))) { >brw_inst_set_src1_reg_type(devinfo, &inst, BRW_HW_REG_TYPE_UD); > } > > @@ -1016,7 +1016,7 @@ precompact(const struct gen_device_info *devinfo, > brw_ins
Re: [Mesa-dev] [PATCH 00/10] anv: Implement VK_KHR_external_fence*
On 2017-08-08 15:45:25, Jason Ekstrand wrote: > This little series adds support for the VK_KHR_external_fence family of > extensions. Most of the real work in implementing these extensions is > actually in the kernel. Once we have a DRM_SYNCOBJ_IOCTL_WAIT that does > what we need, the userspace bits are fairly straightforward. This series > can be found as a branch here: > > https://cgit.freedesktop.org/~jekstrand/mesa/log/?h=wip/anv-external-fence > > The required kernel bits can be found here: > > https://cgit.freedesktop.org/~jekstrand/linux/log/?h=drm-syncobj-wait-submit-v1 > > Cc: Chad Versace > Cc: Chris Wilson > > Jason Ekstrand (10): > anv: Rework fences to work more like BO semaphores > anv/wsi: Use QueueSubmit to trigger the fence in AcquireNextImage > anv: Pull the guts of anv_fence into anv_fence_impl > anv: Rename anv_fence_state to anv_bo_fence_state > vulkan/util: Add a vk_zalloc helper > drm-uapi/drm: Add DRM_IOCTL_SYNCOBJ_WAIT and RESET > anv/gem: Add support for syncobj wait and reset > anv: Use DRM sync objects to back fences whenever possible > anv: Implement VK_KHR_external_fence > anv: Add support for the SYNC_FD handle type for fences > > include/drm-uapi/drm.h | 19 ++ > src/intel/vulkan/anv_batch_chain.c | 59 - > src/intel/vulkan/anv_device.c | 1 + > src/intel/vulkan/anv_extensions.py | 5 + > src/intel/vulkan/anv_gem.c | 56 > src/intel/vulkan/anv_gem_stubs.c | 27 ++ > src/intel/vulkan/anv_private.h | 67 - > src/intel/vulkan/anv_queue.c | 513 > + > src/intel/vulkan/anv_wsi.c | 9 +- > src/vulkan/util/vk_alloc.h | 14 + docs/features.txt > 10 files changed, 651 insertions(+), 119 deletions(-) > > -- > 2.5.0.400.gff86faf > > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH 00/25] i965: Switch to always using logical register types
Matt Turner writes: > The mixture of hardware encodings and logical types has caused lots of > confusion. It's time to fix that. fwiw, apart from the one comment about sizeof V/UV, series is Reviewed-by: Scott D Phillips > ___ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org > https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 102123] [llvmpipe] piglit gl-3.2-layered-rendering-framebuffertexture regression
https://bugs.freedesktop.org/show_bug.cgi?id=102123 Bug ID: 102123 Summary: [llvmpipe] piglit gl-3.2-layered-rendering-framebuffertexture regression Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Keywords: bisected, regression Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org CC: bri...@vmware.com, mar...@gmail.com, srol...@vmware.com mesa: c12c2e40a36f707f733c0d6ad90160472b7a3cf6 (master 17.3.0-devel) $ ./bin/gl-3.2-layered-rendering-framebuffertexture -auto Unexpected framebuffer status! Observed: GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT Expected: GL_FRAMEBUFFER_COMPLETE Texture Type: GL_TEXTURE_2D_MULTISAMPLE. Error during setup. Texture Type: GL_TEXTURE_2D_MULTISAMPLE. FramebufferTexture() Test Failed. Unexpected framebuffer status! Observed: GL_FRAMEBUFFER_INCOMPLETE_ATTACHMENT Expected: GL_FRAMEBUFFER_COMPLETE Texture Type: GL_TEXTURE_2D_MULTISAMPLE_ARRAY. Error during setup. Texture Type: GL_TEXTURE_2D_MULTISAMPLE_ARRAY. FramebufferTexture() Test Failed. PIGLIT: {"result": "fail" } 6839d3369905eb02151334ea7b4cd39ddcfa6770 is the first bad commit commit 6839d3369905eb02151334ea7b4cd39ddcfa6770 Author: Brian Paul Date: Tue Aug 1 21:28:25 2017 -0600 st/mesa: fix handling of NumSamples=1 (v2) In Mesa we use the convention that if gl_renderbuffer::NumSamples or gl_texture_image::NumSamples is zero, it's a non-MSAA surface. Otherwise, it's an MSAA surface. But in gallium nr_samples=1 is a non-MSAA surface. Before, if the user called glRenderbufferStorageMultisample() or glTexImage2DMultisample() with samples=1 we skipped the search for the next higher number of supported samples and asked the gallium driver to create a surface with nr_samples=1. So we got a non-MSAA surface. This failed to meet the expection of the user making those calls. This patch changes the sample count checks in st_AllocTextureStorage() and st_renderbuffer_alloc_storage() to test for samples > 0 instead of > 1. And we now start querying for MSAA support at samples=2 since gallium has no concept of a 1x MSAA surface. A specific example of this problem is the Piglit arb_framebuffer_srgb-blit test. It calls glRenderbufferStorageMultisample() with samples=1 to request an MSAA renderbuffer with the minimum supported number of MSAA samples. Instead of creating a 4x or 8x, etc. MSAA surface, we wound up creating a non-MSAA surface. Finally, add a comment on the gl_renderbuffer::NumSamples field. There is one piglit regression with the VMware driver: ext_framebuffer_multisample-blit-mismatched-formats fails because now we're actually creating 4x MSAA surfaces (the requested sample count is 1) and we're hitting some sort of bug in the blitter code. That will have to be fixed separately. Other drivers may find regressions too now that MSAA surfaces are really being created. v2: start quering for MSAA support with samples=2 instead of 1. Reviewed-by: Roland Scheidegger Reviewed-by: Marek Olšák :04 04 ce8996d82890c153e42edd49df6c4d4a09a10241 2986ed81a0e5f36dfe70d4a3438613251583f4ce M src bisect run success -- You are receiving this mail because: You are the QA Contact for the bug. You are the assignee for the bug.___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH v2 0/2] glsl: interpolateAt*() fixes
Hi Nicolai, I put this series through Intels CI system and it hit a couple of issues. I haven't yet checked if these CTS test regress on radeonsi as well or if its just an i965 thing. Project: deqp-test Test: dEQP- GLES31.functional.shaders.multisample_interpolation.interpolate_at_centroid.nega tive.interpolate_struct_member Status: fail Platform/arch: skl/m64, ivb/m64, bdw/m64, hsw/m64 Project: deqp-test Test: dEQP- GLES31.functional.shaders.multisample_interpolation.interpolate_at_offset.negati ve.interpolate_struct_member Status: fail Platform/arch: skl/m64, ivb/m64, bdw/m64, hsw/m64 Project: deqp-test Test: dEQP- GLES31.functional.shaders.multisample_interpolation.interpolate_at_sample.negati ve.interpolate_struct_member Status: fail Platform/arch: skl/m64, ivb/m64, bdw/m64, hsw/m64 On 01/08/17 20:49, Nicolai Hähnle wrote: Hi all, I sent a v1 of this around ~6 weeks ago. Since then, I brought some related issues up with the OpenGL WG, and we now have a rule clarification in GLSL 4.60 (which is intended to apply to earlier versions as well). This updated series implements that rule clarification. It passes on radeonsi with some additional piglit tests that I'm going to send around in a moment. Please review! Thanks, Nicolai ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH] configure: Trust LLVM >= 4.0 llvm-config --libs for shared libraries
On 09/08/17 03:24 AM, Emil Velikov wrote: > On 8 August 2017 at 08:23, Michel Dänzer wrote: >> From: Michel Dänzer >> >> No need to manually look for the library files anymore with current >> LLVM. This sidesteps the manual method failing when LLVM was built with >> -DLLVM_APPEND_VC_REV=ON. >> > IIRC you recently spotted that the Suse packaging was building LLVM > with separate shared libraries. Apparently that's getting fixed though. > This is something which gets flagged with this (admittedly) quirky test. What do you mean by that? I just double-checked that Mesa with this patch still builds fine against LLVM built with BUILD_SHARED_LIBS=ON, just like without it. > I think we'd want to keep the best of both - just not sure how to > exactly do that. > --libs/--libfiles was completely busted with LLVM 3.9 and is back to > normal with 4.0. > > Could we use that somehow? This patch relies on llvm-config --libs with LLVM >= 4.0. If you mean something else, please be more specific. -- Earthling Michel Dänzer | http://www.amd.com Libre software enthusiast | Mesa and X developer ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [Bug 102125] [softpipe] piglit arb_texture_view-targets regression
https://bugs.freedesktop.org/show_bug.cgi?id=102125 Bug ID: 102125 Summary: [softpipe] piglit arb_texture_view-targets regression Product: Mesa Version: git Hardware: x86-64 (AMD64) OS: Linux (All) Status: NEW Keywords: bisected, regression Severity: normal Priority: medium Component: Mesa core Assignee: mesa-dev@lists.freedesktop.org Reporter: v...@freedesktop.org QA Contact: mesa-dev@lists.freedesktop.org CC: bri...@vmware.com, mar...@gmail.com, srol...@vmware.com mesa: c12c2e40a36f707f733c0d6ad90160472b7a3cf6 (master 17.3.0-devel) $ ./bin/arb_texture_view-targets -auto Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_3D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_RECTANGLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE_ARRAY) PIGLIT: {"subtest": {"1D tex target validity" : "pass"}} Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_3D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_RECTANGLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE_ARRAY) PIGLIT: {"subtest": {"2D tex target validity" : "pass"}} Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_RECTANGLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE_ARRAY) PIGLIT: {"subtest": {"3D tex target validity" : "pass"}} Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_3D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_RECTANGLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE_ARRAY) PIGLIT: {"subtest": {"Cubemap tex target validity" : "pass"}} Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_3D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_1D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_CUBE_MAP_ARRAY) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D_MULTISAMPLE_ARRAY) PIGLIT: {"subtest": {"Rectangle tex target validity" : "pass"}} Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_2D) Mesa: User error: GL_INVALID_OPERATION in glTextureView(illegal target=GL_TEXTURE_3D) Mesa: User error: GL_INVALID_O
Re: [Mesa-dev] [PATCH 1/8] glsl: calculate number of operands in an expression once
On 09/08/17 05:59, Thomas Helland wrote: I'm not quite sure if the increase in size of the base ir class versus the reduced overhead and added code is worth it? There is more code yes, but most of that is asserts and validation code. This should actually make the code more robust than it is currently. Some numbers would be nice. Yeah sorry should have included something. The reduction in time is so small that it is not really measurable. However callgrind was reporting this function as being called just under 34 million times while compiling the Deus Ex shaders (just pre-linking was profiled) with 0.20% spent in this function. IMO considering what the function does that it too high. The other IRs avoid recalculating this type of thing by keeping this stuff in an array of structs which is created only once and shared by all expressions. We could do this here also which would remove the extra member as we would have a pointer for ir_expression_operation and make it a struct rather than an enum. However on a 64-bit system the pointer then negates that saving so it wasn't worth it. If we are worried about size here we should do something about the operands array always having 4 elements. However I can at least change num_operands to a uint8_t. A comment below 2017-08-07 2:18 GMT+00:00 Timothy Arceri : Extra validation is added to ir_validate to make sure this is always updated to the correct numer of operands, as passes like lower_instructions modify the instructions directly rather then generating a new one. --- src/compiler/glsl/glsl_to_nir.cpp | 4 +-- src/compiler/glsl/ir.cpp | 20 +- src/compiler/glsl/ir.h | 13 + src/compiler/glsl/ir_builder_print_visitor.cpp | 8 +++--- src/compiler/glsl/ir_clone.cpp | 2 +- src/compiler/glsl/ir_constant_expression.cpp | 2 +- src/compiler/glsl/ir_equals.cpp| 2 +- src/compiler/glsl/ir_hv_accept.cpp | 2 +- src/compiler/glsl/ir_print_visitor.cpp | 2 +- src/compiler/glsl/ir_rvalue_visitor.cpp| 2 +- src/compiler/glsl/ir_validate.cpp | 8 ++ src/compiler/glsl/lower_instructions.cpp | 32 ++ src/compiler/glsl/lower_int64.cpp | 4 +-- src/compiler/glsl/lower_mat_op_to_vec.cpp | 8 +++--- src/compiler/glsl/lower_ubo_reference.cpp | 2 +- .../glsl/lower_vec_index_to_cond_assign.cpp| 2 +- src/compiler/glsl/lower_vector.cpp | 2 +- src/compiler/glsl/opt_algebraic.cpp| 4 +-- src/compiler/glsl/opt_constant_folding.cpp | 2 +- src/compiler/glsl/opt_tree_grafting.cpp| 2 +- src/mesa/program/ir_to_mesa.cpp| 4 +-- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 ++-- 22 files changed, 102 insertions(+), 31 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 331438a183..e5166855e8 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -1487,25 +1487,25 @@ nir_visitor::visit(ir_expression *ir) } return; } default: break; } nir_ssa_def *srcs[4]; - for (unsigned i = 0; i < ir->get_num_operands(); i++) + for (unsigned i = 0; i < ir->num_operands; i++) srcs[i] = evaluate_rvalue(ir->operands[i]); glsl_base_type types[4]; - for (unsigned i = 0; i < ir->get_num_operands(); i++) + for (unsigned i = 0; i < ir->num_operands; i++) if (supports_ints) types[i] = ir->operands[i]->type->base_type; else types[i] = GLSL_TYPE_FLOAT; glsl_base_type out_type; if (supports_ints) out_type = ir->type->base_type; else out_type = GLSL_TYPE_FLOAT; diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp index 78889bd6d3..d501e19c01 100644 --- a/src/compiler/glsl/ir.cpp +++ b/src/compiler/glsl/ir.cpp @@ -196,38 +196,46 @@ ir_expression::ir_expression(int op, const struct glsl_type *type, ir_rvalue *op0, ir_rvalue *op1, ir_rvalue *op2, ir_rvalue *op3) : ir_rvalue(ir_type_expression) { this->type = type; this->operation = ir_expression_operation(op); this->operands[0] = op0; this->operands[1] = op1; this->operands[2] = op2; this->operands[3] = op3; + init_num_operands(); + #ifndef NDEBUG - int num_operands = get_num_operands(this->operation); for (int i = num_operands; i < 4; i++) { assert(this->operands[i] == NULL); } + + for (unsigned i = 0; i < num_operands; i++) { + assert(this->operands[i] != NULL); + } #endif } It would be nice if the upper for loop iterator used unsigned for consistency with the variable and the other
[Mesa-dev] v2 GLSL compile time improvements
v2: Address various comments from Thomas. All minor but enough to warrant a resend. I've sent all but the last 2 already but they haven't received much feedback yet. This series reduces compile times of the Deus Ex shaders on my Ryzen 1800X from 2m27s -> 2m8s with a cold cache on radeonsi. Note: The above times are from compiling the shaders with shader-db on a single thread, starting the actual game results in around double the time/improvement. The aim of the series was to target overhead in the parser/AST sections of the compiler. I believe there is probably still more low hanging fruit in the AST code. I've been profiling this by compiling the shaders once and then deleting the index file from the cache. Please review. ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 3/8] glsl: stop copying struct and interface member names
We are currently copying the name for each member dereference but we can just share a single instance of the string provided by the type. This change also stops us recalculating the field index repeatedly. --- src/compiler/glsl/ast_array_index.cpp | 14 - src/compiler/glsl/glsl_to_nir.cpp | 2 +- src/compiler/glsl/ir.cpp | 8 ++--- src/compiler/glsl/ir.h | 4 +-- src/compiler/glsl/ir_clone.cpp | 4 ++- src/compiler/glsl/ir_constant_expression.cpp | 4 +-- src/compiler/glsl/ir_print_visitor.cpp | 5 +++- src/compiler/glsl/linker.cpp | 9 +- src/compiler/glsl/lower_buffer_access.cpp | 6 ++-- src/compiler/glsl/lower_named_interface_blocks.cpp | 3 +- src/compiler/glsl/opt_structure_splitting.cpp | 10 ++- src/mesa/program/ir_to_mesa.cpp| 6 ++-- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 34 ++ 13 files changed, 50 insertions(+), 59 deletions(-) diff --git a/src/compiler/glsl/ast_array_index.cpp b/src/compiler/glsl/ast_array_index.cpp index f6b7a64a28..efddbed6ea 100644 --- a/src/compiler/glsl/ast_array_index.cpp +++ b/src/compiler/glsl/ast_array_index.cpp @@ -81,37 +81,37 @@ update_max_array_access(ir_rvalue *ir, int idx, YYLTYPE *loc, while (deref_array != NULL) { deref_array_prev = deref_array; deref_array = deref_array->array->as_dereference_array(); } if (deref_array_prev != NULL) deref_var = deref_array_prev->array->as_dereference_variable(); } if (deref_var != NULL) { if (deref_var->var->is_interface_instance()) { -unsigned field_index = - deref_record->record->type->field_index(deref_record->field); -assert(field_index < deref_var->var->get_interface_type()->length); +unsigned field_idx = deref_record->field_idx; +assert(field_idx < deref_var->var->get_interface_type()->length); int *const max_ifc_array_access = deref_var->var->get_max_ifc_array_access(); assert(max_ifc_array_access != NULL); -if (idx > max_ifc_array_access[field_index]) { - max_ifc_array_access[field_index] = idx; +if (idx > max_ifc_array_access[field_idx]) { + max_ifc_array_access[field_idx] = idx; /* Check whether this access will, as a side effect, implicitly * cause the size of a built-in array to be too large. */ - check_builtin_array_max_size(deref_record->field, idx+1, *loc, -state); + const char *field_name = + deref_record->record->type->fields.structure[field_idx].name; + check_builtin_array_max_size(field_name, idx+1, *loc, state); } } } } } static int get_implicit_array_size(struct _mesa_glsl_parse_state *state, ir_rvalue *array) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index e5166855e8..bb2ba17b22 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -2198,21 +2198,21 @@ nir_visitor::visit(ir_dereference_variable *ir) nir_deref_var *deref = nir_deref_var_create(this->shader, var); this->deref_head = deref; this->deref_tail = &deref->deref; } void nir_visitor::visit(ir_dereference_record *ir) { ir->record->accept(this); - int field_index = this->deref_tail->type->field_index(ir->field); + int field_index = ir->field_idx; assert(field_index >= 0); nir_deref_struct *deref = nir_deref_struct_create(this->deref_tail, field_index); deref->deref.type = ir->type; this->deref_tail->child = &deref->deref; this->deref_tail = &deref->deref; } void nir_visitor::visit(ir_dereference_array *ir) diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp index 98bbd91539..52ca83689e 100644 --- a/src/compiler/glsl/ir.cpp +++ b/src/compiler/glsl/ir.cpp @@ -1097,24 +1097,22 @@ ir_constant::get_array_element(unsigned i) const */ if (int(i) < 0) i = 0; else if (i >= this->type->length) i = this->type->length - 1; return array_elements[i]; } ir_constant * -ir_constant::get_record_field(const char *name) +ir_constant::get_record_field(int idx) { - int idx = this->type->field_index(name); - if (idx < 0) return NULL; if (this->components.is_empty()) return NULL; exec_node *node = this->components.get_head_raw(); for (int i = 0; i < idx; i++) { node = node->next; @@ -1445,34 +1443,34 @@ ir_dereference_array::set_array(ir_rvalue *value) } ir_dereference_record::ir_dereference_record(ir_rvalue *value,
[Mesa-dev] [PATCH 1/8] glsl: calculate number of operands in an expression once
Extra validation is added to ir_validate to make sure this is always updated to the correct numer of operands, as passes like lower_instructions modify the instructions directly rather then generating a new one. The reduction in time is so small that it is not really measurable. However callgrind was reporting this function as being called just under 34 million times while compiling the Deus Ex shaders (just pre-linking was profiled) with 0.20% spent in this function. v2: - make num_operands a unit8_t - fix unsigned/signed mismatches Reviewed-by: Thomas Helland --- src/compiler/glsl/glsl_to_nir.cpp | 4 +-- src/compiler/glsl/ir.cpp | 22 +-- src/compiler/glsl/ir.h | 13 + src/compiler/glsl/ir_builder_print_visitor.cpp | 8 +++--- src/compiler/glsl/ir_clone.cpp | 2 +- src/compiler/glsl/ir_constant_expression.cpp | 2 +- src/compiler/glsl/ir_equals.cpp| 2 +- src/compiler/glsl/ir_hv_accept.cpp | 2 +- src/compiler/glsl/ir_print_visitor.cpp | 2 +- src/compiler/glsl/ir_rvalue_visitor.cpp| 2 +- src/compiler/glsl/ir_validate.cpp | 8 ++ src/compiler/glsl/lower_instructions.cpp | 32 ++ src/compiler/glsl/lower_int64.cpp | 4 +-- src/compiler/glsl/lower_mat_op_to_vec.cpp | 8 +++--- src/compiler/glsl/lower_ubo_reference.cpp | 2 +- .../glsl/lower_vec_index_to_cond_assign.cpp| 2 +- src/compiler/glsl/lower_vector.cpp | 2 +- src/compiler/glsl/opt_algebraic.cpp| 4 +-- src/compiler/glsl/opt_constant_folding.cpp | 2 +- src/compiler/glsl/opt_tree_grafting.cpp| 2 +- src/mesa/program/ir_to_mesa.cpp| 4 +-- src/mesa/state_tracker/st_glsl_to_tgsi.cpp | 6 ++-- 22 files changed, 103 insertions(+), 32 deletions(-) diff --git a/src/compiler/glsl/glsl_to_nir.cpp b/src/compiler/glsl/glsl_to_nir.cpp index 331438a183..e5166855e8 100644 --- a/src/compiler/glsl/glsl_to_nir.cpp +++ b/src/compiler/glsl/glsl_to_nir.cpp @@ -1487,25 +1487,25 @@ nir_visitor::visit(ir_expression *ir) } return; } default: break; } nir_ssa_def *srcs[4]; - for (unsigned i = 0; i < ir->get_num_operands(); i++) + for (unsigned i = 0; i < ir->num_operands; i++) srcs[i] = evaluate_rvalue(ir->operands[i]); glsl_base_type types[4]; - for (unsigned i = 0; i < ir->get_num_operands(); i++) + for (unsigned i = 0; i < ir->num_operands; i++) if (supports_ints) types[i] = ir->operands[i]->type->base_type; else types[i] = GLSL_TYPE_FLOAT; glsl_base_type out_type; if (supports_ints) out_type = ir->type->base_type; else out_type = GLSL_TYPE_FLOAT; diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp index 78889bd6d3..51b875058b 100644 --- a/src/compiler/glsl/ir.cpp +++ b/src/compiler/glsl/ir.cpp @@ -196,38 +196,46 @@ ir_expression::ir_expression(int op, const struct glsl_type *type, ir_rvalue *op0, ir_rvalue *op1, ir_rvalue *op2, ir_rvalue *op3) : ir_rvalue(ir_type_expression) { this->type = type; this->operation = ir_expression_operation(op); this->operands[0] = op0; this->operands[1] = op1; this->operands[2] = op2; this->operands[3] = op3; + init_num_operands(); + #ifndef NDEBUG - int num_operands = get_num_operands(this->operation); - for (int i = num_operands; i < 4; i++) { + for (unsigned i = num_operands; i < 4; i++) { assert(this->operands[i] == NULL); } + + for (unsigned i = 0; i < num_operands; i++) { + assert(this->operands[i] != NULL); + } #endif } ir_expression::ir_expression(int op, ir_rvalue *op0) : ir_rvalue(ir_type_expression) { this->operation = ir_expression_operation(op); this->operands[0] = op0; this->operands[1] = NULL; this->operands[2] = NULL; this->operands[3] = NULL; assert(op <= ir_last_unop); + init_num_operands(); + assert(num_operands == 1); + assert(this->operands[0]); switch (this->operation) { case ir_unop_bit_not: case ir_unop_logic_not: case ir_unop_neg: case ir_unop_abs: case ir_unop_sign: case ir_unop_rcp: case ir_unop_rsq: case ir_unop_sqrt: @@ -418,20 +426,25 @@ ir_expression::ir_expression(int op, ir_rvalue *op0) ir_expression::ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1) : ir_rvalue(ir_type_expression) { this->operation = ir_expression_operation(op); this->operands[0] = op0; this->operands[1] = op1; this->operands[2] = NULL; this->operands[3] = NULL; assert(op > ir_last_unop); + init_num_operands(); + assert(num_operands == 2); + for (unsigned i = 0; i < num_operands; i++) { +
[Mesa-dev] [PATCH 6/8] glsl: stop cloning builtin fuctions _mesa_glsl_find_builtin_function()
The cloning was introduced in f81ede469910d to fixed a problem with shaders including IR that was owned by builtins. However the approach of cloning the whole function each time we reference a builtin lead to a significant reduction in the GLSL IR compilers performance. The previous patch fixes the ownership problem in a more precise way. So we can now remove this cloning. Testing on a Ryzan 7 1800X shows a ~15% decreases in compiling the Deus Ex: Mankind Divided shaders on radeonsi (which take 5min+ on some machines). Looking just at the GLSL IR compiler the speed up is ~40%. Cc: Kenneth Graunke Cc: Lionel Landwerlin Tested-by: Dieter Nützel --- src/compiler/glsl/builtin_functions.cpp | 11 +-- 1 file changed, 1 insertion(+), 10 deletions(-) diff --git a/src/compiler/glsl/builtin_functions.cpp b/src/compiler/glsl/builtin_functions.cpp index a63f545b79..9df9671f13 100644 --- a/src/compiler/glsl/builtin_functions.cpp +++ b/src/compiler/glsl/builtin_functions.cpp @@ -6284,30 +6284,21 @@ _mesa_glsl_release_builtin_functions() ir_function_signature * _mesa_glsl_find_builtin_function(_mesa_glsl_parse_state *state, const char *name, exec_list *actual_parameters) { ir_function_signature *s; mtx_lock(&builtins_lock); s = builtins.find(state, name, actual_parameters); mtx_unlock(&builtins_lock); - if (s == NULL) - return NULL; - - struct hash_table *ht = - _mesa_hash_table_create(NULL, _mesa_hash_pointer, _mesa_key_pointer_equal); - void *mem_ctx = state; - ir_function *f = s->function()->clone(mem_ctx, ht); - _mesa_hash_table_destroy(ht, NULL); - - return f->matching_signature(state, actual_parameters, true); + return s; } bool _mesa_glsl_has_builtin_function(_mesa_glsl_parse_state *state, const char *name) { ir_function *f; bool ret = false; mtx_lock(&builtins_lock); f = builtins.shader->symbols->get_function(name); if (f != NULL) { -- 2.13.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 2/8] glsl: tidy up get_num_operands()
Also add a comment that this should only be used by the ir_reader interface for testing purposes. v2: - fix grammar in comment - use unreachable rather than assert Reviewed-by: Thomas Helland --- src/compiler/glsl/ir.cpp | 9 ++--- src/compiler/glsl/ir.h | 14 +++--- 2 files changed, 9 insertions(+), 14 deletions(-) diff --git a/src/compiler/glsl/ir.cpp b/src/compiler/glsl/ir.cpp index 51b875058b..98bbd91539 100644 --- a/src/compiler/glsl/ir.cpp +++ b/src/compiler/glsl/ir.cpp @@ -549,39 +549,42 @@ ir_expression::ir_expression(int op, ir_rvalue *op0, ir_rvalue *op1, case ir_triop_csel: this->type = op1->type; break; default: assert(!"not reached: missing automatic type setup for ir_expression"); this->type = glsl_type::float_type; } } -unsigned int +/** + * This is only here for ir_reader to used for testing purposes. Please use + * the precomputed num_operands field if you need the number of operands. + */ +unsigned ir_expression::get_num_operands(ir_expression_operation op) { assert(op <= ir_last_opcode); if (op <= ir_last_unop) return 1; if (op <= ir_last_binop) return 2; if (op <= ir_last_triop) return 3; if (op <= ir_last_quadop) return 4; - assert(false); - return 0; + unreachable("Could not calculate number of operands"); } #include "ir_expression_operation_strings.h" const char* depth_layout_string(ir_depth_layout layout) { switch(layout) { case ir_depth_layout_none: return ""; case ir_depth_layout_any: return "depth_any"; diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h index 58e6356566..d53b44304d 100644 --- a/src/compiler/glsl/ir.h +++ b/src/compiler/glsl/ir.h @@ -1531,32 +1531,24 @@ public: * The "variable_context" hash table links ir_variable * to ir_constant * * that represent the variables' values. \c NULL represents an empty * context. * * If the expression cannot be constant folded, this method will return * \c NULL. */ virtual ir_constant *constant_expression_value(struct hash_table *variable_context = NULL); /** -* Determine the number of operands used by an expression +* This is only here for ir_reader to used for testing purposes please use +* the precomputed num_operands field if you need the number of operands. */ - static unsigned int get_num_operands(ir_expression_operation); - - /** -* Determine the number of operands used by an expression -*/ - unsigned int get_num_operands() const - { - return (this->operation == ir_quadop_vector) -? this->type->vector_elements : get_num_operands(operation); - } + static unsigned get_num_operands(ir_expression_operation); /** * Return whether the expression operates on vectors horizontally. */ bool is_horizontal() const { return operation == ir_binop_all_equal || operation == ir_binop_any_nequal || operation == ir_binop_dot || operation == ir_binop_vector_extract || -- 2.13.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 4/8] glsl: remove unused field from ir_call
Reviewed-by: Thomas Helland Tested-by: Dieter Nützel --- src/compiler/glsl/ir.h | 5 - 1 file changed, 5 deletions(-) diff --git a/src/compiler/glsl/ir.h b/src/compiler/glsl/ir.h index ce4ade9e80..170759abeb 100644 --- a/src/compiler/glsl/ir.h +++ b/src/compiler/glsl/ir.h @@ -1595,32 +1595,30 @@ public: */ class ir_call : public ir_instruction { public: ir_call(ir_function_signature *callee, ir_dereference_variable *return_deref, exec_list *actual_parameters) : ir_instruction(ir_type_call), return_deref(return_deref), callee(callee), sub_var(NULL), array_idx(NULL) { assert(callee->return_type != NULL); actual_parameters->move_nodes_to(& this->actual_parameters); - this->use_builtin = callee->is_builtin(); } ir_call(ir_function_signature *callee, ir_dereference_variable *return_deref, exec_list *actual_parameters, ir_variable *var, ir_rvalue *array_idx) : ir_instruction(ir_type_call), return_deref(return_deref), callee(callee), sub_var(var), array_idx(array_idx) { assert(callee->return_type != NULL); actual_parameters->move_nodes_to(& this->actual_parameters); - this->use_builtin = callee->is_builtin(); } virtual ir_call *clone(void *mem_ctx, struct hash_table *ht) const; virtual ir_constant *constant_expression_value(struct hash_table *variable_context = NULL); virtual void accept(ir_visitor *v) { v->visit(this); } @@ -1648,23 +1646,20 @@ public: ir_dereference_variable *return_deref; /** * The specific function signature being called. */ ir_function_signature *callee; /* List of ir_rvalue of paramaters passed in this call. */ exec_list actual_parameters; - /** Should this call only bind to a built-in function? */ - bool use_builtin; - /* * ARB_shader_subroutine support - * the subroutine uniform variable and array index * rvalue to be used in the lowering pass later. */ ir_variable *sub_var; ir_rvalue *array_idx; }; -- 2.13.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 5/8] glsl: clone builtin function constants
f81ede469910d fixed a problem with shaders including IR that was owned by builtins. However the approach of cloning the whole function each time we referenced it lead to a significant reduction in the GLSL IR compiler performance. Everything was already cloned when inlining the function, as far as I can tell this is the only place where we are grabbing IR owned by the builtins without cloning it. The double free can be easily reproduced by compiling the Deus Ex: Mankind Divided shaders with shader db, and then compiling them again after deleting mesa's shader cache index file. This will cause optimisations to never be performed on the IR and which presumably caused this issue to be hidden under normal circumstances. Cc: Kenneth Graunke Cc: Lionel Landwerlin Tested-by: Dieter Nützel --- src/compiler/glsl/ast_function.cpp | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/compiler/glsl/ast_function.cpp b/src/compiler/glsl/ast_function.cpp index f7e90fba5b..73c4c0df7b 100644 --- a/src/compiler/glsl/ast_function.cpp +++ b/src/compiler/glsl/ast_function.cpp @@ -514,21 +514,29 @@ generate_call(exec_list *instructions, ir_function_signature *sig, * return 0 when evaluated inside an initializer with an argument * that is a constant expression." * * If the function call is a constant expression, don't generate any * instructions; just generate an ir_constant. */ if (state->is_version(120, 100)) { ir_constant *value = sig->constant_expression_value(actual_parameters, NULL); if (value != NULL) { - return value; + if (sig->is_builtin()) { +/* This value belongs to a builtin so we must clone it to avoid + * race conditions when freeing shaders and destorying the + * context. + */ +return value->clone(ctx, NULL); + } else { +return value; + } } } ir_dereference_variable *deref = NULL; if (!sig->return_type->is_void()) { /* Create a new temporary to hold the return value. */ char *const name = ir_variable::temporaries_allocate_names ? ralloc_asprintf(ctx, "%s_retval", sig->function_name()) : NULL; -- 2.13.4 ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
[Mesa-dev] [PATCH 8/8] glsl: use ralloc_str_append() rather than ralloc_asprintf_rewrite_tail()
The Deus Ex: Mankind Divided shaders go from spending ~20 seconds in the GLSL IR compilers front-end down to ~18.5 seconds on a Ryzen 1800X. Tested by compiling once with shader-db then deleting the index file from the shader cache and compiling again. v2: - fix rebasing issue in v1 --- src/compiler/glsl/glcpp/glcpp-parse.y | 144 ++ 1 file changed, 113 insertions(+), 31 deletions(-) diff --git a/src/compiler/glsl/glcpp/glcpp-parse.y b/src/compiler/glsl/glcpp/glcpp-parse.y index f1719f90b1..898a26044f 100644 --- a/src/compiler/glsl/glcpp/glcpp-parse.y +++ b/src/compiler/glsl/glcpp/glcpp-parse.y @@ -202,21 +202,26 @@ add_builtin_define(glcpp_parser_t *parser, const char *name, int value); input: /* empty */ | input line ; line: control_line | SPACE control_line | text_line { _glcpp_parser_print_expanded_token_list (parser, $1); - ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "\n"); + const char *newline_str = "\n"; + size_t size = strlen(newline_str); + + ralloc_str_append(&parser->output, newline_str, + parser->output_length, size); + parser->output_length += size; } | expanded_line ; expanded_line: IF_EXPANDED expression NEWLINE { if (parser->is_gles && $2.undefined_macro) glcpp_error(& @1, parser, "undefined macro %s in expression (illegal in GLES)", $2.undefined_macro); _glcpp_parser_skip_stack_push_if (parser, & @1, $2.value); } @@ -252,21 +257,26 @@ define: | FUNC_IDENTIFIER '(' ')' replacement_list NEWLINE { _define_function_macro (parser, & @1, $1, NULL, $4); } | FUNC_IDENTIFIER '(' identifier_list ')' replacement_list NEWLINE { _define_function_macro (parser, & @1, $1, $3, $5); } ; control_line: control_line_success { - ralloc_asprintf_rewrite_tail (&parser->output, &parser->output_length, "\n"); + const char *newline_str = "\n"; + size_t size = strlen(newline_str); + + ralloc_str_append(&parser->output, newline_str, + parser->output_length, size); + parser->output_length += size; } | control_line_error | HASH_TOKEN LINE pp_tokens NEWLINE { if (parser->skip_stack == NULL || parser->skip_stack->type == SKIP_NO_SKIP) { _glcpp_parser_expand_and_lex_from (parser, LINE_EXPANDED, $3, EXPANSION_MODE_IGNORE_DEFINED); @@ -1123,72 +1133,144 @@ _token_list_equal_ignoring_space(token_list_t *a, token_list_t *b) node_b = node_b->next; } return 1; } static void _token_print(char **out, size_t *len, token_t *token) { if (token->type < 256) { - ralloc_asprintf_rewrite_tail (out, len, "%c", token->type); + size_t size = sizeof(char); + + ralloc_str_append(out, (char *) &token->type, *len, size); + *len += size; return; } switch (token->type) { case INTEGER: ralloc_asprintf_rewrite_tail (out, len, "%" PRIiMAX, token->value.ival); break; case IDENTIFIER: case INTEGER_STRING: - case OTHER: - ralloc_asprintf_rewrite_tail (out, len, "%s", token->value.str); + case OTHER: { + size_t size = strlen(token->value.str); + + ralloc_str_append(out, token->value.str, *len, size); + *len += size; break; - case SPACE: - ralloc_asprintf_rewrite_tail (out, len, " "); + } + case SPACE: { + const char *token_str = " "; + size_t size = strlen(token_str); + + ralloc_str_append(out, token_str, *len, size); + *len += size; break; - case LEFT_SHIFT: - ralloc_asprintf_rewrite_tail (out, len, "<<"); + } + case LEFT_SHIFT: { + const char *token_str = "<<"; + size_t size = strlen(token_str); + + ralloc_str_append(out, token_str, *len, size); + *len += size; break; - case RIGHT_SHIFT: - ralloc_asprintf_rewrite_tail (out, len, ">>"); + } + case RIGHT_SHIFT: { + const char *token_str = ">>"; + size_t size = strlen(token_str); + + ralloc_str_append(out, token_str, *len, size); + *len += size; break; - case LESS_OR_EQUAL: - ralloc_asprintf_rewrite_tail (out, len, "<="); + } + case LESS_OR_EQUAL: { + const char *token_str = "<="; + size_t size = strlen(token_str); + + ralloc_str_append(out, token_str, *len, size); + *len += size; break; - case GREATER_OR_EQUAL: - ralloc_asprintf_rewrite_tail (out, len, ">="); + } + case GREATER_OR_EQUAL: { + const char *token_str = ">="; +